Closed Bug 578534 Opened 10 years ago Closed 8 years ago
.cookie Enabled does not take exceptions into account .
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:22.214.171.124) Gecko/20100407 Ubuntu/9.04 Shiretoko/3.5.9 Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:126.96.36.199) Gecko/20100407 Ubuntu/9.04 Shiretoko/3.5.9 If a website tries to check whether it can set cookies by looking at navigator.cookieEnabled when cookies are disabled globally, it will return false even if there is an exception allowing the site. This can result in being redirected to a error page claiming cookies are required, when the site is actually able to set cookies perfectly fine. Reproducible: Always Steps to Reproduce: 1. Uncheck "Accept cookies from sites" in preferences. 2. Add an exception to allow an affected site. 3. Go to the site. Actual Results: navigator.cookieEnabled will return false for the site even though it can set cookies. Expected Results: navigator.cookieEnabled should return true for this site, because it is able to set cookies.
Confirming Mozilla/5.0 (X11; Linux x86_64; en-US; rv:2.0b2pre) Gecko/20100711 Gentoo Firefox/4.0b2pre
Status: UNCONFIRMED → NEW
Ever confirmed: true
I don't know who to request review from.
Attachment #466171 - Flags: review?
Oh great a patch! <sarcasm> Maybe I would have confirmed and marked this bug a dupe of bug 230350 if I would have been allowed to do so. If you follow that link you will probably learn something about 'review process' and 'patience' there... </sarcasm> Seriously: thank you Sindre, this bug is annoying as hell!
Component: General → DOM: Core & HTML
Product: Firefox → Core
QA Contact: general → general
Version: unspecified → Trunk
This version uses nsCOMPtr every time, and resorts to the old behavior if any part of checking for an exception fails (The review in Bug 230350 makes it sound like this is desired). Also changed a bunch of other stuff so it hopefully conforms more with the Mozilla Coding style.
Uses do_QueryInterface instead of QueryInterface.
Library catalogs based on Polaris Library Systems software are one example of a type of site known to be vulnerable to this problem. (You can't even search the catalog if the cookie test fails -- it just tells you to enable cookies. A per-site exception is useless, because the test still fails.) For the same reason, if cookies are allowed globally but a site is blocked, the test will tell the site it can set cookies. (This version of the issue is less likely to cause problems, but I thought I'd mention it anyway.) I also created a testcase, here: http://cgi.galion.lib.oh.us/staff/tests/cookie-enablement.html All this page does is test navigator.cookiesEnabled and report the result (via document.write).
Dan, can you (or anyone else) please have a look? I know there are other important bugs, but this patch had its review requested half a year ago. Thank you. Sorry for bugspam.
Comment on attachment 474447 [details] [diff] [review] Patch v4 + Test v3 Sid, can you review this patch or solicit a fresh one if this has rotted too much? Or pass r? along to a better person? Thanks, /be
Attachment #474447 - Flags: review?(dwitte) → review?(sstamm)
Comment on attachment 474447 [details] [diff] [review] Patch v4 + Test v3 This looks great. A few nits: 1) Declare variables when you need them, not up front. 2) Names starting with 'm' are for member variables. 3) We should probably use the principal URI, just like we do in nsHTMLDocument::SetCookie. 4) The channel is unused by the callee, and the only other caller passes null, so probably better to pass null. I'll make those changes and get this checked in. r=me
Attachment #474447 - Flags: review?(sstamm) → review+
http://hg.mozilla.org/integration/mozilla-inbound/rev/ad8831aa105f Sindre, thank you for the patch! I'm sorry that it took so long to get someone to look at it. :(
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
The documentation of document.cookieEnabled needs to be updated. Notes/suggestions: 1. IE and Chrome also allow exceptions to the global cookie setting. However, IE has this issue as well, Chrome does not. Opera and Safari apparently do not allow exceptions. 2. Since cookieEnabled is not reliable, add a note that setting document.cookie and immediatly trying to read document.cookie is a more reliable method to check whether cookies can be set. P.S. Thank you, Boris!
Docs updated: https://developer.mozilla.org/en/DOM/window.navigator.cookieEnabled And added to Firefox 8 for developers.
You need to log in before you can comment on or make changes to this bug.