Ubuntu

Regression: Firefox 3.6.18 does not set cookie when talking to single letter hostname

Reported by Etienne Goyer on 2011-06-24
24
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Mozilla Firefox
Fix Released
Medium
firefox (Ubuntu)
Undecided
Chris Coulson
Lucid
Medium
Micah Gersten
Maverick
Medium
Micah Gersten

Bug Description

When talking to an hostname consisting of a single letter, Firefox 3.6.18 does not set cookie. Previous version did fine, as well as other browser such as chromium.

How to reproduce:

sudo apt-get install lighttpd
sudo lighttpd-enable-mod cgi
sudo vim /etc/hosts

make 'a' go to 127.0.0.1 as in:

root@machine:/var/www# head -n1 /etc/hosts
127.0.0.1 localhost a

Then simply install the attached python cgi and go to it in
firefox-3.6.18. In our tests it never set a cookie. When testing with
Chrome, it sets the cookie fine.

root@machine:/var/www# lsb_release -rd
Description: Ubuntu 10.04.2 LTS
Release: 10.04

root@machine:/var/www# apt-cache policy firefox
firefox:
 Installed: 3.6.18+build2+nobinonly-0ubuntu0.10.04.1
 Candidate: 3.6.18+build2+nobinonly-0ubuntu0.10.04.1
 Version table:

I expected my cgi script to print something like:
The returned cookie string was "foo=1308946645.14"

and not:

First visit or cookies disabled.

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

Changed in firefox (Ubuntu):
assignee: nobody → Chris Coulson (chrisccoulson)
Micah Gersten (micahg) on 2011-06-24
tags: added: lucid maverick regression-update
Changed in firefox (Ubuntu):
status: New → Invalid
Changed in firefox (Ubuntu Lucid):
assignee: nobody → Chris Coulson (chrisccoulson)
Changed in firefox (Ubuntu):
assignee: Chris Coulson (chrisccoulson) → nobody
Changed in firefox (Ubuntu Maverick):
importance: Undecided → Medium
Chris Coulson (chrisccoulson) wrote :

Not an issue in 5.0

Changed in firefox (Ubuntu):
assignee: nobody → Chris Coulson (chrisccoulson)
Micah Gersten (micahg) on 2011-06-24
Changed in firefox (Ubuntu Lucid):
importance: Undecided → Medium
status: New → Triaged
Changed in firefox (Ubuntu Maverick):
status: New → Triaged
Changed in firefox:
importance: Unknown → Medium
status: Unknown → Confirmed

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

Created attachment 542125
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.

Created attachment 542128
Test for mozilla-central

apparently not a problem on trunk, where we took a different patch for bug 616264

Comment on attachment 542125
Fix bug 667087.patch

Looks great to me. Nice fix! r=dwitte

Comment on attachment 542128
Test for mozilla-central

r=dwitte

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.

Micah Gersten (micahg) on 2011-06-28
summary: - Regression: Firefox 3.6.18 do not set cookie when talking to single
+ Regression: Firefox 3.6.18 does not set cookie when talking to single
letter hostname

Comment on attachment 542125
Fix bug 667087.patch

Approved for 1.9.2.19

Micah Gersten (micahg) wrote :

Chris's patch has been accepted upstream, I'll get the updates built and tested. Upstream has requested we wait to release the fix until the patch actually lands on the 1.9.2 branch before releasing to make sure that the unit tests pass.

Changed in firefox (Ubuntu Lucid):
assignee: Chris Coulson (chrisccoulson) → Micah Gersten (micahg)
importance: Medium → Undecided
status: Triaged → In Progress
importance: Undecided → Medium
Changed in firefox (Ubuntu Maverick):
assignee: nobody → Micah Gersten (micahg)
status: Triaged → In Progress
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package firefox - 3.6.18+build2+nobinonly-0ubuntu0.10.04.2

---------------
firefox (3.6.18+build2+nobinonly-0ubuntu0.10.04.2) lucid-security; urgency=low

  * fix LP: #801778 - Regression: Firefox 3.6.18 does not set cookie when
    talking to single letter hostname
    - add debian/patches/lp801778_bz667087_att542125-fix-cookie-regression.patch
    - update debian/patches/series
 -- Micah Gersten <email address hidden> Tue, 28 Jun 2011 23:14:35 +0100

Changed in firefox (Ubuntu Lucid):
status: In Progress → Fix Released
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package firefox - 3.6.18+build2+nobinonly-0ubuntu0.10.10.2

---------------
firefox (3.6.18+build2+nobinonly-0ubuntu0.10.10.2) maverick-security; urgency=low

  * fix LP: #801778 - Regression: Firefox 3.6.18 does not set cookie when
    talking to single letter hostname
    - add debian/patches/lp801778_bz667087_att542125-fix-cookie-regression.patch
    - update debian/patches/series
 -- Micah Gersten <email address hidden> Tue, 28 Jun 2011 23:21:04 +0100

Changed in firefox (Ubuntu Maverick):
status: In Progress → Fix Released
Changed in firefox:
status: Confirmed → Fix Released

Verified fixed using 1.9.2 testcase for 1.9.2.20.

(In reply to comment #12)
> Verified fixed using 1.9.2 testcase for 1.9.2.20.

Changing the resolution to verified.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.