Refactored qt_normalizePathSegments

There were several use cases that did not work with the old
implementation and it was not really readable.

Task-number: QTBUG-3472
Task-number: QTBUG-40067
Task-number: QTBUG-23892
Change-Id: I1e038792dc54cdc6f8d9bb59d80b11dd3c56fac6
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@digia.com>

src/corelib/io/qdir.cpp

@@ -2018,106 +2018,120 @@ bool QDir::match(const QString &filter, const QString &fileName)
     This method is shared with QUrl, so it doesn't deal with QDir::separator(),
     nor does it remove the trailing slash, if any.
 */
-QString qt_normalizePathSegments(const QString &name, bool allowUncPaths)
+Q_AUTOTEST_EXPORT QString qt_normalizePathSegments(const QString &name, bool allowUncPaths)
 {
-    int used = 0, levels = 0;
     const int len = name.length();
-    QVarLengthArray<QChar> outVector(len);
-    QChar *out = outVector.data();
 
+    if (len == 0)
+        return name;
+
+    int i = len - 1;
+    QVarLengthArray<QChar> outVector(len);
+    int used = len;
+    QChar *out = outVector.data();
     const QChar *p = name.unicode();
-    for (int i = 0, last = -1, iwrite = 0; i < len; ++i) {
+    const QChar *prefix = p;
-        if (p[i] == QLatin1Char('/')) {
+    int up = 0;
-            while (i+1 < len && p[i+1] == QLatin1Char('/')) {
+
-                if (allowUncPaths && i == 0)
+    int prefixLength = 0;
-                    break;
+
-                i++;
+    if (allowUncPaths && len >= 2 && p[1].unicode() == '/' && p[0].unicode() == '/') {
-            }
+        // starts with double slash
-            bool eaten = false;
+        prefixLength = 2;
-            if (i+1 < len && p[i+1] == QLatin1Char('.')) {
+#ifdef Q_OS_WIN
-                int dotcount = 1;
+    } else if (len >= 2 && p[1].unicode() == ':') {
-                if (i+2 < len && p[i+2] == QLatin1Char('.'))
+        // remember the drive letter
-                    dotcount++;
+        prefixLength = (len > 2 && p[2].unicode() == '/') ? 3 : 2;
-                if (i == len - dotcount - 1) {
-                    if (dotcount == 1) {
-                        break;
-                    } else if (levels) {
-                        if (last == -1) {
-                            for (int i2 = iwrite-1; i2 >= 0; i2--) {
-                                if (out[i2] == QLatin1Char('/')) {
-                                    last = i2;
-                                    break;
-                                }
-                            }
-                        }
-                        used -= iwrite - last - 1;
-                        break;
-                    }
-                } else if (p[i+dotcount+1] == QLatin1Char('/')) {
-                    if (dotcount == 2 && levels) {
-                        if (last == -1 || iwrite - last == 1) {
-                            for (int i2 = (last == -1) ? (iwrite-1) : (last-1); i2 >= 0; i2--) {
-                                if (out[i2] == QLatin1Char('/')) {
-                                    eaten = true;
-                                    last = i2;
-                                    break;
-                                }
-                            }
-                        } else {
-                            eaten = true;
-                        }
-                        if (eaten) {
-                            levels--;
-                            used -= iwrite - last;
-                            iwrite = last;
-                            last = -1;
-                        }
-                    } else if (dotcount == 2 && i > 0 && p[i - 1] != QLatin1Char('.')) {
-                        eaten = true;
-                        used -= iwrite - qMax(0, last);
-                        iwrite = qMax(0, last);
-                        last = -1;
-                        ++i;
-                    } else if (dotcount == 1) {
-                        eaten = true;
-                    }
-                    if (eaten)
-                        i += dotcount;
-                } else {
-                    levels++;
-                }
-            } else if (last != -1 && iwrite - last == 1) {
-#if defined(Q_OS_WIN)
-                eaten = (iwrite > 2);
-#else
-                eaten = true;
 #endif
-                last = -1;
+    } else if (p[0].unicode() == '/') {
-            } else {
+        prefixLength = 1;
-                levels++;
+    }
-            }
+    p += prefixLength;
-            if (!eaten)
+    i -= prefixLength;
-                last = i - (i - iwrite);
+
-            else
+    // replicate trailing slash (i > 0 checks for emptiness of input string p)
-                continue;
+    if (i > 0 && p[i].unicode() == '/') {
-        } else if (!i && p[i] == QLatin1Char('.')) {
+        out[--used].unicode() = '/';
-            int dotcount = 1;
+        --i;
-            if (len >= 1 && p[1] == QLatin1Char('.'))
-                dotcount++;
-            if (len >= dotcount && p[dotcount] == QLatin1Char('/')) {
-                if (dotcount == 1) {
-                    i++;
-                    while (i+1 < len-1 && p[i+1] == QLatin1Char('/'))
-                        i++;
-                    continue;
-                }
-            }
-        }
-        out[iwrite++] = p[i];
-        used++;
     }
 
-    QString ret = (used == len ? name : QString(out, used));
+    while (i >= 0) {
+        // remove trailing slashes
+        if (p[i].unicode() == '/') {
+            --i;
+            continue;
+        }
+
+        // remove current directory
+        if (p[i].unicode() == '.' && (i == 0 || p[i-1].unicode() == '/')) {
+            --i;
+            continue;
+        }
+
+        // detect up dir
+        if (i >= 1 && p[i].unicode() == '.' && p[i-1].unicode() == '.'
+                && (i == 1 || (i >= 2 && p[i-2].unicode() == '/'))) {
+            ++up;
+            i -= 2;
+            continue;
+        }
+
+        // prepend a slash before copying when not empty
+        if (!up && used != len && out[used].unicode() != '/')
+            out[--used] = QLatin1Char('/');
+
+        // skip or copy
+        while (i >= 0) {
+            if (p[i].unicode() == '/') { // do not copy slashes
+                --i;
+                break;
+            }
+
+            // actual copy
+            if (!up)
+                out[--used] = p[i];
+            --i;
+        }
+
+        // decrement up after copying/skipping
+        if (up)
+            --up;
+    }
+
+    // add remaining '..'
+    while (up) {
+        if (used != len && out[used].unicode() != '/') // is not empty and there isn't already a '/'
+            out[--used] = QLatin1Char('/');
+        out[--used] = QLatin1Char('.');
+        out[--used] = QLatin1Char('.');
+        --up;
+    }
+
+    bool isEmpty = used == len;
+
+    if (prefixLength) {
+        if (!isEmpty && out[used].unicode() == '/') {
+            // Eventhough 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] = QLatin1Char('.');
+        } else if (out[used].unicode() == '/') {
+            // 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] = QLatin1Char('.');
+        }
+    }
+
+    // If path was not modified return the original value
+    QString ret = (used == 0 ? name : QString(out + used, len - used));
     return ret;
 }
 

tests/auto/corelib/io/qdir/tst_qdir.cpp

@@ -60,11 +60,20 @@
 #define Q_NO_SYMLINKS
 #endif
 
+#ifdef QT_BUILD_INTERNAL
+
+QT_BEGIN_NAMESPACE
+extern Q_AUTOTEST_EXPORT QString qt_normalizePathSegments(const QString &, bool);
+QT_END_NAMESPACE
+
+#endif
+
 class tst_QDir : public QObject
 {
 Q_OBJECT
 
 public:
+    enum UncHandling { HandleUnc, IgnoreUnc };
     tst_QDir();
 
 private slots:
@@ -120,6 +129,11 @@ private slots:
     void cleanPath_data();
     void cleanPath();
 
+#ifdef QT_BUILD_INTERNAL
+    void normalizePathSegments_data();
+    void normalizePathSegments();
+#endif
+
     void compare();
     void QDir_default();
 
@@ -197,6 +211,8 @@ private:
     const QString m_dataPath;
 };
 
+Q_DECLARE_METATYPE(tst_QDir::UncHandling)
+
 tst_QDir::tst_QDir()
     : m_dataPath(QFileInfo(QFINDTESTDATA("testData")).absolutePath())
 {
@@ -980,6 +996,10 @@ void tst_QDir::cd_data()
 
     int index = m_dataPath.lastIndexOf("/");
     QTest::newRow("cdUp") << m_dataPath << ".." << true << m_dataPath.left(index==0?1:index);
+    QTest::newRow("cdUp non existent (relative dir)") << "anonexistingDir" << ".."
+                                                      << true << m_dataPath;
+    QTest::newRow("cdUp non existent (absolute dir)") << m_dataPath + "/anonexistingDir" << ".."
+                                                      << true << m_dataPath;
     QTest::newRow("noChange") << m_dataPath << "." << true << m_dataPath;
 #if defined(Q_OS_WIN)  // on windows QDir::root() is usually c:/ but cd "/" will not force it to be root
     QTest::newRow("absolute") << m_dataPath << "/" << true << "/";
@@ -988,7 +1008,7 @@ void tst_QDir::cd_data()
 #endif
     QTest::newRow("non existant") << "." << "../anonexistingdir" << false << m_dataPath;
     QTest::newRow("self") << "." << (QString("../") + QFileInfo(m_dataPath).fileName()) << true << m_dataPath;
-    QTest::newRow("file") << "." << "qdir.pro" << false << "";
+    QTest::newRow("file") << "." << "qdir.pro" << false << m_dataPath;
 }
 
 void tst_QDir::cd()
@@ -1002,8 +1022,7 @@ void tst_QDir::cd()
     bool notUsed = d.exists(); // make sure we cache this before so we can see if 'cd' fails to flush this
     Q_UNUSED(notUsed);
     QCOMPARE(d.cd(cdDir), successExpected);
-    if (successExpected)
+    QCOMPARE(d.absolutePath(), newDir);
-        QCOMPARE(d.absolutePath(), newDir);
 }
 
 void tst_QDir::setNameFilters_data()
@@ -1061,7 +1080,7 @@ tst_QDir::cleanPath_data()
     QTest::newRow("data6") << "d:\\a\\bc\\def\\../../.." << "d:/";
 #else
     QTest::newRow("data5") << "d:\\a\\bc\\def\\.." << "d:\\a\\bc\\def\\..";
-    QTest::newRow("data6") << "d:\\a\\bc\\def\\../../.." << "d:\\a\\bc\\def\\../../..";
+    QTest::newRow("data6") << "d:\\a\\bc\\def\\../../.." << "..";
 #endif
 #endif
     QTest::newRow("data7") << ".//file1.txt" << "file1.txt";
@@ -1074,6 +1093,30 @@ tst_QDir::cleanPath_data()
     QTest::newRow("data10") << "/:/" << "/:";
 #endif
 #endif
+#ifdef Q_OS_WIN
+    QTest::newRow("data11") << "//foo//bar" << "//foo/bar";
+#endif
+    QTest::newRow("data12") << "ab/a/" << "ab/a"; // Path item with length of 2
+#ifdef Q_OS_WIN
+    QTest::newRow("data13") << "c://" << "c:/";
+#else
+    QTest::newRow("data13") << "c://" << "c:";
+#endif
+
+    QTest::newRow("data14") << "c://foo" << "c:/foo";
+    // Drive letters and unc path in one string
+#ifdef Q_OS_WIN
+    QTest::newRow("data15") << "//c:/foo" << "//c:/foo";
+#else
+    QTest::newRow("data15") << "//c:/foo" << "/c:/foo";
+#endif
+
+    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";
@@ -1089,6 +1132,91 @@ tst_QDir::cleanPath()
     QCOMPARE(cleaned, expected);
 }
 
+#ifdef QT_BUILD_INTERNAL
+void tst_QDir::normalizePathSegments_data()
+{
+    QTest::addColumn<QString>("path");
+    QTest::addColumn<UncHandling>("uncHandling");
+    QTest::addColumn<QString>("expected");
+
+    QTest::newRow("data0") << "/Users/sam/troll/qt4.0//.." << HandleUnc << "/Users/sam/troll";
+    QTest::newRow("data1") << "/Users/sam////troll/qt4.0//.." << HandleUnc << "/Users/sam/troll";
+    QTest::newRow("data2") << "/" << HandleUnc << "/";
+    QTest::newRow("data3") << "//" << HandleUnc << "//";
+    QTest::newRow("data4") << "//" << IgnoreUnc << "/";
+    QTest::newRow("data5") << "/." << HandleUnc << "/";
+    QTest::newRow("data6") << "/./" << HandleUnc << "/";
+    QTest::newRow("data7") << "/.." << HandleUnc << "/..";
+    QTest::newRow("data8") << "/../" << HandleUnc << "/../";
+    QTest::newRow("data9") << "." << HandleUnc << ".";
+    QTest::newRow("data10") << "./" << HandleUnc << "./";
+    QTest::newRow("data11") << "./." << HandleUnc << ".";
+    QTest::newRow("data12") << "././" << HandleUnc << "./";
+    QTest::newRow("data13") << ".." << HandleUnc << "..";
+    QTest::newRow("data14") << "../" << HandleUnc << "../";
+    QTest::newRow("data15") << "../." << HandleUnc << "..";
+    QTest::newRow("data16") << ".././" << HandleUnc << "../";
+    QTest::newRow("data17") << "../.." << HandleUnc << "../..";
+    QTest::newRow("data18") << "../../" << HandleUnc << "../../";
+    QTest::newRow("data19") << ".//file1.txt" << HandleUnc << "file1.txt";
+    QTest::newRow("data20") << "/foo/bar/..//file1.txt" << HandleUnc << "/foo/file1.txt";
+    QTest::newRow("data21") << "foo/.." << HandleUnc << ".";
+    QTest::newRow("data22") << "./foo/.." << HandleUnc << ".";
+    QTest::newRow("data23") << ".foo/.." << HandleUnc << ".";
+    QTest::newRow("data24") << "foo/bar/../.." << HandleUnc << ".";
+    QTest::newRow("data25") << "./foo/bar/../.." << HandleUnc << ".";
+    QTest::newRow("data26") << "../foo/bar" << HandleUnc << "../foo/bar";
+    QTest::newRow("data27") << "./../foo/bar" << HandleUnc << "../foo/bar";
+    QTest::newRow("data28") << "../../foo/../bar" << HandleUnc << "../../bar";
+    QTest::newRow("data29") << "./foo/bar/.././.." << HandleUnc << ".";
+    QTest::newRow("data30") << "/./foo" << HandleUnc << "/foo";
+    QTest::newRow("data31") << "/../foo/" << HandleUnc << "/../foo/";
+    QTest::newRow("data32") << "c:/" << HandleUnc << "c:/";
+    QTest::newRow("data33") << "c://" << HandleUnc << "c:/";
+    QTest::newRow("data34") << "c://foo" << HandleUnc << "c:/foo";
+    QTest::newRow("data35") << "c:" << HandleUnc << "c:";
+    QTest::newRow("data36") << "c:foo/bar" << IgnoreUnc << "c:foo/bar";
+#if defined Q_OS_WIN
+    QTest::newRow("data37") << "c:/." << HandleUnc << "c:/";
+    QTest::newRow("data38") << "c:/.." << HandleUnc << "c:/..";
+    QTest::newRow("data39") << "c:/../" << HandleUnc << "c:/../";
+#else
+    QTest::newRow("data37") << "c:/." << HandleUnc << "c:";
+    QTest::newRow("data38") << "c:/.." << HandleUnc << ".";
+    QTest::newRow("data39") << "c:/../" << HandleUnc << "./";
+#endif
+    QTest::newRow("data40") << "c:/./" << HandleUnc << "c:/";
+    QTest::newRow("data41") << "foo/../foo/.." << HandleUnc << ".";
+    QTest::newRow("data42") << "foo/../foo/../.." << HandleUnc << "..";
+    QTest::newRow("data43") << "..foo.bar/foo" << HandleUnc << "..foo.bar/foo";
+    QTest::newRow("data44") << ".foo./bar/.." << HandleUnc << ".foo.";
+    QTest::newRow("data45") << "foo/..bar.." << HandleUnc << "foo/..bar..";
+    QTest::newRow("data46") << "foo/.bar./.." << HandleUnc << "foo";
+    QTest::newRow("data47") << "//foo//bar" << HandleUnc << "//foo/bar";
+    QTest::newRow("data48") << "..." << HandleUnc << "...";
+    QTest::newRow("data49") << "foo/.../bar" << HandleUnc << "foo/.../bar";
+    QTest::newRow("data50") << "ab/a/" << HandleUnc << "ab/a/"; // Path item with length of 2
+    // Drive letters and unc path in one string. The drive letter isn't handled as a drive letter
+    // but as a host name in this case (even though Windows host names can't contain a ':')
+    QTest::newRow("data51") << "//c:/foo" << HandleUnc << "//c:/foo";
+    QTest::newRow("data52") << "//c:/foo" << IgnoreUnc << "/c:/foo";
+
+    QTest::newRow("resource0") << ":/prefix/foo.bar" << HandleUnc << ":/prefix/foo.bar";
+    QTest::newRow("resource1") << "://prefix/..//prefix/foo.bar" << HandleUnc << ":/prefix/foo.bar";
+}
+
+void tst_QDir::normalizePathSegments()
+{
+    QFETCH(QString, path);
+    QFETCH(UncHandling, uncHandling);
+    QFETCH(QString, expected);
+    QString cleaned = qt_normalizePathSegments(path, uncHandling == HandleUnc);
+    QCOMPARE(cleaned, expected);
+    if (path == expected)
+        QVERIFY2(path.isSharedWith(cleaned), "Strings are same but data is not shared");
+}
+# endif //QT_BUILD_INTERNAL
+
 void tst_QDir::absoluteFilePath_data()
 {
     QTest::addColumn<QString>("path");