1503324 - First test in response-stream-with-broken-then.any.js fails

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Jason Orendorff [:jorendorff]]

I reproduced this in the shell. The following test requires a bit of help to get it to run; see bug 1503012 part 1.


output is:

```
FAIL - then shouldn't be called.
/Users/jorendorff/work/gecko/js/src/tests/non262/ReadableStream/broken-then.js:34:5 Error: Assertion failed: got "bye", expected "hello": The value should be "hello".
```

And here's the test:

```
let failed = false;

async function test() {
    // t.add_cleanup doesn't work when Object.prototype.then is overwritten, so
    // these tests use add_completion_callback for cleanup instead.
    const hello = "hello";
    const bye = "bye";
    const rs = new ReadableStream({
        start(controller) {
            controller.enqueue(hello);
            controller.close();
        }
    });
    const resp = {
        async text() {
            let r = rs.getReader();
            let text = "";
            let x = await r.read();
            while (!x.done) {
                text += x.value;
                x = await r.read();
            }
            return text;
        }
    };
    Object.prototype.then = (onFulfilled) => {
        print("FAIL - then shouldn't be called.");
        failed = true;
        delete Object.prototype.then;
        onFulfilled({done: false, value: bye});
    };
    const text = await resp.text();
    delete Object.prototype.then;
    assertEq(text, 'hello', 'The value should be "hello".');
}

runAsyncTest(test);
assertEq(failed, false);
```

Comment 1

[Jason Orendorff [:jorendorff]]

This is because the spec changed.

It was possible to use Object.prototype.then to spy on (or tamper with) the activity of streams that are supposedly locked:
https://github.com/whatwg/streams/issues/933

The fix was:
https://github.com/whatwg/streams/commit/a27a1fd0f8640d6054e0cbc8b6d7f5464dd096ab

Comment 2

[Jason Orendorff [:jorendorff]]

I have a partial patch. The current sticking point is this fun assumption:
https://searchfox.org/mozilla-central/rev/d850d799a0009f851b5535580e0a8b4bb2c591d7/js/src/builtin/Stream.cpp#1316

The spec added a second field to read requests: [[forAuthorCode]]. So much for that.

There are a few ways to fix it, but I don't want to allocate yet another object per read if I can help it.

Comment 3

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1503324 - Implement ReadableStreamCreateReadResult. r?arai

This prevents author code from using Object.prototype.then to observe or tamper
with a stream that is locked by another consumer.

Comment 4

[Jason Orendorff [:jorendorff]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=fceeca4edaa2781613ba7345064f72712b1d2793

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/817c31467dcb
Implement ReadableStreamCreateReadResult. r=arai

Comment 6

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/817c31467dcb