Skip to content

Commit 07e27ad

Browse files
authored
stream: proxy first own method in Readable.wrap()
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> PR-URL: #64048 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Mattias Buelens <mattias@buelens.com>
1 parent 7a51702 commit 07e27ad

2 files changed

Lines changed: 41 additions & 1 deletion

File tree

lib/internal/streams/readable.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1349,7 +1349,7 @@ Readable.prototype.wrap = function(stream) {
13491349

13501350
// Proxy all the other methods. Important when wrapping filters and duplexes.
13511351
const streamKeys = ObjectKeys(stream);
1352-
for (let j = 1; j < streamKeys.length; j++) {
1352+
for (let j = 0; j < streamKeys.length; j++) {
13531353
const i = streamKeys[j];
13541354
if (this[i] === undefined && typeof stream[i] === 'function') {
13551355
this[i] = stream[i].bind(stream);
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
'use strict';
2+
const common = require('../common');
3+
const assert = require('assert');
4+
const { Readable } = require('stream');
5+
6+
// Readable.prototype.wrap() proxies the methods of the wrapped old-style
7+
// stream onto the new Readable. Regression test for an off-by-one that made
8+
// the proxy loop start at index 1, silently skipping the first own-enumerable
9+
// key returned by Object.keys() — so a method happening to sit at that first
10+
// position was never proxied.
11+
12+
// A minimal old-style stream whose *first* own-enumerable key is a method.
13+
// `Object.keys()` preserves insertion order for string keys, so `firstMethod`
14+
// is `streamKeys[0]` — exactly the slot the bug skipped.
15+
const source = {
16+
firstMethod() { return `first:${this === source}`; },
17+
secondMethod() { return `second:${this === source}`; },
18+
on() { return this; },
19+
pause() {},
20+
resume() {},
21+
};
22+
23+
assert.strictEqual(Object.keys(source)[0], 'firstMethod');
24+
25+
const wrapped = new Readable().wrap(source);
26+
27+
// The method at the first key must be proxied (was `undefined` before the fix).
28+
assert.strictEqual(typeof wrapped.firstMethod, 'function');
29+
assert.strictEqual(typeof wrapped.secondMethod, 'function');
30+
31+
// Proxied methods stay bound to the original stream.
32+
assert.strictEqual(wrapped.firstMethod(), 'first:true');
33+
assert.strictEqual(wrapped.secondMethod(), 'second:true');
34+
35+
// Existing Readable methods must not be clobbered by the proxying.
36+
assert.strictEqual(wrapped.pause, Readable.prototype.pause);
37+
assert.strictEqual(wrapped.resume, Readable.prototype.resume);
38+
39+
wrapped.on('end', common.mustNotCall());
40+
wrapped.destroy();

0 commit comments

Comments
 (0)