Ensure that QUrl::{to,from}LocalPath encode/decode properly

Unlike path(), toLocalFile() isn't reporting a URL component, so it
should decode the percent-encoded characters fully. This extra
decoding pass is meant to catch %00 to %1F, %7F and %25 (the percent
sign itself).

It also catches %80 to %FF, which aren't decoded because they don't
form UTF-8 sequences. That means QUrl::toLocalFile() has undefined
behaviour if the path contained non-UTF8 sequences.

Task-number: QTBUG-25459
Change-Id: Iab5a0ba6afcfc4510e297984f2ffc208cedd752b
Reviewed-by: Shane Kearns <shane.kearns@accenture.com>

2 files changed, 37 insertions, 1 deletions

diff --git a/src/corelib/io/qurl.cpp b/src/corelib/io/qurl.cpp
index 44160b3d8b..aed1be2ffb 100644
--- a/src/corelib/io/qurl.cpp
+++ b/src/corelib/io/qurl.cpp
@@ -279,6 +279,12 @@ static inline char toHex(quint8 c)
     return c > 9 ? c - 10 + 'A' : c + '0';
 }
 
+static inline quint8 fromHex(quint8 c)
+{
+    c |= 0x20;
+    return c >= 'a' ? c - 'a' + 10 : c - '0';
+}
+
 static inline QString ftpScheme()
 {
     return QStringLiteral("ftp");
@@ -2495,6 +2501,9 @@ QUrl QUrl::fromLocalFile(const QString &localFile)
     returned value in the form found on SMB networks (for example,
     "//servername/path/to/file.txt").
 
+    Note: if the path component of this URL contains a non-UTF-8 binary
+    sequence (such as %80), the behaviour of this function is undefined.
+
     \sa fromLocalFile(), isLocalFile()
 */
 QString QUrl::toLocalFile() const
@@ -2519,6 +2528,20 @@ QString QUrl::toLocalFile() const
 #endif
     }
 
+    // check if we need to do one more decoding pass
+    int pct = tmp.indexOf(QLatin1Char('%'));
+    while (pct != -1) {
+        Q_ASSERT(tmp.size() >= pct + 2);
+        ushort char1 = tmp.at(pct + 1).unicode();
+        ushort char2 = tmp.at(pct + 2).unicode();
+
+        Q_ASSERT(isHex(char1) && char1 < 0x80u);
+        Q_ASSERT(isHex(char2) && char2 < 0x80u);
+        tmp.replace(pct, 3, QChar(fromHex(char1) << 4 | fromHex(char2)));
+
+        // next iteration
+        pct = tmp.indexOf(QLatin1Char('%'), pct + 1);
+    }
     return tmp;
 }
 
diff --git a/tests/auto/corelib/io/qurl/tst_qurl.cpp b/tests/auto/corelib/io/qurl/tst_qurl.cpp
index 31acd1aaa7..608bdfb78b 100644
--- a/tests/auto/corelib/io/qurl/tst_qurl.cpp
+++ b/tests/auto/corelib/io/qurl/tst_qurl.cpp
@@ -1028,6 +1028,11 @@ void tst_QUrl::toLocalFile_data()
     QTest::newRow("data9") << QString::fromLatin1("file:////somehost/somedir/somefile") << QString::fromLatin1("//somehost/somedir/somefile");
     QTest::newRow("data10") << QString::fromLatin1("FILE:/a.txt") << QString::fromLatin1("/a.txt");
     QTest::newRow("data11") << QString::fromLatin1("file:///Mambo <%235>.mp3") << QString::fromLatin1("/Mambo <#5>.mp3");
+    QTest::newRow("data12") << QString::fromLatin1("file:///a%25.txt") << QString::fromLatin1("/a%.txt");
+    QTest::newRow("data13") << QString::fromLatin1("file:///a%25%25.txt") << QString::fromLatin1("/a%%.txt");
+    QTest::newRow("data14") << QString::fromLatin1("file:///a%25a%25.txt") << QString::fromLatin1("/a%a%.txt");
+    QTest::newRow("data15") << QString::fromLatin1("file:///a%1f.txt") << QString::fromLatin1("/a\x1f.txt");
+    QTest::newRow("data16") << QString::fromLatin1("file:///%2580.txt") << QString::fromLatin1("/%80.txt");
 
     // and some that result in empty (i.e., not local)
     QTest::newRow("xdata0") << QString::fromLatin1("/a.txt") << QString();
@@ -1064,6 +1069,14 @@ void tst_QUrl::fromLocalFile_data()
                         << QString::fromLatin1("");
     QTest::newRow("data6") << QString::fromLatin1("//somehost/") << QString::fromLatin1("file://somehost/")
                         << QString::fromLatin1("/");
+    QTest::newRow("data7") << QString::fromLatin1("/Mambo <#5>.mp3") << QString::fromLatin1("file:///Mambo <%235>.mp3")
+                           << QString::fromLatin1("/Mambo <#5>.mp3");
+    QTest::newRow("data8") << QString::fromLatin1("/a%.txt") << QString::fromLatin1("file:///a%25.txt")
+                           << QString::fromLatin1("/a%25.txt");
+    QTest::newRow("data9") << QString::fromLatin1("/a%25.txt") << QString::fromLatin1("file:///a%2525.txt")
+                           << QString::fromLatin1("/a%2525.txt");
+    QTest::newRow("data10") << QString::fromLatin1("/%80.txt") << QString::fromLatin1("file:///%2580.txt")
+                            << QString::fromLatin1("/%2580.txt");
 }
 
 void tst_QUrl::fromLocalFile()
@@ -1074,7 +1087,7 @@ void tst_QUrl::fromLocalFile()
 
     QUrl url = QUrl::fromLocalFile(theFile);
 
-    QCOMPARE(url.toString(), theUrl);
+    QCOMPARE(url.toString(QUrl::MostDecoded), theUrl);
     QCOMPARE(url.path(), thePath);
 }