[whatwg/streams] Setting Object.prototype.then permits interfering with pipeTo() internals (#933)

The following test fails in Chromium:
```javascript
promise_test(async () => {
  const intercepted = [];

  const rs = new ReadableStream({
    start(controller) {
      controller.enqueue('a');
      controller.close();
    }
  });
  const ws = recordingWritableStream();

  let callCount = 0;
  Object.prototype.then = function(resolver) {
    if (!this.done) {
      intercepted.push(this.value);
    }
    const retval = Object.create(null);
    retval.done = ++callCount === 3;
    retval.value = callCount;
    resolver(retval);
  };

  await rs.pipeTo(ws);
  delete Object.prototype.then;

  assert_array_equals(intercepted, [], 'nothing should have been intercepted');
  assert_array_equals(ws.events, ['write', 'a', 'close'], 'written chunk should be "a"');
}, 'piping should not be observable');
```
The actual observed values of `intercepted` and `ws.events` are `["a"]` and `["write", 1, "write", 2, "close"]` respectively. This violates the design principle that the internals of pipeTo() should not be observable.

The test passes in the reference implementation, but for the wrong reason: a side-effect of the way the tests are run in Node.js is that the iterator object is created in the wrong realm. I am pretty sure from the definition of [CreateIterResultObject()](https://tc39.github.io/ecma262/#sec-createiterresultobject) that the iterator object is supposed to be created in the realm of the page.

The reason the test works/fails is this step in the standard:
* [Resolve](https://www.w3.org/2001/tag/doc/promises-guide/#resolve-promise) _readRequest_.[[promise]] with ! CreateIterResultObject().

"Resolve" ends up calling https://tc39.github.io/ecma262/#sec-promise-resolve-functions which looks up `"then"` on the prototype chain, ending up at `Object.prototype.then`. This provides a hook where the page can execute arbitrary Javascript, including reading and writing the iterator object. The hook is normally harmless, except in pipeTo().

I think it would be perfectly valid for an implementation of pipeTo() to pass this test even if CreateIterResultObject() created Objects in the current realm. I don't think pipeTo() is required to call "Resolve" or "CreateIterResultObject" at all. An optimised implementation would probably skip those steps. However, setting `Object.prototype.then` makes the optimisation observable.

I think it's important that implementations can optimise pipeTo() without it being observable to the page. I can think of a couple of ways to fix this:
1. Specify pipeTo() as not calling "resolve" or CreateIterResultObject().
2. Use a special variant of CreateIterResultObject() that sets a `null` prototype on the iterator object.

I prefer the second option. I think changing pipeTo() would add a lot of complexity to the standard text, and restrict implementation choice.

-- 
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/933