- From: Anne van Kesteren <notifications@github.com>
- Date: Thu, 24 Nov 2022 07:44:37 -0800
- To: whatwg/fetch <fetch@noreply.github.com>
- Cc: Subscribed <subscribed@noreply.github.com>
- Message-ID: <whatwg/fetch/pull/1520/review/1193316025@github.com>
@annevk commented on this pull request.
I spotted a lot of tiny nits, but I might have missed something bigger therefore. Probably needs another close look once these are addressed. Hope that's understandable.
> @@ -4648,21 +4648,102 @@ steps:
<p class=note>The `<code>GET</code>` <a for=/>method</a> restriction serves no useful purpose
other than being interoperable.
+ <li><p>Set <var>response</var> to a new <a for=/>response</a>.
+
+ <li><p>If <var>request</var>'s <a for=response>header list</a> contains `<code>Range</code>`,
+ set <var>response</var>'s <a for=response>range-requested flag</a>.
Nit:
```suggestion
then set <var>response</var>'s <a for=response>range-requested flag</a>.
```
> + <p>If <var>response</var>'s <a for=response>range-requested flag</a> is unset, run the
+ following steps:
```suggestion
<p>If <var>response</var>'s <a for=response>range-requested flag</a> is unset:
```
> + <p>Otherwise, if <var>response</var>'s <a for=response>range-requested flag</a> is set, run the
+ following steps:
```suggestion
<p>Otherwise:
```
(I was initially thinking we could maybe keep it without the "if" or have it as assert, but since there's only two branches I don't think that's warranted.)
> + <ol>
+ <li><p>Set <var>response</var>'s <a for=response>status message</a> to `<code>OK</code>`.
+
+ <li><p>Set <var>response</var>'s <a for=response>body</a> to <var>body</var>.
+
+ <li><p>Set <var>response</var>'s <a for=response>header list</a> to «
+ (`<code>Content-Length</code>`, <var>fullLength</var>), (`<code>Content-Type</code>`,
+ <var>type</var>) ».
+ </ol>
+
+ <li>
+ <p>Otherwise, if <var>response</var>'s <a for=response>range-requested flag</a> is set, run the
+ following steps:
+
+ <ol>
+ <li><p>Let <var>rangeHeader</var> be the result of <a>extracting header list values</a> given
You want to use "get" or "get, decode, and split". We're trying to get rid of "extracting header list values".
> + <p>Otherwise, if <var>response</var>'s <a for=response>range-requested flag</a> is set, run the
+ 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>Let <var>sliceEndRange</var> be null.
+
+ <li>
+ <p>If <var>rangeValue</var>[1] is non-null, let <var>sliceEndRange</var> be
then set _sliceEndRange_ to*
> + 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>Let <var>sliceEndRange</var> be null.
+
+ <li>
+ <p>If <var>rangeValue</var>[1] is non-null, let <var>sliceEndRange</var> be
+ <var>rangeValue</var>[1] incremented by one.
1 (and maybe even + 1, as Encoding does a bunch)
> + <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>Let <var>sliceEndRange</var> be null.
+
+ <li>
+ <p>If <var>rangeValue</var>[1] is non-null, let <var>sliceEndRange</var> be
+ <var>rangeValue</var>[1] incremented by one.
+
+ <p class="note">A range header denotes an inclusive byte range, while the <a>slice blob</a>
+ algorithm input range does not. To use the <a>slice blob</a> algorithm, we must increment
"must" is a normative term for making a requirement. You cannot use it in a note. And since this is a statement of fact you can say "have to".
> + <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>Let <var>sliceEndRange</var> be null.
+
+ <li>
+ <p>If <var>rangeValue</var>[1] is non-null, let <var>sliceEndRange</var> be
+ <var>rangeValue</var>[1] incremented by one.
+
+ <p class="note">A range header denotes an inclusive byte range, while the <a>slice blob</a>
Nit:
```suggestion
<p class=note>A range header denotes an inclusive byte range, while the <a>slice blob</a>
```
> + algorithm input range does not. To use the <a>slice blob</a> algorithm, we must increment
+ the parsed range header end value.
+
+ <li><p>Let <var>slicedBlob</var> be the result of invoking <a>slice blob</a> given
+ <var>blobURLEntry</var>'s <a for="blob URL entry">object</a>, <var>rangeValue</var>[0],
+ <var>sliceEndRange</var>, and <var>type</var>.
+
+ <li><p>Let <var>slicedBodyWithType</var> be the result of
+ <a for=BodyInit>safely extracting</a> <var>slicedBlob</var>.
+
+ <li><p>Set <var>response</var>'s <a for=response>body</a> to <var>slicedBodyWithType</var>'s
+ <a for="body with type">body</a>.
+
+ <li><p>Let <var>contentRange</var> be `<code>bytes </code>`.
+
+ <li><p>If <var>rangeValue</var>[0] is not null, <a lt="serialize an integer">Serialize</a> and
```suggestion
<li><p>If <var>rangeValue</var>[0] is non-null, then <a lt="serialize an integer">serialize</a> and
```
> + <li><p>Otherwise, if <var>rangeValue</var>[0] is null, append <code>`0`</code> to
+ <var>contentRange</var>.
```suggestion
<li><p>Otherwise, append 0x30 (0) to <var>contentRange</var>.
```
> +
+ <li><p>Set <var>response</var>'s <a for=response>body</a> to <var>slicedBodyWithType</var>'s
+ <a for="body with type">body</a>.
+
+ <li><p>Let <var>contentRange</var> be `<code>bytes </code>`.
+
+ <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>Otherwise, if <var>rangeValue</var>[0] is null, append <code>`0`</code> to
+ <var>contentRange</var>.
+
+ <li><p>Append 0x2D (-) to <var>contentRange</var>.
+
+ <li><p>If <var>sliceEndRange</var> is not null, <a lt="serialize an integer">Serialize</a>
```suggestion
<li><p>If <var>sliceEndRange</var> is non-null, then <a lt="serialize an integer">serialize</a>
```
> + <li><p>Otherwise, if <var>sliceEndRange</var> is null, append <var>fullLength</var> to
+ <var>contentRange</var>.
```suggestion
<li><p>Otherwise, append <var>fullLength</var> to <var>contentRange</var>.
```
> + <var>contentRange</var>.
+
+ <li><p>Otherwise, if <var>sliceEndRange</var> is null, append <var>fullLength</var> to
+ <var>contentRange</var>.
+
+ <li><p>Append 0x2F (/) to <var>contentRange</var>.
+
+ <li><p>Append <var>fullLength</var> to <var>contentRange</var>.
+
+ <li><p>Set <var>response</var>'s <a for=response>status message</a> to
+ `<code>Partial Content</code>`.
+
+ <li><p>Set <var>response</var>'s <a for=response>status</a> to `<code>206</code>`.
+
+ <li><p>Set <var>response</var>'s <a for=response>header list</a> to «
+ (`<code>Content-Length</code>`, <var>length</var>), (`<code>Content-Type</code>`,
Where is `<var>length</var>` defined?
> +
+ <li><p>If <var>sliceEndRange</var> is not null, <a lt="serialize an integer">Serialize</a>
+ and <a>isomorphic encode</a> <var>sliceEndRange</var>, and append the result to
+ <var>contentRange</var>.
+
+ <li><p>Otherwise, if <var>sliceEndRange</var> is null, append <var>fullLength</var> to
+ <var>contentRange</var>.
+
+ <li><p>Append 0x2F (/) to <var>contentRange</var>.
+
+ <li><p>Append <var>fullLength</var> to <var>contentRange</var>.
+
+ <li><p>Set <var>response</var>'s <a for=response>status message</a> to
+ `<code>Partial Content</code>`.
+
+ <li><p>Set <var>response</var>'s <a for=response>status</a> to `<code>206</code>`.
I'm pretty sure this field is an integer.
```suggestion
<li><p>Set <var>response</var>'s <a for=response>status</a> to 206.
```
> +
+ <li><p>Otherwise, if <var>sliceEndRange</var> is null, append <var>fullLength</var> to
+ <var>contentRange</var>.
+
+ <li><p>Append 0x2F (/) to <var>contentRange</var>.
+
+ <li><p>Append <var>fullLength</var> to <var>contentRange</var>.
+
+ <li><p>Set <var>response</var>'s <a for=response>status message</a> to
+ `<code>Partial Content</code>`.
+
+ <li><p>Set <var>response</var>'s <a for=response>status</a> to `<code>206</code>`.
+
+ <li><p>Set <var>response</var>'s <a for=response>header list</a> to «
+ (`<code>Content-Length</code>`, <var>length</var>), (`<code>Content-Type</code>`,
+ <var>type</var>), (`<code>Content-Range</code>`, <var>contentRange</var>) ».
I guess the assumption here is that _type_ couldn't have changed?
> +
+ <li><p>Append 0x2F (/) to <var>contentRange</var>.
+
+ <li><p>Append <var>fullLength</var> to <var>contentRange</var>.
+
+ <li><p>Set <var>response</var>'s <a for=response>status message</a> to
+ `<code>Partial Content</code>`.
+
+ <li><p>Set <var>response</var>'s <a for=response>status</a> to `<code>206</code>`.
+
+ <li><p>Set <var>response</var>'s <a for=response>header list</a> to «
+ (`<code>Content-Length</code>`, <var>length</var>), (`<code>Content-Type</code>`,
+ <var>type</var>), (`<code>Content-Range</code>`, <var>contentRange</var>) ».
+ </ol>
+
+ <li><p> return <var>response</var>.
```suggestion
<li><p>Return <var>response</var>.
```
> +
+ <p class="note">A range header denotes an inclusive byte range, while the <a>slice blob</a>
+ algorithm input range does not. To use the <a>slice blob</a> algorithm, we must increment
+ the parsed range header end value.
+
+ <li><p>Let <var>slicedBlob</var> be the result of invoking <a>slice blob</a> given
+ <var>blobURLEntry</var>'s <a for="blob URL entry">object</a>, <var>rangeValue</var>[0],
+ <var>sliceEndRange</var>, and <var>type</var>.
+
+ <li><p>Let <var>slicedBodyWithType</var> be the result of
+ <a for=BodyInit>safely extracting</a> <var>slicedBlob</var>.
+
+ <li><p>Set <var>response</var>'s <a for=response>body</a> to <var>slicedBodyWithType</var>'s
+ <a for="body with type">body</a>.
+
+ <li><p>Let <var>contentRange</var> be `<code>bytes </code>`.
I think that's fine, but let's make sure it's filed and referenced from an HTML comment?
--
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/fetch/pull/1520#pullrequestreview-1193316025
You are receiving this because you are subscribed to this thread.
Message ID: <whatwg/fetch/pull/1520/review/1193316025@github.com>
Received on Thursday, 24 November 2022 15:44:50 UTC