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.

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

7 files changed, 173 insertions, 27 deletions

diff --git a/src/corelib/ipc/qsharedmemory.cpp b/src/corelib/ipc/qsharedmemory.cpp
index 89bd4ffe0c..8dcdc43e18 100644
--- a/src/corelib/ipc/qsharedmemory.cpp
+++ b/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
 
diff --git a/src/corelib/ipc/qsharedmemory_p.h b/src/corelib/ipc/qsharedmemory_p.h
index 94194babf3..987bb38642 100644
--- a/src/corelib/ipc/qsharedmemory_p.h
+++ b/src/corelib/ipc/qsharedmemory_p.h
@@ -205,8 +205,6 @@ public:
     }
     QNativeIpcKey semaphoreNativeKey() const;
 #endif // QT_CONFIG(systemsemaphore)
-
-    QString legacyKey;  // deprecated
 };
 
 QT_END_NAMESPACE
diff --git a/src/corelib/ipc/qsystemsemaphore.cpp b/src/corelib/ipc/qsystemsemaphore.cpp
index d22e653b5d..520b627c1c 100644
--- a/src/corelib/ipc/qsystemsemaphore.cpp
+++ b/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
 
diff --git a/src/corelib/ipc/qsystemsemaphore_p.h b/src/corelib/ipc/qsystemsemaphore_p.h
index f63315ee81..788c4fb784 100644
--- a/src/corelib/ipc/qsystemsemaphore_p.h
+++ b/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
diff --git a/src/corelib/ipc/qtipccommon.cpp b/src/corelib/ipc/qtipccommon.cpp
index 190cb8c1fd..68176274ef 100644
--- a/src/corelib/ipc/qtipccommon.cpp
+++ b/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
-    the subset that the win/unix kernel allows.
-
-    On Unix this will be a file name
+    Returns a QNativeIpcKey that contains a platform-safe key using rules
+    similar to QtIpcCommon::platformSafeKey() below, but using an algorithm
+    that is compatible with Qt 4.4 to 6.6. Additionally, the returned
+    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))
-        k.setNativeKey(QStandardPaths::writableLocation(QStandardPaths::TempLocation) + u'/' + result);
+    if (isIpcSupported(ipcType, QNativeIpcKey::Type::SystemV)) {
+        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 0;
+    return (QNativeIpcKeyPrivate::legacyKey(lhs) == QNativeIpcKeyPrivate::legacyKey(rhs)) ? 0 : 1;
 }
 
 /*!
@@ -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;
 }
 
diff --git a/src/corelib/ipc/qtipccommon_p.h b/src/corelib/ipc/qtipccommon_p.h
index 3d6ad0903b..72762c5ba7 100644
--- a/src/corelib/ipc/qtipccommon_p.h
+++ b/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 {
diff --git a/tests/auto/corelib/ipc/qnativeipckey/tst_qnativeipckey.cpp b/tests/auto/corelib/ipc/qnativeipckey/tst_qnativeipckey.cpp
index 35e5e1172b..4653d8bbcd 100644
--- a/tests/auto/corelib/ipc/qnativeipckey/tst_qnativeipckey.cpp
+++ b/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"