Request cancellation bug with chrome.webRequest

RESOLVED FIXED in Firefox 55

Status

()

Toolkit
WebExtensions: Request Handling
P2
normal
RESOLVED FIXED
2 months ago
2 months ago

People

(Reporter: Alexei, Unassigned)

Tracking

53 Branch
mozilla55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: [webRequest]triaged)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Reporter)

Description

2 months ago
Created attachment 8862227 [details]
Basic WebExtensions extension to help demonstrate the chrome.webRequest bug

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 https://dnt-test.trackersimulator.org/.well-known/dnt-policy.txt. 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.

Updated

2 months ago
Status: UNCONFIRMED → RESOLVED
Last Resolved: 2 months ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1345893
(Reporter)

Comment 2

2 months ago
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).

Updated

2 months ago
Status: RESOLVED → REOPENED
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: --- → ?

Updated

2 months ago
Priority: -- → P2
Whiteboard: [webRequest]triaged
Comment hidden (mozreview-request)
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 6

2 months ago
mozreview-review
Comment on attachment 8863897 [details]
Bug 1360052 handle cancel during http-on-examine-response,

https://reviewboard.mozilla.org/r/135618/#review139250

::: 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 hidden (mozreview-request)

Comment 9

2 months ago
mozreview-review
Comment on attachment 8863897 [details]
Bug 1360052 handle cancel during http-on-examine-response,

https://reviewboard.mozilla.org/r/135618/#review140114
Attachment #8863897 - Flags: review?(honzab.moz) → review+

Comment 10

2 months ago
Pushed by mixedpuppy@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/0087592da59a
handle cancel during http-on-examine-response, r=mayhemer

Comment 11

2 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0087592da59a
Status: REOPENED → RESOLVED
Last Resolved: 2 months ago2 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.