Skip to content

Commit a48edf4

Browse files
watildeaduh95
authored andcommitted
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 f08b83b commit a48edf4

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
@@ -1332,7 +1332,7 @@ Readable.prototype.wrap = function(stream) {
13321332

13331333
// Proxy all the other methods. Important when wrapping filters and duplexes.
13341334
conststreamKeys=ObjectKeys(stream);
1335-
for(letj=1;j<streamKeys.length;j++){
1335+
for(letj=0;j<streamKeys.length;j++){
13361336
consti=streamKeys[j];
13371337
if(this[i]===undefined&&typeofstream[i]==='function'){
13381338
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+
constcommon=require('../common');
3+
constassert=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+
constsource={
16+
firstMethod(){return`first:${this===source}`;},
17+
secondMethod(){return`second:${this===source}`;},
18+
on(){returnthis;},
19+
pause(){},
20+
resume(){},
21+
};
22+
23+
assert.strictEqual(Object.keys(source)[0],'firstMethod');
24+
25+
constwrapped=newReadable().wrap(source);
26+
27+
// The method at the first key must be proxied (was `undefined` before the fix).
28+
assert.strictEqual(typeofwrapped.firstMethod,'function');
29+
assert.strictEqual(typeofwrapped.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)