[whatwg/fetch] Script-created redirects responses have no location URL (#1146)

(CC @jakearchibald @wanderview who I talked to about this recently.)

Fetch responses have a location URL, computed from the Location header, when they're constructed from the network:
https://fetch.spec.whatwg.org/#ref-for-concept-response-location-url%E2%91%A0

And then the HTTP-redirect fetch algorithm never looks at the Location header and instead follows the location URL value. I also don't see any reference to setting the location URL in the Service Worker spec.
https://fetch.spec.whatwg.org/#concept-http-redirect-fetch

From what I can tell, the main platform-visible consequence of this that, if a Service Worker responds to a navigation FetchEvent of / with `ev.respondWith(fetch("/foo/", {redirect: "manual"})` and the server responds with `Location: bar.html`, the redirect resolves to /foo/bar.html rather than /bar.html.

Looks like Chrome and Firefox don't implement this (resulting in /bar.html) while Safari does. The spec's behavior seems preferable, especially given atomic HTTP redirect handling, so I'm looking at fixing that. In doing so, two fun cases came up:

* [Response.redirect](https://fetch.spec.whatwg.org/#dom-response-redirect) sets the Location header, but not the location URL. That means, if I understand the spec correctly, `respondWith(Response.redirect(...))` does not actually work. We could fix that by adding an extra step to the algorithm, but...
* It's also possible to construct a redirect with `new Response("", {status: 302: headers: {location: "bar.html"}})`. Since you can manipulate the headers after construction, there isn't a very clear place to stick in the location URL computation.

I can see two options here:

1. Add an extra step in `Response.redirect`, but leave `new Response(...302...)` broken.
2. Update HTTP-redirect fetch (or FetchEvent.respondWith) to say, if the response doesn't have a location URL, you rerun the steps to construct it from the status code and Location header and use that instead.

Interestingly, `Response.redirect` works in Chrome/Firefox/Safari, but `new Response(...302...)` only works in Chrome/Firefox. Safari seems to fail the fetch when you `respondWith(new Response(...302...))`.

(1) does mostly match one browser. I say mostly because I don't see anything in the spec that supports an error. It seems the spec would want you to actually commit the response which is really weird. But, relative to Chrome/Firefox, it's a breaking change and I'm a little nervous about compatibility. It also seems generally confusing for `new Response(...302...)` to silently fail. (2) would avoid this.

I think (2) is thus preferable, even though repeating the Location resolution in two places is a little odd.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/fetch/issues/1146

Received on Tuesday, 26 January 2021 19:31:08 UTC