Closed Bug 667087 Opened 8 years ago Closed 8 years ago
Cookies cannot be stored with single character hostnames
I'm reporting this from https://bugs.launchpad.net/ubuntu/+source/firefox/+bug/801778 Since Firefox 3.6.18, cookies cannot be stored with single character hostnames. This is reproducible by installing the extension developer addon and doing this in the JS shell: cm = Cc["@mozilla.org/cookiemanager;1"].getService(Ci.nsICookieManager2); expiry = (Date.now() + 1000) * 1000; cm.countCookiesFromHost("a"); /* Should be 0 */ cm.add("a", "/", "foo", "bar", false, false, true, expiry); cm.countCookiesFromHost("a"); /* Is still 0, but should now be 1 */ This wasn't a problem in 3.6.17. It doesn't seem to be a problem on current nightlies or aurora either (I didn't test 5.0) I see there's a couple of suspect commits between 3.6.17 and 3.6.18, but I'm still waiting for my mozilla-1.9.2 tree to build before I can actually verify that: http://hg.mozilla.org/releases/mozilla-1.9.2/rev/bb935ffe5ff1 http://hg.mozilla.org/releases/mozilla-1.9.2/rev/bb728fdcd717
It's definitely this one which breaks it: http://hg.mozilla.org/releases/mozilla-1.9.2/rev/bb935ffe5ff1 When running the first iteration of the loop in nsCookieService::CountCookiesFromHostInt, (nextDot <= end && *(nextDot + 1) == '\0') always evaluates to true, so it just bails out
Here is a patch which fixes this, with test-case. I'm not sure if it's the best way to fix this without understanding what http://hg.mozilla.org/releases/mozilla-1.9.2/rev/bb728fdcd717 is for.
apparently not a problem on trunk, where we took a different patch for bug 616264
Comment on attachment 542125 [details] [diff] [review] Fix bug 667087.patch Looks great to me. Nice fix! r=dwitte
Attachment #542125 - Flags: review?(dwitte) → review+
Comment on attachment 542128 [details] [diff] [review] Test for mozilla-central r=dwitte
Attachment #542128 - Flags: review?(dwitte) → review+
The ubuntu folks want this badly. Can we get the approval on this one soon, please?
I landed the m-c test on inbound. Please do not mark the bug as FIXED when you merge to central.
Whiteboard: DO NOT MARK AS FIXED WHEN MERGING TO CENTRAL
Comment on attachment 542125 [details] [diff] [review] Fix bug 667087.patch Approved for 220.127.116.11
Attachment #542125 - Flags: approval18.104.22.168? → approval22.214.171.124+
Verified fixed using 1.9.2 testcase for 126.96.36.199.
(In reply to comment #12) > Verified fixed using 1.9.2 testcase for 188.8.131.52. Changing the resolution to verified.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.