From Bugzilla Helper: User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:0.9.3) Gecko/20010802 BuildID: When a 401 is sent with a cookie in the headers mozilla doesn't send back the cookie until you hit cancel and reload the page. Reproducible: Always Steps to Reproduce: 1. Send 401 header and a cookie 2. Cookie doesn't get sent back until cancel is pressed and page is reloaded. Actual Results: Cookie not sent. Expected Results: Cookie should be sent. This basiclly breaks the a few select few auth schemes.
Need more details here. In particular a detailed set of steps for reproducing this, including the specific URL used.
http://www.burst.net/~nick/mozilla_cookie_bug/setcookie.cgi Wrote a script to demo this.
Confirming and accepting
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Target Milestone: --- → mozilla0.9.4
Here's what's happening: 1. Go to http://www.burst.net/~nick/mozilla_cookie_bug/setcookie.cgi 2. Click on "here" 3. This causes a GET to http://www.burst.net/~nick/mozilla_cookie_bug/auth.php 4. An authentication dialog is brought up although the page has not yet finished loading 5. Fill in the authentication ("test", "test") and press OK 6. The nsHTTPChannel::SetRequestHeader is entered to set cookie header 7. mIsPending is true so this routine does not execute (contains NS_ENSURE_TRUE) 8. Therefore the cookie header has not been updated and the server gets the wrong value for the cookie. So the problem is caused by mIsPending being true in step 7. It was set to TRUE by a preceding call to nsHTTPChannel::AsyncOpen and should have been set back to FALSE by a call to nsHTTPChannel::OnStopRequest. But the call to OnStopRequest never occured because the page never finished loading. A simple patch that would fix this is simply to remove the following line at the head of nsHttpChannel::SetRequestHeader NS_ENSURE_TRUE(!mIsPending, NS_ERROR_IN_PROGRESS); So I'm requesting a review of this patch. If the reviewer feels that there is a more fundamental problem here (such as why the page never stops loading), please suggest a patch to fix the underlying problem. cc'ing some networking people since the patch is in nsHTTPChannel, as well as some layout people since the problem is caused by the page not finishing loading at the time that the authentication dialog is displayed. rpotts, darin, please review
that assertion is there for a very good reason... a better fix would be to toggle the mIsPending flag around the call to OnModifyRequest.
Yes, this latter patch also fixes the problem. r=morse rpotts, please sr. Thanks.
Is this the only situation where an existing channel is reused? I'm wondering if there are any other situations where it is allowable to modify 'request attributes' after a connection has been initiated... If not, then i guess this approach is ok... Do we need to worry about the possibility of an OnModifyRequest listener calling Cancel(...) ? If so, could mIsPending get incorrectly set to PR_TRUE? Or will the proxy listener code defer calling nsHttpChannel::OnStopRequest(...) until after nsHttpChannel::ProcessAuthentication() has returned... If Cancelling is not an issue, then r/sr=rpotts
darin, can you answer the questions that rick raised?
authentication is the only case in which we reuse an existing http channel.. i also confirmed that it is the only other place in which we call OnModifyRequest. cancelation during an OnModifyRequest callback is almost a NOP because both mTransaction and mCacheReadRequest would be NULL (mStatus is however set). nsHttpChannel::Cancel should probably return an error if !mIsPending, since it doesn't make any sense to cancel a transaction that is not pending, or we could just make it a NOP.
nav triage team: Marking p3
Priority: -- → P3
Fix checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
we'll need this on the 0.9.2 branch as well (after a day or so of shakeout on the trunk).
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
taking from morse since he's going on vacation
Assignee: morse → pchen
Status: REOPENED → NEW
fix checked into branch
Status: NEW → RESOLVED
Last Resolved: 17 years ago → 17 years ago
Resolution: --- → FIXED
verified-on-trunk: WinNT4 2001082803 Linux rh6 2001082908 Mac os9 2001082908
*** Bug 99092 has been marked as a duplicate of this bug. ***
verified: Linux rh6 2001-09-24-04-0.9.4
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.