diff --git a/webenginepart/autotests/CMakeLists.txt b/webenginepart/autotests/CMakeLists.txt --- a/webenginepart/autotests/CMakeLists.txt +++ b/webenginepart/autotests/CMakeLists.txt @@ -1,3 +1,5 @@ +add_definitions(-DBUILD_TESTING) + include(ECMAddTests) find_package(Qt5Test ${QT_MIN_VERSION} CONFIG REQUIRED) diff --git a/webenginepart/autotests/webenginepartcookiejar_test.h b/webenginepart/autotests/webenginepartcookiejar_test.h --- a/webenginepart/autotests/webenginepartcookiejar_test.h +++ b/webenginepart/autotests/webenginepartcookiejar_test.h @@ -26,6 +26,7 @@ #include #include +#include class QWebEngineCookieStore; class WebEnginePartCookieJar; @@ -60,13 +61,30 @@ void testCookieAddedToStoreAreAddedToKCookieServer(); void testCookieRemovedFromStoreAreRemovedFromKCookieServer_data(); void testCookieRemovedFromStoreAreRemovedFromKCookieServer(); + void testPersistentCookiesAreAddedToStoreOnCreation(); + void testSessionCookiesAreNotAddedToStoreOnCreation(); private: + /** + * @brief Adds a cookie to KCookieServer + * + * The cookie is supposed to be in `QWebEngineStore` "format", that is its domain must not be empty; + * a domain not starting with a dot means that the domain field wasn't given in the `Set-Cookie` header. + * + * @param _cookie the cookie to add + * @param host the host where the cookie come from + * @return QDBusError the error returned by DBus when adding the cookie. If no error occurred, this object + * will be invalid + */ + QDBusError addCookieToKCookieServer(const QNetworkCookie &_cookie, const QString &host); void deleteCookies(const QList &cookies); QList findTestCookies(); - QString m_cookieName; + //Cookie expiration dates returned by KCookieServer always have msecs set to 0 + static QDateTime currentDateTime(){return QDateTime::fromSecsSinceEpoch(QDateTime::currentMSecsSinceEpoch()/1000);} + + QString m_cookieName; QWebEngineCookieStore *m_store; WebEnginePartCookieJar *m_jar; QWebEngineProfile *m_profile; 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 @@ -32,6 +32,23 @@ #include #include +#include + +namespace QTest { + template <> + char *toString(const QNetworkCookie &cookie){ + QByteArray ba = "QNetworkCookie{"; + ba += "\nname: " + cookie.name(); + ba += "\ndomain: " + (cookie.domain().isEmpty() ? "" : cookie.domain()); + ba += "\npath: " + (cookie.path().isEmpty() ? "" : cookie.path()); + ba += "\nvalue: " + cookie.value(); + ba += "\nexpiration: " + (cookie.expirationDate().isValid() ? QString::number(cookie.expirationDate().toMSecsSinceEpoch()) : ""); + ba += "\nsecure: " + QString::number(cookie.isSecure()); + ba += "\nhttp: only" + QString::number(cookie.isHttpOnly()); + return qstrdup(ba.data()); + } +} + QTEST_MAIN(TestWebEnginePartCookieJar); void TestWebEnginePartCookieJar::initTestCase() @@ -246,20 +263,9 @@ QFETCH(const QString, domain); QFETCH(const QString, host); - const QString url = "https://" + host; - - //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(QString()); - } - 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())); + QDBusError e = addCookieToKCookieServer(cookie, host); + QVERIFY2(!e.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, host, "", ""); @@ -274,3 +280,76 @@ cookies = reply.value(); QVERIFY2(!cookies.contains(name), "Cookie wasn't removed from server"); } + +QDBusError TestWebEnginePartCookieJar::addCookieToKCookieServer(const QNetworkCookie& _cookie, const QString& host) +{ + QNetworkCookie cookie(_cookie); + QUrl url; + url.setHost(host); + url.setScheme(cookie.isSecure() ? "https" : "http"); + if (!cookie.domain().startsWith('.')) { + cookie.setDomain(QString()); + } + const QByteArray setCookie = "Set-Cookie: " + cookie.toRawForm(); + m_server->call(QDBus::Block, "addCookies", url.toString(), setCookie, static_cast(0)); + return m_server->lastError(); +} + +void TestWebEnginePartCookieJar::testPersistentCookiesAreAddedToStoreOnCreation() +{ + delete m_jar; + QDateTime exp = QDateTime::currentDateTime().addYears(1); + QString baseCookieName = m_cookieName + "-startup"; + QList data { + {baseCookieName + "-persistent", "test-value", ".yyy.xxx.com", "/abc/def/", "zzz.yyy.xxx.com", currentDateTime().addYears(1), true}, + {baseCookieName + "-no-path", "test-value", ".yyy.xxx.com", "", "zzz.yyy.xxx.com", currentDateTime().addYears(1), true}, + {baseCookieName + "-no-domain", "test-value", "", "/abc/def/", "zzz.yyy.xxx.com", currentDateTime().addYears(1), true}, + {baseCookieName + "-no-secure", "test-value", ".yyy.xxx.com", "/abc/def/", "zzz.yyy.xxx.com", currentDateTime().addYears(1), false} + }; + QList expected; + for(const CookieData &d: data){ + QNetworkCookie c = d.cookie(); + QDBusError e = addCookieToKCookieServer(c, d.host); + QVERIFY2(!e.isValid(), qPrintable(e.message())); + //In case of an empty domain, WebEnginePartCookieJar will use QNetworkCookie::normalize on the cookie + if (c.domain().isEmpty()) { + c.setDomain(d.host); + } + expected << c; + } + m_jar = new WebEnginePartCookieJar(m_profile, this); + QList cookiesInsertedIntoJar; + for(const QNetworkCookie &c: qAsConst(m_jar->m_testCookies)){ + if(QString(c.name()).startsWith(baseCookieName)) { + cookiesInsertedIntoJar << c; + } + } + + //Ensure that cookies in the two lists are in the same order before comparing them + //(the order in cookiesInsertedIntoJar depends on the order KCookieServer::findCookies + //returns them) + auto sortLambda = [](const QNetworkCookie &c1, const QNetworkCookie &c2){ + return c1.name() < c2.name(); + }; + std::sort(cookiesInsertedIntoJar.begin(), cookiesInsertedIntoJar.end(), sortLambda); + std::sort(expected.begin(), expected.end(), sortLambda); + + QCOMPARE(cookiesInsertedIntoJar, expected); +} + +void TestWebEnginePartCookieJar::testSessionCookiesAreNotAddedToStoreOnCreation() +{ + delete m_jar; + CookieData data{m_cookieName + "-startup-session", "test-value", ".yyy.xxx.com", "/abc/def", "zzz.yyy.xxx.com", QDateTime(), true}; + QDBusError e = addCookieToKCookieServer(data.cookie(), data.host); + QVERIFY2(!e.isValid(), qPrintable(e.message())); + m_jar = new WebEnginePartCookieJar(m_profile, this); + QList cookiesInsertedIntoJar; + for(const QNetworkCookie &c: qAsConst(m_jar->m_testCookies)) { + if (c.name() == data.name) { + cookiesInsertedIntoJar << c; + } + } + QVERIFY2(cookiesInsertedIntoJar.isEmpty(), "Session cookies inserted into cookie store"); +} + diff --git a/webenginepart/src/CMakeLists.txt b/webenginepart/src/CMakeLists.txt --- a/webenginepart/src/CMakeLists.txt +++ b/webenginepart/src/CMakeLists.txt @@ -3,6 +3,10 @@ add_definitions(-DTRANSLATION_DOMAIN=\"webenginepart\") +if(BUILD_TESTING) + add_definitions(-DBUILD_TESTING) +endif(BUILD_TESTING) + include_directories(${CMAKE_CURRENT_SOURCE_DIR} ${CMAKE_BINARY_DIR}) set(kwebenginepartlib_LIB_SRCS diff --git a/webenginepart/src/webenginepartcookiejar.h b/webenginepart/src/webenginepartcookiejar.h --- a/webenginepart/src/webenginepartcookiejar.h +++ b/webenginepart/src/webenginepartcookiejar.h @@ -332,6 +332,11 @@ */ CookieList m_cookiesLoadedFromKCookieServer; +#ifdef BUILD_TESTING + QList m_testCookies; + friend class TestWebEnginePartCookieJar; +#endif + }; /** diff --git a/webenginepart/src/webenginepartcookiejar.cpp b/webenginepart/src/webenginepartcookiejar.cpp --- a/webenginepart/src/webenginepartcookiejar.cpp +++ b/webenginepart/src/webenginepartcookiejar.cpp @@ -70,6 +70,7 @@ if(!m_cookieServer.isValid()){ qDebug() << "Couldn't connect to KCookieServer"; } + loadKIOCookies(); //QWebEngineCookieStore::setCookieFilter only exists from Qt 5.11.0 @@ -148,6 +149,10 @@ return; } +#ifdef BUILD_TESTING + m_testCookies.clear(); +#endif + QNetworkCookie cookie(_cookie); CookieIdentifier id(cookie); @@ -291,6 +296,9 @@ continue; } m_cookiesLoadedFromKCookieServer << cookie; +#ifdef BUILD_TESTING + m_testCookies << cookie; +#endif m_cookieStore->setCookie(cookie); } }