Fix QUrl operator== and operator<

Don't crash when either side is null but not both sides.

Also make sure operator< is working properly and satisfies the basic
conditions of a type (such as that if A < B, then !(B < A)).

Change-Id: Idd9e9fc593e1a7781d9f4f2b13a1024b643926fd
Reviewed-by: Giuseppe D'Angelo <dangelog@gmail.com>
Reviewed-by: Lars Knoll <lars.knoll@nokia.com>

diff --git a/src/corelib/io/qurl.cpp b/src/corelib/io/qurl.cpp
index a83d6eb..a716369 100644 (file)
--- a/src/corelib/io/qurl.cpp
+++ b/src/corelib/io/qurl.cpp
@@ -2197,24 +2197,44 @@ QByteArray QUrl::toAce(const QString &domain)
 */
 bool QUrl::operator <(const QUrl &url) const
 {
-    if (!d) return url.d;
-    if (d->scheme < url.d->scheme)
-        return true;
-    if (d->userName < url.d->userName)
-        return true;
-    if (d->password < url.d->password)
-        return true;
-    if (d->host < url.d->host)
-        return true;
-    if (d->port < url.d->port)
-        return true;
-    if (d->path < url.d->path)
-        return true;
-    if (d->query < url.d->query)
-        return true;
-    if (d->fragment < url.d->fragment)
-        return true;
-    return false;
+    if (!d || !url.d) {
+        bool thisIsEmpty = !d || d->isEmpty();
+        bool thatIsEmpty = !url.d || url.d->isEmpty();
+
+        // sort an empty URL first
+        return thisIsEmpty && !thatIsEmpty;
+    }
+
+    int cmp;
+    cmp = d->scheme.compare(url.d->scheme);
+    if (cmp != 0)
+        return cmp < 0;
+
+    cmp = d->userName.compare(url.d->userName);
+    if (cmp != 0)
+        return cmp < 0;
+
+    cmp = d->password.compare(url.d->password);
+    if (cmp != 0)
+        return cmp < 0;
+
+    cmp = d->host.compare(url.d->host);
+    if (cmp != 0)
+        return cmp < 0;
+
+    if (d->port != url.d->port)
+        return d->port < url.d->port;
+
+    cmp = d->path.compare(url.d->path);
+    if (cmp != 0)
+        return cmp < 0;
+
+    cmp = d->query.compare(url.d->query);
+    if (cmp != 0)
+        return cmp < 0;
+
+    cmp = d->fragment.compare(url.d->fragment);
+    return cmp < 0;
 }
 
 /*!
@@ -2223,8 +2243,12 @@ bool QUrl::operator <(const QUrl &url) const
 */
 bool QUrl::operator ==(const QUrl &url) const
 {
-    if (!d || !url.d)
-        return d == url.d;
+    if (!d && !url.d)
+        return true;
+    if (!d)
+        return url.d->isEmpty();
+    if (!url.d)
+        return d->isEmpty();
     return d->scheme == url.d->scheme &&
             d->userName == url.d->userName &&
             d->password == url.d->password &&

diff --git a/src/corelib/io/qurl_p.h b/src/corelib/io/qurl_p.h
index fb54d74..2333809 100644 (file)
--- a/src/corelib/io/qurl_p.h
+++ b/src/corelib/io/qurl_p.h
@@ -107,6 +107,8 @@ public:
 
     void parse(const QString &url, QUrl::ParsingMode parsingMode);
     void clear();
+    bool isEmpty() const
+    { return sectionIsPresent == 0 && port == -1 && path.isEmpty(); }
 
     // no QString scheme() const;
     void appendAuthority(QString &appendTo, QUrl::FormattingOptions options) const;

diff --git a/tests/auto/corelib/io/qurl/tst_qurl.cpp b/tests/auto/corelib/io/qurl/tst_qurl.cpp
index 5c9e20b..f0081df 100644 (file)
--- a/tests/auto/corelib/io/qurl/tst_qurl.cpp
+++ b/tests/auto/corelib/io/qurl/tst_qurl.cpp
@@ -66,6 +66,8 @@ private slots:
     void unc();
     void assignment();
     void comparison();
+    void comparison2_data();
+    void comparison2();
     void copying();
     void setUrl();
     void i18n_data();
@@ -286,6 +288,55 @@ void tst_QUrl::comparison()
     QVERIFY(url5 == url6);
 }
 
+void tst_QUrl::comparison2_data()
+{
+    QTest::addColumn<QUrl>("url1");
+    QTest::addColumn<QUrl>("url2");
+    QTest::addColumn<int>("ordering"); // like strcmp
+
+    QTest::newRow("null-null") << QUrl() << QUrl() << 0;
+
+    QUrl empty;
+    empty.setPath("/hello"); // ensure it has detached
+    empty.setPath(QString());
+    QTest::newRow("null-empty") << QUrl() << empty << 0;
+
+    QTest::newRow("scheme-null") << QUrl("x:") << QUrl() << 1;
+    QTest::newRow("samescheme") << QUrl("x:") << QUrl("x:") << 0;
+
+    // the following three are by choice
+    // the order could be the opposite and it would still be correct
+    QTest::newRow("scheme-path") << QUrl("x:") << QUrl("/tmp") << +1;
+    QTest::newRow("fragment-path") << QUrl("#foo") << QUrl("/tmp") << -1;
+    QTest::newRow("fragment-scheme") << QUrl("#foo") << QUrl("x:") << -1;
+
+    QTest::newRow("noport-zeroport") << QUrl("http://example.com") << QUrl("http://example.com:0") << -1;
+}
+
+void tst_QUrl::comparison2()
+{
+    QFETCH(QUrl, url1);
+    QFETCH(QUrl, url2);
+    QFETCH(int, ordering);
+
+    QCOMPARE(url1.toString() == url2.toString(), ordering == 0);
+    QCOMPARE(url1 == url2, ordering == 0);
+    QCOMPARE(url1 != url2, ordering != 0);
+    if (ordering == 0)
+        QCOMPARE(qHash(url1), qHash(url2));
+
+    QCOMPARE(url1 < url2, ordering < 0);
+    QCOMPARE(!(url1 < url2), ordering >= 0);
+
+    QCOMPARE(url2 < url1, ordering > 0);
+    QCOMPARE(!(url2 < url1), ordering <= 0);
+
+    // redundant checks (the above should catch these)
+    QCOMPARE(url1 < url2 || url2 < url1, ordering != 0);
+    QVERIFY(!(url1 < url2 && url2 < url1));
+    QVERIFY(url1 < url2 || url1 == url2 || url2 < url1);
+}
+
 void tst_QUrl::copying()
 {
     QUrl url("http://qt.nokia.com/");