Summary: | new domain-specific cookie policy: treat as session cookie | ||
---|---|---|---|
Product: | [Applications] konqueror | Reporter: | Gerd v. Egidy <gerd> |
Component: | kcookiejar | Assignee: | Dawit Alemayehu <adawit> |
Status: | RESOLVED FIXED | ||
Severity: | wishlist | CC: | co, DavidLBarbour, gaaf, greg_g, howland, krzysiek, yellow |
Priority: | NOR | ||
Version: | unspecified | ||
Target Milestone: | --- | ||
Platform: | RedHat Enterprise Linux | ||
OS: | Linux | ||
Latest Commit: | http://commits.kde.org/kde-baseapps/930e19b9a7244b45b9fc830b34a09fdc1b2fb3c8 | Version Fixed In: | 4.10 |
Sentry Crash Report: | |||
Attachments: |
kdelibs-cookie-policy.patch
kdebase-cookie-policy.patch cookie-policy-snapshot.png kdelibs-cookie-policy.patch kdebase-cookie-policy.patch kdelibs_Add-accept-for-session-as-a-new-policy-for-cookies.patch kde-baseapps_Adapt-cookie-configuration-dialog-to-new-policy.patch Add "accept for session" as a new policy for cookies Adapt cookie configuration dialog to new policy "accept for session" |
Description
Gerd v. Egidy
2003-02-08 17:03:23 UTC
Just wanted to add my support for this bug. In principle I'd like to reject all cookies except for a few. And those few I usually want to expire after the session. Perhaps it would be more logical to not let the "Treat all cookies as session cookies" option disable the "Site Policy". A way of dealing with this would be to add an 'expire' option to each "Site Policy". Let the user choose between "Expire after n-days" or "Expire after session". *** This bug has been confirmed by popular vote. *** *** Bug 97036 has been marked as a duplicate of this bug. *** With the current google+gmail mess using the same cookies to connect search info to emails, this wishlist item becomes more needed/wanted. Is there any progress/incentive on implementing this? This is the one feature that I return to Mozilla for - co-ercing cookies into session cookies on a per-site basis. It's useful when navigating to sites that just don't work without cookies, yet which have no business keeping track of what I do for all time. Obviously the workaround is to clear out cookies manually on a regular basis. I think that this feature would be very usefull. According to this request a wonderfull feature would be a little icon in status bar like the adblock one which is shown everytime a site wants to or saves a cookie. If you click on this icon you can choose to accept, reject, session or ask which is saved in settings. Regarding the interface, why not replace the current "Policy" tab's global settings with the following? Only accept cookies from originating server: Checked/Unchecked Long-duration cookies: Ask/Accept/Reject/Convert to Session Cookies Session cookies: Ask/Accept/Reject If you then copied that in the per-site policy "New.../Change..." dialog and added a "Use Global Settings" option, you'd have all the flexibility we love in Firefox's pre-extension setup. (With extensions, you get fun stuff like CustomizeGoogle's ability to make it more difficult for Google to track things) I think Stephan got it right in #7. At least it would cover my needs. Personally, I would set: Only accept cookies from originating server: Checked Long-duration cookies: Convert to Session Cookies Session cookies: Accept Then, in the per-site policy, I'd have a list of trusted sites (such as bugs.kde.org) that are allowed to store long-duration cookies. And a blacklist of sites that are not allowed to store any cookies at all. (My apologies if you don't care how I would configure my system: the point is that Stephan's suggestion seems workable.) Created attachment 31049 [details]
kdelibs-cookie-policy.patch
I think that it would be very convenient to set the "Treat as Session Cookie" policy for specific sites, or to set it as default with exceptions for specific sites. So I propose a patch to do that.
Main points of the patch (here for kdelibs, I will attach the kdebase part later):
- Currently the policy types are "Ask, Accept, Reject". With the patch a new policy "AcceptForSession" (treat as session cookie) is added.
- Currently the policy acts on two levels: there's the domain specific policy, and if that's unset there's the global policy. With the patch the policy acts on three levels: first on a single cookie level, then domain, then global. However the policy set for the single cookies is only meaningful for storing the AcceptForSession value, and is not saved on disk.
- The patch removes the "Treat as session cookies" global option, as it is not needed anymore. It also removes the "Auto accept session cookies" option. The latter is debatable, since one can think of a scenario where it could still be useful (accepting session cookies _but_ not accepting persistent cookies, not even for the session only), but this doesn't fit cleanly so I opted to remove it.
Hope this helps.
Created attachment 31050 [details]
kdebase-cookie-policy.patch
kdebase part of the patch.
Note: here I removed the use of KUrl::fromAce and KUrl::toAce functions (encoding and decoding of domain names). This was needed for a different problem: when the settings module uses those functions to read the configuration file and to write it back, it modifies the domain names by removing the leading dot. So, messing with the settings module will cause problems and testing the patch would be impossible. I will see how to deal with that separately.
Created attachment 31051 [details]
cookie-policy-snapshot.png
Snapshot to get a quick look at the effect of the patch.
Admittedly the "Do you want to accept..." dialog is really ugly... :/
Hi Gregorio, thank you for coding this up! I look forward to try it, I hope I'll find some time this week to set up the compile-environment. Kind regards, Gerd Created attachment 32505 [details] kdelibs-cookie-policy.patch Slightly updated patch against current svn. Added comment, whatstis, set a button icon for the dialog, removed the fromAce/toAce part (see bug 183720). kdelibs part of the patch, against r947557. Created attachment 32506 [details]
kdebase-cookie-policy.patch
kdebase part of the patch, against r947557.
*** Bug 155835 has been marked as a duplicate of this bug. *** Created attachment 57213 [details]
kdelibs_Add-accept-for-session-as-a-new-policy-for-cookies.patch
For the record, here are the patches regenerated to apply cleanly in 4.6.0, in 'git format-patch' format.
This is the patch for the kdelibs repository.
Created attachment 57214 [details]
kde-baseapps_Adapt-cookie-configuration-dialog-to-new-policy.patch
Patch for the kde-baseapps repository.
Gregorio, First thanks for providing a patch. It is rare that people provide their own patch for feature they want to have.;) Second, I had plans to commit a modified version of your patch to KDE 4.6 (see the feature plans page for KDE 4.6), but I did not have the time to do it. Unfortunately I doubt I will have the time to do it for KDE 4.7 either since there is way too much on my plate. However, I will try my best to get it in for KDE 4.8. In the mean time, there is one thing I do not like with your patch: You removed the ability to automatically accept session cookies. That is wrong. For example, I should be able to choose "Ask for confirmation" and still have the ability to automatically accept "session cookies". Changing the check box that treats all cookies as session cookies to a selectable option that does the same on per cookie basis should not deprive the other use cases of this functionality. If you believe otherwise, please explain why that should not be the case. I guess you removed it because it conflicted with the new option when you should have simply ignored it when the new option is selected. You can even disable the checkbox associated with it when the user selects the new option in the config dialog box. The other issue is the missing test cases. All those unit tests in kdelibs/kioslave/http/kcookiejar/tests have to be modified to reflect these changes. Anyhow, this should not be that much of a big deal to go through. The above issue however is important. Hi Dawit, thanks for your reply, it's really appreciated to get feedback after so much time. :) About the option to automatically accept session cookies, at the time I wrote the patch I thought a bit about possible use cases for keeping it, but didn't came up with anything really compelling and eventually left it out (there was some reasoning about it somewhere in comment #9). Actually, the problem was that it didn't fit nicely. I set the goal to have the same set of policies available both globally and per-site for cleanliness, and that was an exception as it was only global. But I agree that in fact there are possible use cases and it should be reintroduced. As you are planning to reimplement the feature, maybe you have some ideas on the best way to readd the option to accept session cookies? As a global checkbox above the policy panels as it is now, or maybe something different, even splitting the "default policy" panel into one policy for session cookies and one for persistent cookies? Anyway, if you give an indication on what you are planning and you are not already working on the code, I can give it a try. Thanks again! Created attachment 64647 [details]
Add "accept for session" as a new policy for cookies
I took some time to update and refine the patch for this feature. It would be very nice if this was considered for the current development cycle, either 4.8 or whatever the next version will be (this touches both kdelibs and kde-baseapps/konqueror).
I summarize here the main points of the patch:
- Currently the policy types are "Ask, Accept, Reject". With the patch a new
policy "AcceptForSession" (treat as session cookie) is added, with matching chages to the kcookiejar server (kdelibs) and to the configuration dialog (kde-baseapps/konqueror).
- The option "Treat as session cookies" is removed.
With respect to the previous version of the patch:
- The option "Auto accept session cookies" is retained and works as intended.
- I added specific test code to check for end-of-session situations, and added appropriate test cases for the patch functionalities.
- I reorganized the code a bit for consistency: before, the function cookieAdvice() was called only once before putting the cookies in the jar, then cookies inside the jar were treated in no uniform way (all cookies were Accepted once in the jar, there was no distinction between cookies treated as session-cookies and persistent cookies). Now, cookieAdvice() is used throughout the code to provide uniform treatment of the cookies in the jar based on configured policy: to decide whether to delete them at the end of the session, to send them back to a server, to save them on disk.
This is the kdelibs part of the patch. The kde-baseapps part will follow.
Created attachment 64648 [details]
Adapt cookie configuration dialog to new policy "accept for session"
kde-baseapps part of the patch.
By the way, while testing the patch I stumbled on a couple of small bugs in current git which I reported as bug 284297 and bug 284299 (with patches). Maybe Dawit could take a look at them as they relate to the work he is doing in konqueror. (In reply to comment #20) > Created an attachment (id=64647) [details] > Add "accept for session" as a new policy for cookies > > I took some time to update and refine the patch for this feature. It would be > very nice if this was considered for the current development cycle, either 4.8 > or whatever the next version will be (this touches both kdelibs and > kde-baseapps/konqueror). > > I summarize here the main points of the patch: > - Currently the policy types are "Ask, Accept, Reject". With the patch a new > policy "AcceptForSession" (treat as session cookie) is added, with matching > chages to the kcookiejar server (kdelibs) and to the configuration dialog > (kde-baseapps/konqueror). > - The option "Treat as session cookies" is removed. > > With respect to the previous version of the patch: > - The option "Auto accept session cookies" is retained and works as intended. > - I added specific test code to check for end-of-session situations, and added > appropriate test cases for the patch functionalities. > - I reorganized the code a bit for consistency: before, the function > cookieAdvice() was called only once before putting the cookies in the jar, then > cookies inside the jar were treated in no uniform way (all cookies were > Accepted once in the jar, there was no distinction between cookies treated as > session-cookies and persistent cookies). Now, cookieAdvice() is used throughout > the code to provide uniform treatment of the cookies in the jar based on > configured policy: to decide whether to delete them at the end of the session, > to send them back to a server, to save them on disk. > > This is the kdelibs part of the patch. The kde-baseapps part will follow. Thanks for your great work Gregorio! You have even added a unit test for testing your changes! Very nice. Makes our job that much easier. :) I am going to make sure this gets merged into whatever is going to become KDE 4.8 release as soon as I finish testing it on my system. I also have to first find out which branch will be used as a basis for KDE 4.8. It is probably going to be the frameworks branch, but I have to find that out too. (In reply to comment #20) > Created an attachment (id=64647) [details] > Add "accept for session" as a new policy for cookies > > I took some time to update and refine the patch for this feature. It would be > very nice if this was considered for the current development cycle, either 4.8 > or whatever the next version will be (this touches both kdelibs and > kde-baseapps/konqueror). > > I summarize here the main points of the patch: > - Currently the policy types are "Ask, Accept, Reject". With the patch a new > policy "AcceptForSession" (treat as session cookie) is added, with matching > chages to the kcookiejar server (kdelibs) and to the configuration dialog > (kde-baseapps/konqueror). > - The option "Treat as session cookies" is removed. > > With respect to the previous version of the patch: > - The option "Auto accept session cookies" is retained and works as intended. > - I added specific test code to check for end-of-session situations, and added > appropriate test cases for the patch functionalities. > - I reorganized the code a bit for consistency: before, the function > cookieAdvice() was called only once before putting the cookies in the jar, then > cookies inside the jar were treated in no uniform way (all cookies were > Accepted once in the jar, there was no distinction between cookies treated as > session-cookies and persistent cookies). Now, cookieAdvice() is used throughout > the code to provide uniform treatment of the cookies in the jar based on > configured policy: to decide whether to delete them at the end of the session, > to send them back to a server, to save them on disk. > > This is the kdelibs part of the patch. The kde-baseapps part will follow. Thanks for your great work Gregorio! You have even added a unit test for testing your changes! Very nice. Makes our job that much easier. :) I am going to make sure this gets merged into whatever is going to become KDE 4.8 release as soon as I finish testing it on my system. I also have to first find out which branch will be used as a basis for KDE 4.8. It is probably going to be the frameworks branch, but I have to find that out too. Git commit ef269fdee33113354776693089c26f73093658a5 by Dawit Alemayehu. Committed on 24/09/2012 at 08:31. Pushed by adawit into branch 'KDE/4.10'. Changed the global "Treat all cookies as session cookies" option to a per domain/cookie policy setting called "AcceptForSession". As the name implies the policy allows a user to accept a given cookie that would automatically expire it at the end of the current session regardless of the cookies' expiration date. The patch has a corresponding patch that takes care of the cookie configuration module and was provided by Regorio Guidi. CCMAIL:gregorio.guidi@gmail.com REVIEW:106618 M +1 -0 kioslave/http/kcookiejar/CMakeLists.txt M +49 -37 kioslave/http/kcookiejar/kcookiejar.cpp M +26 -11 kioslave/http/kcookiejar/kcookiejar.h A +11 -0 kioslave/http/kcookiejar/kcookiescfg.pl M +6 -0 kioslave/http/kcookiejar/kcookiescfg.upd M +6 -0 kioslave/http/kcookiejar/kcookieserver.cpp M +89 -48 kioslave/http/kcookiejar/kcookiewin.cpp M +5 -3 kioslave/http/kcookiejar/kcookiewin.h A +51 -0 kioslave/http/kcookiejar/tests/cookie_session.test M +3 -7 kioslave/http/kcookiejar/tests/cookie_settings.test M +11 -2 kioslave/http/kcookiejar/tests/kcookiejartest.cpp http://commits.kde.org/kdelibs/ef269fdee33113354776693089c26f73093658a5 Git commit 930e19b9a7244b45b9fc830b34a09fdc1b2fb3c8 by Dawit Alemayehu. Committed on 24/09/2012 at 08:59. Pushed by adawit into branch 'master'. Added configuration option for accepting cookies until end of session per globally (default) or per domain. This is the configuration part of the change that was made to the cookiejar with the following commit: http://commits.kde.org/kdelibs/ef269fdee33113354776693089c26f73093658a5 FIXED-IN:4.10 REVIEW:106619 M +31 -59 konqueror/settings/kio/kcookiespolicies.cpp M +2 -4 konqueror/settings/kio/kcookiespolicies.h M +192 -201 konqueror/settings/kio/kcookiespolicies.ui M +12 -6 konqueror/settings/kio/kcookiespolicyselectiondlg.h M +7 -0 konqueror/settings/kio/kcookiespolicyselectiondlg.ui http://commits.kde.org/kde-baseapps/930e19b9a7244b45b9fc830b34a09fdc1b2fb3c8 After all this time, I see you did not forget about this patch. I really appreciate your dedication and skills, seeing also all the other work you do for konqueror. Thanks Dawit! |