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

Redirected range requests and preflights. #145

Open
mikewest opened this issue Oct 27, 2015 · 17 comments
Open

Redirected range requests and preflights. #145

mikewest opened this issue Oct 27, 2015 · 17 comments
Labels
security/privacy There are security or privacy implications topic: orb topic: redirects

Comments

@mikewest
Copy link
Member

Chrome has some funky behavior around HTMLMediaElement + redirected range requests.

https://codereview.chromium.org/1220963004 denied responses to range requests if their origin is distinct from the origin response for the initial request.

https://codereview.chromium.org/1356353003 relaxes that restriction to accept responses to range requests if they're CORS-same-origin with the origin response from the initial request. It also treats "range" as a simple header for the purposes of preflights if the request is CORS enabled (e.g. <video crossorigin ...>).

It would be nice to spec this out in a sane way. :)

@mikewest
Copy link
Member Author

+@tyoshino

@mcmanus
Copy link

mcmanus commented Oct 27, 2015

cc @mnot - I'm a bit confused on the context - is this saying that 2 different uris with 206 responses should be stitched together just because they both had the same original uri before redirection? (and if they pass cors). That seems odd - they're different resources.

@tyoshino
Copy link
Member

This scheme is already in use widely by CDNs. Chrome's HTMLMediaElement is stitching fragments served for different URLs together (see the opening comment of https://code.google.com/p/chromium/issues/detail?id=532569 by strobe). Chrome's resource loader in general doesn't.

Given the situation, it seems we could document requirements for such an approach to make sure it's secure. It doesn't necessarily require all "fetching" on the web platform to do the stitching.

@mnot
Copy link
Member

mnot commented Oct 29, 2015

Doing it generically would indeed be very broken.

To do this for a specific application (e.g., HTMLMediaElement), you need a really explicit assertion that not only are the two resources equivalent, but also that the two specific representations are exactly the same -- e.g., ETag sharing. Even then, this is not something happening in HTTP -- it has to be built on top.

See:
http://httpwg.github.io/specs/rfc7233.html#combining.byte.ranges
http://httpwg.github.io/specs/rfc7234.html#combining.responses

@annevk
Copy link
Member

annevk commented Oct 30, 2015

Are we doing this @rocallahan?

@rocallahan
Copy link

When our media resource loader takes over an HTTP load, it uses the final post-all-redirects URI as its canonical URI for the resource. All subsequent range requests start with that URI; if further redirects occur, they are honoured. The principal(s) associated with the media data are gathered from all final-URIs. If these are different origins that's generally OK: we'll still play the media, though (since at least one of those origins must not be same-origin with the page) certain APIs will be affected (e.g. after drawing a video frame to a canvas, the canvas will be tainted).

I'm not familiar with the CDN setup described in https://code.google.com/p/chromium/issues/detail?id=532569, but I assume the CDN has a canonical URI which redirects quasi-randomly to one of many mirror URIs, and the mirror URIs never do any more redirects. If so, then by using the final URI from the first load for every subsequent range request we're avoiding any issues.

@annevk
Copy link
Member

annevk commented Nov 3, 2015

Okay, so it sounds like the HTML standard would need to do this for media elements. @foolip, have you looked into doing this? It would perhaps also require some overrides then to make sure Fetch does not do anything bad upstream.

@foolip
Copy link
Member

foolip commented Nov 4, 2015

I haven't given this any thought in the spec, no. What I do know is that media elements integrate with the network layer in a rather unique way, that seems to be true of all implementations, and certainly was in Presto.

The problem of knowing that the resource is the same when requesting a second range isn't unique to redirects, even when the same server responds you in principle need some sanity checks. I doubt that these are interoperable today, and I doubt even more that doing the strict checks that would actually make sense (ETag) would really be web compatible.

@tyoshino
Copy link
Member

tyoshino commented Nov 4, 2015

Just to make sure, the proposal by @rocallahan is that once the UA receives any body bytes back from the server, it stops following further redirects?

@tyoshino
Copy link
Member

tyoshino commented Nov 9, 2015

Seems the model doesn't work for some CDNs. See this post by strobe@ from YouTube https://code.google.com/p/chromium/issues/detail?id=532569#c33

@rocallahan
Copy link

Just to make sure, the proposal by @rocallahan is that once the UA receives any body bytes back from the server, it stops following further redirects?

Sorry, I thought I was pretty clear and I'm not sure how to make it clearer:

All subsequent range requests start with that URI; if further redirects occur, they are honoured.

...

Seems the model doesn't work for some CDNs. See this post by strobe@ from YouTube https://code.google.com/p/chromium/issues/detail?id=532569#c33

That seems to be based on a misunderstanding of what I said.

@tyoshino
Copy link
Member

tyoshino commented Nov 9, 2015

I wanted to make sure I'm understanding what you said in the second paragraph correctly. It was my mistake that I referred to the paragraph by "proposal".

Thanks for replying to the crbug thread.

manno added a commit to voc/voctoweb that referenced this issue Jan 3, 2016
chrome changed its behaviour with redirects and multiple origins.
As a result playback is broken, probably since version 47:

* https://code.google.com/p/chromium/issues/detail?id=532569
* whatwg/fetch#145
@annevk annevk added the security/privacy There are security or privacy implications label Feb 26, 2018
@jakearchibald
Copy link
Collaborator

https://jewel-chair.glitch.me/same-origin.html

  • Contains an <audio> that points to /audio-redirect-second-part.
  • If the request has a Range that starts at an offset other than 0, the server redirects to /audio-normal.

Chrome: Observes the redirect. Subsequent requests go to /audio-normal.
Firefox: Observes the redirect. Subsequent requests go to /audio-redirect-second-part.

https://jewel-chair.glitch.me/same-origin-immediate-redirect.html

  • Contains an <audio> that points to /audio-redirect-first-part.
  • Always redirects to /audio-normal.

Chrome: Observes the redirect. Subsequent requests go to /audio-normal.
Firefox: Observes the redirect. Subsequent requests go to /audio-normal.

I'm looking to spec the correct behaviour here, and I'd like to do the same for other range requests like downloads.

Initially, the Firefox behaviour seems inconsistent. But, if a browser were to request multiple ranges in parallel, Chrome's behaviour could be racey.

I'm not familiar with the CDN pattern @tyoshino mentioned. Are there any further details? Do these CDNs tend to redirect for the initial range, or do they perform multiple redirects for different parts of the media resource?

annevk added a commit to web-platform-tests/wpt that referenced this issue Jan 19, 2021
Plus some minor cleanup.

We will likely have to treat Range as a special case for media elements (see whatwg/fetch#145) so creating this to ensure that only happens when Range is set by the user agent.
@annevk
Copy link
Member

annevk commented Jan 19, 2021

Range is already allowed to be set by media elements due to https://fetch.spec.whatwg.org/#unsafe-request-flag. Not necessarily great as it allows poking holes in the same-origin policy (see also #568), but that is how it is.

@annevk
Copy link
Member

annevk commented Jan 20, 2021

@horo-t @mikewest it seems Chrome has the strictest handling of media element range requests thanks to your efforts:

  • For the initial request, redirects are followed, resulting in an opaque response at a "final URL".
  • Subsequent requests are made directly to the "final URL". If this "final URL" redirects:
    • If the redirect crosses the origin boundary, error. (Firefox has this too, though it does not make subsequent requests directly to the "final URL".)
    • If the redirect does not end up at the "final URL" (e.g., directly or via another redirect), error. (Firefox does not have this.)

Given that rather weird behavior it seems we might be able to outlaw redirects for subsequent requests completely. This would also help https://github.com/annevk/orb, though it does not matter much. Is there a reason they are allowed? And if not, are you interested in simplifying that logic?

cc @padenot @anforowicz

@mikewest
Copy link
Member Author

If we can get away with dropping redirects entirely, I'd be happy too. @jakearchibald might have more context on how we landed on the current behavior?

@dalecurtis
Copy link

IIRC, the subsequent redirects are sometimes used to reauthenticate the resource. I.e., you watch a video for some time and then walk away for a couple hours, upon clicking play again the provider may need to reauthenticate your session (for content license requirements) which may redirect through some validation before going back to the final redirected URL.

jgraham pushed a commit to web-platform-tests/wpt that referenced this issue Feb 4, 2021
Plus some minor cleanup.

We will likely have to treat Range as a special case for media elements (see whatwg/fetch#145) so creating this to ensure that only happens when Range is set by the user agent.
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Feb 9, 2021
…Authorization/Range, a=testonly

Automatic update from web-platform-tests
Fetch: ensure preflight is required for Authorization/Range

Plus some minor cleanup.

We will likely have to treat Range as a special case for media elements (see whatwg/fetch#145) so creating this to ensure that only happens when Range is set by the user agent.

--

wpt-commits: 56116583e8a403e4b9d410c1429a83fbb96397f4
wpt-pr: 27240
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Feb 10, 2021
…Authorization/Range, a=testonly

Automatic update from web-platform-tests
Fetch: ensure preflight is required for Authorization/Range

Plus some minor cleanup.

We will likely have to treat Range as a special case for media elements (see whatwg/fetch#145) so creating this to ensure that only happens when Range is set by the user agent.

--

wpt-commits: 56116583e8a403e4b9d410c1429a83fbb96397f4
wpt-pr: 27240

UltraBlame original commit: fb94e67baa08bbced0bebf19114b1f7c8dc255a6
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Feb 10, 2021
…Authorization/Range, a=testonly

Automatic update from web-platform-tests
Fetch: ensure preflight is required for Authorization/Range

Plus some minor cleanup.

We will likely have to treat Range as a special case for media elements (see whatwg/fetch#145) so creating this to ensure that only happens when Range is set by the user agent.

--

wpt-commits: 56116583e8a403e4b9d410c1429a83fbb96397f4
wpt-pr: 27240

UltraBlame original commit: fb94e67baa08bbced0bebf19114b1f7c8dc255a6
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Feb 10, 2021
…Authorization/Range, a=testonly

Automatic update from web-platform-tests
Fetch: ensure preflight is required for Authorization/Range

Plus some minor cleanup.

We will likely have to treat Range as a special case for media elements (see whatwg/fetch#145) so creating this to ensure that only happens when Range is set by the user agent.

--

wpt-commits: 56116583e8a403e4b9d410c1429a83fbb96397f4
wpt-pr: 27240

UltraBlame original commit: fb94e67baa08bbced0bebf19114b1f7c8dc255a6
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Feb 11, 2021
…Authorization/Range, a=testonly

Automatic update from web-platform-tests
Fetch: ensure preflight is required for Authorization/Range

Plus some minor cleanup.

We will likely have to treat Range as a special case for media elements (see whatwg/fetch#145) so creating this to ensure that only happens when Range is set by the user agent.

--

wpt-commits: 56116583e8a403e4b9d410c1429a83fbb96397f4
wpt-pr: 27240
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
security/privacy There are security or privacy implications topic: orb topic: redirects
Development

No branches or pull requests

9 participants