Skip to content

stream: proxy first own method in Readable.wrap()#64048

Open
watilde wants to merge 1 commit into
nodejs:mainfrom
watilde:stream-fix-wrap-proxy
Open

stream: proxy first own method in Readable.wrap()#64048
watilde wants to merge 1 commit into
nodejs:mainfrom
watilde:stream-fix-wrap-proxy

Conversation

@watilde

@watilde watilde commented Jun 21, 2026

Copy link
Copy Markdown
Member

The method-proxying loop in Readable.prototype.wrap() started at index 1, silently skipping streamKeys[0] and leaving a method at the first own-enumerable key unproxied.

This was introduced in ee9e2a2 when ArrayPrototypeForEach( ObjectKeys(stream), ...) — which iterates from index 0 — was rewritten as a manual for loop for performance. The faithful translation starts at 0; a sibling loop converted in the same commit correctly does so. The bug stayed hidden because keys[0] is usually _events (a non-function), which the loop's typeof guard skips anyway.

The method-proxying loop in Readable.prototype.wrap() started at
index 1, silently skipping streamKeys[0] and leaving a method at the
first own-enumerable key unproxied.

This was introduced in ee9e2a2 when ArrayPrototypeForEach(
ObjectKeys(stream), ...) — which iterates from index 0 — was
rewritten as a manual for loop for performance. The faithful
translation starts at 0; a sibling loop converted in the same commit
correctly does so. The bug stayed hidden because keys[0] is usually
_events (a non-function), which the loop's typeof guard skips anyway.

Signed-off-by: Daijiro Wachi <daijiro.wachi@gmail.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/streams

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. stream Issues and PRs related to the stream subsystem. labels Jun 21, 2026

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 22, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 22, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@Renegade334 Renegade334 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. labels Jun 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. needs-ci PRs that need a full CI run. stream Issues and PRs related to the stream subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants