We have one unit test for webRequest.onBeforeRequest of WebSocket URLs (ws:): https://searchfox.org/mozilla-central/rev/dc4560dcaafd79375b9411fdbbaaebb0a59a93ac/toolkit/components/extensions/test/xpcshell/test_ext_webRequest_webSocket.js

To avoid regressions, we need to expand our test coverage:

• Add webRequestBlocking / "blocking" extraInfoSpec and verify that the WebSocket request can be blocked.
• Add listeners for events other than webRequest.onBeforeRequest and verify that they are triggered (or not triggered). The behavior should explicitly be tested to make sure that the implementation is stable.

In https://bugzilla.mozilla.org/show_bug.cgi?id=1631933#c13 it has been pointed out that the documentation of supported schemes (at https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/webRequest/RequestFilter) does not mention ws(s).

Once this bug has been resolved, we should double-check that ws(s) is mentioned as a supported scheme.

Are you interested in working on it to submit a patch? It is a very good first bug to work on, as the task is clear and it's a test-only change in a domain in which you are already familiar. I'll mentor you on the bug if you are interested.

In terms of the webRequest api ws/wss are aliases for http/https, in fact we only mangle the url we get from the channel[1] and handle wss? in filters/etc. That test is there just to ensure the alias works, otherwise this is purely http and we have plenty of coverage. The only place where additional testing should be necessary is if there is any special-case code paths in nsHttpChannel that I'm unaware of.

[1] https://searchfox.org/mozilla-central/rev/dc4560dcaafd79375b9411fdbbaaebb0a59a93ac/toolkit/components/extensions/webrequest/ChannelWrapper.cpp#532-536

I still think that it's worthwhile to have an explicit test for all possible events + supporting blocking response {cancel:true}, even if "ws(s):" is just an alias for "http(s):" internally.

WebSockets are kind of long-lived responses with keep-alive pings and (message) frames. Without testing I wouldn't claim that all events are fired as expected. And I wonder how webRequest.filterResponseData fits in there - would it see all frames? Could it interfere with the protocol? I have no idea.

Hi! I am working on implementing the tests described in this bug.

I was successfully able to implement a test confirming that the onBeforeSendHeaders event is triggered for web sockets. However, I am not yet able to trigger onSendHeaders event. I am still working on determining whether this is the behavior of the system or a failure of the test code.

I'm unclear on the process here; should I attempt to make a single patch that include tests for many webRequest events, or would it be preferable to split apart into smaller pieces such as individual tests?

Thanks for being patient with me, this would be my first contribution to the codebase.

(In reply to 0xACE from comment #4)

I'm unclear on the process here; should I attempt to make a single patch that include tests for many webRequest events, or would it be preferable to split apart into smaller pieces such as individual tests?

If unsure, create one big patch. If needed I can always ask to split later (or possibly suggest to reduce the amount of code duplication).

Thanks for being patient with me, this would be my first contribution to the codebase.

If you aren't aware of it yet, I suggest to read https://wiki.mozilla.org/WebExtensions/Contribution_Onramp to familiarize yourself with the patch development process, and especially the "Submitting a patch" section.

The bug assignee is inactive on Bugzilla, so the assignee is being reset.