Re: [whatwg/fetch] Allow range header to be set by APIs (#560)

annevk commented on this pull request.

Sorry, I have more nits and this needs rebasing. I'm okay with landing this before HTML takes a dependency given that this describes existing necessary infrastructure and not a new feature.

Some tests would be good though, but it seems you've worked on this already.

> @@ -446,6 +446,20 @@ documented in <a href=#cors-protocol-exceptions>CORS protocol exceptions</a>.
 <p>A <dfn export>CORS non-wildcard request-header name</dfn> is a <a>byte-case-insensitive</a> match
 for `<code>Authorization</code>`.
 
+<p>A <dfn export>privileged no-cors request-header name</dfn> is a <a for=/>header</a>
+<a for=header>name</a> that is a <a>byte-case-insensitive</a> match for one of
+
+<ul class=brief>
+ <li>`<code>Range</code>`
+</ul>
+
+<div class="note no-backref">
+ <p>These are headers that can be set by privileged APIs, and will be preserved if their associated
+ request object is copied, but will be removed if the request is modified in any way.

Should we enumerate some of the known suspects here?

> @@ -1277,6 +1291,29 @@ or "<code>worker</code>".
   </ol>
 </ol>
 
+<hr>
+
+<p>To <dfn export for=request id=concept-request-add-range-header>add a range header</dfn> to a <a
+for=/>request</a> <var>request</var>, with an integer <var>first</var>, and an optional integer
+<var>last</var>, run these steps:
+
+<ol>
+ <li><p>Let <var>rangeValue</var> be `<code>bytes </code>`.
+
+ <li><p><a>UTF-8 encode</a> <var>first</var> and append the result to <var>rangeValue</var>.

You cannot encode an integer (same below).

> +for=/>request</a> <var>request</var>, with an integer <var>first</var>, and an optional integer
+<var>last</var>, run these steps:
+
+<ol>
+ <li><p>Let <var>rangeValue</var> be `<code>bytes </code>`.
+
+ <li><p><a>UTF-8 encode</a> <var>first</var> and append the result to <var>rangeValue</var>.
+
+ <li><p>Append `<code>-</code>` to <var>rangeValue</var>.
+
+ <li><p>If <var>last</var> is provided, then <a>UTF-8 encode</a> it and append the result to
+ <var>rangeValue</var>.
+
+ <li><p><a for="header list">Append</a> `<code>Range</code>`/<var>rangeValue</var> to
+ <var>request</var>'s <a for=request>header list</a>.
+</ol>

Maybe mention here that if you need this primitive you need to seek security review?

> @@ -1360,6 +1394,10 @@ specified. [[!CSP]]
 `<a http-header><code>Access-Control-Expose-Headers</code></a>` header. This list is used by a
 <a>CORS filtered response</a> to determine which headers to expose.
 
+<p>A <a for=/>response</a> has an associated
+<dfn for=response id=concept-response-range-requested-flag>range requested flag</dfn>, which is
+initially unset.

This still needs a note about the purpose and be changed to "range-requested" (note the hyphen).

> +  <div class=note>
+   <p>Traditionally, APIs accept a ranged response even if a range wasn't requested. However, we
+   need to prevent a partial response from an earlier ranged request being provided to an API that
+   didn't make a range request.
+
+   <p>The above step prevents the following attack attack:
+
+   <p>A media element is used to request a range of a cross-origin HTML resource. Although this is
+   invalid media, a reference to a clone of the response can be retained in a service worker. This
+   can later be used as the response to a script element's fetch. If the partial response is valid
+   JavaScript (even though the whole resource is not), executing it would leak private data.
+
+   <p>The response is replaced with an "ok" response to avoid triggering error events or rejections
+   in APIs, as this would also leak information about the response.
+  </div>
+

I wonder if this should copy some fields as we established in the CORB PR, to keep CSP and such working. I guess maybe not since it happens at a later point?

> @@ -4509,9 +4597,9 @@ method, when invoked, must run these steps:
  "<code>request</code>" and <var>name</var> is a <a>forbidden header name</a>,
  return.
 
- <li><p>Otherwise, if <a for=Headers>guard</a> is
- "<code>request-no-cors</code>" and <var>name</var>/`<code>invalid</code>` is
- not a <a>CORS-safelisted request-header</a>, then return.
+ <li><p>Otherwise, if <a for=Headers>guard</a> is "<code>request-no-cors</code>",

Given the note below we should put this `<p>` on a new line. (I realize this was already there. I can fix it therefore in a nit comment if you want.)

-- 
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/pull/560#pullrequestreview-117971626