137079 - Account management does not seem to work

Status: VERIFIED FIXED

(Core :: Networking: Cookies, defect, P1)

Description

[Michael Sinz]

It seems like a cookie problem - but after logging into your account, which does
work, any other operation (such as lookup up the tracking information on a past
order) fails.

This fails under a build just before the 1.0 branch (I did the build a few days
before the branch).  It does have the fix for bug 134203 (another cookie
problem) however, it may be related since the problem only happens when the
cookie security setting is set to "Originating site only"

Once again, this is a regression from 0.9.9 release due to the fact that the
cookie rule is stricter than it was in 0.9.9.

More debugging to come in a day or so - I have some other commitments to keep.

Comment 1

[Michael Sinz]

Note that this could be a problem with the way Buy.com does its cookies but I
always run with this setting on any browser that lets me, including IE, and this
is the first I noticed it.

I checked, and the problem also existed before the patch for bug 134203 was done
so that fix did not break this.

I would think that this is something that needs to be worked on for the 1.0
release in addition to the trunk...

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

I added this to the Make RC1 Not Suck list.

Comment 3

[Michael Sinz]

What is the status on this bug?  I have not had a chance to dig deeper into it
(busy at work) but this looks like a potential major problem.

Comment 4

[Stephen P. Morse]

Attaching a patch, but first let me give some background explaination.

In 4.x, originating-server-only pref controlled whether or not a site could set 
a cookie.  But a non-originating-server could always be sent a cookie that had 
already been set.  This was actually a privacy loophole, reported to us by 
Richard Smith.

That loophole was never closed in 4.x but we did decide to correct for it in 
mozilla.  So the current codebase tests for originating server before sending as 
well as setting a cookie.

Furthermore, the test for determining whether we have the originating server 
before sending a cookie (in nsCookies.cpp) is as follows:

  if ((cookie_GetBehaviorPref() == PERMISSION_DontAcceptForeign) &&
      (!firstAddress || cookie_isForeign(address, firstAddress, ioService))) {

So if firstAddress is null, we consider this to be the same as not the 
originating server and therefore do not send the cookie.

The problem occuring in this bug report is that firstAddress was coming up null 
in certain cases in which we indeed had the originating server.  The fix (in the 
patch I'm about to attach) is to be more agressive in obtaining the firstAddress 
if it comes up null in our first attempt.

So let's consider risk.  The thing we don't want to change is the testing for 
non-originating server when it comes to setting cookies.  The patch presented 
here does not affect that code.  It affects the test when sending cookies only.  
And we are free to change the sending behavior because it is new in 6.0.  
Furthermore, the change is fail safe -- it increases rather than decreases the 
number of cases in which the originating-server-only test will pass.

Comment 5

[Stephen P. Morse]

Attachment: more aggessive test for determining originating server before sending cookies

Comment 6

[Stephen P. Morse]

There are numerous other bugs reported recently that have different symtpoms 
but also involve originating-server-only pref setting.  Most of them do not have 
test cases so I can't check them out, but I would expect that the patch posted 
here will correct those as well.  The other bugs are:

bug 136891: External javascript requests aren't passing cookie headers
bug 138560: Dynamic Stylesheets cause loss of cookies
bug 134625: cookies not always being sent when "originating server only"
bug 133817: Linked css files ignored when the href is a call to a script
bug 137979: Flash XML.load() method refuses to send cookie
bug 140668: Login and password not working properly, once logged in
bug 137200: save as webpage-complete saves the wrong CSS and JavaScript files
bug 129717: Cookies are not being past to the server when getting <LINK> href

Comment 7

[Stephen P. Morse]

Add one more bug to the list above:

bug 141246: cookies don't work if set to "originating server only"

Comment 8

[Stephen P. Morse]

Two more are:

bug 139866: Cookies not sent for session management
bug 133496: Cookies, javascript and "Enable cookies originating web site only"

Comment 9

[Stephen P. Morse]

and yet another:

bug 137931: Google Preferences not saved

Comment 10

[Stephen P. Morse]

and another

bug 144085: Mozilla looses session cookie

Comment 11

[Michael Sinz]

Well, I have tried the patch on the two sites I had problems with (buy.com being
the public one) and Mozilla now works (as in I can use the restrictive cookie
setting)

I would nominate this as an important fix as it really addresses the originating
server problem.  (The sites really were originating but something within the
browser got FirstURL wrong)

Comment 12

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 83241 [details] [diff] [review]
more aggessive test for determining originating server before sending cookies

r=pink

Comment 13

[Alec Flett]

Comment on attachment 83241 [details] [diff] [review]
more aggessive test for determining originating server before sending cookies

sr=alecf

Comment 14

[Randell Jesup [:jesup] (needinfo me)]

Please check this in ASAP to trunk as soon as it opens.  Please also request
approval from drivers and include the discussion of risk (and reward) for
incorporating this patch.

Comment 15

[Stephen P. Morse]

And another two:

bug 139072: Cookie policy changed between beta and RC1
bug 144161: No longer accumulate points at iWon

Comment 16

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 83241 [details] [diff] [review]
more aggessive test for determining originating server before sending cookies

a=[e-mail]
Please check into branch and trunk ASAP, or we'll need to rip this feature from
1.0

Comment 17

[Stephen P. Morse]

Checked in on trunk.  Awaiting pdt approval for branch checkin.

Comment 18

[Stephen P. Morse]

Still one more:

bug 144269: Save link target doesn't work in Yahoo mail

Comment 19

[Peter Trudelle]

nsbeta1+ per Nav triage team.

Comment 20

[Peter Trudelle]

fixed

Comment 21

[Michael Sinz]

I have verified that this fix works - both by manually applying the patch and
doing a fresh build from the trunk (I have not yet done the 1.0 branch build
test but then I don't know if it was approved to go into the branch)

Comment 22

[Michael Sinz]

See bug 144988 for ongoing issue for ongoing questions as to FirstURL
(nsIHttpChannel.GetDocumentURI()) being null.

Comment 23

[scottputterman]

adding adt1.0.0+ for checkin to the 1.0 branch. 

Comment 24

[Mike Shaver (:shaver emeritus)]

I checked this in on the branch.

Comment 25

[Tom Everingham]

verified branch

Comment 26

[Matthew Litwin]

I can confim this for www.nytimes.com

After an (apparently successful) login, no cookie is stored in the Manage Stored
Cookies list, and you can't access the rest of the site.

This is under the latest FreeBSD port of mozilla:

Mozilla/5.0 (X11; U; FreeBSD i386; en-US; rv:1.0.0) Gecko/20020715