Skip to content

Commit 3486489

Browse files
committed
stream: ensure pipeToSync requires synchronous close
Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode PR-URL: #65652 Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent c7447bf commit 3486489

4 files changed

Lines changed: 36 additions & 17 deletions

File tree

lib/internal/streams/iter/pull.js

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ const {
2424
codes: {
2525
ERR_INVALID_ARG_TYPE,
2626
ERR_INVALID_ARG_VALUE,
27+
ERR_INVALID_STATE,
2728
ERR_OUT_OF_RANGE,
2829
},
2930
} = require('internal/errors');
@@ -981,6 +982,13 @@ function pullWithConsumerCleanup(source, transforms, signal) {
981982
*/
982983
function pipeToSync(source, ...args) {
983984
const { transforms, writer, options } = parsePipeToArgs(args, 'writeSync');
985+
const hasWritevSync = typeof writer.writevSync === 'function';
986+
const endSync = writer.endSync;
987+
988+
if (!options?.preventClose && typeof endSync !== 'function') {
989+
throw new ERR_INVALID_ARG_TYPE(
990+
'writer.endSync', 'Function', endSync);
991+
}
984992

985993
// Normalize source and create pipeline
986994
const normalized = fromSync(source);
@@ -989,8 +997,6 @@ function pipeToSync(source, ...args) {
989997
normalized;
990998

991999
let totalBytes = 0;
992-
const hasWritevSync = typeof writer.writevSync === 'function';
993-
const hasEndSync = typeof writer.endSync === 'function';
9941000

9951001
try {
9961002
for (const batch of pipeline) {
@@ -1019,8 +1025,9 @@ function pipeToSync(source, ...args) {
10191025
}
10201026

10211027
if (!options?.preventClose) {
1022-
if (!hasEndSync || writer.endSync() < 0) {
1023-
writer.end?.();
1028+
if (FunctionPrototypeCall(endSync, writer) < 0) {
1029+
throw new ERR_INVALID_STATE(
1030+
'Writer could not be closed synchronously');
10241031
}
10251032
}
10261033
} catch (error) {

test/parallel/test-stream-iter-pipeto-edge.js

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,41 @@
11
// Flags: --experimental-stream-iter
22
'use strict';
33

4-
// Edge case tests for pipeToSync: endSync fallback, preventFail.
4+
// Edge case tests for pipeToSync close and failure behavior.
55

66
const common = require('../common');
77
const assert = require('assert');
88
const { pipeToSync, fromSync } = require('stream/iter');
99

10-
// pipeToSync endSync returns negative → falls back to end()
11-
async function testPipeToSyncEndSyncFallback() {
10+
// pipeToSync cannot complete when endSync() requires async fallback.
11+
async function testPipeToSyncEndSyncFailure() {
1212
let endCalled = false;
1313
const writer = {
1414
writeSync() { return true; },
15-
endSync() { return -1; }, // Negative → triggers end() fallback
15+
endSync() { return -1; },
1616
end() { endCalled = true; },
1717
};
18-
pipeToSync(fromSync('data'), writer);
19-
assert.strictEqual(endCalled, true);
18+
assert.throws(
19+
() => pipeToSync(fromSync('data'), writer, { preventFail: true }),
20+
{ code: 'ERR_INVALID_STATE' },
21+
);
22+
assert.strictEqual(endCalled, false);
2023
}
2124

22-
// pipeToSync endSync missing → falls back to end()
25+
// pipeToSync requires endSync() when closing is enabled.
2326
async function testPipeToSyncNoEndSync() {
27+
let writeCalled = false;
2428
let endCalled = false;
2529
const writer = {
26-
writeSync() { return true; },
30+
writeSync() { writeCalled = true; return true; },
2731
end() { endCalled = true; },
2832
};
29-
pipeToSync(fromSync('data'), writer);
30-
assert.strictEqual(endCalled, true);
33+
assert.throws(
34+
() => pipeToSync(fromSync('data'), writer),
35+
{ code: 'ERR_INVALID_ARG_TYPE' },
36+
);
37+
assert.strictEqual(writeCalled, false);
38+
assert.strictEqual(endCalled, false);
3139
}
3240

3341
// pipeToSync with preventFail: true — source error does NOT call fail()
@@ -61,7 +69,7 @@ async function testPipeToSyncPreventClose() {
6169
}
6270

6371
Promise.all([
64-
testPipeToSyncEndSyncFallback(),
72+
testPipeToSyncEndSyncFailure(),
6573
testPipeToSyncNoEndSync(),
6674
testPipeToSyncPreventFail(),
6775
testPipeToSyncPreventClose(),

test/parallel/test-stream-iter-pipeto.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ async function testPipeToSyncSourceError() {
7171
let failCalled = false;
7272
const writer = {
7373
writeSync() { return true; },
74+
endSync: common.mustNotCall(),
7475
fail(reason) { failCalled = true; },
7576
};
7677
function* failingSource() {
@@ -127,6 +128,7 @@ async function testPipeToSyncWithTransforms() {
127128
const chunks = [];
128129
const writer = {
129130
writeSync(chunk) { chunks.push(new TextDecoder().decode(chunk)); return true; },
131+
endSync() { return 0; },
130132
};
131133
const upper = (batch) => {
132134
if (batch === null) return null;
@@ -160,6 +162,7 @@ async function testPipeToSyncWriterTransformMethodIgnored() {
160162
chunks.push(new TextDecoder().decode(chunk));
161163
return true;
162164
},
165+
endSync() { return 0; },
163166
};
164167

165168
pipeToSync(fromSync('hello'), writer);
@@ -240,7 +243,7 @@ async function testPipeToSyncMinimalWriter() {
240243
},
241244
};
242245

243-
pipeToSync(fromSync('minimal-sync'), minimalWriter);
246+
pipeToSync(fromSync('minimal-sync'), minimalWriter, { preventClose: true });
244247
assert.strictEqual(chunks.length > 0, true);
245248
}
246249

test/parallel/test-stream-iter-resizable-buffers.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,8 @@ async function testPipeRejectsWriterResize() {
161161
fail: common.mustCall(),
162162
};
163163
assert.throws(
164-
() => pipeToSync([new Uint8Array(syncBuffer)], syncWriter),
164+
() => pipeToSync(
165+
[new Uint8Array(syncBuffer)], syncWriter, { preventClose: true }),
165166
kResizeError,
166167
);
167168
}

0 commit comments

Comments
 (0)