fix(ext/node): start proxy loop at index 0 in Readable.prototype.wrap() - #36566
Open
badgerbees wants to merge 1 commit into
Open
fix(ext/node): start proxy loop at index 0 in Readable.prototype.wrap()#36566badgerbees wants to merge 1 commit into
badgerbees wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ports the upstream Node.js PR #64048, which fixes the method-proxying loop in
Readable.prototype.wrap()to start at index 0 (nodejs/node#64048).Previously, the method-proxying loop in
Readable.prototype.wrap()started at index 1, silently skippingstreamKeys[0]and leaving the method at the first own-enumerable key of the wrapped stream unproxied. The bug was introduced in nodejs/node@ee9e2a2 whenArrayPrototypeForEach(ObjectKeys(stream), ...)— which iterates from index 0 — was rewritten as a manualforloop for performance. The faithful translation starts at 0; a sibling loop converted in the same commit already correctly does so. The bug stayed hidden becausekeys[0]is usually_events(a non-function), which the loop'stypeofguard skips anyway.This structurally aligns Deno's
ext/node/polyfills/internal/streams/readable.jswith Node's by changingfor (let j = 1; ...)tofor (let j = 0; ...)so the proxy loop iterates over every own-enumerable key of the wrapped stream.The proxying logic is otherwise unchanged: a key is still only proxied when
this[i] === undefined && typeof stream[i] === "function", so the previously-skipped first key (typically_events) continues to be safely ignored while any real first method is now correctly bound.Enables the upstream
parallel/test-stream2-readable-wrap-proxy-methods.js, which asserts that a function on the first own-enumerable key of the wrapped stream is proxied onto theReadable.AI Disclosure:
This PR was prepared with the assistance of an AI coding tool. The underlying code port was authored by a human; the AI was used to audit and analyze the change, compare it against the upstream Node.js PR, and draft this description.