Re: [whatwg/fetch] Add chunk type checking of requestForServiceWorker's body's stream (#1199)

@ricea commented on this pull request.



> @@ -4156,6 +4156,33 @@ these steps:
    <li><p>Let <var>requestForServiceWorker</var> be a <a for=request>clone</a> of
    <var>request</var>.
 
+   <li>
+    <p> If <var>requestForServiceWorker</var>'s <a for=/>body</a> is non-null, then:
+
+    <ol>
+     <li> Let <var>transformAlgorithm</var> given <var>chunk</var> be these steps:
+      <ol>
+        <li><p>If the ongoing fetch is <a for=fetch>terminated</a>, then abort these steps.
+
+        <li><p>If <var>chunk</var> is not a {{Uint8Array}} object, then
+         <a lt=terminated for=fetch>terminate</a> the ongoing fetch.
+
+        <li>Otherwise, <a for=ReadableStream>enqueue</a> <var>chunk</var>. The user agent may split
+         the chunk in <a>implementation-defined</a> practical size and

I think it would be easier to understand if it was "may split the chunk _into_ implementation-defined practical _sizes_ and _enqueue_ each of them".

Do we also need to say that implementations can concatenate chunks?

> @@ -4156,6 +4156,33 @@ these steps:
    <li><p>Let <var>requestForServiceWorker</var> be a <a for=request>clone</a> of
    <var>request</var>.
 
+   <li>
+    <p> If <var>requestForServiceWorker</var>'s <a for=/>body</a> is non-null, then:
+
+    <ol>
+     <li> Let <var>transformAlgorithm</var> given <var>chunk</var> be these steps:

My understanding is that we should have a `<p>` after the `<li>` but I will defer to @annevk's judgement.

> +     <li> Let <var>transformAlgorithm</var> given <var>chunk</var> be these steps:
+      <ol>
+        <li><p>If the ongoing fetch is <a for=fetch>terminated</a>, then abort these steps.
+
+        <li><p>If <var>chunk</var> is not a {{Uint8Array}} object, then
+         <a lt=terminated for=fetch>terminate</a> the ongoing fetch.
+
+        <li>Otherwise, <a for=ReadableStream>enqueue</a> <var>chunk</var>. The user agent may split
+         the chunk in <a>implementation-defined</a> practical size and
+         <a for=ReadableStream>enqueues</a> each of them.
+      </ol>
+
+     <li> Let <var>transformStream</var> be result of <a for=TransformStream>setting up</a> a
+      {{TransformStream}} with <var>transformAlgorithm</var>.
+
+     <li> Perform <a href=https://streams.spec.whatwg.org/#readable-stream-pipe-to>ReadableStreamPipeTo</a>(

If you wait a little longer we can land https://github.com/whatwg/streams/pull/1113 which will make this cleaner.

-- 
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/1199#pullrequestreview-627982569

Received on Monday, 5 April 2021 17:07:07 UTC