Location of this check

[domenic]

The current spec draft locates this after:

• checks
  • local-URLs-only
  • CSP report-only reports
  • bad port
  • mixed content
  • CSP enforce
  • Integrity policy
• request modifications
  • upgrade insecure requests
  • upgrade mixed content requests

It thus misses at least the following:

• checks
  • "same-origin" mode check
  • non-HTTP(S) schemes check
• request modifications
  • referrer determination
  • HSTS

as well as possibly other harder-to-classify stuff in step 12, e.g. response tainting for "navigate", "websocket", or "no-cors" cases

Although a carefully-written OverrideResponseForRequest might be fine with these, e.g. if it makes sure to never inspect the referrer, include both http: and https: cases to deal with HSTS upgrades, never reacts to non-HTTP(S) URLs, etc., I think it would be better spec architecture if the hook was inserted later.

My suggestion is that it should be located near the top of HTTP fetch, but I don't guarantee this is the very best place.

[domenic]

(The most observable of these, I believe, is the "same-origin" mode check. That is, I think in the draft as-written, fetch("https://evil.example/shimmed", { mode: "same-origin" }) is likely to return a shimmed body, instead of a network error. Which would be pretty bad!)

[mikewest]

Excellent feedback, thank you.

I think I agree with you about the positioning. Folks like @annevk / @sysrqb and @bvandersloot-mozilla / @tomrittervg might have more expert opinions, but some quick testing shows that Safari and Firefox both position their content blocking mechanisms somewhere around here Your mode: 'same-origin' example rejects, even for resources Firefox would otherwise shim, and neither browser seems to hand the request off to a Service Worker prior to blocking.

[mikewest]

b3da890 adjusts this to patch into both Scheme Fetch and HTTP Fetch (because Main Fetch calls directly to HTTP Fetch in some cases, and we'd like to ensure that we can intervene against data:, blob:, et al.). It also proposes an alternative which might be clearer? I'm not sure which approach is more maintainable in the long term.

I plan to put this up as a PR against Fetch in the somewhat near future, since it's behavior multiple user agents have already shipped that would be ideal to explain. If folks have opinions about better approaches than the one proposed in the doc at this point, I'd appreciate hearing about them. :)

[mikewest]

https://explainers-by-googlers.github.io/script-blocking/#fetch <-- is the new text

[annevk]

Nit: please don't introduce new abstract operations. In general web platform specifications use "this is an algorithm" as name style.

I think it would be clearer if we initialized overrideResponse in a step and then a subsequent step checked it for null-ness and does the return.

Why would we not check this in "main fetch" where we also check most other policies?

Do all browsers have some kind of shimming? cc @sysrqb @valenting

[mikewest]

Nit: please don't introduce new abstract operations. In general web platform specifications use "this is an algorithm" as name style.

I think it would be clearer if we initialized overrideResponse in a step and then a subsequent step checked it for null-ness and does the return.

Adjusted both of these in ac5a869, thanks!

Why would we not check this in "main fetch" where we also check most other policies?

The note at the bottom of https://explainers-by-googlers.github.io/script-blocking/#fetch tries to address this. TL;DR: We can absolutely put it into Main Fetch, it will simply require us to extract the potential network errors that can be generated in the cascade of step 12 (e.g. cross-origin requests with mode same-origin, non-http(s) schemes other than data:, maybe other stuff I'm not seeing?) so that they happen before the response could be replaced by a confused user agent.

Do all browsers have some kind of shimming?

Firefox certainly does: see https://searchfox.org/mozilla-central/source/browser/extensions/webcompat/shims. I'm fairly certain that DuckDuckGo and Brave also do. I'd defer to you on whether WebKit's site-specific quirks replace any responses.

[annevk]

I see, tricky. Another approach could be that we introduce "override fetch" which can dispatch to "scheme fetch" or "HTTP fetch" and we ensure that we only ever exit "main fetch" through "override fetch". I kinda like that as it makes the contract more explicit.

[mikewest]

I think the only places we call into "scheme fetch" or "HTTP fetch" from "main fetch" are in step 12. You're suggesting that we replace those calls with calls to "override fetch" (and then presumably pass it a type ("scheme" or "http"), fetchParams, and makeCORSPreflight (defaulting to false))? That's certainly doable. I can sketch it out to see what you think.

Or do you mean extracting step 12 entirely into an "override fetch" that would return a response or a network error?

[annevk]

The former is what I was thinking. It seems okay to leave that for eventual upstreaming though.

[mikewest]

Thanks for the feedback, all: I put a patch up at whatwg/fetch#1840, and I'll close this out in favor of a discussion on that PR.