Unify behavior for long path or UNC prefix removal

Split the code out of QDir::fromNativeSeparator into a separate reusable function to remove the above-mentioned prefixes. Fixes and unifies behavior if the prefix was given with slashes instead of backslashes. Add a couple more test cases. Fixes: QTBUG-93868 Pick-to: 5.15 6.0 6.1 Change-Id: Ibd94ae283e2fb113f9c2db97475fbc7d89522bbf Reviewed-by: Edward Welbourne <[email protected]> Reviewed-by: Oliver Wolff <[email protected]>
zoldalis · Jun 4, 2021 · 0564ebd · 0564ebd
1 parent a1d9673
commit 0564ebd
Show file tree

Hide file tree

Showing 9 changed files with 102 additions and 47 deletions.
diff --git a/src/corelib/io/qdir.cpp b/src/corelib/io/qdir.cpp
@@ -953,35 +953,10 @@ QString QDir::toNativeSeparators(const QString &pathName)
 QString QDir::fromNativeSeparators(const QString &pathName)
 {
 #if defined(Q_OS_WIN)
-    const QChar nativeSeparator = u'\\';
-    int i = pathName.indexOf(nativeSeparator);
-    if (i != -1) {
-        QString n(pathName);
-        const QStringView uncPrefix(uR"(\\?\UNC\)");
-        const QStringView extendedLengthPathPrefix(uR"(\\?\)");
-        if (n.startsWith(uncPrefix)) {
-            // Keep the initial double-slash, chop out the rest of the prefix.
-            n = n.remove(2, uncPrefix.size() - 2);
-            if ((i = n.indexOf(nativeSeparator)) == -1)
-                return n;
-        } else if (n.startsWith(extendedLengthPathPrefix)) {
-            n = n.sliced(extendedLengthPathPrefix.size());
-            if ((i = n.indexOf(nativeSeparator)) == -1)
-                return n;
-        }
-
-        QChar * const data = n.data();
-        data[i++] = u'/';
-
-        for (; i < n.length(); ++i) {
-            if (data[i] == nativeSeparator)
-                data[i] = u'/';
-        }
-
-        return n;
-    }
-#endif
+    return QFileSystemEntry::removeUncOrLongPathPrefix(pathName).replace(u'\\', u'/');
+#else
     return pathName;
+#endif
 }
 
 static QString qt_cleanPath(const QString &path, bool *ok = nullptr);

diff --git a/src/corelib/io/qfilesystemengine_win.cpp b/src/corelib/io/qfilesystemengine_win.cpp
@@ -297,18 +297,8 @@ static QString readSymLink(const QFileSystemEntry &link)
                 const wchar_t* PathBuffer = &rdb->SymbolicLinkReparseBuffer.PathBuffer[offset];
                 result = QString::fromWCharArray(PathBuffer, length);
             }
-            // cut-off "\\?\" and "\??\"
-            if (result.size() > 4
-                    && result.at(0) == QLatin1Char('\\')
-                    && result.at(2) == QLatin1Char('?')
-                    && result.at(3) == QLatin1Char('\\')) {
-                result = result.mid(4);
-                // cut off UNC in addition when the link points at a UNC share
-                // in which case we need to prepend another backslash to get \\server\share
-                if (QStringView{result}.left(3) == QLatin1String("UNC")) {
-                    result.replace(0, 3, QLatin1Char('\\'));
-                }
-            }
+            // remove "\\?\", "\??\" or "\\?\UNC\"
+            result = QFileSystemEntry::removeUncOrLongPathPrefix(result);
         }
         free(rdb);
         CloseHandle(handle);

diff --git a/src/corelib/io/qfilesystementry.cpp b/src/corelib/io/qfilesystementry.cpp
@@ -288,6 +288,34 @@ bool QFileSystemEntry::isDriveRootPath(const QString &path)
            && path.at(0).isLetter() && path.at(1) == QLatin1Char(':')
            && path.at(2) == QLatin1Char('/'));
 }
+
+QString QFileSystemEntry::removeUncOrLongPathPrefix(QString path)
+{
+    constexpr qsizetype minPrefixSize = 4;
+    if (path.size() < minPrefixSize)
+        return path;
+
+    auto data = path.data();
+    const auto slash = path[0];
+    if (slash != u'\\' && slash != u'/')
+        return path;
+
+    // check for "//?/" or "/??/"
+    if (data[2] == u'?' && data[3] == slash && (data[1] == slash || data[1] == u'?')) {
+        path = path.sliced(minPrefixSize);
+
+        // check for a possible "UNC/" prefix left-over
+        if (path.size() >= 4) {
+            data = path.data();
+            if (data[0] == u'U' && data[1] == u'N' && data[2] == u'C' && data[3] == slash) {
+                data[2] = slash;
+                return path.sliced(2);
+            }
+        }
+    }
+
+    return path;
+}
 #endif // Q_OS_WIN
 
 bool QFileSystemEntry::isRootPath(const QString &path)

diff --git a/src/corelib/io/qfilesystementry_p.h b/src/corelib/io/qfilesystementry_p.h
@@ -91,6 +91,7 @@ class QFileSystemEntry
 #if defined(Q_OS_WIN)
     bool isDriveRoot() const;
     static bool isDriveRootPath(const QString &path);
+    static QString removeUncOrLongPathPrefix(QString path);
 #endif
     bool isRoot() const;
 

diff --git a/tests/auto/corelib/io/qdir/tst_qdir.cpp b/tests/auto/corelib/io/qdir/tst_qdir.cpp
@@ -1268,9 +1268,20 @@ tst_QDir::cleanPath_data()
     QTest::newRow("drive-above-root") << "A:/.." << "A:/..";
     QTest::newRow("unc-server-up") << "//server/path/.." << "//server";
     QTest::newRow("unc-server-above-root") << "//server/.." << "//server/..";
-    QTest::newRow("longpath") << "\\\\?\\d:\\" << "d:/";
+
+    QTest::newRow("longpath") << uR"(\\?\d:\)"_qs << u"d:/"_qs;
+    QTest::newRow("longpath-slash") << u"//?/d:/"_qs << u"d:/"_qs;
+    QTest::newRow("longpath-mixed-slashes") << uR"(//?/d:\)"_qs << u"d:/"_qs;
+    QTest::newRow("longpath-mixed-slashes-2") << uR"(\\?\d:/)"_qs << u"d:/"_qs;
+
     QTest::newRow("unc-network-share") << uR"(\\?\UNC\localhost\c$\tmp.txt)"_qs
         << u"//localhost/c$/tmp.txt"_qs;
+    QTest::newRow("unc-network-share-slash") << u"//?/UNC/localhost/c$/tmp.txt"_qs
+        << u"//localhost/c$/tmp.txt"_qs;
+    QTest::newRow("unc-network-share-mixed-slashes") << uR"(//?/UNC/localhost\c$\tmp.txt)"_qs
+        << u"//localhost/c$/tmp.txt"_qs;
+    QTest::newRow("unc-network-share-mixed-slashes-2") << uR"(\\?\UNC\localhost/c$/tmp.txt)"_qs
+        << u"//localhost/c$/tmp.txt"_qs;
 #else
     QTest::newRow("data15") << "//c:/foo" << "/c:/foo";
 #endif // non-windows
@@ -1748,6 +1759,8 @@ void tst_QDir::nativeSeparators()
     QCOMPARE(QDir::fromNativeSeparators(QLatin1String("\\\\?\\C:\\")), QString("C:/"));
     QCOMPARE(QDir::fromNativeSeparators(uR"(\\?\UNC\localhost\c$\tmp.txt)"_qs),
              u"//localhost/c$/tmp.txt"_qs);
+    QCOMPARE(QDir::fromNativeSeparators(uR"(//?/UNC/localhost\c$\tmp.txt)"_qs),
+             u"//localhost/c$/tmp.txt"_qs);
 #else
     QCOMPARE(QDir::toNativeSeparators(QLatin1String("/")), QString("/"));
     QCOMPARE(QDir::toNativeSeparators(QLatin1String("\\")), QString("\\"));

diff --git a/tests/auto/corelib/io/qtemporarydir/CMakeLists.txt b/tests/auto/corelib/io/qtemporarydir/CMakeLists.txt
@@ -10,3 +10,8 @@ qt_internal_add_test(tst_qtemporarydir
     PUBLIC_LIBRARIES
         Qt::TestPrivate
 )
+
+qt_internal_extend_target(tst_qtemporarydir CONDITION WIN32
+    LIBRARIES
+        shlwapi
+)
diff --git a/tests/auto/corelib/io/qtemporarydir/tst_qtemporarydir.cpp b/tests/auto/corelib/io/qtemporarydir/tst_qtemporarydir.cpp
@@ -37,6 +37,7 @@
 #include <qset.h>
 #include <QtTest/private/qtesthelpers_p.h>
 #ifdef Q_OS_WIN
+# include <shlwapi.h>
 # include <windows.h>
 #endif
 #ifdef Q_OS_UNIX // for geteuid()
@@ -159,9 +160,25 @@ void tst_QTemporaryDir::fileTemplate_data()
     }
 
 #ifdef Q_OS_WIN
-    const auto tmp = QDir::toNativeSeparators(QDir::tempPath()).sliced(QDir::rootPath().size());
-    QTest::newRow("UNC") << uR"(\\localhost\C$\)"_qs + tmp + uR"(\UNC.XXXXXX.tmpDir)"_qs
-                         << "UNC." << ".tmpDir";
+    auto tmp = QDir::toNativeSeparators(QDir::tempPath());
+    if (PathGetDriveNumber((const wchar_t *) tmp.utf16()) < 0)
+        return; // skip if we have no drive letter
+
+    tmp.data()[1] = u'$';
+    const auto tmpPath = tmp + uR"(\UNC.XXXXXX.tmpDir)"_qs;
+
+    QTest::newRow("UNC-backslash")
+            << uR"(\\localhost\)"_qs + tmpPath << "UNC."
+            << ".tmpDir";
+    QTest::newRow("UNC-prefix")
+            << uR"(\\?\UNC\localhost\)"_qs + tmpPath << "UNC."
+            << ".tmpDir";
+    QTest::newRow("UNC-slash")
+            << u"//localhost/"_qs + QDir::fromNativeSeparators(tmpPath) << "UNC."
+            << ".tmpDir";
+    QTest::newRow("UNC-prefix-slash")
+            << uR"(//?/UNC/localhost/)"_qs + QDir::fromNativeSeparators(tmpPath) << "UNC."
+            << ".tmpDir";
 #endif
 }
 

diff --git a/tests/auto/corelib/io/qtemporaryfile/CMakeLists.txt b/tests/auto/corelib/io/qtemporaryfile/CMakeLists.txt
@@ -45,3 +45,8 @@ if(ANDROID AND NOT ANDROID_EMBEDDED)
             ${android_testdata_resource_files}
     )
 endif()
+
+qt_internal_extend_target(tst_qtemporaryfile CONDITION WIN32
+    LIBRARIES
+        shlwapi
+)
diff --git a/tests/auto/corelib/io/qtemporaryfile/tst_qtemporaryfile.cpp b/tests/auto/corelib/io/qtemporaryfile/tst_qtemporaryfile.cpp
@@ -40,6 +40,7 @@
 #include <QtTest/private/qtesthelpers_p.h>
 
 #if defined(Q_OS_WIN)
+# include <shlwapi.h>
 # include <windows.h>
 #endif
 #if defined(Q_OS_UNIX)
@@ -208,9 +209,29 @@ void tst_QTemporaryFile::fileTemplate_data()
     }
 
 #ifdef Q_OS_WIN
-    const auto tmp = QDir::toNativeSeparators(QDir::tempPath()).sliced(QDir::rootPath().size());
-    QTest::newRow("UNC") << uR"(\\localhost\C$\)"_qs + tmp + uR"(\QTBUG-74291.XXXXXX.tmpFile)"_qs
-                         << "QTBUG-74291." << ".tmpFile" << "";
+    auto tmp = QDir::toNativeSeparators(QDir::tempPath());
+    if (PathGetDriveNumber((const wchar_t *) tmp.utf16()) < 0)
+        return; // skip if we have no drive letter
+
+    tmp.data()[1] = u'$';
+    const auto tmpPath = tmp + uR"(\QTBUG-74291.XXXXXX.tmpFile)"_qs;
+
+    QTest::newRow("UNC-backslash")
+            << uR"(\\localhost\)"_qs + tmpPath << "QTBUG-74291."
+            << ".tmpFile"
+            << "";
+    QTest::newRow("UNC-prefix")
+            << uR"(\\?\UNC\localhost\)"_qs + tmpPath << "QTBUG-74291."
+            << ".tmpFile"
+            << "";
+    QTest::newRow("UNC-slash")
+            << u"//localhost/"_qs + QDir::fromNativeSeparators(tmpPath) << "QTBUG-74291."
+            << ".tmpFile"
+            << "";
+    QTest::newRow("UNC-prefix-slash")
+            << uR"(//?/UNC/localhost/)"_qs + QDir::fromNativeSeparators(tmpPath) << "QTBUG-74291."
+            << ".tmpFile"
+            << "";
 #endif
 }