21197 - [dogfood] Cookies register o.k., but too late for browser to recognize? (asynch/cookie issue?)

Status: VERIFIED FIXED

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

Description

[rxsherm]

First let me describe the typical Netscape 4.5, IE behavior :

If I visit the URL above, and enter the user id "mozilla" followed by the
password of "ummm" and click "login" it calls a CGI (stlogin.cgi) which sets
three cookies : LOGIN, USER, and PASSWD and then immediate sets a redirect (ala
location: ) to another CGI (stmain.cgi) which checks that the LOGIN cookie is
set to YES. If it is, (which it should now be) then the CGI will continue
processing and display a "console" web page. If it is not set to YES (or not set
at all) then the CGI will send another redirect to throw the user back at the
main screen (because it assumes that the user is trying to access the cgi
directly without first authenticating)

When I do this in Netscape 4.5 or IE, I go from the login screen to the console
screen with no problem.

When I do this with Mozilla (Build ID: 1999120808) the stmain.cgi cannot get the
cookie information that has been set, and redirects me back to the main page.
Using "Show cookies" reveals that the cookies are there... And if I go back to
the login URL and try again with the same user name and password, the second
attempt lets me in...

It almost seems that mozilla is handling the redirect from the stlogin.cgi and
starts working on the stmain.cgi before it has completely processed the "set
cookie" calls.

Comment 1

[leger]

tever, can you provide more info...what latest build show...morse, any input?

Comment 2

[Stephen P. Morse]

It's happening in today's build

Comment 3

[Stephen P. Morse]

rxsherm's guess as to what is happening is indeed correct.  From a sniffer I see
that:

1. http request is made to stlogin.cgi
2. http response for stlogin.cgi sets three cookies and specifies a redirect to
stmain.cgi
3. http request to stmain.cgi does not include any cookies.

From the debugger I see that:

1. No cookies were found in cookie list at the point of making http request to
stlogin.cgi

2. No cookies were found in the cookie list at the point of making http request
to stmain.cgi

3. Three cookies were set *after* the http request to stmain.cgi was made

This sounds like a necko problem in that necko is notifying its listeners too
late that a response is available, and consequently the cookies are getting set
too late.  I seem to recall that we had this problem before and that Judson
fixed it.  Jud, can you shed some light here?

Comment 4

[leger]

Putting on PDT- radar.  But if this happens with a top 100 site, please
re-submit to PDT.

Comment 5

[chris hofmann]

moving off the m12 radar.  let me know if a good understanding
of the problem and fix appears in the next couple of days.

Comment 6

[Judson Valeski]

This test case is fried. I'm getting a 500 error back from the server in 4.x and
5.0

Comment 7

[rxsherm]

It's an ISP outage... I'm working on getting it resolved.

--Roby

Comment 8

[rxsherm]

It's back up now... A syad must have been dinking with the web server config.
Sorry about that. --Roby

Comment 9

[Judson Valeski]

I have a fix for this bug regression impact may be large. I'm assuming ownership
of this bug.

Here' the fix:

nsHTTPChannel::FinishedResponseHeaders()
...
    // Notify the consumer that headers are available...
    OnHeadersAvailable();
    mFiredOnHeadersAvailable = PR_TRUE;

    //
    // Check the status code to see if any special processing is necessary.
    //
    // If a redirect (ie. 30x) occurs, the mResponseDataListener is
    // released and a new request is issued...
    //
    rv = ProcessStatusCode();
...
}

we used to processStatusCode() *before* handling headers.

Comment 10

[rpotts (gone)]

Hey Jud,
I think that this fix looks safe (and correct :-).  The main impact of this
change is that nsIHTTPNotify::ExamineAsyncResponse(...) will be called for all
URIs (including those with 30x or 40x status codes).  Since the cookie code is
the only consumer of nsIHTTPNotify::ExamineAsyncResponse(...) I believe that
there should be little impact - other than more cookies being saved :-)

-- rick

Comment 11

[Stephen P. Morse]

Jud, thanks for taking this bug from me.  But when doing so, you need to either
notify the old owner that you are doing it, or put the old owner on the cc list.
There's a bug in bugzilla whereby the person being removed from owner does not
get any further notifications whenever changes are made to the bug report,
including the change involving his removal.  I was following this bug and then
silently got removed from the discussion and I couldn't even find the bug again
without doing a search.

I just got assigned another bug which, after analyzing it, realized that it is a
dup of this one -- cookies getting set after the redirect.  Will mark it as a
dup.

Comment 12

[Stephen P. Morse]

*** Bug 21046 has been marked as a duplicate of this bug. ***

Comment 13

[Stephen P. Morse]

Removing PDT- designation so that this bug gets reconsidered.  I don't know if
it occurs on any of the top 100 sites, but it is going to occur on a lot of
sites out there and make it hard to be able to use the product as dogfood.
There have already been two bug reports filed on this so at least two testers
have run into two independent sites of this type.

Comment 14

[leger]

Putting on the PDT- radar.  We need specific sites that are critical and/or
numerous.

Comment 15

[Paul MacQuiddy]

my.yahoo.com is an example of a site that gives you a cookie error when you
register, I am sure it is the same problem. Removing PDT for further
consideration, you can't get much bigger than yahoo

Comment 16

[James Everingham]

Ok, I've had an emergency PDT council meeting for this bug specifically.
Approved.

Comment 17

[Judson Valeski]

fix checked in 12/14/99 3:05 pm pac time.

Comment 18

[varada]

Marking as verified on Windows build - 1999121508

Comment 19

[Scott Furman]

This was my bad.  The change not to fire OnHeadersAvailable in the HTTP protocol
handler was deliberate, so as to make redirects transparent to clients of HTTP.