Allow HTTP scheme fetches to make CORS preflight for navigations

[alexpetros]

This allows navigations to make CORS preflight requests, if the navigation contains a request that is not safelisted. Navigation does not (yet) provide APIs for non-safelisted requests; this update to the fetch spec is a prerequisite for HTML spec changes that might add those APIs (i.e. PUT method support in forms).

No web platform tests or MDN updates are included, as no changes to browser functionality are required by this update alone.

This change supports the work outlined in this WHATWG Issue #3577 comment.

• At least two implementers are interested (and none opposed):
  • …
  • …
• Tests are written and can be reviewed and commented upon at:
  • N/A
• Implementation bugs are filed:
  • Chromium: …
  • Gecko: …
  • WebKit: …
  • Deno (not for CORS changes): …
• MDN issue is filed: N/A
• The top of this comment includes a clear commit message to use.

(See WHATWG Working Mode: Changes for more details.)

---

Preview | Diff

[domfarolino]

So the idea with this PR is that <var>makeCORSPreflight</var> variable is really closer to "makeCORSPreflight if needed", and can be set to true because it won't trigger preflights except for when requests otherwise demand them by using an unsafe method or request header?

But I think this PR will have side effects on other requests that would suddenly start triggering preflights, when they shouldn't. For example:

fetch("https://google.com", {method: 'GET', mode: 'no-cors', headers: {'Content-Type': 'application/json'}})

... uses an unsafe request header, which would ordinarily trigger a preflight, but for the no-cors mode which never sends preflights¹. I think this PR would change that, and preflights would be sent for these kinds of no-cors requests, at the very least.

Another way to achieve this is to set the use-CORS-preflight flag on the relevant kinds of navigation requests that we want to subject to preflights. Unfortunately, that wouldn't integrate cleanly with "Main fetch" today, given how mode=navigate fetches are handled before we consider the use-CORS-preflight flag, just a few conditions down.

So maybe the best option is to modify scheme fetch to only turn a false "makeCORSPreflight" to true if the request mode is navigate (instead of unconditionally as you're doing now)?

Footnotes

1. This is because scheme fetch is invoked from the no-cors path, which today calls HTTP fetch without the makeCORSPreflight variable set to true, so we would never even test the method or request headers. ↩

[alexpetros]

@domfarolino First of all, thank you so much for the thoughtful review!

So the idea with this PR is that makeCORSPreflight variable is really closer to "makeCORSPreflight if needed", and can be set to true because it won't trigger preflights except for when requests otherwise demand them by using an unsafe method or request header?

Yes, this is my understanding of HTTP Fetch Step 4.1, which only issues the preflight if the request has attributes (method or headers) that are not CORS-safelisted (or if the use-CORS-Preflight flag is set); otherwise, it proceeds as normal (the path navigation takes today). I'm happy to rename it, but my instinct is to touch as little as possible.

Another way to achieve this is to set the use-CORS-preflight flag on the relevant kinds of navigation requests that we want to subject to preflights. Unfortunately, that wouldn't integrate cleanly with "Main fetch" today, given how mode=navigate fetches are handled before we consider the use-CORS-preflight flag, just a few conditions down.

I agree that this is not ideal. It would also create some confusion around when exactly CORS is supposed to be checked in the request lifecycle. Since it already happens in one path that the navigation fetch will definitely reach (HTTP fetch), I'd like to re-use that logic if possible.

So maybe the best option is to modify scheme fetch to only turn a false "makeCORSPreflight" to true if the request mode is navigate (instead of unconditionally as you're doing now)?

This seems great, and more narrow. There's also precedent for inspecting the request mode after that main switch. I will go ahead and make that change!

[domfarolino]

I think the current approach is at least safe, and avoids the scary side effects pointed out further above. I think it'd be great to get a gut check from @annevk on this now.

[domfarolino]

Non-editor LGTM