Skip to content

Commit aa6913c

Browse files
trivikraduh95
authored andcommitted
stream: validate fromWritable() options before cache
Validate options before returning a cached fromWritable() adapter so invalid later options still throw. Cache adapters by backpressure policy as well as Writable instance, since the policy changes write behavior. Fixes: #63277 Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com> Assisted-by: openai:gpt-5.5 PR-URL: #63278Fixes: #63277 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
1 parent 6c53ddb commit aa6913c

3 files changed

Lines changed: 61 additions & 7 deletions

File tree

‎doc/api/stream_iter.md‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1517,8 +1517,9 @@ the synchronous Writer methods (`writeSync`, `writevSync`, `endSync`) always
15171517
return `false` or `-1`, deferring to the async path. The per-write
15181518
`options.signal` parameter from the Writer interface is also ignored.
15191519

1520-
The result is cached per instance -- calling `fromWritable()` twice with the
1521-
same stream returns the same Writer.
1520+
The result is cached per instance and backpressure policy -- calling
1521+
`fromWritable()` twice with the same stream and `backpressure` option returns
1522+
the same Writer.
15221523

15231524
For duck-typed streams that do not expose `writableHighWaterMark`,
15241525
`writableLength`, or similar properties, sensible defaults are used.

‎lib/internal/streams/iter/classic.js‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ const {
2121
PromiseReject,
2222
PromiseResolve,
2323
PromiseWithResolvers,
24+
SafeMap,
2425
SafeWeakMap,
2526
SymbolAsyncDispose,
2627
SymbolAsyncIterator,
@@ -427,10 +428,6 @@ function fromWritable(writable, options = kNullPrototype) {
427428
thrownewERR_INVALID_ARG_TYPE('writable','Writable',writable);
428429
}
429430

430-
// Return cached adapter if available.
431-
constcached=fromWritableCache.get(writable);
432-
if(cached!==undefined)returncached;
433-
434431
validateObject(options,'options');
435432
const{
436433
backpressure ='strict',
@@ -459,6 +456,17 @@ function fromWritable(writable, options = kNullPrototype) {
459456
'drop-oldest is not supported for classic stream.Writable');
460457
}
461458

459+
// Return cached adapter if available. Backpressure policy changes writer
460+
// behavior, so cache one adapter per policy.
461+
letcachedByBackpressure=fromWritableCache.get(writable);
462+
if(cachedByBackpressure!==undefined){
463+
constcached=cachedByBackpressure.get(backpressure);
464+
if(cached!==undefined)returncached;
465+
}else{
466+
cachedByBackpressure=newSafeMap();
467+
fromWritableCache.set(writable,cachedByBackpressure);
468+
}
469+
462470
// Fall back to sensible defaults for duck-typed streams that may not
463471
// expose the full stream.Writable property set.
464472
consthwm=writable.writableHighWaterMark??16384;
@@ -710,7 +718,7 @@ function fromWritable(writable, options = kNullPrototype) {
710718
returnpromise;
711719
};
712720

713-
fromWritableCache.set(writable,writer);
721+
cachedByBackpressure.set(backpressure,writer);
714722
returnwriter;
715723
}
716724

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
// Flags: --experimental-stream-iter
2+
'use strict';
3+
4+
constcommon=require('../common');
5+
constassert=require('assert');
6+
const{ Writable }=require('stream');
7+
const{ fromWritable }=require('stream/iter');
8+
9+
{
10+
constwritable=newWritable({write(){}});
11+
12+
fromWritable(writable);
13+
14+
assert.throws(
15+
()=>fromWritable(writable,{backpressure: 'invalid'}),
16+
{code: 'ERR_INVALID_ARG_VALUE'},
17+
);
18+
19+
writable.destroy();
20+
}
21+
22+
asyncfunctiontestCachedWritableUsesLaterBackpressureOptions(){
23+
constchunks=[];
24+
constwritable=newWritable({
25+
highWaterMark: 1,
26+
write(chunk,encoding,callback){
27+
chunks.push(Buffer.from(chunk));
28+
},
29+
});
30+
31+
fromWritable(writable);
32+
constwriter=fromWritable(writable,{backpressure: 'drop-newest'});
33+
34+
awaitwriter.write('a');
35+
awaitwriter.write('b');
36+
37+
assert.deepStrictEqual(
38+
chunks.map((chunk)=>chunk.toString()),
39+
['a'],
40+
);
41+
42+
writable.destroy();
43+
}
44+
45+
testCachedWritableUsesLaterBackpressureOptions().then(common.mustCall());

0 commit comments

Comments
 (0)