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

RESOLVED FIXED in Firefox 65

Status

()

defect
P1
normal
RESOLVED FIXED
7 months ago
4 months ago

People

(Reporter: jorendorff, Assigned: jorendorff)

Tracking

(Blocks 1 bug)

unspecified
mozilla65
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox65 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

7 months ago
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);
```
Assignee

Updated

7 months ago
Priority: -- → P1
Assignee

Comment 1

6 months ago
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
Assignee

Comment 2

6 months ago
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.
Assignee

Comment 3

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

Comment 5

6 months ago
Pushed by jorendorff@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/817c31467dcb
Implement ReadableStreamCreateReadResult. r=arai

Comment 6

6 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/817c31467dcb
Status: NEW → RESOLVED
Last Resolved: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Assignee: nobody → jorendorff
Depends on: 1515816
You need to log in before you can comment on or make changes to this bug.