Skip to content
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

Add ServiceWorker Static Routing API support #1737

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

sisidovski
Copy link

@sisidovski sisidovski commented Feb 16, 2024

We propose ServiceWorker static routing API, which allows developers to configure routing, and offload things ServiceWorkers do. The main spec update is in w3c/ServiceWorker#1701, but some behavior involves the fetch spec change (specifically, "race-network-and-fetch-handler" option).

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


Preview | Diff

fetch.bs Outdated Show resolved Hide resolved
@sisidovski sisidovski changed the title Support ServiceWorker Static Routing API Add ServiceWorker Static Routing API support Feb 22, 2024
Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-editor LGTM

fetch.bs Show resolved Hide resolved
Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't this return a response without any filtering getting applied? I guess the idea is that these are already filtered responses? Can that be asserted?

@@ -4506,6 +4506,10 @@ steps:

<li><p>If <var>recursive</var> is false, then run the remaining steps <a>in parallel</a>.

<li><p>Let |raceResponse| be the result of <a for=/>lookup race response</a> given <var>request</var>.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can tell https://w3c.github.io/ServiceWorker/#lookup-race-response-algorithm ends up waiting, which would delay this algorithm. Is that really the intent?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but actually this wouldn't delay the algorithm. When |raceResponse| is not null, the corresponding request is at least in-flight status. The map in "Lookup Race Response" is a map only contains in-flight requests/responses. At this point in 4.1. Main fetch, the actual fetch is not performed yet, we expect waiting for the in-flight request is faster in most cases.

@sisidovski
Copy link
Author

Wouldn't this return a response without any filtering getting applied? I guess the idea is that these are already filtered responses? Can that be asserted?

@annevk Sorry to be the slow response. This response is the result of "fetch" in this step in the service worker spec. So these should be filtered appropriately.

@annevk
Copy link
Member

annevk commented Oct 1, 2024

Okay, I think that makes sense. Still have a question about the Service Worker side of things though:

If raceNetworkRequestResponse’s status is ok status, then:

How does this work for "no-cors" fetches?

cc @asutherland

@sisidovski
Copy link
Author

So, ok status will be filtered for "no-cors" fetches?

If raceNetworkRequestResponse’s status is ok status, then:

What about updating this paragraph to refer internal response? Not sure referring the ok status in internal response is concerned for information leaks, but there're a couple of lines referring internal response in the ServiceWorker spec.

@annevk
Copy link
Member

annevk commented Oct 2, 2024

Yes, as per https://fetch.spec.whatwg.org/#concept-filtered-response-opaque status will be 0. It does seem very concerning this would tell you for every subresource whether it has an ok status or not. That's not information we reveal for images today, for instance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants