Closed Bug 11610 Opened 25 years ago Closed 25 years ago

cookie failure - redirect processing should occur after header processing

Categories

(Core :: Networking, defect, P3)

defect

Tracking

()

VERIFIED FIXED

People

(Reporter: hshaw, Assigned: gagan)

References

()

Details

Our current processing of HTTP redirects causes many ecommerce sites to work poorly with necko. The sequence of events for many sites is (let's use login as example) o present form with user/password fields o user enters data/submits o server processes form data, authenticates user, responds with set-cookie and redirects to real login CGI o we process the redirect before firing the OnHeadersAvailable() so the set-cookie is lost and the real login CGI thinks cookies are disabled. See mozilla/netwerk/protocol/http/src/nsHTTPResponseListener::FinishedResponseHeade rs for more details. Suggest processing headers available before processing redirect. - Hubie
If we call FireOnHeaderAvail() before the ProcessRedirect(), we'll wind up notifying the user's eventSink twice, which is not good. One option would be to propagate the response headers over to the "new" response (the one coming from the redirect); then we'd need to work out some kind of precedence scheme for headers.
One thing to keep in mind is that (at least in the case of cookies) you need need the Set-Cookie: headers to be processed (by the cookie module) before the new redirect request goes out, else the new redirect request won't have the correct Cookie: headers, causing login operation (the example in this case) to fail. BTW Just to see what would happen I stuck a FireOnHeaderAvail() before the ProcessRedirect(). It's probably doing bad things as you describe, but logons were working there weren't any catastrophic crashes.
Assignee: gagan → rpotts
Target Milestone: M10
I think it is reasonable to call FireOnHeadersAvailable twice. A client may have other listeners for any header and hence it does need to know what the original headers and the new (redirected) one's headers are. Assigning to rpotts and marking for M10.
Assignee: rpotts → gagan
Target Milestone: M10 → M9
This should be handled for M9. Taking it over and fixing...
Status: NEW → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Patch mailed to rpotts.
*** Bug 12262 has been marked as a duplicate of this bug. ***
Status: RESOLVED → REOPENED
Has this patch been checked in? I am using 8/20 M9 candidate builds and still can not login to the URL above. Also bug 12262, which was marked as a dup, still goes into an infinite loop. Re-opening bug. My guess is that the patch was not checked in?
Resolution: FIXED → ---
Clearing Fixed resolution due to reopen.
Gagan, Looks like potts couldn't get your patch in for whatever reason. Here's how you can do it yourself. checkout the pertinant files from the branch. cvs co -r SeaMonkey_M9_BRANCH mozilla/netwerk/protocol/http/whatever.x (you can do this with entire dirs or modules too) apply your changes to those files. cvs stat the one's you changed to confirm that they've got the M9 sticky tag. cvs commit them. Jud
When I looked at the M9 branch for netwerk/protocol/http/src/nsHTTPResponseListener.cpp It looked like Rick did get the patch in revision 1.47.2.3 date: 1999/08/21 08:54:21; author: rpotts%netscape.com; state: Exp; lines: +4 -3 bug #11610. Make sure that the OnHeadersAvailable notification is fired *even* if a http redirect will occur... The good news, is most of the cookies aren't lost anymore, since FireOnHeadersAvailable() gets called before the redirect. The bad news is the Set-Cookie:'s that come after the redirect (from the subsequent redirect request) are now lost when they weren't before. Or maybe I'm tired and am not reading the code properly. Re: bug #12262 , I think it may have been closed as a duplicate a little too soon. daria.mtv.com is a somewhat complicated page that includes java. I'm not sure if cookies are really the problem or something else. The cookies appeared to be set properly when I traced the code. I think that one requires more investigation and should be reopened. On a positive note, I took the patches from bug #'s 11610 and 11630 and put them in my local tip build (didn't check it in) and I can login to various sites I couldn't before. The particular site in this URL returns Set-Cookie: headers with the original HTTP response *and* with the redirected HTTP response. - Hubie
Target Milestone: M9 → M10
Moving to M10. We need to get M9 out the door and this will give you folks time to investigate. paulmac, please provide M9 Release Note.
hey gagan, I got your patch but I couldn't tell what was going on since it wasn't a context diff :-( It looked like you moved the call to ProcessStatusCode(...) after the call to FireOnHeadersAvailable(...)but I wasn't sure... Let me know what should be changed and I'll check it in... -- rick
I would think this is pretty serious since we are breaking many sites that used to work in M8. IMO, this should not be pushed to M10 but should be considered an M9 show stopper.
The current patch fixes most of the sites I've encountered. There's far fewer sites which set cookies after a redirect. I don't think fixing the problem with setting cookies after the redirect would be difficult to fix, but if you are really pressed for time, what's currently there should work for most sites.
*** Bug 12087 has been marked as a duplicate of this bug. ***
Blocks: 12833
Target Milestone: M10 → M12
paulmac why was this reopened? are we still seeing this problem?
Status: REOPENED → RESOLVED
Closed: 25 years ago25 years ago
Resolution: --- → FIXED
this was re-opened because the stated URL above was not working. However, it is working now with 9/22 seamonkey, so marking fixed.
Status: RESOLVED → VERIFIED
Bulk move of all Necko (to be deleted component) bugs to new Networking component.
You need to log in before you can comment on or make changes to this bug.