1900362 - Do not override the Priority header when set by the user

Status: RESOLVED FIXED

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

Description

[Valentin Gosu [:valentin] (he/him) {{ FYI: OOO 14 June - 4 August }}]

See conversation about chrome behaviour Define behavior for Priority request header · Issue 1718 · whatwgfetch

At least in the case of Chrome, if the Priority header is set before it gets to the networking stack then the user-supplied value will be used (like was discussed above).

I think the fix would be rather simple:
https://searchfox.org/mozilla-central/rev/dc08a7321a22918d5a26b7641f9b10cd2a09d98e/netwerk/protocol/http/nsHttpChannel.cpp#488-491

void nsHttpChannel::SetPriorityHeader() {
  nsAutoCString userSetPriority;
  Unused << GetRequestHeader("Priority"_ns, userSetPriority);
  if (!userSetPriority.IsEmpty()) {
     // insert comment here :)
     return;
  }

  uint8_t urgency =
      nsHttpHandler::UrgencyFromCoSFlags(mClassOfService.Flags(), mPriority);
  bool incremental = mClassOfService.Incremental();

Comment 1

[Valentin Gosu [:valentin] (he/him) {{ FYI: OOO 14 June - 4 August }}]

See also https://github.com/whatwg/fetch/issues/1718#issuecomment-2145347776

The Priority header implementation in Chrome and Firefox ignores the existence of the request filtering engine available to extensions. declarativeNetRequest and webRequest do not see the header because it is set by the browser after request filtering occurs, so extensions cannot remove the header, they will only be able to set a custom value.

We should also consider moving the call to SetPriorityHeader a bit earlier to allow webextensions to change/remove the value.

Comment 2

[Sean Kim]

Attachment: Bug 1900362 - Do not override the Priority header when set by the user r=kershaw,#necko

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6a92033df002
Do not override the Priority header when set by the user and move SetPriorityHeader call before onModifyRequest r=necko-reviewers,kershaw,devtools-reviewers,bomsy

Comment 4

[Iulian Moraru]

Backed out for causing multiple failures.

• Push with failures

• Failure log for mochitest-remote failures

• Failure log for mochitest-plain failures

• Failure log for wpt failures example 1

• Failure log for wpt failures example 2

• Failure log for wpt failures example 3

• Backout link

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2f58630eb7af
Do not override the Priority header when set by the user r=necko-reviewers,kershaw,devtools-reviewers,bomsy,webdriver-reviewers,whimboo

Comment 6

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/2f58630eb7af