1817450 - Missing webRequest events on the page that uses service worker

Status: REOPENED

(WebExtensions :: Request Handling, defect, P3)

Description

[Vladimir Pastukhov]

Attachment: Test extension

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:109.0) Gecko/20100101 Firefox/110.0

Steps to reproduce:

Response-related webRequest events (onHeadersReceived and the following) are not fired if the website uses service worker for loading its pages. One notable example: YouTube.

1. Open about:debugging#/runtime/this-firefox
2. Unregister YouTube service worker if you have one in the list.
3. Load the attached extension, which simply logs webRequest events.
4. Click Inspect and select Console.
5. Open https://www.youtube.com/ in the new tab, save the log.
6. Verify that the YouTube service worker is registered now.
7. Close the tab, clear the console.
8. Repeat step 5.

Actual results:

The test extension logs event name, request id and type, tab id/frame id, and the URL.
Only https://www.youtube.com/ URL is of interest, just filter out the others.

After step 5:
22:08:19.625 onBeforeRequest : 23 main_frame 2/0 https://www.youtube.com/
22:08:19.630 onBeforeSendHeaders : 23 main_frame 2/0 https://www.youtube.com/
22:08:19.900 onHeadersReceived : 23 main_frame 2/0 https://www.youtube.com/
22:08:19.902 onResponseStarted : 23 main_frame 2/0 https://www.youtube.com/
22:08:20.529 onCompleted : 23 main_frame 2/0 https://www.youtube.com/

After step 8:
22:10:07.306 onBeforeRequest : 187 main_frame 3/0 https://www.youtube.com/
22:10:07.306 onBeforeSendHeaders : 187 main_frame 3/0 https://www.youtube.com/
22:10:07.316 onBeforeRequest : 188 main_frame -1/0 https://www.youtube.com/
22:10:07.317 onBeforeSendHeaders : 188 main_frame -1/0 https://www.youtube.com/
22:10:07.471 onHeadersReceived : 188 main_frame -1/0 https://www.youtube.com/
22:10:07.471 onResponseStarted : 188 main_frame -1/0 https://www.youtube.com/
22:10:08.172 onCompleted : 188 main_frame -1/0 https://www.youtube.com/

Expected results:

As you can see, the request # 187 from my log is never completed. According to the web extensions API spec, either onCompleted or onErrorOccurred event must be fired for all requests. The request # 187 is initiated by the browser itself, whilst # 188 - by the service worker (tab id -1 suggest this).

When browser extensions keep records of the active requests and rely on onCompleted & onErrorOccurred to release objects, this flaw can lead to memory leaks, not to mention other malfunctions. I also suppose this bug can affect other request types than just main_frame.

I must note here that in Chrome this second request doesn't even exist. Either Chrome manages it transparently, or YouTube works differently in Chrome. Personally, as the extensions developer, I would prefer the Chrome's behavior, because I don't see why I could be interested in having these duplicate requests.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'WebExtensions::Request Handling' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Alex Cornestean]

Hello,

I reproduced the issue on the latest Nightly (112.0a1/20230219211858), Beta (111.0b3/20230219190218) and Release (110.0/20230214051806) under Windows 10 x64 and Ubuntu 16.04 LTS.

I can confirm that after Step (8) a request (#412 in my case) is never completed. See below example:

After Step (5):

onBeforeRequest : 220 main_frame 3/0 https://www.youtube.com/ background.js:4:34
onBeforeSendHeaders : 220 main_frame 3/0 https://www.youtube.com/ background.js:4:34
onHeadersReceived : 220 main_frame 3/0 https://www.youtube.com/ background.js:4:34
onResponseStarted : 220 main_frame 3/0 https://www.youtube.com/ background.js:4:34
onCompleted : 220 main_frame 3/0 https://www.youtube.com/ background.js:4:34

This second set of requests is logged as a results of me accepting the cookie policy. If I don’t do this the YouTube service worker will not be registered as part of Step (6)

onBeforeRequest : 310 main_frame 3/0 https://www.youtube.com/ background.js:4:34
onBeforeSendHeaders : 310 main_frame 3/0 https://www.youtube.com/ background.js:4:34
onHeadersReceived : 310 main_frame 3/0 https://www.youtube.com/ background.js:4:34
onResponseStarted : 310 main_frame 3/0 https://www.youtube.com/ background.js:4:34
onCompleted : 310 main_frame 3/0 https://www.youtube.com/ background.js:4:34

After Step (8):

onBeforeRequest : 412 main_frame 4/0 https://www.youtube.com/ background.js:4:34
onBeforeSendHeaders : 412 main_frame 4/0 https://www.youtube.com/ background.js:4:34

onBeforeRequest : 413 main_frame -1/0 https://www.youtube.com/ background.js:4:34
onBeforeSendHeaders : 413 main_frame -1/0 https://www.youtube.com/ background.js:4:34
onHeadersReceived : 413 main_frame -1/0 https://www.youtube.com/ background.js:4:34
onResponseStarted : 413 main_frame -1/0 https://www.youtube.com/ background.js:4:34
onCompleted : 413 main_frame -1/0 https://www.youtube.com/ background.js:4:34

Comment 4

[Vladimir Pastukhov]

Rob,
How come you changed the status to RESOLVED when this bug is easily reproducible, see comment #2 ?

Comment 5

[Rob Wu [:robwu]]

(In reply to Vladimir Pastukhov from comment #4)

Rob,
How come you changed the status to RESOLVED when this bug is easily reproducible, see comment #2 ?

I misread the bug and thought that it was claiming that service worker requests are not intercepted. But that was added in bug 1599941, so I marked it as a duplicate. But after reading the STR again, it looks like a different issue.

The extension to reproduce the bug is missing one event listener. When I modify your STR and add the onBeforeRedirect event, I do see a more understandable sequence of events.

I used the following STR:

1. Load extension (with onBeforeRedirect added to the list of events in background.js)
2. Visit https://www.youtube.com/ and approve all cookies in YouTube's cookie banner. This is required before YouTube registers a service worker.
3. Close YouTube, clear the console to start anew.
4. Visit https://www.youtube.com/robots.txt (a simple page without subresources, so that it is easier to spot requests of interest without noise).
5. Look at the console.

Result:

onBeforeRequest     : 656 main_frame 8/0 https://www.youtube.com/robots.txt background.js:3:38
onBeforeSendHeaders : 656 main_frame 8/0 https://www.youtube.com/robots.txt background.js:3:38
onBeforeRedirect    : 656 main_frame 8/0 https://www.youtube.com/robots.txt background.js:3:38
onBeforeRequest     : 657 main_frame -1/0 https://www.youtube.com/robots.txt background.js:3:38
onBeforeSendHeaders : 657 main_frame -1/0 https://www.youtube.com/robots.txt background.js:3:38
onHeadersReceived   : 657 main_frame -1/0 https://www.youtube.com/robots.txt background.js:3:38
onResponseStarted   : 657 main_frame -1/0 https://www.youtube.com/robots.txt background.js:3:38
onCompleted         : 657 main_frame -1/0 https://www.youtube.com/robots.txt

What happens is that the request is internally redirected to the service worker.
As you noted, the webRequest API usually ends a request for a given requestId with either onCompleted or onErrorOccurred.
In this case, that is not happening, which is a bug.

Comment 6

[Vladimir Pastukhov]

(In reply to Rob Wu [:robwu] from comment #5)

According to the onBeforeRedirect API specification this event is "Fired when a server-initiated redirect is about to occur". In this case with the service worker, no server-initiated redirect is performed, and this internal redirect with identical source and target URLs and missing HTTP response code makes no sense at all.

Moreover, this sequence does not comply with the established event life cycle. onBeforeRedirect should be issued after onHeadersReceived (or onBeforeRequest in case of extension-initiated redirection), and should be followed by another onBeforeRequest having the new URL and the same request Id. Whoever invented the current behavior did not adhere to the spec.

Therefore, the whole existence of onBeforeRedirect in the event chain of the request in question, can itself be considered as a separate bug.

Comment 7

[Rob Wu [:robwu]]

(In reply to Vladimir Pastukhov from comment #6)

(In reply to Rob Wu [:robwu] from comment #5)

According to the onBeforeRedirect API specification this event is "Fired when a server-initiated redirect is about to occur". In this case with the service worker, no server-initiated redirect is performed, and this internal redirect with identical source and target URLs and missing HTTP response code makes no sense at all.

While originally designed to handle true server-side redirects, there are APIs to simulate server-side redirects:

• blocking webRequest event handler that returns redirectUrl (after onBeforeRequest, onBeforeSendHeaders, onHeadersReceived).
• declarativeNetRequest API with redirect action.
• Service worker interception.
• https upgrades (e.g. HSTS, CSP upgrade-insecure-requests, HTTPS First)

The reported onBeforeRedirect event does unfortunately not offer enough detail for service worker intercepted responses.

Moreover, this sequence does not comply with the established event life cycle. onBeforeRedirect should be issued after onHeadersReceived (or onBeforeRequest in case of extension-initiated redirection), and should be followed by another onBeforeRequest having the new URL and the same request Id. Whoever invented the current behavior did not adhere to the spec.

Underneath the diagram with the typical behavior is the message "onErrorOccurred can fire at any time during the request. Also, note that sometimes the sequence of events may differ from this. For example, in Firefox, on an HSTS upgrade, the onBeforeRedirect event is triggered immediately after onBeforeRequest". We should improve the documentation to emphasize that onBeforeRedirect can fire at any stage before onResponseStarted.

Therefore, the whole existence of onBeforeRedirect in the event chain of the request in question, can itself be considered as a separate bug.

The existence of the onBeforeRedirect event is not a bug. The fact that there is no onCompleted / onErrorOccurred event is a bug.