diff --git a/webenginepart/autotests/webenginepartcookiejar_test.cpp b/webenginepart/autotests/webenginepartcookiejar_test.cpp --- a/webenginepart/autotests/webenginepartcookiejar_test.cpp +++ b/webenginepart/autotests/webenginepartcookiejar_test.cpp @@ -25,6 +25,7 @@ #include #include + #include #include #include @@ -133,14 +134,15 @@ QVERIFY2(!m_server->lastError().isValid(), m_server->lastError().message().toLatin1()); QStringList resFields = res.value(); - QEXPECT_FAIL("persistent cookie with path and no domain", "Handling of cookies without domain is currently broken", Abort); QVERIFY(!resFields.isEmpty()); QCOMPARE(fields.count(), resFields.count()); QCOMPARE(resFields.at(0), domain); QCOMPARE(resFields.at(1), path); QCOMPARE(resFields.at(2), name); - QEXPECT_FAIL("", "The value returned by KCookieServer strips the leftmost part of the fqdn. Why?", Continue); + if (!domain.isEmpty()){ + QEXPECT_FAIL("", "The value returned by KCookieServer strips the leftmost part of the fqdn. Why?", Continue); + } QCOMPARE(resFields.at(3), host); QCOMPARE(resFields.at(4), value); const int secsSinceEpoch = resFields.at(5).toInt(); @@ -245,17 +247,24 @@ QFETCH(const QString, host); const QString url = "https://" + host; - const QByteArray setCookie = "Set-Cookie: " + cookie.toRawForm(); + + //cookie is in the "format" used by QWebEngineCookieStore, which means that, if the domain should be empty, + //it is stored as a domain not starting with a dot. KCookieServer, instead, wants cookies without domains + //to actually have no domain, so we have to change it + QNetworkCookie kcookieServerCookie(cookie); + if (!kcookieServerCookie.domain().startsWith(".")) { + kcookieServerCookie.setDomain(""); + } + const QByteArray setCookie = "Set-Cookie: " + kcookieServerCookie.toRawForm(); //Add cookie to KCookieServer QDBusMessage rep = m_server->call(QDBus::Block, "addCookies", url, setCookie, static_cast(0)); QVERIFY2(!m_server->lastError().isValid(), qPrintable(m_server->lastError().message())); //Ensure cookie has been added to KCookieServer - QDBusReply reply = m_server->call(QDBus::Block, "findCookies", QVariant::fromValue(QList{2}), domain, "", "", ""); + QDBusReply reply = m_server->call(QDBus::Block, "findCookies", QVariant::fromValue(QList{2}), domain, host, "", ""); QVERIFY2(reply.isValid(), qPrintable(reply.error().message())); QStringList cookies = reply.value(); - QEXPECT_FAIL("remove persistent cookie with path and no domain", "Handling of cookies without domain is currently broken", Abort); QVERIFY2(cookies.contains(name), "Cookie wasn't added to server"); //Emit QWebEngineCookieStore::cookieRemoved signal and check that cookie has indeed been removed diff --git a/webenginepart/src/webenginepartcookiejar.h b/webenginepart/src/webenginepartcookiejar.h --- a/webenginepart/src/webenginepartcookiejar.h +++ b/webenginepart/src/webenginepartcookiejar.h @@ -266,16 +266,17 @@ #endif //QTWEBENGINE_VERSION >= QT_VERSION_CHECK(5,11,0) /** - * @brief Adds a dot in front of a domain if it's not already there + * @brief Removes the domain from the cookie if the domain doesn't start with a dot * - * @param dom the domain - * @return `dom` if it already starts with a dot and `dom` preceded by a dot otherwise + * The cookies managed by QWebEngine never have an empty domain; instead, their domain starts with a dot if it was explicitly given and doesn't start + * with a dot in case it wasn't explicitly given. `KCookieServer::addCookies` and `KCookieServer::deleteCookie`, instead, require an empty domain if the + * domain wasn't explicitly given. This function transforms a cookie of the first form to one of the second form. * - * @internal - * This function is needed because KCookieJar prepends a dot to all domains not starting with one (according - * to `KCookieJar::makeCookies` in `kcookiejar.cpp`) + * If the cookie's domain starts with a dot, this function does nothing. + * + * @param cookie the cookie to remove the domain from. This cookie is modified in place */ - static inline QString prependDotToDomain(const QString &dom) {return dom.startsWith(".") ? dom : "." + dom;} + static void removeCookieDomain(QNetworkCookie &cookie); /** * @brief Tries to construct an Url for the given cookie diff --git a/webenginepart/src/webenginepartcookiejar.cpp b/webenginepart/src/webenginepartcookiejar.cpp --- a/webenginepart/src/webenginepartcookiejar.cpp +++ b/webenginepart/src/webenginepartcookiejar.cpp @@ -130,6 +130,13 @@ return 0; } +void WebEnginePartCookieJar::removeCookieDomain(QNetworkCookie& cookie) +{ + if (!cookie.domain().startsWith('.')) { + cookie.setDomain(QString()); + } +} + void WebEnginePartCookieJar::addCookie(const QNetworkCookie& _cookie) { //If the added cookie is in m_cookiesLoadedFromKCookieServer, it means @@ -163,6 +170,9 @@ if (url.isEmpty()) { return; } + //NOTE: the removal of the domain (when not starting with a dot) must be done *after* creating + //the URL, as constructUrlForCookie needs the domain + removeCookieDomain(cookie); QByteArray header("Set-Cookie: "); header += cookie.toRawForm(); header += "\n"; @@ -211,12 +221,11 @@ } } -bool WebEnginePartCookieJar::cookieInKCookieJar(const WebEnginePartCookieJar::CookieIdentifier& _id, const QUrl& url) +bool WebEnginePartCookieJar::cookieInKCookieJar(const WebEnginePartCookieJar::CookieIdentifier& id, const QUrl& url) { if (!m_cookieServer.isValid()) { return false; } - CookieIdentifier id(_id.name, prependDotToDomain(_id.domain), _id.path); QList fields = { static_cast(CookieDetails::name), static_cast(CookieDetails::domain), @@ -236,11 +245,10 @@ return false; } -void WebEnginePartCookieJar::removeCookie(const QNetworkCookie& cookie) +void WebEnginePartCookieJar::removeCookie(const QNetworkCookie& _cookie) { - CookieIdentifier id(cookie); - int pos = m_pendingRejectedCookies.indexOf(id); + int pos = m_pendingRejectedCookies.indexOf(CookieIdentifier(_cookie)); //Ignore pending cookies if (pos >= 0) { m_pendingRejectedCookies.takeAt(pos); @@ -251,15 +259,15 @@ return; } + QNetworkCookie cookie(_cookie); QUrl url = constructUrlForCookie(cookie); if(url.isEmpty()){ qDebug() << "Can't remove cookie" << cookie.name() << "because its URL isn't known"; return; } + removeCookieDomain(cookie); - //Add leading dot to domain, if necessary - id.domain = prependDotToDomain(id.domain); - QDBusPendingCall pcall = m_cookieServer.asyncCall("deleteCookie", id.domain, constructUrlForCookie(cookie).toString(), cookie.path(), QString(cookie.name())); + QDBusPendingCall pcall = m_cookieServer.asyncCall("deleteCookie", cookie.domain(), url.toString(), cookie.path(), QString(cookie.name())); QDBusPendingCallWatcher *w = new QDBusPendingCallWatcher(pcall, this); connect(w, &QDBusPendingCallWatcher::finished, this, &WebEnginePartCookieJar::cookieRemovalFailed); } @@ -324,6 +332,13 @@ c.setPath(extractField(CookieDetails::path).toUtf8()); c.setSecure(extractField(CookieDetails::secure).toInt()); //1 for true, 0 for false c.setValue(extractField(CookieDetails::value).toUtf8()); + if (c.domain().isEmpty()) { + QString host = extractField(CookieDetails::host); + QUrl url; + url.setScheme(c.isSecure() ? "https" : "http"); + url.setHost(host); + c.normalize(url); + } return c; }