Re: [whatwg/fetch] Add support for blob range requests (PR #1520)

@annevk commented on this pull request.



> +     <li><p>If <var>response</var>'s <a for=response>range-requested flag</a> is unset, run the
+     following steps:
+
+     <ol>
+       <li><p>Set <var>response</var>'s <a for=response>status message</a> to `<code>OK</code>`

High-level formatting nit (this does not just apply here):
```suggestion
     <li>
      <p>If <var>response</var>'s <a for=response>range-requested flag</a> is unset, then:

      <ol>
       <li><p>Set <var>response</var>'s <a for=response>status message</a> to `<code>OK</code>`.
```

> +
+       <li><p>Append 0x2D (-) to <var>contentRange</var>.
+
+       <li><p>If <var>rangeValue</var>[1] is not null, <a lt="serialize an integer">Serialize</a> and
+       <a>isomorphic encode</a> <var>rangeValue</var>[1], and append the result to
+       <var>contentRange</var>.
+
+       <li><p>If <var>rangeValue</var>[1] is null, append <var>bodyLength</var> to
+       <var>contentRange</var>
+
+       <li><p>Append 0x2F (/) to <var>contentRange</var>.
+
+       <li><p>Append <var>bodyLength</var> to <var>contentRange</var>.
+
+       <li><p>Set <var>response</var>'s <a for=response>status message</a> to
+       `<code>Partial Content</code>`

You want to set status to 206 as well here. (It might be okay to not set status message at all and let it be the empty byte sequence as it would be in HTTP/2 and up, but I don't care strongly.)

> +     following steps:
+
+     <ol>
+       <li><p>Let <var>rangeHeader</var> be the result of <a>extracting header list values</a> given
+       `<code>Range</code>` and <var>request</var>'s <a for=response>header list</a>.
+
+       <li><p>Let <var>rangeValue</var> be the result of <a>parsing a single range header value</a>
+       given <var>rangeHeader</var>.
+
+       <li><p>If <var>rangeValue</var> is failure, then return a <a>network error</a>.
+
+       <li>
+         <p>blob.slice(<var>rangeStart</var>[0], <var>rangeValue</var>[1], <var>type</var>) here
+         on <var>body</var>.
+
+         <p class="note">TODO(dlrobertson): This requires a lot more thought.

Primarily we don't want to use the public JS API here but instead call an internal algorithm.

> +         on <var>body</var>.
+
+         <p class="note">TODO(dlrobertson): This requires a lot more thought.
+
+       <li>
+         <p>Let <var>contentRange</var> be `<code>bytes </code>`.
+
+         <p class="note">TODO(dlrobertson): Split this out into something like
+         <a>add a range header</a>
+
+       <li><p>If <var>rangeValue</var>[0] is not null, <a lt="serialize an integer">Serialize</a> and
+       <a>isomorphic encode</a> <var>rangeValue</var>[0], and append the result to
+       <var>contentRange</var>.
+
+       <li>
+         <p>If <var>rangeValue</var>[0] is null, <a lt="serialize an integer">Serialize</a> and

Maybe use "Otherwise" here instead to make it clear it's one or the other?

>       <li><p>Let <var>bodyWithType</var> be the result of <a for=BodyInit>safely extracting</a>
      <var>blobURLEntry</var>'s <a for="blob URL entry">object</a>.
 
      <li><p>Let <var>body</var> be <var>bodyWithType</var>'s <a for="body with type">body</a>.
 
-     <li><p>Let <var>length</var> be <var>body</var>'s <a for=body>length</a>,
+     <li><p>Let <var>bodyLength</var> be <var>body</var>'s <a for=body>length</a>,

Maybe `initialLength`/`originalLength`/`fullLength` is clearer?

-- 
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/fetch/pull/1520#pullrequestreview-1165389704
You are receiving this because you are subscribed to this thread.

Message ID: <whatwg/fetch/pull/1520/review/1165389704@github.com>