Re: [whatwg/streams] Resolving promises with byte sequences results in UB (Issue #1178)

> In our prototype implementation of this, what happens is we end up calling ReadableStreamDefaultReaderRead as part of the stream processing, which in turn calls ReadableStreamDefaultController.[[PullSteps]], then the [readRequest's chunk steps](https://streams.spec.whatwg.org/#default-reader-read), which resolves the promise with " «[ "value" → chunk, "done" → false ]»."

The last step you mention (resolving a promise) *should not happen* if you're using [ReadableStreamDefaultReaderRead](https://streams.spec.whatwg.org/#readable-stream-default-reader-read). That's only part of [ReadableStreamDefaultReader.read()](https://streams.spec.whatwg.org/#default-reader-read).

`ReadableStreamDefaultReader.read()` *should* return a `Promise` with a `ReadableStreamDefaultReadResult`, and since such a read result is converted by Web IDL to a JavaScript `Object`, then it's unavoidable that `Object.prototype.then` is accessed. This is expected behavior.

However, other specifications (such as Fetch) shouldn't interact with this API, they should use [read a chunk](https://streams.spec.whatwg.org/#readablestreamdefaultreader-read-a-chunk) instead. This algorithm does *not* return a promise, instead the caller must provide a [read request](https://streams.spec.whatwg.org/#read-request) which will receive the result. Since there are no promises involved, `Object.prototype.then` is never accessed.

---

For [read all bytes](https://streams.spec.whatwg.org/#readablestreamdefaultreader-read-all-bytes), the problem is that we resolve a (supposedly internal) promise with a byte sequence. A byte sequence is not a Web IDL type or an ECMAScript value, hence the undefined behavior mentioned in the OP. But even if we "fix" it by resolving with a `Uint8Array` instead, we still end up with an object (a `Uint8Array`) that could have a modified `.then` method (either through `Uint8Array.prototype.then` or `Object.prototype.then`). Thus, the resolution value of this *internal* promise can be manipulated by author code, which is *not good*.

One way to solve this is by resolving with an object that *cannot possibly* have a modified `.then` method. For example, this would work (and that's what we used to do with `forAuthorCode`):
```javascript
const result = Object.create(null);
Object.defineProperty(result, 'bytes', { value: new Uint8Array(bytes) });
resolve(result);
```
Fetch could then read the bytes in its fulfillment handler from `result.bytes`, and be confident that it hasn't been tampered with by author code.

However, this isn't very pretty. It's weird that an internal algorithm would need to construct a wrapper JavaScript object first just to resolve a promise. That's why we suggested replacing the promise with a callback, or introducing a "special" type of internal promises that are separate from ECMAScript `Promise`s.

-- 
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/issues/1178#issuecomment-948085571