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)
Core
Networking
Tracking
()
VERIFIED
FIXED
M12
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
Comment 1•25 years ago
|
||
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.
Reporter | ||
Comment 2•25 years ago
|
||
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.
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.
Status: NEW → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Updated•25 years ago
|
Status: RESOLVED → REOPENED
Comment 7•25 years ago
|
||
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?
Comment 9•25 years ago
|
||
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
Reporter | ||
Comment 10•25 years ago
|
||
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
Comment 11•25 years ago
|
||
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.
Comment 12•25 years ago
|
||
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
Comment 13•25 years ago
|
||
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.
Reporter | ||
Comment 14•25 years ago
|
||
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.
Comment 15•25 years ago
|
||
*** Bug 12087 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 16•25 years ago
|
||
paulmac why was this reopened? are we still seeing this problem?
Updated•25 years ago
|
Status: REOPENED → RESOLVED
Closed: 25 years ago → 25 years ago
Resolution: --- → FIXED
Comment 17•25 years ago
|
||
this was re-opened because the stated URL above was not working. However, it is
working now with 9/22 seamonkey, so marking fixed.
Updated•25 years ago
|
Status: RESOLVED → VERIFIED
Comment 18•25 years ago
|
||
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.
Description
•