Closed Bug 1360052 Opened 6 years ago Closed 6 years ago

Request cancellation bug with chrome.webRequest


(WebExtensions :: Request Handling, defect, P2)

53 Branch


(firefox55 fixed)

Tracking Status
firefox55 --- fixed
webextensions ?


(Reporter: alexeiatyahoodotcom+mzllbgzll, Unassigned)


(Whiteboard: [webRequest]triaged)


(2 files)

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.81 Safari/537.36

Steps to reproduce:

Cancelling a 3xx redirect request that also includes a Set-Cookie header still produces a cookie, even though the request gets cancelled. No cookies get set in this scenario in Chrome.

To reproduce, please review and install the attached WebExtension and then visit A cookie will get set even though the request was cancelled and no redirect took place.

The URL above responds with a 302 status code and a Location header. It also includes a Set-Cookie header.

The attached extension filters requests inside a blocking chrome.webRequest.onHeadersReceived listener. Redirects get cancelled and Set-Cookie headers get stripped.

I have not checked if chrome.webRequest.onBeforeRequest is similarly affected, it may or not be.

Actual results:

Cookie(s) get set.

Expected results:

No cookie(s) get set.
Closed: 6 years ago
Resolution: --- → DUPLICATE
I don't think this is a duplicate, as this is still an issue on firefox-54.0b2 (Beta) and firefox-55.0a1.en-US.linux-x86_64 (Nightly).
Ever confirmed: true
Flags: needinfo?(mixedpuppy)
Resolution: DUPLICATE → ---
I agree, bug 1345893 was specific to handling suspended channels during http-on-modify-request (onBeforeRequest), this appears to be specific to http-on-examine-* and may be unrelated to suspending the channel (ie. suspend is handled for on-examine-response at least).  

Observers apparently not handling async suspend in nsHttpChannel:

http-on-opening-request (gHttpHandler->OnOpeningRequest in httpchannel)
http-on-examine-merged-response (gHttpHandler->OnExamineMergedResponse in httpchannel)

I'm not sure yet if the issue here is async handling of merged-response, or if cookie handling happens in a different order on Firefox than on Chrome.

Because onHeadersReceived is the first potential place to examine a response, I think a webextension should have the opportunity to do something before cookies are handled.
Flags: needinfo?(mixedpuppy)
webextensions: --- → ?
Priority: -- → P2
Whiteboard: [webRequest]triaged
Comment on attachment 8863897 [details]
Bug 1360052 handle cancel during http-on-examine-response,

Honza, I haven't written a test for this (using the addon in this bug to test manually), just want feedback to see if this is fine then I'll add a test.

Specifically we're not handling cancel right after http-on-examine-response, so cookies still get set, then the cancel is handled at some point afterwards.
Attachment #8863897 - Flags: feedback?(honzab.moz)
Comment on attachment 8863897 [details]
Bug 1360052 handle cancel during http-on-examine-response,

::: netwerk/protocol/http/nsHttpChannel.cpp:2089
(Diff revision 1)
>          return NS_OK;
>      }
> +    // Check if request was cancelled during http-on-examine-response.
> +    if (mCanceled) {
> +        return mStatus;

No.  Please do this:

        if (mCanceled) {
            return CallOnStartRequest();
Attachment #8863897 - Flags: review-
Comment on attachment 8863897 [details]
Bug 1360052 handle cancel during http-on-examine-response,

rb is sooooooooooooo stupid!
Attachment #8863897 - Flags: review-
Attachment #8863897 - Flags: feedback?(honzab.moz)
Attachment #8863897 - Flags: feedback-
Comment on attachment 8863897 [details]
Bug 1360052 handle cancel during http-on-examine-response,
Attachment #8863897 - Flags: review?(honzab.moz) → review+
Pushed by
handle cancel during http-on-examine-response, r=mayhemer
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.