QNetworkCookie: Use RFC6265 rules for parsing Set-Cookie

The ';' separator takes priority even inside a quoted string.
Quotation marks have no special meaning, they are not parsed and
regenerated anymore. This means it is not possible to include
the ';' character inside a cookie value.
Other characters are returned transparently, including [",\]

Task-number: QTBUG-15794
Task-number: QTBUG-26002
Task-number: QTBUG-11641
Change-Id: I4eefef5c6ac7753d5a21c226169e264578521fe9
Reviewed-by: Richard J. Moore <rich@kde.org>

diff --git a/src/network/access/qnetworkcookie.cpp b/src/network/access/qnetworkcookie.cpp
index 89e6f8a..306195a 100644 (file)
--- a/src/network/access/qnetworkcookie.cpp
+++ b/src/network/access/qnetworkcookie.cpp
@@ -379,85 +379,27 @@ static QPair<QByteArray, QByteArray> nextField(const QByteArray &text, int &posi
     //    (1)  token
     //    (2)  token = token
     //    (3)  token = quoted-string
-    int i;
     const int length = text.length();
     position = nextNonWhitespace(text, position);
 
-    // parse the first part, before the equal sign
-    for (i = position; i < length; ++i) {
-        register char c = text.at(i);
-        if (c == ';' || c == '=')
-            break;
-    }
-
-    QByteArray first = text.mid(position, i - position).trimmed();
-    position = i;
-
-    if (first.isEmpty())
-        return qMakePair(QByteArray(), QByteArray());
-    if (i == length || text.at(i) != '=')
-        // no equal sign, we found format (1)
-        return qMakePair(first, QByteArray());
-
-    QByteArray second;
-    second.reserve(32);         // arbitrary but works for most cases
-
-    i = nextNonWhitespace(text, position + 1);
-    if (i < length && text.at(i) == '"') {
-        // a quote, we found format (3), where:
-        // quoted-string  = ( <"> *(qdtext | quoted-pair ) <"> )
-        // qdtext         = <any TEXT except <">>
-        // quoted-pair    = "\" CHAR
+    int semiColonPosition = text.indexOf(';', position);
+    if (semiColonPosition < 0)
+        semiColonPosition = length; //no ';' means take everything to end of string
 
-        // If it is NAME=VALUE, retain the value as is
-        // refer to http://bugreports.qt-project.org/browse/QTBUG-17746
+    int equalsPosition = text.indexOf('=', position);
+    if (equalsPosition < 0 || equalsPosition > semiColonPosition) {
         if (isNameValue)
-            second += '"';
-        ++i;
-        while (i < length) {
-            register char c = text.at(i);
-            if (c == '"') {
-                // end of quoted text
-                if (isNameValue)
-                    second += '"';
-                break;
-            } else if (c == '\\') {
-                if (isNameValue)
-                    second += '\\';
-                ++i;
-                if (i >= length)
-                    // broken line
-                    return qMakePair(QByteArray(), QByteArray());
-                c = text.at(i);
-            }
-
-            second += c;
-            ++i;
-        }
-
-        for ( ; i < length; ++i) {
-            register char c = text.at(i);
-            if (c == ';')
-                break;
-        }
-        position = i;
-    } else {
-        // no quote, we found format (2)
-        position = i;
-        for ( ; i < length; ++i) {
-            register char c = text.at(i);
-            // for name value pairs, we want to parse until reaching the next ';'
-            // and not break when reaching a space char
-            if (c == ';' || ((isNameValue && (c == '\n' || c == '\r')) || (!isNameValue && isLWS(c))))
-                break;
-        }
-
-        second = text.mid(position, i - position).trimmed();
-        position = i;
+            return qMakePair(QByteArray(), QByteArray()); //'=' is required for name-value-pair (RFC6265 section 5.2, rule 2)
+        equalsPosition = semiColonPosition; //no '=' means there is an attribute-name but no attribute-value
     }
 
-    if (second.isNull())
-        second.resize(0); // turns into empty-but-not-null
+    QByteArray first = text.mid(position, equalsPosition - position).trimmed();
+    QByteArray second;
+    int secondLength = semiColonPosition - equalsPosition - 1;
+    if (secondLength > 0)
+        second = text.mid(equalsPosition + 1, secondLength).trimmed();
+
+    position = semiColonPosition;
     return qMakePair(first, second);
 }
 
@@ -500,20 +442,7 @@ QByteArray QNetworkCookie::toRawForm(RawForm form) const
 
     result = d->name;
     result += '=';
-    if ((d->value.contains(';') ||
-        d->value.contains('"')) &&
-        (!d->value.startsWith('"') &&
-        !d->value.endsWith('"'))) {
-        result += '"';
-
-        QByteArray value = d->value;
-        value.replace('"', "\\\"");
-        result += value;
-
-        result += '"';
-    } else {
-        result += d->value;
-    }
+    result += d->value;
 
     if (form == Full) {
         // same as above, but encoding everything back
@@ -972,7 +901,7 @@ QList<QNetworkCookie> QNetworkCookiePrivate::parseSetCookieHeaderLine(const QByt
 
         // The first part is always the "NAME=VALUE" part
         QPair<QByteArray,QByteArray> field = nextField(cookieString, position, true);
-        if (field.first.isEmpty() || field.second.isNull())
+        if (field.first.isEmpty())
             // parsing error
             break;
         cookie.setName(field.first);

diff --git a/tests/auto/network/access/qnetworkcookie/tst_qnetworkcookie.cpp b/tests/auto/network/access/qnetworkcookie/tst_qnetworkcookie.cpp
index 77dfa18..77afd23 100644 (file)
--- a/tests/auto/network/access/qnetworkcookie/tst_qnetworkcookie.cpp
+++ b/tests/auto/network/access/qnetworkcookie/tst_qnetworkcookie.cpp
@@ -173,13 +173,13 @@ void tst_QNetworkCookie::parseSingleCookie_data()
 
     cookie.setValue("\",\"");
     QTest::newRow("with-value-with-special1") << "a = \",\" " << cookie;
-    cookie.setValue("\";\"");
+    cookie.setValue("\"");
     QTest::newRow("with-value-with-special2") << "a = \";\" " << cookie;
     cookie.setValue("\" \"");
     QTest::newRow("with-value-with-special3") << "a = \" \" " << cookie;
     cookie.setValue("\"\\\"\"");
     QTest::newRow("with-value-with-special4") << "a = \"\\\"\" " << cookie;
-    cookie.setValue("\"\\\"a, b; c\\\"\"");
+    cookie.setValue("\"\\\"a, b");
     QTest::newRow("with-value-with-special5") << "a = \"\\\"a, b; c\\\"\"" << cookie;
     // RFC6265 states that cookie values shouldn't contain commas, but we still allow them
     // since they are only reserved for future compatibility and other browsers do the same.
@@ -238,11 +238,13 @@ void tst_QNetworkCookie::parseSingleCookie_data()
     cookie.setPath("/with spaces");
     QTest::newRow("path-with-spaces") << "a=b;path=/with%20spaces" << cookie;
     QTest::newRow("path-with-spaces2") << "a=b; path=/with%20spaces " << cookie;
+    cookie.setPath("\"/with spaces\"");
     QTest::newRow("path-with-spaces3") << "a=b; path=\"/with spaces\"" << cookie;
     QTest::newRow("path-with-spaces4") << "a=b; path = \"/with spaces\" " << cookie;
 
     cookie.setPath("/with\"Quotes");
     QTest::newRow("path-with-quotes") << "a=b; path = /with%22Quotes" << cookie;
+    cookie.setPath("\"/with\\\"Quotes\"");
     QTest::newRow("path-with-quotes2") << "a=b; path = \"/with\\\"Quotes\"" << cookie;
 
     cookie.setPath(QString::fromUtf8("/R\303\251sum\303\251"));
@@ -595,7 +597,11 @@ void tst_QNetworkCookie::parseSingleCookie_data()
     cookie.setExpirationDate(QDateTime(QDate(2011, 3, 1), QTime(10, 51, 14), Qt::UTC));
     QTest::newRow("network5") << "leo_auth_token=\"GST:UroVXaxYA3sVSkoVjMNH9bj4dZxVzK2yekgrAUxMfUsyLTNyPjoP60:1298974875:b675566ae32ab36d7a708c0efbf446a5c22b9fca\"; Version=1; Max-Age=1799; Expires=Tue, 01-Mar-2011 10:51:14 GMT; Path=/" << cookie;
 
-
+    // cookie containing JSON data (illegal for server, client should accept) - QTBUG-26002
+    cookie = QNetworkCookie("xploreCookies", "{\"desktopReportingUrl\":\"null\",\"userIds\":\"1938850\",\"contactEmail\":\"NA\",\"contactName\":\"NA\",\"enterpriseLicenseId\":\"0\",\"openUrlTxt\":\"NA\",\"customerSurvey\":\"NA\",\"standardsLicenseId\":\"0\",\"openUrl\":\"NA\",\"smallBusinessLicenseId\":\"0\", \"instImage\":\"1938850_univ skovde.gif\",\"isMember\":\"false\",\"products\":\"IEL|VDE|\",\"openUrlImgLoc\":\"NA\",\"isIp\":\"true\",\"instName\": \"University of XXXXXX\",\"oldSessionKey\":\"LmZ8hlXo5a9uZx2Fnyw1564T1ZOWMnf3Dk*oDx2FQHwbg6RYefyrhC8PL2wx3Dx3D-18x2d8723DyqXRnkILyGpmx2Fh9wgx3Dx3Dc2lAOhHqGSKT78xxGwXZxxCgx3Dx3D-XrL4FnIlW2OPkqtVJq0LkQx3Dx3D-tujOLwhFqtX7Pa7HGqmCXQx3Dx3D\", \"isChargebackUser\":\"false\",\"isInst\":\"true\"}");
+    cookie.setDomain(".ieee.org");
+    cookie.setPath("/");
+    QTest::newRow("network6") << "xploreCookies={\"desktopReportingUrl\":\"null\",\"userIds\":\"1938850\",\"contactEmail\":\"NA\",\"contactName\":\"NA\",\"enterpriseLicenseId\":\"0\",\"openUrlTxt\":\"NA\",\"customerSurvey\":\"NA\",\"standardsLicenseId\":\"0\",\"openUrl\":\"NA\",\"smallBusinessLicenseId\":\"0\", \"instImage\":\"1938850_univ skovde.gif\",\"isMember\":\"false\",\"products\":\"IEL|VDE|\",\"openUrlImgLoc\":\"NA\",\"isIp\":\"true\",\"instName\": \"University of XXXXXX\",\"oldSessionKey\":\"LmZ8hlXo5a9uZx2Fnyw1564T1ZOWMnf3Dk*oDx2FQHwbg6RYefyrhC8PL2wx3Dx3D-18x2d8723DyqXRnkILyGpmx2Fh9wgx3Dx3Dc2lAOhHqGSKT78xxGwXZxxCgx3Dx3D-XrL4FnIlW2OPkqtVJq0LkQx3Dx3D-tujOLwhFqtX7Pa7HGqmCXQx3Dx3D\", \"isChargebackUser\":\"false\",\"isInst\":\"true\"}; domain=.ieee.org; path=/" << cookie;
 }
 
 void tst_QNetworkCookie::parseSingleCookie()