[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-core-devel
Subject: Re: KIO / KWebView and PrivateBrowsing (Cookies)
From: Dawit A <adawit () kde ! org>
Date: 2012-04-23 23:38:16
Message-ID: CALa28R63yHf-C6w=dhXoyrQUi7PnPonQpCCh+asTHSZH4LUo3w () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
On Mon, Apr 23, 2012 at 1:12 PM, Alex Fiestas <afiestas@kde.org> wrote:
> KWebView can't do Private Browsing because of KIO integration, need advice
> on
> how to fix it but first let me explain the issue.
>
> What I consider PrivateBrowsing:
> Private Browsing should have an empty CookieJar that will keep cookies
> only as
> long as the private browsing is activated, after that cookies will be
> removed.
What is happening now?
> While in PrivateBrowsing all * new * cokies are created with an expiration
> date of "right now" and stored in kcookiejar. This is done by calling
> AccessManager::setDisablecookieStorage(false) and stored in
> CookieJar::setCookiesFromUrl.
>
> File: kdelibs/kio/kio/accessmanager.cpp
>
> All existing cookies will still be available to KWebPage, so even though
> I'm
> in PrivateBrowser authentifications will remain as well as any other
> information stored in the cookies.
>
As a practical usecase, in akonadi-google we need an empty cookieJar so any
> previous authentification on GMail will be ignored (for example I can be
> authentificated with X account on rekonq while configuring Y account on
> akonadi-google).
> Where is the problem located?
>
The problem is the integration class not doing the right thing right now.
As you already discovered in private browsing mode the cookies are merely
set to session cookies and added to KDE's central cookiejar which violates
the privateness of any cookie.
> As far as I understood the code there are two problems:
>
> -kio_http uses KCookieJar directly, and it doesn't have a Cookie mode like
> the
> one we need. It has: none, manual, auto we will need another one
> (private?).
>
Nope. That is not necessary. The reason the "cookies: manual" meta-data
exists is to allow the client application to deal with cookie management
itself. In this case the client is KIO::Integration::AccessManager.
However, since that class is not doing the right thing, you have this
particular issue.
-kio_http won't use any new QNetworkCookieJar we set via
> AccessManager::setNetworkCookieJar. This is more difficult to fix since
> kio_http is out of process. Maybe we can use some kind of DBus magic?
>
None of this is necessary. What should happen in private browsing mode is
the "cookies" metadata should be set to "manual" to disable cookie handling
kio_http. The KIO::Integration::AccessManager can be modified to send its
own cookie header instead. This would give you want you want. I have
attached an untested patch that does just that. Test it and let me know if
it works okay for you.
The only question I have and which the patch does not address is whether or
not cookies stored outside of the private-browsing mode should be used
(read: sent back to the server) during private browsing mode. Reading the
up on the definition of private browsing mode, it seems to me that we
should only stop saving information once in private browsing mode, not stop
sending data that was previously stored before "private browsing mode" was
initiated.
[Attachment #5 (text/html)]
<div class="gmail_extra"><div class="gmail_quote">On Mon, Apr 23, 2012 at 1:12 PM, \
Alex Fiestas <span dir="ltr"><<a href="mailto:afiestas@kde.org" \
target="_blank">afiestas@kde.org</a>></span> wrote:<br><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex">
KWebView can't do Private Browsing because of KIO integration, need advice on<br>
how to fix it but first let me explain the issue.<br>
<br>
What I consider PrivateBrowsing:<br>
Private Browsing should have an empty CookieJar that will keep cookies only as<br>
long as the private browsing is activated, after that cookies will be \
removed.</blockquote><div><br></div><blockquote class="gmail_quote" style="margin:0 0 \
0 .8ex;border-left:1px #ccc solid;padding-left:1ex">What is happening now?<br>
While in PrivateBrowsing all * new * cokies are created with an expiration<br>
date of "right now" and stored in kcookiejar. This is done by calling<br>
AccessManager::setDisablecookieStorage(false) and stored in<br>
CookieJar::setCookiesFromUrl.<br>
<br>
File: kdelibs/kio/kio/accessmanager.cpp<br>
<br>
All existing cookies will still be available to KWebPage, so even though I'm<br>
in PrivateBrowser authentifications will remain as well as any other<br>
information stored in the cookies.<br></blockquote><div><br></div><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> As a practical usecase, in akonadi-google we need an empty \
cookieJar so any<br> previous authentification on GMail will be ignored (for \
example I can be<br> authentificated with X account on rekonq while configuring Y \
account on<br> akonadi-google).</blockquote><div> </div><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex">Where is the problem \
located?<br></blockquote><div><br></div><div>The problem is the integration class not \
doing the right thing right now. As you already discovered in private browsing mode \
the cookies are merely set to session cookies and added to KDE's central \
cookiejar which violates the privateness of any cookie.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px \
#ccc solid;padding-left:1ex"> As far as I understood the code there are two \
problems:<br> <br>
-kio_http uses KCookieJar directly, and it doesn't have a Cookie mode like \
the<br> one we need. It has: none, manual, auto we will need another one \
(private?).<br></blockquote><div><br></div><div>Nope. That is not necessary. The \
reason the "cookies: manual" meta-data exists is to allow the client \
application to deal with cookie management itself. In this case the client is \
KIO::Integration::AccessManager. However, since that class is not doing the right \
thing, you have this particular issue.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 \
.8ex;border-left:1px #ccc solid;padding-left:1ex">
-kio_http won't use any new QNetworkCookieJar we set via<br>
AccessManager::setNetworkCookieJar. This is more difficult to fix since<br>
kio_http is out of process. Maybe we can use some kind of DBus \
magic?<br></blockquote><div><br></div><div>None of this is necessary. What should \
happen in private browsing mode is the "cookies" metadata should be set to \
"manual" to disable cookie handling kio_http. The \
KIO::Integration::AccessManager can be modified to send its own cookie header \
instead. This would give you want you want. I have attached an untested patch that \
does just that. Test it and let me know if it works okay for you.</div>
<div><br></div><div>The only question I have and which the patch does not address is \
whether or not cookies stored outside of the private-browsing mode should be used \
(read: sent back to the server) during private browsing mode. Reading the up on the \
definition of private browsing mode, it seems to me that we should only stop saving \
information once in private browsing mode, not stop sending data that was previously \
stored before "private browsing mode" was initiated.</div>
<div><br></div>
<div><br></div><div><br></div></div></div>
--e89a8f502906442d2e04be61241c--
["kio_accessmanager_private_browsing_mode.patch" (application/octet-stream)]
diff --git a/kio/kio/accessmanager.cpp b/kio/kio/accessmanager.cpp
index e535c8a..a64bb53 100644
--- a/kio/kio/accessmanager.cpp
+++ b/kio/kio/accessmanager.cpp
@@ -331,6 +331,21 @@ QNetworkReply *AccessManager::createRequest(Operation op, const \
QNetworkRequest break;
}
+ // If permanent storage of cookies is disabled, set "cookies" meta-data to
+ Integration::CookieJar* jar = \
qobject_cast<Integration::CookieJar*>(cookieJar()); + if (jar && \
jar->isCookieStorageDisabled()) { + metaData.insert(QL1S("cookies"), \
QL1S("manual")); + QStringList cookieValues;
+ Q_FOREACH(const QNetworkCookie& cookie, jar->cookiesForUrl(req.url())) {
+ cookieValues << cookie.toRawForm(QNetworkCookie::NameAndValueOnly);
+ }
+ if (!cookieValues.isEmpty()) {
+ QString cookieStr (QL1S("Cookie: "));
+ cookieStr += cookieValues.join(QL1S("; "));
+ metaData.insert(QL1S("setcookies"), cookieStr);
+ }
+ }
+
// Set the meta data for this job...
kioJob->setMetaData(metaData);
@@ -483,25 +498,30 @@ QList<QNetworkCookie> CookieJar::cookiesForUrl(const QUrl &url) \
const if (!d->isEnabled) {
return cookieList;
}
- QDBusInterface kcookiejar("org.kde.kded", "/modules/kcookiejar", \
"org.kde.KCookieServer");
- QDBusReply<QString> reply = kcookiejar.call("findDOMCookies", \
url.toString(QUrl::RemoveUserInfo), (qlonglong)d->windowId);
- if (!reply.isValid()) {
- kWarning(7044) << "Unable to communicate with the cookiejar!";
- return cookieList;
- }
+ if (!d->isStorageDisabled) {
+ QDBusInterface kcookiejar("org.kde.kded", "/modules/kcookiejar", \
"org.kde.KCookieServer"); + QDBusReply<QString> reply = \
kcookiejar.call("findDOMCookies", url.toString(QUrl::RemoveUserInfo), \
(qlonglong)d->windowId);
- const QString cookieStr = reply.value();
- const QStringList cookies = cookieStr.split(QL1S("; "), \
QString::SkipEmptyParts);
- Q_FOREACH(const QString& cookie, cookies) {
- const int index = cookie.indexOf(QL1C('='));
- const QString name = cookie.left(index);
- const QString value = cookie.right((cookie.length() - index - 1));
- cookieList << QNetworkCookie(name.toUtf8(), value.toUtf8());
- //kDebug(7044) << "cookie: name=" << name << ", value=" << value;
+ if (!reply.isValid()) {
+ kWarning(7044) << "Unable to communicate with the cookiejar!";
+ return cookieList;
+ }
+
+ const QString cookieStr = reply.value();
+ const QStringList cookies = cookieStr.split(QL1S("; "), \
QString::SkipEmptyParts); + Q_FOREACH(const QString& cookie, cookies) {
+ const int index = cookie.indexOf(QL1C('='));
+ const QString name = cookie.left(index);
+ const QString value = cookie.right((cookie.length() - index - 1));
+ cookieList << QNetworkCookie(name.toUtf8(), value.toUtf8());
+ //kDebug(7044) << "cookie: name=" << name << ", value=" << value;
+ }
+
+ return cookieList;
}
- return cookieList;
+ return QNetworkCookieJar::cookiesForUrl(url);
}
bool CookieJar::setCookiesFromUrl(const QList<QNetworkCookie> &cookieList, const \
QUrl &url) @@ -510,21 +530,20 @@ bool CookieJar::setCookiesFromUrl(const \
QList<QNetworkCookie> &cookieList, const return false;
}
- QDBusInterface kcookiejar("org.kde.kded", "/modules/kcookiejar", \
"org.kde.KCookieServer");
- Q_FOREACH(const QNetworkCookie &cookie, cookieList) {
- QByteArray cookieHeader ("Set-Cookie: ");
- if (d->isStorageDisabled && !cookie.isSessionCookie()) {
- QNetworkCookie sessionCookie(cookie);
- sessionCookie.setExpirationDate(QDateTime());
- cookieHeader += sessionCookie.toRawForm();
- } else {
+ if (!d->isStorageDisabled) {
+ QDBusInterface kcookiejar("org.kde.kded", "/modules/kcookiejar", \
"org.kde.KCookieServer"); +
+ Q_FOREACH(const QNetworkCookie &cookie, cookieList) {
+ QByteArray cookieHeader ("Set-Cookie: ");
cookieHeader += cookie.toRawForm();
+ kcookiejar.call("addCookies", url.toString(QUrl::RemoveUserInfo), \
cookieHeader, (qlonglong)d->windowId); + //kDebug(7044) << "[" << \
d->windowId << "]" << cookieHeader << " from " << url; }
- kcookiejar.call("addCookies", url.toString(QUrl::RemoveUserInfo), \
cookieHeader, (qlonglong)d->windowId);
- //kDebug(7044) << "[" << d->windowId << "]" << cookieHeader << " from " << \
url; +
+ return !kcookiejar.lastError().isValid();
}
- return !kcookiejar.lastError().isValid();
+ return QNetworkCookieJar::setCookiesFromUrl(cookieList, url);
}
void CookieJar::setDisableCookieStorage(bool disable)
diff --git a/kio/kio/accessmanager.h b/kio/kio/accessmanager.h
index 1b755b2..de91e8d 100644
--- a/kio/kio/accessmanager.h
+++ b/kio/kio/accessmanager.h
@@ -329,7 +329,7 @@ public:
bool setCookiesFromUrl(const QList<QNetworkCookie> &cookieList, const QUrl \
&url);
/**
- * Returns true if persistent caching of cookies is disabled.
+ * Returns true if permanent cookie storage is disabled.
*
* @see setDisableCookieStorage
* @since 4.6
@@ -339,9 +339,9 @@ public:
/**
* Prevent persistent storage of cookies.
*
- * Call this function if you do not want cookies to be stored locally for
- * later access without disabling the cookiejar. All cookies will be discarded
- * once the sessions that are using the cookie are done.
+ * Call this function if you do not want cookies to be stored permanently
+ * without completely disabling the cookiejar. All cookies will be discarded
+ * once this cookiejar instance is removed.
*
* @since 4.6
*/
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic