Re: [whatwg/streams] Support teeing readable byte streams (#1114)

@MattiasBuelens commented on this pull request.



> +      |r|).
+   1. If |canceled1| is false or |canceled2| is false, [=resolve=] |cancelPromise| with undefined.
+ 1. Let |pullWithDefaultReader| be the following steps:
+  1. If |reader| [=implements=] {{ReadableStreamBYOBReader}},
+   1. Assert: |reader|.[=ReadableStreamBYOBReader/[[readIntoRequests]]=] is [=list/is empty|empty=].
+   1. Perform ! [$ReadableStreamReaderGenericRelease$](|reader|).
+   1. Set |reader| to ! [$AcquireReadableStreamDefaultReader$](|stream|).
+   1. Perform |forwardReaderError|, given |reader|.
+  1. Let |readRequest| be a [=read request=] with the following [=struct/items=]:
+   : [=read request/chunk steps=], given |chunk|
+   ::
+    1. [=Queue a microtask=] to perform the following steps:
+     1. Set |reading| to false.
+     1. Let |chunk1| and |chunk2| be |chunk|.
+     1. If |canceled1| is false and |canceled2| is false, set |chunk2| to !
+        [$StructuredDeserialize$](? [$StructuredSerialize$](|chunk|), [=the current Realm=]).

Okay, this is a difficult one...

If we want to guarantee that [the tee wrapper algorithm structurally clones every chunk](https://streams.spec.whatwg.org/commit-snapshots/dd76a17a3738d78708a8dfd8f0508e717d6a1988/#readablestream-tee), then we *have* to copy the entire `ArrayBuffer`.

But this buffer comes from the BYOB request of one of the branches. So if we copy it in its entirety, we might leak data from one branch to the other branch!
```javascript
const rs = new ReadableStream({
  type: 'bytes',
  pull(c) {
    c.byobRequest.view[0] = 1;
    c.byobRequest.respond(1);
  }
});

const [branch1, branch2] = rs.tee();

// The first reader reads into its own buffer with a BYOB reader
const reader1 = branch1.getReader({ mode: 'byob' });
const buffer1 = new Uint8Array([42, 42, 42]);
const result1 = await reader1.read(buffer);
result1.value; // [1]
new Uint8Array(result1.value.buffer); // [1, 42, 42]

// The second reader reads the same data, but using a default reader
const reader2 = branch2.getReader();
const result2 = await reader2.read();
// The read chunk has the expected data
result2.value; // [1]
// ...but its backing buffer was copied from the first reader's buffer!
new Uint8Array(result2.value.buffer); // [1, 42, 42]
```
This can be bad if the first reader also puts other, potentially sensitive information inside `buffer1`. 😱 

Thoughts? Personally, I think it'd be more efficient and safer if we only copied the bytes within `chunk`'s view. So in the previous example, the last line would log `[1]` instead.

-- 
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/streams/pull/1114#discussion_r600560386