whatwg / fetch

Fetch Standard
https://fetch.spec.whatwg.org/
Other
2.12k stars 332 forks source link

Service workers and URL fragments #1175

Open annevk opened 3 years ago

annevk commented 3 years ago

505 and #696 have unearthed that dealing with URL fragments correctly for service workers is rather tricky so I'm splitting that out into this issue. This is tracked on the service worker side in https://github.com/w3c/ServiceWorker/issues/1564.

There's a couple of options from those discussion threads I'll try to summarize, but ideally @davidben and @wanderview would come to some agreement here I think as Chrome would need to experiment with changes to the status quo:

Tests:

See also #1167 (for exposing fragments on responses).

wanderview commented 3 years ago

Just thinking this through out loud here....

I don't like status quo because it deviates from redirects.

I would be for alternative 2, except its a breaking a change. Fragments are exposed on FetchEvent.request.url today and its hard to know if there are service workers out there customizing logic based on the fragment.

I guess alternative 1 also has some backward incompatibility today. If there is an existing service worker that progressively caches responses it could stumble into the cache poisoning scenario you outline above.

The site could probably more easily fix alternative 1 issue than they could fix alternative 2, though. The site could just do new Response(response_with_fragment) when adding to cache_storage to avoid the poisoning. If their fetch handler logic depends on request fragments, though, that would require architectural changes to fix.

Of course, alternative 1 problem is much more likely than alternative 2.

From a launching perspective I think we could probably craft metrics to determine how often alternative 1 problem occurs. I'm not sure how to craft a metric to detect fragment-specific logic in alternative 2 problem, though.

We could try to measure how possible alternative 1 is before deciding. Measurements would require remembering the response fragment (even if not exposed yet) and then counting how many pages return a cache_storage originated response with a different fragment than FetchEvent.request.url.

If the measurements show the issue does not happen in practice today then I would argue to go ahead with alternative 1. If the issue is real, though, then maybe we could be default to not storing fragments in cache_storage and provide a preserveFragment:true option in cache_storage to opt in to storing it.

I think my main concern with this plan would be the cost of getting measurements in place and this not being a prioritized effort.

What do other folks think? @jakearchibald @asutherland @youennf @mfalken