Last Comment Bug 667087 - Cookies cannot be stored with single character hostnames
: Cookies cannot be stored with single character hostnames
Status: VERIFIED FIXED
: regression, reproducible, verified1.9.2
Product: Core
Classification: Components
Component: Networking: Cookies (show other bugs)
: 1.9.2 Branch
: All Linux
: -- normal (vote)
: ---
Assigned To: :Ehsan Akhgari
:
: Patrick McManus [:mcmanus]
Mentors:
Depends on:
Blocks: 650522
  Show dependency treegraph
 
Reported: 2011-06-24 15:23 PDT by Chris Coulson
Modified: 2011-08-04 07:23 PDT (History)
14 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected
.20+
.20-fixed


Attachments
Fix bug 667087.patch (3.06 KB, patch)
2011-06-27 04:45 PDT, Chris Coulson
dwitte: review+
christian: approval1.9.2.20+
Details | Diff | Splinter Review
Test for mozilla-central (921 bytes, patch)
2011-06-27 04:59 PDT, Chris Coulson
dwitte: review+
Details | Diff | Splinter Review

Description Chris Coulson 2011-06-24 15:23:42 PDT
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
Comment 1 Chris Coulson 2011-06-24 17:31:52 PDT
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
Comment 2 Chris Coulson 2011-06-27 04:45:48 PDT
Created attachment 542125 [details] [diff] [review]
Fix bug 667087.patch

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.
Comment 3 Chris Coulson 2011-06-27 04:59:27 PDT
Created attachment 542128 [details] [diff] [review]
Test for mozilla-central
Comment 4 Daniel Veditz [:dveditz] 2011-06-27 10:26:05 PDT
apparently not a problem on trunk, where we took a different patch for bug 616264
Comment 5 dwitte@gmail.com 2011-06-28 13:06:57 PDT
Comment on attachment 542125 [details] [diff] [review]
Fix bug 667087.patch

Looks great to me. Nice fix! r=dwitte
Comment 6 dwitte@gmail.com 2011-06-28 13:07:20 PDT
Comment on attachment 542128 [details] [diff] [review]
Test for mozilla-central

r=dwitte
Comment 7 :Ehsan Akhgari 2011-06-28 14:35:23 PDT
The ubuntu folks want this badly.  Can we get the approval on this one soon, please?
Comment 8 :Ehsan Akhgari 2011-06-28 14:41:35 PDT
I landed the m-c test on inbound.  Please do not mark the bug as FIXED when you merge to central.
Comment 9 christian 2011-06-28 14:46:13 PDT
Comment on attachment 542125 [details] [diff] [review]
Fix bug 667087.patch

Approved for 1.9.2.19
Comment 10 Marco Bonardo [::mak] 2011-06-29 02:22:44 PDT
test merged http://hg.mozilla.org/mozilla-central/rev/beadf61d53a7
Comment 12 Al Billings [:abillings] 2011-07-26 16:29:53 PDT
Verified fixed using 1.9.2 testcase for 1.9.2.20.
Comment 13 AndreiD[QA] 2011-08-04 07:23:00 PDT
(In reply to comment #12)
> Verified fixed using 1.9.2 testcase for 1.9.2.20.

Changing the resolution to verified.

Note You need to log in before you can comment on or make changes to this bug.