Integrate with Fetch Metadata. #993

mikewest · 2020-01-10T08:05:14Z

This patch integrates Fetch Metadata processing into Fetch's "main
fetch" algorithm, and defines a "user activation flag" on requests that
will be populated during HTML's "process a navigate fetch" algorithm.

Closes #885.

At least two implementers are interested (and none opposed):
- Chrome is shipping this as of 76.
- Mozilla has labeled the feature as "worth prototyping".
Tests are written and can be reviewed and commented upon at:
- https://wpt.fyi/results/fetch/metadata?label=experimental&label=master&aligned
- @mikewest is writing a test for the specific bit around Service Worker's access to the headers in WPT: Sec-Fetch-* headers aren't accessible in service workers. web-platform-tests/wpt#27857.
Implementation bugs are filed:
- Chrome: Shipped.
- Firefox: https://bugzilla.mozilla.org/show_bug.cgi?id=1695911
- Safari: https://bugs.webkit.org/show_bug.cgi?id=204744

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

Preview | Diff

This patch integrates Fetch Metadata processing into Fetch's "main fetch" algorithm, and defines a "user activation flag" on requests that will be populated during HTML's "process a navigate fetch" algorithm. Closes #885.

mikewest · 2020-01-10T08:05:34Z

As discussed on public-webappsec@. WDYT, @annevk?

fetch.bs

mikewest · 2020-02-11T06:46:26Z

Coming back to this as a prereq to whatwg/html#5203.

WDYT?

annevk

As Yoav once discovered, this does not work. As https://fetch.spec.whatwg.org/#cors-unsafe-request-header-names is currently invoked, setting Sec- headers before the network layer forces a preflight. We'd have to add an exception there (or its caller).

I think there is agreement that adding an exception is fine, provided the Sec- headers have reasonable restrictions on length and attacker-controlled data. (Perhaps we ought to document that in the process of enshrining this exception.)

fetch.bs

mikewest · 2020-02-11T13:35:15Z

As Yoav once discovered, this does not work. As https://fetch.spec.whatwg.org/#cors-unsafe-request-header-names is currently invoked, setting Sec- headers before the network layer forces a preflight. We'd have to add an exception there (or its caller).

Would you like me to put these headers specifically into https://fetch.spec.whatwg.org/#cors-safelisted-request-header? Or would you like us to work out a Sec-* carveout generally?

annevk · 2020-02-11T14:09:18Z

I think we should carve it out generically, right?

Now actually copying @yoavweiss so he can share any thoughts as to why carving out Sec- generically might be bad.

mikewest · 2020-02-14T08:53:05Z

Would you like me to carve it out generically in this CL, or in a separate patch? Probably the latter, right? #1000

As discussed in #993.

annevk · 2020-02-18T15:30:52Z

This also does not address w3c/webappsec-fetch-metadata#29 as far as I can tell. We need a solution for that.

Also, w3c/webappsec-fetch-metadata#38 changed right as this PR would expose them to service workers?

Could you maybe do another triage round so I know what upstream issues might end up impacting Fetch still?

mikewest · 2021-03-02T12:32:38Z

I merged the last few months of changes in f780097, and moved the integration out of "main fetch" and into "HTTP-network-or-cache" fetch in 58d9d18. I believe that addresses the following concerns:

We do not expose the headers to the Request.headers accessor in Service workers, as they're added after SW have a shot at handling the request. That should allow us to resolve Exposing Sec-Fetch headers to Service Workers via Request().Headers() w3c/webappsec-fetch-metadata#38.
The HTTPS->HTTP->HTTPS redirect behavior described in the spec (and in WPT) should fall naturally out of this change for the same reason (we recalculate on each hop), which should allow us to resolve Update Redirect section with HTTPS->HTTP behavior w3c/webappsec-fetch-metadata#29.
We can punt the question of a more generic Sec-* carveout for CORS preflights, as this change ensures that headers are added after the point at which we would have send a preflight (in HTTP fetch).

I've skimmed very quickly through other bugs, but nothing other than whatwg/html#5203 jumps out at me as needing to be resolved before landing this integration. I'll try to label things accordingly if I can find some time.

WDYT?

annevk

Thanks for getting around to updating this. The plan looks good to me, I only have nits.

fetch.bs

annevk · 2021-03-02T13:18:06Z

I've also added the pull request template to OP as this is a normative change. Mainly so we can find tests and bugs later. I think this has support from Chrome/Firefox.

Maybe @youennf can weigh in for Safari?

annevk

Yeah this looks good, thanks.

Based on feedback in whatwg/fetch#993.

mikewest · 2021-03-02T15:32:45Z

Updated dependent PRs and specs, and filled out the template above.

As requested in whatwg/fetch#993. Change-Id: Ie6096154ad9f6af73e2c26e0bb0c8f72a2a7a99a

As requested in whatwg/fetch#993. Change-Id: Ie6096154ad9f6af73e2c26e0bb0c8f72a2a7a99a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2727263 Reviewed-by: Matt Falkenhagen <[email protected]> Commit-Queue: Mike West <[email protected]> Cr-Commit-Position: refs/heads/master@{#859345}

annevk · 2021-03-04T08:19:26Z

Thanks @mikewest!

annevk · 2021-03-04T08:20:03Z

Unfortunately, I just realized I forgot to note whatwg/html#5203 in the commit message. Oh well.

In order to support Fetch Metadata Request Headers's Sec-Fetch-User header, this sets navigation request's user-activation if the navigation is triggered while the source browsing context has transient activation. See also whatwg/fetch#993.

…ible in service workers., a=testonly Automatic update from web-platform-tests WPT: `Sec-Fetch-*` headers aren't accessible in service workers. As requested in whatwg/fetch#993. Change-Id: Ie6096154ad9f6af73e2c26e0bb0c8f72a2a7a99a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2727263 Reviewed-by: Matt Falkenhagen <[email protected]> Commit-Queue: Mike West <[email protected]> Cr-Commit-Position: refs/heads/master@{#859345} -- wpt-commits: df6a144d964283f2929eeb937af2806d9aafec62 wpt-pr: 27857

…ible in service workers., a=testonly Automatic update from web-platform-tests WPT: `Sec-Fetch-*` headers aren't accessible in service workers. As requested in whatwg/fetch#993. Change-Id: Ie6096154ad9f6af73e2c26e0bb0c8f72a2a7a99a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2727263 Reviewed-by: Matt Falkenhagen <falkenchromium.org> Commit-Queue: Mike West <mkwstchromium.org> Cr-Commit-Position: refs/heads/master{#859345} -- wpt-commits: df6a144d964283f2929eeb937af2806d9aafec62 wpt-pr: 27857 UltraBlame original commit: cc8aa5885f9fc1c5d8b014047501861d5af169e6

After whatwg/fetch#948, whatwg/fetch#993, and whatwg/html#5203, the integration with Fetch and HTML is complete. This patch points to those integration points rather than claiming that there's still work to be done. Closes #73.

As requested in whatwg/fetch#993. Change-Id: Ie6096154ad9f6af73e2c26e0bb0c8f72a2a7a99a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2727263 Reviewed-by: Matt Falkenhagen <[email protected]> Commit-Queue: Mike West <[email protected]> Cr-Commit-Position: refs/heads/master@{#859345} GitOrigin-RevId: 371392c37dfd0b6830aba1b3648dbfcaab5ff1af

Integrate with Fetch Metadata.

160c7ec

This patch integrates Fetch Metadata processing into Fetch's "main fetch" algorithm, and defines a "user activation flag" on requests that will be populated during HTML's "process a navigate fetch" algorithm. Closes #885.

mikewest mentioned this pull request Jan 10, 2020

Set requests' user activation flag during navigation. whatwg/html#5203

Merged

3 tasks

annevk reviewed Jan 10, 2020

View reviewed changes

fetch.bs Outdated Show resolved Hide resolved

mikewest mentioned this pull request Jan 10, 2020

Add Fetch Metadata to bikeshed's index. speced/bikeshed#1584

Closed

mikewest added 2 commits February 11, 2020 07:37

Merge remote-tracking branch 'origin/master' into fetch-metadata

4b66321

fixup bikeshedding

ab1d7b0

annevk reviewed Feb 11, 2020

View reviewed changes

fetch.bs Outdated Show resolved Hide resolved

mikewest added a commit that referenced this pull request Feb 14, 2020

Accept 'sec-'-prefixed headers as CORS-safelisted.

9c64c16

As discussed in #993.

mikewest mentioned this pull request Feb 14, 2020

Accept 'sec-'-prefixed headers as CORS-safelisted. #1000

Closed

annevk mentioned this pull request Jul 6, 2020

Fetch integration w3c/webappsec-fetch-metadata#58

Closed

2 tasks

ghostd mentioned this pull request Oct 14, 2020

Set CORS preflight requests' mode to cors. servo/servo#23780

Closed

Base automatically changed from master to main January 15, 2021 07:38

mikewest added 2 commits March 2, 2021 13:24

Merge remote-tracking branch 'origin/main' into fetch-metadata

f780097

fixup move integration to HTTP-network-or-cache fetch

58d9d18

annevk reviewed Mar 2, 2021

View reviewed changes

fetch.bs Outdated Show resolved Hide resolved

fetch.bs Outdated Show resolved Hide resolved

fixup annevk's feedback

c005062

annevk approved these changes Mar 2, 2021

View reviewed changes

mikewest added a commit to w3c/webappsec-fetch-metadata that referenced this pull request Mar 2, 2021

Rename algorithm from set => append.

bbc55cd

Based on feedback in whatwg/fetch#993.

chromium-wpt-export-bot mentioned this pull request Mar 2, 2021

WPT: Sec-Fetch-* headers aren't accessible in service workers. web-platform-tests/wpt#27857

Merged

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Mar 2, 2021

WPT: Sec-Fetch-* headers aren't accessible in service workers.

ab60433

As requested in whatwg/fetch#993. Change-Id: Ie6096154ad9f6af73e2c26e0bb0c8f72a2a7a99a

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Mar 2, 2021

WPT: Sec-Fetch-* headers aren't accessible in service workers.

ae15033

As requested in whatwg/fetch#993. Change-Id: Ie6096154ad9f6af73e2c26e0bb0c8f72a2a7a99a

nits

b36a125

annevk merged commit 51a664d into main Mar 4, 2021

annevk deleted the fetch-metadata branch March 4, 2021 08:19

mikewest mentioned this pull request Jul 13, 2021

Clean up the integration with HTML and Fetch. w3c/webappsec-fetch-metadata#75

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrate with Fetch Metadata. #993

Integrate with Fetch Metadata. #993

mikewest commented Jan 10, 2020 •

edited by pr-preview bot

Loading

mikewest commented Jan 10, 2020

mikewest commented Feb 11, 2020

annevk left a comment

mikewest commented Feb 11, 2020

annevk commented Feb 11, 2020

mikewest commented Feb 14, 2020 •

edited

Loading

annevk commented Feb 18, 2020

mikewest commented Mar 2, 2021

annevk left a comment

annevk commented Mar 2, 2021

annevk left a comment

mikewest commented Mar 2, 2021

annevk commented Mar 4, 2021

annevk commented Mar 4, 2021

Integrate with Fetch Metadata. #993

Integrate with Fetch Metadata. #993

Conversation

mikewest commented Jan 10, 2020 • edited by pr-preview bot Loading

mikewest commented Jan 10, 2020

mikewest commented Feb 11, 2020

annevk left a comment

Choose a reason for hiding this comment

mikewest commented Feb 11, 2020

annevk commented Feb 11, 2020

mikewest commented Feb 14, 2020 • edited Loading

annevk commented Feb 18, 2020

mikewest commented Mar 2, 2021

annevk left a comment

Choose a reason for hiding this comment

annevk commented Mar 2, 2021

annevk left a comment

Choose a reason for hiding this comment

mikewest commented Mar 2, 2021

annevk commented Mar 4, 2021

annevk commented Mar 4, 2021

mikewest commented Jan 10, 2020 •

edited by pr-preview bot

Loading

mikewest commented Feb 14, 2020 •

edited

Loading