Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Web Streams: commit pull-into descriptors after filling from queue #56044

Closed
MattiasBuelens opened this issue Nov 27, 2024 · 0 comments
Closed

Web Streams: commit pull-into descriptors after filling from queue #56044

MattiasBuelens opened this issue Nov 27, 2024 · 0 comments
Assignees
@MattiasBuelens
Labels
web streams web-standards Issues and PRs related to Web APIs

Comments

@MattiasBuelens
Copy link
Contributor

In GHSA-p5g2-876g-95h9, we discovered that in Chromium, a user could run JavaScript code synchronously during ReadableStreamFulfillReadIntoRequest by patching Object.prototype.then, and use this gadget to break some invariants within ReadableByteStreamControllerProcessPullIntoDescriptorsUsingQueue. Fortunately, Node.js seems unaffected.

The Streams standard has been updated with a proper fix for this case. We now postpone all calls to ReadableByteStreamControllerCommitPullIntoDescriptor until after all pull-into descriptors have been filled up by ReadableByteStreamControllerProcessPullIntoDescriptorsUsingQueue. This way, we won't trigger any patched then() method until the stream is in a stable state.
@MattiasBuelens MattiasBuelens mentioned this issue Nov 27, 2024
Merged
5 tasks
@aduh95 aduh95 added the web-standards Issues and PRs related to Web APIs label Nov 28, 2024
@MattiasBuelens MattiasBuelens self-assigned this Nov 28, 2024
MattiasBuelens added a commit to MattiasBuelens/node that referenced this issue Nov 29, 2024
@MattiasBuelens
stream: commit pull-into descriptors after filling from queue
285dbf0 
Fixes: nodejs#56044
@MattiasBuelens MattiasBuelens mentioned this issue Nov 29, 2024
Closed
aduh95 pushed a commit that referenced this issue Dec 10, 2024
@MattiasBuelens @aduh95
stream: commit pull-into descriptors after filling from queue
a4a8361 
Fixes: #56044
PR-URL: #56072
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
ruyadorno pushed a commit that referenced this issue Dec 20, 2024
@MattiasBuelens @ruyadorno
stream: commit pull-into descriptors after filling from queue
1691e62 
Fixes: #56044
PR-URL: #56072
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@MattiasBuelens MattiasBuelens

Labels
web streams web-standards Issues and PRs related to Web APIs
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

web streams: commit pull-into descriptors after filling from queue MattiasBuelens/node
2 participants
@MattiasBuelens @aduh95