Utils: Make path cleaning platform agnostic

Unfortunately, QDir::cleanPath() only cleans according to the rules
of the host system, which can be wrong in remote setups.

As the implementation is not accessible/tweakable from the outside,
copy the relevant code and remove the platform #ifdef's.

Change-Id: Ife9a925412a12d3cef21ed3721a387c61c152ddf
Reviewed-by: Christian Stenger <christian.stenger@qt.io>

src/libs/utils/filepath.cpp

@@ -1660,6 +1660,15 @@ FilePath FilePath::resolvePath(const QString &tail) const
    return resolvePath(tailPath);
 }
 
+// Cleans path part similar to QDir::cleanPath()
+//  - directory separators normalized (that is, platform-native
+//    separators converted to "/") and redundant ones removed, and "."s and ".."s
+//    resolved (as far as possible).
+//   Symbolic links are kept. This function does not return the
+//    canonical path, but rather the simplest version of the input.
+//    For example, "./local" becomes "local", "local/../bin" becomes
+//    "bin" and "/local/usr/../bin" becomes "/local/bin".
+
 FilePath FilePath::cleanPath() const
 {
     return withNewPath(doCleanPath(path()));
@@ -1670,6 +1679,150 @@ QTextStream &operator<<(QTextStream &s, const FilePath &fn)
     return s << fn.toString();
 }
 
+static QString normalizePathSegmentHelper(const QString &name)
+{
+    const int len = name.length();
+
+    if (len == 0)
+        return name;
+
+    int i = len - 1;
+    QVarLengthArray<char16_t> outVector(len);
+    int used = len;
+    char16_t *out = outVector.data();
+    const ushort *p = reinterpret_cast<const ushort *>(name.data());
+    const ushort *prefix = p;
+    int up = 0;
+
+    const int prefixLength =  name.at(0) == u'/' ? 1 : 0;
+
+    p += prefixLength;
+    i -= prefixLength;
+
+    // replicate trailing slash (i > 0 checks for emptiness of input string p)
+    // except for remote paths because there can be /../ or /./ ending
+    if (i > 0 && p[i] == '/') {
+        out[--used] = '/';
+        --i;
+    }
+
+    while (i >= 0) {
+        if (p[i] == '/') {
+            --i;
+            continue;
+        }
+
+        // remove current directory
+        if (p[i] == '.' && (i == 0 || p[i-1] == '/')) {
+            --i;
+            continue;
+        }
+
+        // detect up dir
+        if (i >= 1 && p[i] == '.' && p[i-1] == '.' && (i < 2 || p[i - 2] == '/')) {
+            ++up;
+            i -= i >= 2 ? 3 : 2;
+            continue;
+        }
+
+        // prepend a slash before copying when not empty
+        if (!up && used != len && out[used] != '/')
+            out[--used] = '/';
+
+        // skip or copy
+        while (i >= 0) {
+            if (p[i] == '/') {
+                --i;
+                break;
+            }
+
+            // actual copy
+            if (!up)
+                out[--used] = p[i];
+            --i;
+        }
+
+        // decrement up after copying/skipping
+        if (up)
+            --up;
+    }
+
+    // Indicate failure when ".." are left over for an absolute path.
+//    if (ok)
+//        *ok = prefixLength == 0 || up == 0;
+
+    // add remaining '..'
+    while (up) {
+        if (used != len && out[used] != '/') // is not empty and there isn't already a '/'
+            out[--used] = '/';
+        out[--used] = '.';
+        out[--used] = '.';
+        --up;
+    }
+
+    bool isEmpty = used == len;
+
+    if (prefixLength) {
+        if (!isEmpty && out[used] == '/') {
+            // Even though there is a prefix the out string is a slash. This happens, if the input
+            // string only consists of a prefix followed by one or more slashes. Just skip the slash.
+            ++used;
+        }
+        for (int i = prefixLength - 1; i >= 0; --i)
+            out[--used] = prefix[i];
+    } else {
+        if (isEmpty) {
+            // After resolving the input path, the resulting string is empty (e.g. "foo/.."). Return
+            // a dot in that case.
+            out[--used] = '.';
+        } else if (out[used] == '/') {
+            // After parsing the input string, out only contains a slash. That happens whenever all
+            // parts are resolved and there is a trailing slash ("./" or "foo/../" for example).
+            // Prepend a dot to have the correct return value.
+            out[--used] = '.';
+        }
+    }
+
+    // If path was not modified return the original value
+    if (used == 0)
+        return name;
+    return QString::fromUtf16(out + used, len - used);
+}
+
+QString doCleanPath(const QString &input_)
+{
+    QString input = input_;
+    if (input.contains('\\'))
+        input.replace('\\', '/');
+
+    if (input.startsWith("//?/")) {
+        input = input.mid(4);
+        if (input.startsWith("UNC/"))
+            input = '/' + input.mid(3);  // trick it into reporting two slashs at start
+    }
+
+    int prefixLen = 0;
+    const int shLen = FilePath::schemeAndHostLength(input);
+    if (shLen > 0) {
+        prefixLen = shLen + FilePath::rootLength(input.mid(shLen));
+    } else {
+        prefixLen = FilePath::rootLength(input);
+        if (prefixLen > 0 && input.at(prefixLen - 1) == '/')
+            --prefixLen;
+    }
+
+    QString path = normalizePathSegmentHelper(input.mid(prefixLen));
+
+    // Strip away last slash except for root directories
+    if (path.size() > 1 && path.endsWith(u'/'))
+        path.chop(1);
+
+    return input.left(prefixLen) + path;
+}
+
+
+// FileFilter
+
 FileFilter::FileFilter(const QStringList &nameFilters,
                        const QDir::Filters fileFilters,
                        const QDirIterator::IteratorFlags flags)

src/libs/utils/filepath.h

@@ -205,7 +205,10 @@ public:
 
     [[nodiscard]] bool ensureReachable(const FilePath &other) const;
 
-    QString toFSPathString() const;
+    [[nodiscard]] QString toFSPathString() const;
+
+    [[nodiscard]] static int rootLength(const QStringView path); // Assumes no scheme and host
+    [[nodiscard]] static int schemeAndHostLength(const QStringView path);
 
 private:
     friend class ::tst_fileutils;
@@ -213,9 +216,6 @@ private:
     void setPath(QStringView path);
     void setFromString(const QString &filepath);
 
-    static int rootLength(const QStringView path);
-    static int schemeAndHostLength(const QStringView path);
-
     [[nodiscard]] QString mapToDevicePath() const;
     [[nodiscard]] QString encodedHost() const;
 

src/libs/utils/fileutils.cpp

@@ -763,25 +763,6 @@ bool FileUtils::isRelativePath(const QString &path)
     return isRelativePathHelper(path, HostOsInfo::hostOs());
 }
 
-// Cleans part after optional :// scheme separator, similar to QDir::cleanPath()
-//  - directory separators normalized (that is, platform-native
-//    separators converted to "/") and redundant ones removed, and "."s and ".."s
-//    resolved (as far as possible).
-//   Symbolic links are kept. This function does not return the
-//    canonical path, but rather the simplest version of the input.
-//    For example, "./local" becomes "local", "local/../bin" becomes
-//    "bin" and "/local/usr/../bin" becomes "/local/bin".
-
-// FIXME: This should not use the host-platform dependent QDir::cleanPath()
-
-QString doCleanPath(const QString &input)
-{
-    const int pos = input.indexOf("://");
-    if (pos == -1)
-        return QDir::cleanPath(input);
-    return input.left(pos + 3) + QDir::cleanPath(input.mid(pos + 3));
-}
-
 FilePath FileUtils::commonPath(const FilePath &oldCommonPath, const FilePath &filePath)
 {
     FilePath newCommonPath = oldCommonPath;

src/libs/utils/fileutils.h

@@ -268,7 +268,10 @@ private:
 QTCREATOR_UTILS_EXPORT QTextStream &operator<<(QTextStream &s, const FilePath &fn);
 
 bool isRelativePathHelper(const QString &path, OsType osType);
-QString doCleanPath(const QString &input);
+
+// For testing
+QTCREATOR_UTILS_EXPORT QString doCleanPath(const QString &input);
+QTCREATOR_UTILS_EXPORT QString cleanPathHelper(const QString &path);
 
 } // namespace Utils
 

tests/auto/utils/fileutils/tst_fileutils.cpp

@@ -29,37 +29,53 @@ signals:
 
 private slots:
     void initTestCase();
+
     void parentDir_data();
     void parentDir();
+
     void isChildOf_data();
     void isChildOf();
+
     void fileName_data();
     void fileName();
+
     void calcRelativePath_data();
     void calcRelativePath();
+
     void relativePath_specials();
     void relativePath_data();
     void relativePath();
+
     void fromToString_data();
     void fromToString();
+
     void fromString_data();
     void fromString();
+
     void toString_data();
     void toString();
+
     void toFSPathString_data();
     void toFSPathString();
+
     void comparison_data();
     void comparison();
+
     void linkFromString_data();
     void linkFromString();
+
     void pathAppended_data();
     void pathAppended();
+
     void commonPath_data();
     void commonPath();
+
     void resolvePath_data();
     void resolvePath();
+
     void relativeChildPath_data();
     void relativeChildPath();
+
     void bytesAvailableFromDF_data();
     void bytesAvailableFromDF();
     void rootLength_data();
@@ -70,13 +86,18 @@ private slots:
     void asyncLocalCopy();
     void startsWithDriveLetter();
     void startsWithDriveLetter_data();
-    void onDevice();
+
     void onDevice_data();
+    void onDevice();
+
     void plus();
     void plus_data();
     void url();
     void url_data();
 
+    void cleanPath_data();
+    void cleanPath();
+
 private:
     QTemporaryDir tempDir;
     QString rootPath;
@@ -127,18 +148,16 @@ void tst_fileutils::parentDir_data()
     QTest::newRow("relativepath") << "relativepath" << "." << "";
 
     // Windows stuff:
-#ifdef Q_OS_WIN
     QTest::newRow("C:/data") << "C:/data" << "C:/" << "";
     QTest::newRow("C:/") << "C:/" << "" << "";
-    QTest::newRow("//./com1") << "//./com1" << "//." << "";
+    QTest::newRow("//./com1") << "//./com1" << "//./" << "";
     QTest::newRow("//?/path") << "//?/path" << "/" << "Qt 4 cannot handle this path.";
     QTest::newRow("/Global?\?/UNC/host") << "/Global?\?/UNC/host" << "/Global?\?/UNC/host"
                                         << "Qt 4 cannot handle this path.";
     QTest::newRow("//server/directory/file")
             << "//server/directory/file" << "//server/directory" << "";
-    QTest::newRow("//server/directory") << "//server/directory" << "//server" << "";
+    QTest::newRow("//server/directory") << "//server/directory" << "//server/" << "";
     QTest::newRow("//server") << "//server" << "" << "";
-#endif
 }
 
 void tst_fileutils::parentDir()
@@ -974,5 +993,74 @@ void tst_fileutils::bytesAvailableFromDF()
     QCOMPARE(result, expected);
 }
 
+void tst_fileutils::cleanPath_data()
+{
+    QTest::addColumn<QString>("path");
+    QTest::addColumn<QString>("expected");
+
+    QTest::newRow("data0") << "/Users/sam/troll/qt4.0//.." << "/Users/sam/troll";
+    QTest::newRow("data1") << "/Users/sam////troll/qt4.0//.." << "/Users/sam/troll";
+    QTest::newRow("data2") << "/" << "/";
+    QTest::newRow("data2-up") << "/path/.." << "/";
+    QTest::newRow("data2-above-root") << "/.." << "/..";
+    QTest::newRow("data3") << QDir::cleanPath("../.") << "..";
+    QTest::newRow("data4") << QDir::cleanPath("../..") << "../..";
+    QTest::newRow("data5") << "d:\\a\\bc\\def\\.." << "d:/a/bc"; // QDir/Linux had:  "d:\\a\\bc\\def\\.."
+    QTest::newRow("data6") << "d:\\a\\bc\\def\\../../.." << "d:/"; // QDir/Linux had: ".."
+    QTest::newRow("data7") << ".//file1.txt" << "file1.txt";
+    QTest::newRow("data8") << "/foo/bar/..//file1.txt" << "/foo/file1.txt";
+    QTest::newRow("data9") << "//" << "//";  // QDir had: "/"
+    QTest::newRow("data10w") << "c:\\" << "c:/";
+    QTest::newRow("data10l") << "/:/" << "/:";
+    QTest::newRow("data11") << "//foo//bar" << "//foo/bar";  // QDir/Win had: "//foo/bar"
+    QTest::newRow("data12") << "ab/a/" << "ab/a"; // Path item with length of 2
+    QTest::newRow("data13w") << "c:/" << "c:/";
+    QTest::newRow("data13w2") << "c:\\" << "c:/";
+    //QTest::newRow("data13l") << "c://" << "c:";
+
+//   QTest::newRow("data14") << "c://foo" << "c:/foo";
+    QTest::newRow("data15") << "//c:/foo" << "//c:/foo"; // QDir/Lin had: "/c:/foo";
+    QTest::newRow("drive-up") << "A:/path/.." << "A:/";
+    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-slash") << "//?/d:/" << "d:/";
+    QTest::newRow("longpath-mixed-slashes") << "//?/d:\\" << "d:/";
+    QTest::newRow("longpath-mixed-slashes-2") << "\\\\?\\d:/" << "d:/";
+
+    QTest::newRow("unc-network-share") << "\\\\?\\UNC\\localhost\\c$\\tmp.txt"
+        << "//localhost/c$/tmp.txt";
+    QTest::newRow("unc-network-share-slash") << "//?/UNC/localhost/c$/tmp.txt"
+        << "//localhost/c$/tmp.txt";
+    QTest::newRow("unc-network-share-mixed-slashes") << "//?/UNC/localhost\\c$\\tmp.txt"
+        << "//localhost/c$/tmp.txt";
+    QTest::newRow("unc-network-share-mixed-slashes-2") << "\\\\?\\UNC\\localhost/c$/tmp.txt"
+        << "//localhost/c$/tmp.txt";
+
+    QTest::newRow("QTBUG-23892_0") << "foo/.." << ".";
+    QTest::newRow("QTBUG-23892_1") << "foo/../" << ".";
+
+    QTest::newRow("QTBUG-3472_0") << "/foo/./bar" << "/foo/bar";
+    QTest::newRow("QTBUG-3472_1") << "./foo/.." << ".";
+    QTest::newRow("QTBUG-3472_2") << "./foo/../" << ".";
+
+    QTest::newRow("resource0") << ":/prefix/foo.bar" << ":/prefix/foo.bar";
+    QTest::newRow("resource1") << ":/prefix/..//prefix/foo.bar" << ":/prefix/foo.bar";
+
+    QTest::newRow("ssh") << "ssh://host/prefix/../foo.bar" << "ssh://host/foo.bar";
+    QTest::newRow("ssh2") << "ssh://host/../foo.bar" << "ssh://host/../foo.bar";
+}
+
+void tst_fileutils::cleanPath()
+{
+    QFETCH(QString, path);
+    QFETCH(QString, expected);
+    QString cleaned = doCleanPath(path);
+    QCOMPARE(cleaned, expected);
+}
+
 QTEST_GUILESS_MAIN(tst_fileutils)
+
 #include "tst_fileutils.moc"