IPC: Move the legacy key to the QNativeIpcKey

This is needed to support passing it to other processes so they can
enable legacy, compatibility mode. Right now, there's no such code, but
I am 90% certain we'll need it soon in 6.6.x, if not for compatibility
changes in the future.

There's a bug in passing a QNativeIpcKey to another process that causes
QSharedMemory to use the wrong QSystemSemaphore for control (a feature
that should never have existed in the first place, but we're 15 years
too late on that). I have not yet investigated a fix for this, but it
will likely involve knowing the original legacy key.

Pick-to: 6.6.0
Change-Id: Idd5e1bb52be047d7b4fffffd1750b547013cb336
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
(cherry picked from commit 9edb835904a7fa856e482464a7258019d5766333)

src/corelib/ipc/qsharedmemory.cpp

@@ -140,7 +140,6 @@ QSharedMemory::QSharedMemory(const QNativeIpcKey &key, QObject *parent)
 QSharedMemory::QSharedMemory(const QString &key, QObject *parent)
     : QSharedMemory(legacyNativeKey(key), parent)
 {
-    d_func()->legacyKey = key;
 }
 #endif
 
@@ -185,9 +184,7 @@ QSharedMemory::~QSharedMemory()
 */
 void QSharedMemory::setKey(const QString &key)
 {
-    Q_D(QSharedMemory);
     setNativeKey(legacyNativeKey(key));
-    d->legacyKey = key;
 }
 #endif
 
@@ -245,7 +242,6 @@ void QSharedMemory::setNativeKey(const QNativeIpcKey &key)
     if (isAttached())
         detach();
     d->cleanHandle();
-    d->legacyKey = QString();
     if (key.type() == d->nativeKey.type()) {
         // we can reuse the backend
         d->nativeKey = key;
@@ -314,7 +310,7 @@ bool QSharedMemoryPrivate::initKey(SemaphoreAccessMode mode)
 QString QSharedMemory::key() const
 {
     Q_D(const QSharedMemory);
-    return d->legacyKey;
+    return QNativeIpcKeyPrivate::legacyKey(d->nativeKey);
 }
 #endif
 

src/corelib/ipc/qsharedmemory_p.h

@@ -205,8 +205,6 @@ public:
     }
     QNativeIpcKey semaphoreNativeKey() const;
 #endif // QT_CONFIG(systemsemaphore)
-
-    QString legacyKey;  // deprecated
 };
 
 QT_END_NAMESPACE

src/corelib/ipc/qsystemsemaphore.cpp

@@ -84,7 +84,6 @@ inline void QSystemSemaphorePrivate::destructBackend()
 QSystemSemaphore::QSystemSemaphore(const QString &key, int initialValue, AccessMode mode)
     : QSystemSemaphore(legacyNativeKey(key), initialValue, mode)
 {
-    d->legacyKey = key;
 }
 #endif
 
@@ -210,8 +209,6 @@ void QSystemSemaphore::setNativeKey(const QNativeIpcKey &key, int initialValue,
     }
     d->initialValue = initialValue;
     d->handle(mode);
-
-    d->legacyKey.clear();
 }
 
 /*!
@@ -244,7 +241,6 @@ QNativeIpcKey QSystemSemaphore::nativeIpcKey() const
 void QSystemSemaphore::setKey(const QString &key, int initialValue, AccessMode mode)
 {
     setNativeKey(legacyNativeKey(key), initialValue, mode);
-    d->legacyKey = key;
 }
 
 /*!
@@ -256,7 +252,7 @@ void QSystemSemaphore::setKey(const QString &key, int initialValue, AccessMode m
  */
 QString QSystemSemaphore::key() const
 {
-    return d->legacyKey;
+    return QNativeIpcKeyPrivate::legacyKey(d->nativeKey);
 }
 #endif
 

src/corelib/ipc/qsystemsemaphore_p.h

@@ -142,8 +142,6 @@ public:
     {
         return visit([&](auto p) { return p->modifySemaphore(this, count); });
     }
-
-    QString legacyKey;  // deprecated
 };
 
 QT_END_NAMESPACE

src/corelib/ipc/qtipccommon.cpp

@@ -8,6 +8,7 @@
 #include <qstandardpaths.h>
 #include <qstringconverter.h>
 #include <qurl.h>
+#include <qurlquery.h>
 
 #if defined(Q_OS_DARWIN)
 #  include "private/qcore_mac_p.h"
@@ -91,10 +92,11 @@ static QNativeIpcKey::Type stringToType(QStringView typeString)
     Legacy: this exists for compatibility with QSharedMemory and
     QSystemSemaphore between 4.4 and 6.6.
 
-    Generate a string from the key which can be any unicode string into
+    Returns a QNativeIpcKey that contains a platform-safe key using rules
-    the subset that the win/unix kernel allows.
+    similar to QtIpcCommon::platformSafeKey() below, but using an algorithm
-
+    that is compatible with Qt 4.4 to 6.6. Additionally, the returned
-    On Unix this will be a file name
+    QNativeIpcKey will record the input \a key so it can be included in the
+    string form if necessary to pass to other processes.
 */
 QNativeIpcKey QtIpcCommon::legacyPlatformSafeKey(const QString &key, QtIpcCommon::IpcType ipcType,
                                                  QNativeIpcKey::Type type)
@@ -112,13 +114,14 @@ QNativeIpcKey QtIpcCommon::legacyPlatformSafeKey(const QString &key, QtIpcCommon
             // to be in the form <application group identifier>/<custom identifier>.
             // Since we don't know which application group identifier the user wants
             // to apply, we instead document that requirement, and use the key directly.
-            k.setNativeKey(key);
+            QNativeIpcKeyPrivate::setNativeAndLegacyKeys(k, key, key);
         } else {
             // The shared memory name limit on Apple platforms is very low (30 characters),
             // so we can't use the logic below of combining the prefix, key, and a hash,
             // to ensure a unique and valid name. Instead we use the first part of the
             // hash, which should still long enough to avoid collisions in practice.
-            k.setNativeKey(u'/' + QLatin1StringView(hex).left(SHM_NAME_MAX - 1));
+            QString native = u'/' + QLatin1StringView(hex).left(SHM_NAME_MAX - 1);
+            QNativeIpcKeyPrivate::setNativeAndLegacyKeys(k, native, key);
         }
         return k;
 #endif
@@ -145,20 +148,30 @@ QNativeIpcKey QtIpcCommon::legacyPlatformSafeKey(const QString &key, QtIpcCommon
     switch (type) {
     case QNativeIpcKey::Type::Windows:
         if (isIpcSupported(ipcType, QNativeIpcKey::Type::Windows))
-            k.setNativeKey(result);
+            QNativeIpcKeyPrivate::setNativeAndLegacyKeys(k, result, key);
         return k;
     case QNativeIpcKey::Type::PosixRealtime:
+        result.prepend(u'/');
         if (isIpcSupported(ipcType, QNativeIpcKey::Type::PosixRealtime))
-            k.setNativeKey(result.prepend(u'/'));
+            QNativeIpcKeyPrivate::setNativeAndLegacyKeys(k, result, key);
         return k;
     case QNativeIpcKey::Type::SystemV:
         break;
     }
-    if (isIpcSupported(ipcType, QNativeIpcKey::Type::SystemV))
+    if (isIpcSupported(ipcType, QNativeIpcKey::Type::SystemV)) {
-        k.setNativeKey(QStandardPaths::writableLocation(QStandardPaths::TempLocation) + u'/' + result);
+        result = QStandardPaths::writableLocation(QStandardPaths::TempLocation) + u'/' + result;
+        QNativeIpcKeyPrivate::setNativeAndLegacyKeys(k, result, key);
+    }
     return k;
 }
 
+/*!
+    \internal
+    Returns a QNativeIpcKey of type \a type, suitable for QSystemSemaphore or
+    QSharedMemory depending on \a ipcType. The returned native key is generated
+    from the Unicode input \a key and is safe for use on for the key type in
+    question in the current OS.
+*/
 QNativeIpcKey QtIpcCommon::platformSafeKey(const QString &key, QtIpcCommon::IpcType ipcType,
                                            QNativeIpcKey::Type type)
 {
@@ -478,6 +491,7 @@ void QNativeIpcKey::setType_internal(Type type)
 */
 void QNativeIpcKey::setNativeKey_internal(const QString &)
 {
+    d->legacyKey_.clear();
 }
 
 /*!
@@ -493,6 +507,8 @@ void QNativeIpcKey::setNativeKey_internal(const QString &)
 */
 size_t qHash(const QNativeIpcKey &ipcKey, size_t seed) noexcept
 {
+    // by *choice*, we're not including d->legacyKey_ in the hash -- it's
+    // already partially encoded in the key
     return qHashMulti(seed, ipcKey.key, ipcKey.type());
 }
 
@@ -504,8 +520,7 @@ size_t qHash(const QNativeIpcKey &ipcKey, size_t seed) noexcept
 */
 int QNativeIpcKey::compare_internal(const QNativeIpcKey &lhs, const QNativeIpcKey &rhs) noexcept
 {
-    Q_UNUSED(lhs); Q_UNUSED(rhs);
+    return (QNativeIpcKeyPrivate::legacyKey(lhs) == QNativeIpcKeyPrivate::legacyKey(rhs)) ? 0 : 1;
-    return 0;
 }
 
 /*!
@@ -534,6 +549,12 @@ QString QNativeIpcKey::toString() const
     QUrl u;
     u.setScheme(prefix);
     u.setPath(copy, QUrl::TolerantMode);
+    if (isSlowPath()) {
+        QUrlQuery q;
+        if (!d->legacyKey_.isEmpty())
+            q.addQueryItem(u"legacyKey"_s, QString(d->legacyKey_).replace(u'%', "%25"_L1));
+        u.setQuery(q);
+    }
     return u.toString(QUrl::DecodeReserved);
 }
 
@@ -553,7 +574,7 @@ QNativeIpcKey QNativeIpcKey::fromString(const QString &text)
     Type invalidType = {};
     Type type = stringToType(u.scheme());
     if (type == invalidType || !u.isValid() || !u.userInfo().isEmpty() || !u.host().isEmpty()
-            || u.port() != -1 || u.hasQuery())
+            || u.port() != -1)
         return QNativeIpcKey(invalidType);
 
     QNativeIpcKey result(QString(), type);
@@ -563,6 +584,18 @@ QNativeIpcKey QNativeIpcKey::fromString(const QString &text)
     // decode the payload
     result.setNativeKey(u.path());
 
+    if (u.hasQuery()) {
+        const QList items = QUrlQuery(u).queryItems();
+        for (const auto &item : items) {
+            if (item.first == u"legacyKey"_s) {
+                QString legacyKey = QUrl::fromPercentEncoding(item.second.toUtf8());
+                QNativeIpcKeyPrivate::setLegacyKey(result, std::move(legacyKey));
+            } else {
+                // unknown query item
+                return QNativeIpcKey(invalidType);
+            }
+        }
+    }
     return result;
 }
 

src/corelib/ipc/qtipccommon_p.h

@@ -31,6 +31,32 @@ QT_BEGIN_NAMESPACE
 class QNativeIpcKeyPrivate
 {
 public:
+    QString legacyKey_;
+
+    static QString legacyKey(const QNativeIpcKey &key)
+    {
+        if (key.isSlowPath())
+            return key.d->legacyKey_;
+        return QString();
+    }
+    static void setLegacyKey(QNativeIpcKey &key, const QString &legacyKey)
+    {
+        QNativeIpcKeyPrivate::makeExtended(key)->legacyKey_ = legacyKey;
+    }
+    static void setNativeAndLegacyKeys(QNativeIpcKey &key, const QString &nativeKey,
+                                       const QString &legacyKey)
+    {
+        key.setNativeKey(nativeKey);
+        setLegacyKey(key, legacyKey);
+    }
+
+private:
+    static QNativeIpcKeyPrivate *makeExtended(QNativeIpcKey &key)
+    {
+        if (!key.isSlowPath())
+            key.d = new QNativeIpcKeyPrivate;
+        return key.d;
+    }
 };
 
 namespace QtIpcCommon {

tests/auto/corelib/ipc/qnativeipckey/tst_qnativeipckey.cpp

@@ -6,6 +6,19 @@
 
 #include "../ipctestcommon.h"
 
+#if QT_CONFIG(sharedmemory)
+#  include <qsharedmemory.h>
+#endif
+#if QT_CONFIG(systemsemaphore)
+#  include <qsystemsemaphore.h>
+#endif
+
+#if QT_CONFIG(sharedmemory)
+static const auto makeLegacyKey = QSharedMemory::legacyNativeKey;
+#else
+static const auto makeLegacyKey = QSystemSemaphore::legacyNativeKey;
+#endif
+
 using namespace Qt::StringLiterals;
 
 class tst_QNativeIpcKey : public QObject
@@ -22,6 +35,8 @@ private slots:
     void toString();
     void fromString_data();
     void fromString();
+    void legacyKeys_data();
+    void legacyKeys();
 };
 
 void tst_QNativeIpcKey::defaultTypes()
@@ -181,6 +196,19 @@ void tst_QNativeIpcKey::equality()
     key2.setType(QNativeIpcKey::DefaultTypeForOs);
     QCOMPARE(key1, key2);
     QVERIFY(!(key1 != key2));
+
+    key1 = makeLegacyKey("key1", QNativeIpcKey::DefaultTypeForOs);
+    QCOMPARE_NE(key1, key2);
+    QVERIFY(!(key1 == key2));
+
+    key2 = key1;
+    QCOMPARE(key1, key2);
+    QVERIFY(!(key1 != key2));
+
+    // just setting the native key won't make them equal again!
+    key2.setNativeKey(key1.nativeKey());
+    QCOMPARE_NE(key1, key2);
+    QVERIFY(!(key1 == key2));
 }
 
 void tst_QNativeIpcKey::hash()
@@ -215,6 +243,12 @@ void tst_QNativeIpcKey::swap()
     QCOMPARE(key1.type(), QNativeIpcKey::Type::PosixRealtime);
     QCOMPARE(key2.nativeKey(), "key2");
     QCOMPARE(key2.type(), QNativeIpcKey::Type::Windows);
+
+    key1 = makeLegacyKey("key1", QNativeIpcKey::DefaultTypeForOs);
+    QCOMPARE(key1.type(), QNativeIpcKey::DefaultTypeForOs);
+    key1.swap(key2);
+    QCOMPARE(key1.type(), QNativeIpcKey::Type::Windows);
+    QCOMPARE(key2.type(), QNativeIpcKey::DefaultTypeForOs);
 }
 
 void tst_QNativeIpcKey::toString_data()
@@ -323,5 +357,70 @@ void tst_QNativeIpcKey::fromString()
     QCOMPARE(QNativeIpcKey::fromString(string), key);
 }
 
+void tst_QNativeIpcKey::legacyKeys_data()
+{
+    QTest::addColumn<QNativeIpcKey::Type>("type");
+    QTest::addColumn<QString>("legacyKey");
+    auto addRows = [](QNativeIpcKey::Type type) {
+        const char *label = "<unknown-type>";
+        switch (type) {
+        case QNativeIpcKey::Type::SystemV:
+            label = "systemv";
+            break;
+        case QNativeIpcKey::Type::PosixRealtime:
+            label = "posix";
+            break;
+        case QNativeIpcKey::Type::Windows:
+            label = "windows";
+            break;
+        }
+        auto add = [=](const char *name, const QString &legacyKey) {
+            QTest::addRow("%s-%s", label, name) << type << legacyKey;
+        };
+        add("empty", {});
+        add("text", "foobar"_L1);
+        add("pathlike", "/sometext"_L1);
+        add("objectlike", "Global\\sometext"_L1);
+        add("colon-slash", ":/"_L1);
+        add("slash-colon", "/:"_L1);
+        add("percent", "%"_L1);
+        add("question-hash", "?#"_L1);
+        add("hash-question", "#?"_L1);
+        add("double-slash", "//"_L1);
+        add("triple-slash", "///"_L1);
+        add("non-ascii", "\xe9"_L1);
+        add("non-utf8", "\xa0\xff"_L1);
+        add("non-latin1", u":\u0100.\u2000.\U00010000"_s);
+    };
+
+    addRows(QNativeIpcKey::DefaultTypeForOs);
+    if (auto type = QNativeIpcKey::legacyDefaultTypeForOs();
+            type != QNativeIpcKey::DefaultTypeForOs)
+        addRows(type);
+}
+
+void tst_QNativeIpcKey::legacyKeys()
+{
+    QFETCH(QNativeIpcKey::Type, type);
+    QFETCH(QString, legacyKey);
+
+    QNativeIpcKey key = makeLegacyKey(legacyKey, type);
+    QCOMPARE(key.type(), type);
+
+    QString string = key.toString();
+    QNativeIpcKey key2 = QNativeIpcKey::fromString(string);
+    QCOMPARE(key2, key);
+
+    if (!legacyKey.isEmpty()) {
+        // confirm it shows up in the encoded form
+        Q_ASSERT(!legacyKey.contains(u'&'));    // needs extra encoding
+        QUrl u;
+        u.setQuery("legacyKey="_L1 + legacyKey, QUrl::DecodedMode);
+        QString encodedLegacyKey = u.toString(QUrl::RemoveScheme | QUrl::RemoveAuthority
+                                              | QUrl::DecodeReserved);
+        QVERIFY2(string.contains(encodedLegacyKey), qPrintable(string));
+    }
+}
+
 QTEST_MAIN(tst_QNativeIpcKey)
 #include "tst_qnativeipckey.moc"