1819592 - Don't set Sec-Fetch-* headers if a channel is used for a download

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect, P2)

Description

[Kai Engert [:KaiE:]]

Bug 1803739 needs an additional load flag, to selectively prevent that Sec-Fetch-* headers are sent.

Unfortunately there is no more room more additional flags in the existing nsIRequest.loadFlags attribute:

0:00.96 xpidl.xpidl.IDLError: error: xpidl constants must fit within uint32_t, /home/user/moz/commcent/mozilla/netwerk/base/nsIChannel.idl line 292:4
 0:00.96     const unsigned long LOAD_NO_SEC_FETCH_HEADERS = 1 << 32;

This patch suggests to introduce an additional loadFlags2 attribute for storing additional flags.

Comment 1

[Kai Engert [:KaiE:]]

Attachment: Bug 1819592 - New loadFlag LOAD2_NO_SEC_FETCH_HEADERS and make room for more load flags. r=valentin

Comment 2

[Kai Engert [:KaiE:]]

Valentin explained we don't need a new load flag, but can instead use nsIHttpChannelInternal.

And in the meantime, in bug 1803739, Freddy suggested we should use a more general flag like "this is a download".

Combining these, I see that nsIHttpChannelInternal already has an attribute channelIsForDownload.

Based on Freddy's suggestion, we could omit the sec-fetch-* headers if channelIsForDownload is set to true.

Comment 3

[Kai Engert [:KaiE:]]

Attachment: Bug 1819592 - Add flag noSecurityContext to nsIHttpChannelInternal, don't send Sec-Fetch-* headers if set. r=valentin

Comment 4

[Kai Engert [:KaiE:]]

Would someone from the Necko team be able to point us to an existing test, which would be suited as an example for adding a test?

IIUC, we need a test that creates an XMLHttpRequest, see the new flag, then receive the headers that are generated by the request, and check that these headers aren't present?

Probably the test will need to be even more advanced, because it needs to operate in a context that would normally produce the Sec-Fetch-* flags? Do we need to have a related parent request?

Any pointers/suggestions would be very much appreciated.

Comment 5

[Kai Engert [:KaiE:]]

I found toolkit/components/extensions/test/xpcshell/test_ext_secfetch.js
Would that be a good starting point?
It uses "fetch" (not XMLHttpRequest). Is there a way to set an attribute of nsIHttpChannelInternal when using fetch?

Comment 6

[Frederik Braun [:freddy]]

dom/xhr/tests/test_XHR_anon.html is a test where XMLHttpRequest() is making use of advanced preferences.
dom/xhr/tests/test_XHR_parameters.html is testing the privilege questions.

Comment 7

[Kai Engert [:KaiE:]]

Sorry I cannot make the remaining work a priority right now.

Comment 8

[Magnus Melin [:mkmelin] (away, back Aug 4)]

Attachment: Bug 1819592 - Don't set Sec- headers for system requests. r=freddyb,ckerschb

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d8a8eaf13f78
Don't set Sec- headers for system requests. r=freddyb,ckerschb

Comment 10

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/d8a8eaf13f78

Comment 11

[Magnus Melin [:mkmelin] (away, back Aug 4)]

Comment on attachment 9339039 [details]
Bug 1819592 - Don't set Sec- headers for system requests. r=freddyb,ckerschb

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Makes reading of rss feeds from some major sites like Feedburner work in Thunderbird.
• User impact if declined: Feed reading won't work.
• Fix Landed on Version: 117
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): It only affects requests/fetches from privileged code, in combination with the server reacting differently to Sec- headers.
  Probably such cases do not exist for Firefox.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9339039 [details]
Bug 1819592 - Don't set Sec- headers for system requests. r=freddyb,ckerschb

Approved for 115.2esr.

Comment 13

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/6a0e335faac3