1797404 - [DNR] Implement modifyHeaders action

Status: RESOLVED FIXED

(WebExtensions :: Request Handling, enhancement, P2)

Description

[Rob Wu [:robwu]]

Implement the following DNR action: modifyHeaders

Comment 1

[Rob Wu [:robwu]]

Note, the current modifyHeaders action does not permit the "append" operation for request headers.
This is because it was documented to not be allowed at https://source.chromium.org/chromium/chromium/src/+/main:extensions/common/api/declarative_net_request.idl;l=54-55;drc=60039d4d4bd70512e21a2dfe586602aca1d9d35e:

   // This describes the possible operations for a "modifyHeaders" rule.
  enum HeaderOperation {
    // Adds a new entry for the specified header. This operation is not
    // supported for request headers.
    append,

While most of Chromium's IDL is usually rendered in auto-generated documentation, the comments in that highlighted section are not.

It is not clear why it is unsupported, so if feasible we should just support "append" (along with "set" and "remove").

There is demand for this feature, e.g. as seen at https://crbug.com/1270273

Comment 2

[Rob Wu [:robwu]]

Attachment: Bug 1797404 - Implement DNR modifyHeaders action

Comment 3

[Rob Wu [:robwu]]

Attachment: Bug 1797404 - Allow "append" operation to be used with requestHeaders

Comment 4

[Rob Wu [:robwu]]

I just checked again, and it looks like Chrome does now support "append" for some headers (based on an allowlist). I linked the details from https://crbug.com/1117475#c7 . In the patch above I enabled "append" for any header, without restricting which headers can be appended to, since any limitation would be somewhat arbitrary.

Comment 5

[Luca Greco [:rpl] [:luca] [:lgreco]]

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

I just checked again, and it looks like Chrome does now support "append" for some headers (based on an allowlist). I linked the details from https://crbug.com/1117475#c7 . In the patch above I enabled "append" for any header, without restricting which headers can be appended to, since any limitation would be somewhat arbitrary.

I see, that clarify a bit the possible motivation for not supporting it first and then supporting it only on specific headers, would you mind to file a bug to remind us to reconsider if we should do something similar (I'd like us to also confirm what happens for other vendors, e.g. if and in which form Safari supports or intend to support this particular corner case).

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7703f8e13f31
Implement DNR modifyHeaders action r=rpl
https://hg.mozilla.org/integration/autoland/rev/8d4ff6d7ca86
Allow "append" operation to be used with requestHeaders r=rpl

Comment 7

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/7703f8e13f31
https://hg.mozilla.org/mozilla-central/rev/8d4ff6d7ca86