-
Notifications
You must be signed in to change notification settings - Fork 342
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
Allow "manual" redirect fetches with caveats #66
Comments
Also, I don't see the risk in exposing |
Did you have it analyzed? There's a reason redirects are indistinguishable from network errors today. |
We Google Docs/Drive ran into these issues in the following cases: For case 2:We have a service worker with scope //docs.google.com/ and one with scope //docs.google.com/spreadsheets/. We have a general //docs.google.com/open?id=... url that can open any item in Google Drive. If the id turns out to be a spreadsheet we send a 302 server side for //docs.google.com/spreadsheets/d/... This means it will miss out on any service worker features that are spreadsheets specific. The workaround with double hitting would also be big issue due to logging, resources and latency for the page load. For case 3:A user is logged out and hits a service worker controlled endpoint on "docs.google.com". We basically do a fetchEvent.respondWith(fetch(fetchEvent.request)) but catch errors on the fetch so we can fall back to a offline response. If the user is logged out he gets send to //accounts.google.com to log in. If that happens we show an error and we don't have any clue that this login redirect happened so we can't send a 302 to the page. For this case we haven't really found a good workaround. |
Here's a demo https://jakearchibald.github.io/isserviceworkerready/demos/redirect/. The link on the page points to |
Jake tells me that we're already distinguishing opaque responses from network errors; could we simply store some internal data on the opaque response that would kick in once we fell back into the navigation algorithm in order to execute the redirect as though the SW wasn't involved? |
Although opaque responses are opaque to JS, they're visible to the receiver (if the receiver is cool with opaque responses). So in the same way |
If |
That is a good question. Another question is whether the response object can be abused elsewhere. E.g. if the page then makes a request that does follow redirects and the SW replies with that response. And this request has different characteristics, such as a |
I can't imagine that we're leaking "too much" information here. It's visible in other ways (iframe onload handlers in some browsers, etc.). The specific issue here is 2-part:
Another thought: one thing that Docs might be able to do in this case is to set a TTL on resposne document that's not zero if we can know the eventual URL isn't on the same origin. Hopefully if CORS+redirect can surface that, we might be able to use knowledge of the eventual URL + the browser's cache to keep from falling through to the network a second time. Thoughts? |
Sorry, but CORS is not going to do anything here, it's not visible otherwise, and I'm not sure how that addresses any of the security review questions. |
CORS on the final response would at least allow you to view the eventual URL (or should if it doesn't already). That's enough to solve this issue with the hack i mention in the second bullet. But the larger issue remains: we've decided to hide redirect status to keep certain properties of redirects from bubbling through; so how do we allow control over this? If it isn't CORS, what is it? |
I guess we could have some header that indicates it's okay to surface the redirect to script. Combined with setting the redirect mode to manual that would allow surfacing it then... (And if the redirect is on another origin it would also need to specify CORS headers, of course.) If we want this that should probably be a separate issue. @jakearchibald had a point that if it's okay to expose a redirect as an opaque response you're already pretty far in knowing it's a redirect, except that you don't know where it goes (or what it contains). Would be great to have security review for this. |
To be clear, note that CORS on the final response is not sufficient here. The request would need to be initiated with mode "cors" rather than "no-cors" too, and that's not how the navigate algorithm does its fetching. So the service worker would need to override that. |
Slight, but relevant tangent: understanding impact of redirects, preflights, etc, is important for performance telemetry -- see w3c/resource-timing#21. The lack of access and visibility into these stages is a common complaint against current Nav+ResourceTiming APIs. It'd be nice if we could rationalize all this stuff between the various use cases. |
@igrigorik if we want to expose more about redirects we need a new protocol similar to CORS. As I said before, that warrants a new issue. I've been trying to evaluate how we can minimize the complexity here (please verify my analysis):
Since you're guaranteed it's a redirect, given an option to disable them, we should just expose it as a "redirect" (new type of response, identical to opaque except for its type and exposure of url as that is identical to the request url). We could then enforce that you can only return such a "redirect" response to navigation request contexts (that have redirect mode on manual), coupled with a same-origin restriction. ( I don't see any obvious issues with this, other than perhaps the difficulty for the "navigate" algorithm to handle redirects by itself:
None of that is well defined at the moment. |
Hmm, no... this applies to all contexts. Timing and performance data on redirects, preflights, etc, is important for all fetches:
If the browser emits multiple requests to satisfy the fetch request, I should be able to observe what those were and the cost of each -- subject to TAO and other restrictions. |
@igrigorik again, that is a different problem, please file a new issue. Most requests don't have the redirect mode set to manual, only those whose context is a navigation request context. |
Can't figure out why this is needed/useful.
Do we need to enforce this? If the SW returns an opaque redirect, the skip-service-worker flag could be set on the request. Doesn't this do the right thing?
We have to do that anyway though, right? Doesn't that algorithm have to handle redirects already? |
So you have some kind of identifer for the object if you decide to store it. I fully expect
Isn't that the opposite of what we want to be doing?
Not opaque ones. |
My bad, I'd assumed navigate created a new request for each phase of the redirect, so it wouldn't retain the Is there any value in making an opaque redirect response work in response to a "follow" request? It could work like this: (using https://fetch.spec.whatwg.org/#http-fetch) Request with redirect mode "follow"
Request with redirect mode "manual"
|
What you outline for 'Request with redirect mode "follow"' is not what we do today for redirects created by the service worker. It seems very confusing to create a different model just because the redirect happens to be opaque. Failing seems much saner. |
Yeah, now I've written it out, you're right, failing is less of a gotcha. |
Gecko impl bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1184607 |
annevk@ |
Thank you @horo-t. Addressed. |
For the fossil record: https://bugs.chromium.org/p/chromium/issues/detail?id=510650 |
Why can we not inspect |
See #601 (which you found, but adding the link here for others). |
Because none of the stakeholders care about this... it doesn't get done. It leaves us poor developers to work around missing basic functionality |
any updates? |
https://fetch.spec.whatwg.org/#dom-request step 19:
This is causing problems for service worker users doing
event.respondWith(fetch(event.request))
in the following situations:While w3c/ServiceWorker#607 will deal with 1, it won't solve the others.
request.url
andresponse.url
& returningResponse.redirect(response.url)
, but this results in a double-requestCould we allow manual redirects with fetch, but where the response has some degree of opaqueness for security? This would involve hiding the
location
response header at least.The text was updated successfully, but these errors were encountered: