Merge lp:~mardy/webbrowser-app/cookie-domain2 into lp:webbrowser-app

[Alberto Mardegan]

Fix handling of host cookies

Oxide (and chromium) expect host cookies to be treated in a special way: the domain in the cookie must be left empty, or an initial "." will always be added to the cookie domain.

Fix handling of host cookies

Oxide (and chromium) expect host cookies to be treated in a special way: the domain in the cookie must be left empty, or an initial "." will always be added to the cookie domain.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:718
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1127/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/5473/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/3910
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-amd64-ci/326
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-armhf-ci/326
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-armhf-ci/326/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-i386-ci/326
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/5174/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/6725
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/6725/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/14115
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/3286
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/4227
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/4227/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/1127/rebuild

review: Needs Fixing (continuous-integration)

[Olivier Tilloy (osomon)]

LGTM.

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:718
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1128/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/5478
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/3912
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-amd64-ci/327
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-armhf-ci/327
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-armhf-ci/327/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-i386-ci/327
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/5176
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/6730
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/6730/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/14117
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/3288
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/4229
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/4229/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/1128/rebuild

review: Approve (continuous-integration)

=== modified file 'src/app/webcontainer/oxide-cookie-helper.cpp'
--- src/app/webcontainer/oxide-cookie-helper.cpp	2014-10-03 12:49:16 +0000
+++ src/app/webcontainer/oxide-cookie-helper.cpp	2014-10-03 12:49:16 +0000
@@ -22,7 +22,6 @@
 #include <QDebug>
 #include <QMap>
 #include <QMetaMethod>
-#include <QSet>
 #include <QUrl>
 
 typedef QList<QNetworkCookie> Cookies;
@@ -36,13 +35,15 @@
     OxideCookieHelperPrivate(OxideCookieHelper* q);
 
     void setCookies(const QList<QNetworkCookie>& cookies);
+    QList<QNetworkCookie> cookiesWithDomain(const QList<QNetworkCookie>& cookies,
+                                            const QString& domain);
 
 private Q_SLOTS:
-    void oxideCookiesUpdated(int requestId, const QVariant& failedCookies);
+    void oxideCookiesUpdated(int requestId, const QVariant& failedCookiesVariant);
 
 private:
     QObject* m_backend;
-    QSet<int> m_pendingCalls;
+    QMap<int, QString> m_pendingCalls;
     QList<QNetworkCookie> m_failedCookies;
     mutable OxideCookieHelper* q_ptr;
 };
@@ -81,29 +82,38 @@
     /* Since Oxide does not support setting cookies for different domains in a
      * single call to setCookies(), we group the cookies by their domain, and
      * perform a separate call to Oxide's setCookies() for each domain.
+     *
+     * Cookies whose domain doesn't start with a "." are host cookies, and need
+     * to be treated specially: we will use their domain as host (that is, we
+     * will pass it as first argument in the setNetworkCookies() call), and
+     * will unset their domain in the cookie.
      */
-    QMap<QString, Cookies> cookiesPerDomain;
+    QMap<QString, Cookies> cookiesPerHost;
     Q_FOREACH(const QNetworkCookie &cookie, cookies) {
-        /* This creates an empty list if the domain is new in the map */
+        QNetworkCookie c(cookie);
+        /* We use the domain (without any starting dot) as host */
+        QString host = c.domain();
+        if (host.startsWith('.')) {
+            host = host.mid(1);
+        } else {
+            /* No starting dot => this is a host cookie */
+            c.setDomain(QString());
+        }
+        /* This creates an empty list if the host is new in the map */
         QList<QNetworkCookie> &domainCookies =
-            cookiesPerDomain[cookie.domain()];
+            cookiesPerHost[host];
 
-        domainCookies.append(cookie);
+        domainCookies.append(c);
     }
 
     /* Grouping done, perform the calls */
-    QMapIterator<QString, Cookies> it(cookiesPerDomain);
+    QMapIterator<QString, Cookies> it(cookiesPerHost);
     while (it.hasNext()) {
         it.next();
 
-        QString domain = it.key();
-        if (domain.startsWith('.')) {
-            domain = domain.mid(1);
-        }
-
         QUrl url;
         url.setScheme("http");
-        url.setHost(domain);
+        url.setHost(it.key());
 
         int requestId = -1;
         QMetaObject::invokeMethod(m_backend, "setNetworkCookies",
@@ -112,9 +122,9 @@
                                   Q_ARG(const QUrl&, url),
                                   Q_ARG(const QList<QNetworkCookie>&, it.value()));
         if (Q_UNLIKELY(requestId == -1)) {
-            m_failedCookies.append(it.value());
+            m_failedCookies.append(cookiesWithDomain(it.value(), url.host()));
         } else {
-            m_pendingCalls.insert(requestId);
+            m_pendingCalls.insert(requestId, url.host());
         }
     }
 
@@ -127,12 +137,30 @@
     }
 }
 
+QList<QNetworkCookie>
+OxideCookieHelperPrivate::cookiesWithDomain(const QList<QNetworkCookie>& cookies,
+                                            const QString& domain)
+{
+    QList<QNetworkCookie> restoredCookies;
+    Q_FOREACH(const QNetworkCookie& cookie, cookies) {
+        QNetworkCookie c(cookie);
+        if (c.domain().isEmpty()) {
+            c.setDomain(domain);
+        }
+        restoredCookies.append(c);
+    }
+    return restoredCookies;
+}
+
 void OxideCookieHelperPrivate::oxideCookiesUpdated(int requestId,
-                                                   const QVariant& failedCookies)
+                                                   const QVariant& failedCookiesVariant)
 {
     Q_Q(OxideCookieHelper);
 
-    m_failedCookies.append(OxideCookieHelper::cookiesFromVariant(failedCookies));
+    QString host = m_pendingCalls.value(requestId);
+    QList<QNetworkCookie> failedCookies =
+        OxideCookieHelper::cookiesFromVariant(failedCookiesVariant);
+    m_failedCookies.append(cookiesWithDomain(failedCookies, host));
     m_pendingCalls.remove(requestId);
 
     if (m_pendingCalls.isEmpty()) {

=== modified file 'tests/unittests/oxide-cookie-helper/tst_OxideCookieHelper.cpp'
--- tests/unittests/oxide-cookie-helper/tst_OxideCookieHelper.cpp	2014-10-03 12:49:16 +0000
+++ tests/unittests/oxide-cookie-helper/tst_OxideCookieHelper.cpp	2014-10-03 12:49:16 +0000
@@ -53,7 +53,16 @@
     int setNetworkCookies(const QUrl& url, const QList<QNetworkCookie>& cookies) {
         if (m_failUrls.contains(url)) return -1;
         m_lastRequestId++;
-        Q_EMIT setNetworkCookiesCalled(m_lastRequestId, url, cookies);
+        /* Handle host cookies */
+        QList<QNetworkCookie> restoredCookies;
+        Q_FOREACH(const QNetworkCookie& cookie, cookies) {
+            QNetworkCookie c(cookie);
+            if (c.domain().isEmpty()) {
+                c.setDomain(url.host());
+            }
+            restoredCookies.append(c);
+        }
+        Q_EMIT setNetworkCookiesCalled(m_lastRequestId, url, restoredCookies);
         return m_lastRequestId;
     }