-
Notifications
You must be signed in to change notification settings - Fork 56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Proposal: declarativeNetRequest: matching based on response headers #460
Comments
Hi @Celsius273 Thank you for consulting with the community. We need to be able to modify the response headers by using rules that take into account the existing response header. From what I understand, your proposal will not allow this, but sounds like it makes it more possible to happen in the future as DNR rules will now also evaluate during the onHeadersReceived. Did you think about this requested functionality? |
Hi @nir-walkme DNR currently already has the ability to modify headers: not just remove them, but override their value or append a value onto them, which could be useful for CSP. See the ModifyHeaderInfo object for how to do this. However, it does not currently have the ability to substitute headers (i.e. replace parts of a header's value with something else). Note that even though modifyHeaders rules can be matched in the onBeforeRequest stage, their actions are still executed in the onHeadersReceived stage. This proposal is essentially a "v1" of matching on response headers: we want to add/implement a viable base that satisfies some use cases before exploring something more complex such as header substitution, which seems like the use case you're suggesting. Thanks |
Thanks @Celsius273 I have one request regarding your existing proposal: Add the ability to match a header by value exactly and not by "contains". Use case exampleIn order to solve partially the CSP problem described before, we could use predefined DNR rules that replaces a CSP header value completely with another value. Let's assume we want to set a rule that adds We will create the following DNR rule: The reason that a 'contains' rule is not enough is that if we had set the following rule: SummaryThe above request would help improve your "v1" matching proposal. |
Copying a few examples here: examples 1 is relatively basic, 2a/2b deals with allow rules, 3 deals with modifyHeaders rules: Going to add a few examples here: Note about modifyHeaders (MH) rules: MH rule interactions are a bit difficult to reason about since multiple rules can match and rules can specify different operations.
Example 1
In this (trivial) example, a request from abc.com will get blocked even though rule 2 has a higher priority, since it matches on a later request stage. Example 2a
A request from abc.com with a set-cookie header will go through since rule 3 (allow) has a higher priority than rule 4 (block) and prevents rule 4 from matching. Example 2b
A request from abc.com with a set-cookie header will get blocked from rule 4, since rule 3 (allow) has a lower priority and does not prevent rule 4 (higher priority) from matching. Example 3
A request from abc.com with the set-cookie header “bad-cookie” will be blocked by rule 6 since it matches based on the header’s value. Note that this match is done on the header’s original value before rule 5 has a chance to modify it (block actions have higher precedence than modifyHeaders actions). Thanks, |
Hello, one piece of the design for response header matching rules that we’d like to get some feedback from is the execution of modifyheaders rules. Option 1:
Option 2:
In both cases, response header conditions match based on the original headers before any modifications by DNR or webRequest. We believe that option 2 produces more intuitive behavior that can be better adapted to more use cases. Feedback and example use cases would be greatly appreciated, thanks! |
Using DNR, any slightly complex modification strategy is impossible. We have the same need as @nir-walkme of being able to modify the CSP directives (add specific URLs in some directives, add 'unsafe-inline' when needed, remove 'none' in some case...). In Manifest v2 we implemented it simply using blocking webRequest.onHeadersReceived. Would is be possible to get CSP modification tools? Or a way to delegate some treatement of DNR rules to some code? If we could tell "modify this header using this helper function (defined in the webextension), with this parameter", we could simply tailor the CSP as we would like. You could put some restriction on the code to not have access to any API excepted some simple ones (no web extension API call, no fetch, no async...) to ensure it remains fast. |
Hello, I have proposed an edit to the RuleCondition and HeaderCondition schema in the opening comment Namely, excludedResponseHeaders will be specified as a list of HeaderCondition instead of just a list of header names (strings). The allows a rule to be not matched if a request contains a header with a specific value (vs before where the rule is not matched if a request just contains that header). e.g. if a rule specifies the following in excludedResponseHeaders:
^the rule is not matched if:
FAQ: What's the difference between specifying a header's value in responseHeaders/excluded values vs excludedResponseHeaders/values ? A:
In this example, if the request contains the header "h1: [bar]", then other conditions in responseHeaders are still evaluated, and the rule will match if the request contains the header "h2". However, the request contains the header "h3: [bar]", then conditions in responseHeaders will not be evaluated and the rule will not match. |
@Celsius273 If DNR gains matching during the For example, to block particular redirects, something like: {
id: 1,
action: { type: "block" },
condition: {
responseHeaders: [{ header: "Location" }],
// response status code conditions combine via AND, not OR?
// however it's done, the big idea is we need to be able to specify status code ranges
responseStatusCode: [
{ operator: ">=", value: 300 },
{ operator: "<", value: 400 }
],
urlFilter: "abc",
resourceTypes: ["xmlhttprequest"]
}
}, |
Re #460 (comment) about the order of modifyHeader rule matching. It took some attempts to read it before I understood what you meant. To rephrase, the proposed options were:
I too agree that option 2 is preferred over option 1. P.S. I am currently exploring edge cases and will post another comment later this week. |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Mozilla bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1877486 |
(I typed this many weeks ago, just submitting now for context) @Celsius273 I have tried the in-progress implementation in Chrome 127.0.6503.0 (associated with https://crbug.com/40727004 ), and it looks like the condition only matches if the header value is (case-insensitively) equal to any of the specified The input is specified by: https://source.chromium.org/chromium/chromium/src/+/main:extensions/common/api/declarative_net_request.idl;l=173-184;drc=24a80999fbc083276e0fbef7f9e2eec36429f6a8
And the implementation is in According to the implementation, a request only matches if the header is (case-insensitively) identical to any value in Example: Test caseI confirmed the behavior by loading an extension with the await chrome.declarativeNetRequest.updateSessionRules({
removeRuleIds: [1],
addRules:[
{
id: 1,
condition: {
regexFilter: ".*",
resourceTypes: ["main_frame", "sub_frame"],
responseHeaders: [
{ header: "content-Type", values: ["application/json"] },
],
},
action: { type: "redirect", redirect: { regexSubstitution: "https://example.com/?\\0." } }
},
],
}); Then, consider the following test cases:
My expectation is that the first 5 requests are matched (and the last three not), because these are More examplesFor a better picture, here are more examples: Semicolons:
Semicolons as delimiter mixed with semicolons that are not a delimiter:
Commas:
Proposed enhancementTo support the use case, it should be possible to match a substring of header values. This can range from supporting to match wildcards, left/right anchoring (startsWith/endsWith), or even regular expressions. Another option could be to expose the parsed header values for specific headers (e.g. |
Hello, The feature is now enabled by default on Canary and dev. Please try it out and reach out for any feedback, future enhancement ideas and bugs. re: Rob, more flexible matching on header values is next on my plate: a similar issue has been raised and I can perhaps build off of that? Unfortunately more flexible matching will require the use of a new filter type (likely regex), mainly because in Chromium's HTTPResponseHeaders class, convenience methods which retrieve multiple values from a header assume the use of a comma as a delimiter. The addition of a regex value filter should be able to satisfy your use case? potential schema:
Thanks, |
Your linked
Chromium's DNR implementation currently uses Based on this, I think that there is no implementation constraint to use the same (consistent) value for matching by
Yes, it would satisfy the Content-Type use case that is needed by PDF.js
I suggest to not make them mutually exclusive, but rather stack its effect on top of
|
I had a discussion with @Rob--W on various ways to avoid doing a full string match on Option 1: Splitting headers based on delimitersImagine a rule Option 2: Substring match up to first delimiterImagine a rule Option 3: General substring matchImagine a rule Note: In option 1 and 2, we would need a mapping of headers to delimiters. In Chromium, the append header allow list contains some of this but it would need to be more complete. ComparisonMatching by Content-TypeAs described, option 1 and 2 would support a condition for the application/pdf Content-Type (or similar use cases for JSON or epub extensions). Option 3 would provide an early exit for this but a regular expression would also be required to check the value is not actually Matching based on a specific cookie attributeFor the header Matching based on cookie nameThis is trivially possible with option 2 since it always appears at the start of a string, followed by an equals sign. It is not possible with option 1 or 3 since the name may appear elsewhere in the header and not actually be the cookie name. Matching based on CSPIt is unlikely that any of these options are sufficient and a regular expression would likely be required. There are simply too many ways in which directives can be ordered. Matching based on referrer headerThis is a use case which has come up, particularly in order to trim the header and remove (for example) a path from the URL. However, to do replacement like this a regular expression would be required so this is unlikely to be addressed solely by any of the options. ConclusionI lean slightly towards option 1. Like the other options, it solves the use cases we know about, including In conversation with @Rob--W, I believe he prefers option 2 or 3. Option 3 partly addresses the |
This comment was marked as resolved.
This comment was marked as resolved.
Hi... I apologize for this oversight but it should be enabled by default on canary and in the next dev build after crrev.com/c/5624732 lands. For now, you'll still need to enable the I tested your rule with the flag enabled and it works as intended |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Hi Daniel, re: feature detection: the current intention is for the feature to be available by default on canary/dev for testing, and it will be enabled in the next stable release (M128). Unfortunately we don't have a great way to detect said features in DNR unless the extension can access feature flag values from within Chrome. I do agree that a way to give feedback for the extension whether certain fields are supported or not, would be helpful. re: empty header value: in the example listed, it mentions the absence of the content-type header. That said, I can change that code to allow for empty header values (header is present, but value is empty), though ideally, header values shouldn't be empty when possible. Thanks, |
This comment was marked as resolved.
This comment was marked as resolved.
crrev.com/c/5634852 will allow for empty header values to be specified in response header conditions... Perhaps not an ideal solution but checking the user agent of the browser for a Chrome version may help detect if response header conditions are supported? @Rob--W @oliverdunk One solution for more flexible header value matching without resorting to regex could be to use the MatchPattern function for strings? It supports Let me know if this will satisfy your use cases, or if there are any I've missed that won't require regex. Thanks, |
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as duplicate.
This comment was marked as duplicate.
@Rob--W In the last meeting, you mentioned that you'd need to profile before indicating Firefox's stance on using glob patterns for header matching. Would you be able to take a look and follow-up with your thoughts? |
Since the expected number of header matching rules is low, I don't expect a significant performance impact with the use of globs for matching headers. |
^Ack, glob support has been added in crrev.com/c/5671762 |
And with the last CL submitted, as seen in crrev.com/c/5739758 and crbug.com/40727004 this feature is now complete! Please try out response header matching in DNR and forward feedback to me or oliverdunk@ |
Hello!
My name is Kelvin Jiang and I am part of the Chrome Extensions team and I'll be working on adding the ability for declarativeNetRequest (DNR) rules to match based on response headers which is tracked in this crbug
From what I've gathered (requirements):
Add the ability to match on:
^to support this, I propose adding the following fields to RuleCondition:
Context: request stage for webRequest and DNR
Currently, DNR rules are matched during the onBeforeRequest stage. For rules that match on response headers, the earliest stage that they can be matched is onHeadersReceived when we receive the response headers from the request.
A few more details:
Let me know if this looks good or if there should any changes to the above proposal. I'm looking forward to working with all of you and I know that this feature has been requested for quite a while!
The text was updated successfully, but these errors were encountered: