Specify how media elements make and validate range requests #2814
Conversation
<span>CORS-cross-origin</span> responses aren't mixed with responses from other URLs.</p> | ||
|
||
<p>A <span>media resource</span> has a | ||
<dfn data-x="concept-media-resource-uses-opaque-data-flag">uses opaque data flag</dfn>, |
annevk
Jul 6, 2017
Member
I like "uses opaque response flag". We don't go more granular thus far than a Response
for that kind of data and I doubt we'll ever will so it seems reasonable?
I like "uses opaque response flag". We don't go more granular thus far than a Response
for that kind of data and I doubt we'll ever will so it seems reasonable?
@@ -31982,6 +31992,18 @@ interface <dfn>HTMLMediaElement</dfn> : <span>HTMLElement</span> { | |||
|
|||
</p> | |||
|
|||
<p>A <span>media resource</span> has a | |||
<dfn data-x="concept-media-resource-response-urls">list of response urls</dfn>, a | |||
<span>list</span>, which is initially empty.</p> |
jakearchibald
Jul 5, 2017
•
Author
Collaborator
Should this be an ordered set? It doesn't really matter in terms of behaviour, but an implementation using a set would use less memory.
Should this be an ordered set? It doesn't really matter in terms of behaviour, but an implementation using a set would use less memory.
annevk
Jul 6, 2017
•
Member
Say you have: URL -> URL2 -> URL where the first URL redirects but the last one doesn't due the a cookie. Would it matter if that ends up as (URL, URL2)?
Say you have: URL -> URL2 -> URL where the first URL redirects but the last one doesn't due the a cookie. Would it matter if that ends up as (URL, URL2)?
jakearchibald
Jul 6, 2017
Author
Collaborator
I don't think so. The order isn't important here either.
I want to ensure that all chunks of a particular media resource were requested from the same endpoint if opaque responses are involved. If the server redirects, that's fine. If all the responses are not-opaque, anything goes.
The attack I'm wanting to avoid is:
- Video src is
https://personal.example.com
, which contains personal data and is not a valid media element.
- Media element issues request for
0-
.
- Service worker responds with
0-100
from a resource which is a known to be a video header, where the 101st byte represents the media length.
- Media element issues request for
101-
.
- Service worker responds with
fetch(event.request)
.
Since we've already given the media element a valid video header, the additional bytes from https://personal.example.com
may leak as the video length etc etc.
However, this PR would see that the initial response url of null doesn't match https://personal.example.com
, and return a network error.
I don't think so. The order isn't important here either.
I want to ensure that all chunks of a particular media resource were requested from the same endpoint if opaque responses are involved. If the server redirects, that's fine. If all the responses are not-opaque, anything goes.
The attack I'm wanting to avoid is:
- Video src is
https://personal.example.com
, which contains personal data and is not a valid media element. - Media element issues request for
0-
. - Service worker responds with
0-100
from a resource which is a known to be a video header, where the 101st byte represents the media length. - Media element issues request for
101-
. - Service worker responds with
fetch(event.request)
.
Since we've already given the media element a valid video header, the additional bytes from https://personal.example.com
may leak as the video length etc etc.
However, this PR would see that the initial response url of null doesn't match https://personal.example.com
, and return a network error.
annevk
Jul 6, 2017
Member
So why would you store a list of URLs? Wouldn't you just stay opaque the moment you encounter it once?
So why would you store a list of URLs? Wouldn't you just stay opaque the moment you encounter it once?
jakearchibald
Jul 6, 2017
•
Author
Collaborator
"opaque" isn't enough information. All the responses could be opaque but from different places. As a result, a load of non-video data could become valid video data and leak information.
I guess I could replace this with "initial response url list", which is the url list of the first response used, and a "mixed response sources flag".
Then, if the first entry of an additional response's url list doesn't match the first entry in the "initial response url list", set the "mixed response sources flag".
If the "uses opaque data flag" is set, or the response is opaque, and "mixed response sources flag" is set, return a network error.
"opaque" isn't enough information. All the responses could be opaque but from different places. As a result, a load of non-video data could become valid video data and leak information.
I guess I could replace this with "initial response url list", which is the url list of the first response used, and a "mixed response sources flag".
Then, if the first entry of an additional response's url list doesn't match the first entry in the "initial response url list", set the "mixed response sources flag".
If the "uses opaque data flag" is set, or the response is opaque, and "mixed response sources flag" is set, return a network error.
annevk
Jul 6, 2017
Member
I see.
I see.
jakearchibald
Jul 7, 2017
Author
Collaborator
I think I'm breaking HLS by doing this. Will rethink my approach. I think "If opaque data is used, every response's (including ones previously used by this resource) first entry in the url list must match its request url" works.
I think I'm breaking HLS by doing this. Will rethink my approach. I think "If opaque data is used, every response's (including ones previously used by this resource) first entry in the url list must match its request url" works.
jakearchibald
Jul 11, 2017
Author
Collaborator
Should be fine now
Should be fine now
data-x="concept-media-resource-uses-opaque-data-flag">uses opaque data flag</span> affects | ||
whether subtitles referenced in the <span>media data</span> are exposed in the API and, for | ||
<code>video</code> elements, whether a <code>canvas</code> gets tainted when the video is | ||
drawn on it.</p> |
jakearchibald
Jul 5, 2017
Author
Collaborator
Those last few lines are adapted from some text that already existed. I'm a little concerned about how hand-wavey they are, especially as the canvas spec seems to contradict it (#2813).
Those last few lines are adapted from some text that already existed. I'm a little concerned about how hand-wavey they are, especially as the canvas spec seems to contradict it (#2813).
jakearchibald
Jul 7, 2017
Author
Collaborator
Yeah, the text here matches what browsers do. The canvas section is wrong.
Yeah, the text here matches what browsers do. The canvas section is wrong.
data-x="concept-media-ranged-fetch-steps">ranged fetch steps</span> to | ||
perform HTTP range retrieval requests for a given start and end range, or switching to a | ||
streaming protocol. The user agent must consider a resource erroneous only if it has given | ||
up trying to fetch it.</p> |
jakearchibald
Jul 5, 2017
Author
Collaborator
Again, I'm worried about the hand-waving here, but I'm not sure how to improve it without defining how each codec works.
Again, I'm worried about the hand-waving here, but I'm not sure how to improve it without defining how each codec works.
annevk
Jul 6, 2017
Member
How does fetching behavior relate to the codec?
How does fetching behavior relate to the codec?
jakearchibald
Jul 6, 2017
Author
Collaborator
- Video starts fetching.
- Container/codec decoding happens.
- Through knowledge of the codec, the browser knows the metadata sits in the last 1000 bytes of the resource.
- Fetch
Range: 123456-
Or
- User seeks resource to 10 minutes in.
- Through knowledge of the codec, the browser knows the content at 10:00 sits 9000000 bytes in.
- Fetch
Range: 9000000-
.
Without defining "Through knowledge of the codec", I'm just providing the steps for making a range request, but never really defining when it's called.
- Video starts fetching.
- Container/codec decoding happens.
- Through knowledge of the codec, the browser knows the metadata sits in the last 1000 bytes of the resource.
- Fetch
Range: 123456-
Or
- User seeks resource to 10 minutes in.
- Through knowledge of the codec, the browser knows the content at 10:00 sits 9000000 bytes in.
- Fetch
Range: 9000000-
.
Without defining "Through knowledge of the codec", I'm just providing the steps for making a range request, but never really defining when it's called.
annevk
Jul 6, 2017
Member
It might be good to call out that codec contract a little bit. So it's more clear what we're not defining. That will also make it easier for folks to tell us we're making the wrong assumptions.
It might be good to call out that codec contract a little bit. So it's more clear what we're not defining. That will also make it easier for folks to tell us we're making the wrong assumptions.
jakearchibald
Jul 6, 2017
Author
Collaborator
👍
@@ -33015,6 +33053,115 @@ interface <dfn>HTMLMediaElement</dfn> : <span>HTMLElement</span> { | |||
will end up firing a <code data-x="event-media-suspend">suspend</code> event, as described | |||
earlier.</p> | |||
|
|||
<p>The <dfn data-x="concept-media-ranged-fetch-steps">ranged fetch steps</dfn> with a | |||
<var>start</var> and optional <var>end</var> are the following steps:</p> |
jakearchibald
Jul 5, 2017
Author
Collaborator
I've written this like an inner function that can access variables defined in the parent steps. Is this in any way ok?
I've written this like an inner function that can access variables defined in the parent steps. Is this in any way ok?
annevk
Jul 6, 2017
•
Member
Probably a question for the Infra Standard, but I think generally we try to avoid that and pass parameters explicitly.
Probably a question for the Infra Standard, but I think generally we try to avoid that and pass parameters explicitly.
jakearchibald
Jul 6, 2017
Author
Collaborator
I placed it within the resource fetch algorithm because that runs for the entire load of the media element. As in, it only reaches the final step if the whole media finishes loading.
I placed it within the resource fetch algorithm because that runs for the entire load of the media element. As in, it only reaches the final step if the whole media finishes loading.
domenic
Jul 6, 2017
Member
Given the way in this this is referenced, it seems totally fine to me. It's barely a function; it's more annotating some steps by saying /* begin ranged fetch steps */
... /* end range fetch steps */
.
Given the way in this this is referenced, it seems totally fine to me. It's barely a function; it's more annotating some steps by saying /* begin ranged fetch steps */
... /* end range fetch steps */
.
list</span>'s <span data-x="concept-header-list-allow-privileged-headers">allow privilaged | ||
headers flag</span>.</p></li> | ||
|
||
<li><p>Let <var>rangeValue</var> be `<code data-x="">bytes </code>`.</p></li> |
jakearchibald
Jul 5, 2017
Author
Collaborator
I think I'm going to move these "creating a range header" steps into the fetch spec, to avoid these lines being duplicated in every spec that wants to make a range request.
I think I'm going to move these "creating a range header" steps into the fetch spec, to avoid these lines being duplicated in every spec that wants to make a range request.
annevk
Jul 6, 2017
Member
If there's at least two callers (or expected to be) that seems reasonable.
If there's at least two callers (or expected to be) that seems reasonable.
jakearchibald
Jul 6, 2017
Author
Collaborator
It'll be needed for downloads, and probably background fetch.
It'll be needed for downloads, and probably background fetch.
<li><p>Let <var>rangeValue</var> be `<code data-x="">bytes </code>`.</p></li> | ||
|
||
<li><p><span data-x="UTF-8 encode">Encode</span> <var>start</var> and append the result to | ||
<var>rangeValue</var></p></li> |
jakearchibald
Jul 5, 2017
Author
Collaborator
I wasn't really sure how to build this header. Is "append" correct here?
I wasn't really sure how to build this header. Is "append" correct here?
annevk
Jul 6, 2017
Member
For building a byte sequence that seems fine. There's some debate still over whether they should maybe be immutable, but this will probably end up working.
For building a byte sequence that seems fine. There's some debate still over whether they should maybe be immutable, but this will probably end up working.
|
||
<ol> | ||
<li><p>If <var>firstResponseUrl</var> is null, and <var>url</var> is not null, return a | ||
<span>network error</span>.</p></li> |
jakearchibald
Jul 5, 2017
Author
Collaborator
Wait, this is wrong. brb
Wait, this is wrong. brb
annevk
Jan 15, 2021
Member
Do you remember why you said this was wrong? I am seeing the same in Chrome. Firefox is more lenient (though does require 2xx). Not sure exactly what Safari wants here, I cannot get 200 to work there.
Do you remember why you said this was wrong? I am seeing the same in Chrome. Firefox is more lenient (though does require 2xx). Not sure exactly what Safari wants here, I cannot get 200 to work there.
jakearchibald
Jan 15, 2021
Author
Collaborator
I can't remember why this is wrong. Safari doesn't like 200 here, but I think it should work (as described in the steps below).
There are other issues with this PR though, which I'll detail in a follow-up comment.
I can't remember why this is wrong. Safari doesn't like 200 here, but I think it should work (as described in the steps below).
There are other issues with this PR though, which I'll detail in a follow-up comment.
<span data-x="concept-request-header-list">header list</span>.</p></li> | ||
|
||
<li><p>If <var>range</var>'s first-byte-pos doesn't equal <var>start</var>, return a | ||
<span>network error</span>.</p></li> |
jakearchibald
Jul 5, 2017
Author
Collaborator
Can I refer to first-byte-pos like this? It's from https://tools.ietf.org/html/rfc7233#section-4.2.
Can I refer to first-byte-pos like this? It's from https://tools.ietf.org/html/rfc7233#section-4.2.
annevk
Jul 6, 2017
Member
It might be good to cross-reference that term. Given the tight integration with HTTP uplifting to Fetch makes more sense to me now, even if this is the only caller.
It might be good to cross-reference that term. Given the tight integration with HTTP uplifting to Fetch makes more sense to me now, even if this is the only caller.
response even if it requested a range. It <!--non-normative-->may terminate fetches | ||
that are (or become) redundant, e.g. if the returned range is already covered by a | ||
previous or in-flight response, or the media is seeked as such that active fetches are | ||
unlikely to be useful.</p> |
jakearchibald
Jul 5, 2017
Author
Collaborator
Is this too much detail for a note? It's difficult since the process of aborting fetches isn't really covered.
Is this too much detail for a note? It's difficult since the process of aborting fetches isn't really covered.
domenic
Jul 6, 2017
Member
I think it needs to not use should and may in a note. You can e.g. substitute "encouraged" and "could". Otherwise it does seem helpful.
I think it needs to not use should and may in a note. You can e.g. substitute "encouraged" and "could". Otherwise it does seem helpful.
@foolip Could you take a look at this? |
Going on vacation soon and won't have time to finish all things, like this. I'll check back early in August to see if this is still open. |
associated: | ||
<ul class="brief"> | ||
<li><dfn data-x="concept-header-list-allow-privileged-headers" data-x-href="https://fetch.spec.whatwg.org/#concept-header-list-allow-privileged-headers">allow privileged headers flag</dfn></li> | ||
<li><dfn data-x="concept-header-list-append" data-x-href="https://fetch.spec.whatwg.org/#concept-header-list-append">append</dfn> method</li> |
domenic
Jul 6, 2017
Member
Nit: s/method/algorithm, I think.
Nit: s/method/algorithm, I think.
<span>CORS-cross-origin</span> responses aren't mixed with responses from other URLs.</p> | ||
|
||
<p>A <span>media resource</span> has a | ||
<dfn data-x="concept-media-resource-uses-opaque-data-flag">uses opaque data flag</dfn>, |
domenic
Jul 6, 2017
Member
Nit: <dfn
goes on previous line. The Great Re-Wrapper can help by turning unwrapped lines into wrapped ones.
Nit: <dfn
goes on previous line. The Great Re-Wrapper can help by turning unwrapped lines into wrapped ones.
jakearchibald
Jul 7, 2017
Author
Collaborator
I wasn't aware of that tool, cheers!
(fwiw I'd rather we just relied on editors for this kind of wrapping. Breaking mid-line makes it hard to search for terms across the document)
I wasn't aware of that tool, cheers!
(fwiw I'd rather we just relied on editors for this kind of wrapping. Breaking mid-line makes it hard to search for terms across the document)
@@ -31982,6 +31992,18 @@ interface <dfn>HTMLMediaElement</dfn> : <span>HTMLElement</span> { | |||
|
|||
</p> | |||
|
|||
<p>A <span>media resource</span> has a |
domenic
Jul 6, 2017
Member
Do you think media resource is the right place to store these things? So far in the spec it seems less like a spec data structure with fields and more like a concept. Maybe putting them on the media element might be more appropriate.
Do you think media resource is the right place to store these things? So far in the spec it seems less like a spec data structure with fields and more like a concept. Maybe putting them on the media element might be more appropriate.
jakearchibald
Jul 6, 2017
Author
Collaborator
I use media resource as it changes when the src changes. If that happens, we want the properties to reset.
I use media resource as it changes when the src changes. If that happens, we want the properties to reset.
domenic
Jul 6, 2017
Member
Fair enough.
Fair enough.
<span>CORS-cross-origin</span> responses aren't mixed with responses from other URLs.</p> | ||
|
||
<p>A <span>media resource</span> has a | ||
<dfn data-x="concept-media-resource-uses-opaque-data-flag">uses opaque data flag</dfn>, |
domenic
Jul 6, 2017
Member
Since we're trying to move away from flags and to booleans, maybe this should be a boolean? OK either way though; maybe we should only do an en-masse conversion instead of mixing up both into the spec.
Since we're trying to move away from flags and to booleans, maybe this should be a boolean? OK either way though; maybe we should only do an en-masse conversion instead of mixing up both into the spec.
jakearchibald
Jul 7, 2017
Author
Collaborator
I wasn't aware of the flag->boolean change. Happy to change it here.
I wasn't aware of the flag->boolean change. Happy to change it here.
@@ -32933,11 +32955,25 @@ interface <dfn>HTMLMediaElement</dfn> : <span>HTMLElement</span> { | |||
|
|||
<!--FETCH--><p><span data-x="concept-fetch">Fetch</span> <var>request</var>. |
domenic
Jul 6, 2017
Member
What is the relation between this request and the one possibly created by the range request steps? It seems like we're normatively requiring this one, and then if the browser wants to do a range request, it needs to do a second request. Is that correct?
What is the relation between this request and the one possibly created by the range request steps? It seems like we're normatively requiring this one, and then if the browser wants to do a range request, it needs to do a second request. Is that correct?
jakearchibald
Jul 7, 2017
Author
Collaborator
Yeah, that's what I intended. It feels like browsers should make a non-range request first to get the start of the resource, but also to determine if ranges are accepted.
Yeah, that's what I intended. It feels like browsers should make a non-range request first to get the start of the resource, but also to determine if ranges are accepted.
<p>If <var>responseUrlList</var>[0] <span data-x="list contains">exists</span>, <span | ||
data-x="list append">append</span> <var>responseUrlList</var>[0] to the <var>current media | ||
resource</var>'s <span data-x="concept-media-resource-response-urls">list of response | ||
urls</span>.</p> |
domenic
Jul 6, 2017
Member
It is a bit surprising we append the first, not the last (i.e. not the response's url). A note explaining that we are intentionally doing this for the purpose of tracking redirects would be good, IMO.
It is a bit surprising we append the first, not the last (i.e. not the response's url). A note explaining that we are intentionally doing this for the purpose of tracking redirects would be good, IMO.
data-x="concept-media-resource-uses-opaque-data-flag">uses opaque data flag</span> affects | ||
whether subtitles referenced in the <span>media data</span> are exposed in the API and, for | ||
<code>video</code> elements, whether a <code>canvas</code> gets tainted when the video is | ||
drawn on it.</p> |
data-x="concept-media-ranged-fetch-steps">ranged fetch steps</span> to | ||
perform HTTP range retrieval requests for a given start and end range, or switching to a | ||
streaming protocol. The user agent must consider a resource erroneous only if it has given | ||
up trying to fetch it.</p> |
domenic
Jul 6, 2017
Member
I have a separate worry about the hand-waving here, which is that it gives the user agent license to do literally anything. For example a user agent could issue range requests but not use the ranged fetch steps. Maybe the intent is clear enough though that we don't need to change it.
I have a separate worry about the hand-waving here, which is that it gives the user agent license to do literally anything. For example a user agent could issue range requests but not use the ranged fetch steps. Maybe the intent is clear enough though that we don't need to change it.
jakearchibald
Jul 7, 2017
Author
Collaborator
I'm worried about the "streaming protocol" bit, in that it might not follow the rules in terms of mixing opaque & non-opaque responses.
I'm worried about the "streaming protocol" bit, in that it might not follow the rules in terms of mixing opaque & non-opaque responses.
<span>environment settings object</span> and | ||
<span data-x="concept-request-type">type</span> to "<code data-x="">audio</code>" if the | ||
<span>media element</span> is an <code>audio</code> element and to "<code | ||
data-x="">video</code>" otherwise.</p></li> |
domenic
Jul 6, 2017
Member
This would be clearer as two steps IMO.
This would be clearer as two steps IMO.
response even if it requested a range. It <!--non-normative-->may terminate fetches | ||
that are (or become) redundant, e.g. if the returned range is already covered by a | ||
previous or in-flight response, or the media is seeked as such that active fetches are | ||
unlikely to be useful.</p> |
domenic
Jul 6, 2017
Member
I think it needs to not use should and may in a note. You can e.g. substitute "encouraged" and "could". Otherwise it does seem helpful.
I think it needs to not use should and may in a note. You can e.g. substitute "encouraged" and "could". Otherwise it does seem helpful.
I'm going to move on to range requests & downloads, but in the mean time… any ideas how I could write tests? I think all browsers will issue ranges requests for a simple wav file that's seeked, which I can automate in browsers that support autoplay. However, the spec doesn't require browsers to make range requests, so it feels like a bad approach. Is there any kind of "unable to test" warning I could create in this situation. Or, should I make it a manual test, where you have to seek a media element to generate range requests? |
data-x="concept-request-header-list">header list</span>.</p></li> | ||
|
||
<li><p>If <var>range</var>'s <a | ||
href="https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.16">first-byte-pos</a> |
jakearchibald
Jul 11, 2017
Author
Collaborator
@annevk Is there a better way to link to this? The HTTP spec doesn't allow linking to the term directly.
@annevk Is there a better way to link to this? The HTTP spec doesn't allow linking to the term directly.
domenic
Jul 11, 2017
Member
Well, it should be abstracted out into a <span>
with the appropriate <dfn data-x-href="">
in the dependencies section, for one, but that's just editorial...
Well, it should be abstracted out into a <span>
with the appropriate <dfn data-x-href="">
in the dependencies section, for one, but that's just editorial...
annevk
Jul 12, 2017
Member
Yeah, other than that I was thinking that maybe this should be in Fetch given the tight integration with HTTP?
Yeah, other than that I was thinking that maybe this should be in Fetch given the tight integration with HTTP?
jakearchibald
Jul 12, 2017
Author
Collaborator
Wouldn't that mean duplicating HTTP's definition?
Wouldn't that mean duplicating HTTP's definition?
annevk
Jul 12, 2017
Member
We'd still reference HTTP directly in Fetch as you did here, but for everyone else in the platform we'd have it abstracted.
We'd still reference HTTP directly in Fetch as you did here, but for everyone else in the platform we'd have it abstracted.
jakearchibald
Jul 12, 2017
•
Author
Collaborator
It's be nice to have a header list algorithm that took a header name, and returned the parsed parts as defined by HTTP. It might be nice to expose this on the headers object too.
Eg: Let parsedRange be the result of parsing a header given headerList and Range
.
Where parsedRange would be a structure like:
[
{
"ranges-specifier": "bytes=0-",
"byte-ranges-specifier": "bytes=0-",
"bytes-unit": "bytes",
"byte-range-set": "0-",
"suffix-byte-range-spec": null,
"byte-range-spec": "0-",
"first-byte-pos": "0",
"last-byte-pos": null
}
]
It's be nice to have a header list algorithm that took a header name, and returned the parsed parts as defined by HTTP. It might be nice to expose this on the headers object too.
Eg: Let parsedRange be the result of parsing a header given headerList and Range
.
Where parsedRange would be a structure like:
[
{
"ranges-specifier": "bytes=0-",
"byte-ranges-specifier": "bytes=0-",
"bytes-unit": "bytes",
"byte-range-set": "0-",
"suffix-byte-range-spec": null,
"byte-range-spec": "0-",
"first-byte-pos": "0",
"last-byte-pos": null
}
]
annevk
Jul 12, 2017
Member
That would be great, but that would only work for headers the browser supports (and might be tricky given where that code lives).
That would be great, but that would only work for headers the browser supports (and might be tricky given where that code lives).
In general it's OK to write tests that aren't fully required by the spec. This is especially true if all browsers pass them. (Or intend to pass them, e.g. all use range requests.) |
Will pick this up again soon. |
There are security issues with this PR, especially the way it checks only the first response URL. The final response URL is the important one, although there might need to be further restrictions if the redirect goes cross-origin then comes back to the same final url, unless we want that to be observable. whatwg/fetch#144 (comment) is a better summary of the issues and solutions. Background fetch has some algorithms for validating partial responses, which might be useful here, although those are CORS requests https://wicg.github.io/background-fetch/#validate-partial-response-algorithm, the validation for opaque responses will need to be stricter. I'm happy for this PR to close. It's probably easier to start again. |
I'm looking for early feedback on this. I'm going to add some comments to highlight parts I'm not sure about.
PR Preview failed to build. (Last tried on Jan 21, 2021, 12:09 PM UTC).
More
PR Preview relies on a number of web services to run. There seems to be an issue with the following one:
If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please file an issue.