QUrl: Make sure we do reject URLs for which IDNA nameprep failed

qt_nameprep() already reset the string to its original length to
indicate failure, but we didn't handle that in qt_ACE_do(). So make it
have a return value whcih makes it easier to handle that case and do
handle it.

[ChangeLog][QtCore][QUrl] Fixed a bug that caused URLs whose hostnames
contained unassigned or prohibited Unicode codepoints to report
isValid() = true, despite clearing the hostname.

Change-Id: I41e7b3bced5944239f41fffd1545b7274c4b419d
Reviewed-by: David Faure <david.faure@kdab.com>

3 files changed, 30 insertions, 8 deletions

diff --git a/src/corelib/io/qurl_p.h b/src/corelib/io/qurl_p.h
index cb88cac35e..1b9237e58a 100644
--- a/src/corelib/io/qurl_p.h
+++ b/src/corelib/io/qurl_p.h
@@ -65,7 +65,7 @@ extern Q_AUTOTEST_EXPORT int qt_urlRecode(QString &appendTo, const QChar *begin,
 enum AceLeadingDot { AllowLeadingDot, ForbidLeadingDot };
 enum AceOperation { ToAceOnly, NormalizeAce };
 extern QString qt_ACE_do(const QString &domain, AceOperation op, AceLeadingDot dot);
-extern Q_AUTOTEST_EXPORT void qt_nameprep(QString *source, int from);
+extern Q_AUTOTEST_EXPORT bool qt_nameprep(QString *source, int from);
 extern Q_AUTOTEST_EXPORT bool qt_check_std3rules(const QChar *uc, int len);
 extern Q_AUTOTEST_EXPORT void qt_punycodeEncoder(const QChar *s, int ucLength, QString *output);
 extern Q_AUTOTEST_EXPORT QString qt_punycodeDecoder(const QString &pc);
diff --git a/src/corelib/io/qurlidna.cpp b/src/corelib/io/qurlidna.cpp
index 2f8bd91f6e..2305e66407 100644
--- a/src/corelib/io/qurlidna.cpp
+++ b/src/corelib/io/qurlidna.cpp
@@ -2021,7 +2021,7 @@ static bool isBidirectionalL(uint uc)
     return false;
 }
 
-Q_AUTOTEST_EXPORT void qt_nameprep(QString *source, int from)
+Q_AUTOTEST_EXPORT bool qt_nameprep(QString *source, int from)
 {
     QChar *src = source->data(); // causes a detach, so we're sure the only one using it
     QChar *out = src + from;
@@ -2036,7 +2036,7 @@ Q_AUTOTEST_EXPORT void qt_nameprep(QString *source, int from)
         }
     }
     if (out == e)
-        return; // everything was mapped easily (lowercased, actually)
+        return true;    // everything was mapped easily (lowercased, actually)
     int firstNonAscii = out - src;
 
     // Characters unassigned in Unicode 3.2 are not allowed in "stored string" scheme
@@ -2059,7 +2059,7 @@ Q_AUTOTEST_EXPORT void qt_nameprep(QString *source, int from)
             QChar::UnicodeVersion version = QChar::unicodeVersion(uc);
             if (version == QChar::Unicode_Unassigned || version > QChar::Unicode_3_2) {
                 source->resize(from); // not allowed, clear the label
-                return;
+                return false;
             }
         }
         if (!isMappedToNothing(uc)) {
@@ -2086,7 +2086,7 @@ Q_AUTOTEST_EXPORT void qt_nameprep(QString *source, int from)
     // Strip prohibited output
     if (containsProhibitedOuptut(source, firstNonAscii)) {
         source->resize(from);
-        return;
+        return false;
     }
 
     // Check for valid bidirectional characters
@@ -2110,9 +2110,13 @@ Q_AUTOTEST_EXPORT void qt_nameprep(QString *source, int from)
     }
     if (containsRandALCat) {
         if (containsLCat || (!isBidirectionalRorAL(src[from].unicode())
-                             || !isBidirectionalRorAL(e[-1].unicode())))
+                             || !isBidirectionalRorAL(e[-1].unicode()))) {
             source->resize(from); // not allowed, clear the label
+            return false;
+        }
     }
+
+    return true;
 }
 
 static const QChar *qt_find_nonstd3(const QChar *uc, int len, Qt::CaseSensitivity cs)
@@ -2553,7 +2557,8 @@ QString qt_ACE_do(const QString &domain, AceOperation op, AceLeadingDot dot)
         } else {
             // Punycode encoding and decoding cannot be done in-place
             // That means we need one or two temporaries
-            qt_nameprep(&result, prevLen);
+            if (!qt_nameprep(&result, prevLen))
+                return QString();   // failed
             labelLength = result.length() - prevLen;
             int toReserve = labelLength + 4 + 6; // "xn--" plus some extra bytes
             aceForm.resize(0);
diff --git a/tests/auto/corelib/io/qurl/tst_qurl.cpp b/tests/auto/corelib/io/qurl/tst_qurl.cpp
index 20282068cb..09aefcee91 100644
--- a/tests/auto/corelib/io/qurl/tst_qurl.cpp
+++ b/tests/auto/corelib/io/qurl/tst_qurl.cpp
@@ -2048,6 +2048,21 @@ void tst_QUrl::isValid()
     }
 
     {
+        QUrl url = QUrl::fromEncoded("foo://%f0%9f%93%99.example.la/g");
+        QVERIFY(!url.isValid());
+        QVERIFY(url.toString().isEmpty());
+        QCOMPARE(url.path(), QString("/g"));
+        url.setHost("%f0%9f%93%99.example.la/");
+        QVERIFY(!url.isValid());
+        QVERIFY(url.toString().isEmpty());
+        url.setHost("\xf0\x9f\x93\x99.example.la/");
+        QVERIFY(!url.isValid());
+        QVERIFY(url.toString().isEmpty());
+        QVERIFY2(url.errorString().contains("Invalid hostname"),
+                 qPrintable(url.errorString()));
+    }
+
+    {
         QUrl url("http://example.com");
         QVERIFY(url.isValid());
         QVERIFY(!url.toString().isEmpty());
@@ -2778,7 +2793,9 @@ void tst_QUrl::hosts()
 {
     QFETCH(QString, url);
 
-    QTEST(QUrl(url).host(), "host");
+    QUrl u(url);
+    QTEST(u.host(), "host");
+    QVERIFY(u.isEmpty() || u.isValid());
 }
 
 void tst_QUrl::hostFlags_data()