Skip to content

Commit 823fb00

Browse files
jasnellcjihrig
authored andcommitted
quic: properly pass readable/writable constructor options
PR-URL: #34283 Reviewed-By: Anna Henningsen <anna@addaleax.net>
1 parent d2917a5 commit 823fb00

File tree

3 files changed

+15
-26
lines changed

3 files changed

+15
-26
lines changed

lib/internal/quic/core.js

+7-23
Original file line numberDiff line numberDiff line change
@@ -472,21 +472,11 @@ function onStreamReady(streamHandle, id, push_id) {
472472
// state because new streams should not have been accepted at the C++
473473
// level.
474474
assert(!session.closing);
475-
476-
// TODO(@jasnell): Get default options from session
477-
const uni = id & 0b10;
478-
const {
479-
highWaterMark,
480-
defaultEncoding,
481-
} = session[kStreamOptions];
482475
const stream = new QuicStream({
483-
writable: !uni,
484-
highWaterMark,
485-
defaultEncoding,
476+
...session[kStreamOptions],
477+
writable: !(id & 0b10),
486478
}, session, push_id);
487479
stream[kSetHandle](streamHandle);
488-
if (uni)
489-
stream.end();
490480
session[kAddStream](id, stream);
491481
process.nextTick(emit.bind(session, 'stream', stream));
492482
}
@@ -2145,12 +2135,6 @@ class QuicSession extends EventEmitter {
21452135
readable: !halfOpen
21462136
}, this);
21472137

2148-
// TODO(@jasnell): This really shouldn't be necessary
2149-
if (halfOpen) {
2150-
stream.push(null);
2151-
stream.read();
2152-
}
2153-
21542138
state.pendingStreams.add(stream);
21552139

21562140
// If early data is being used, we can create the internal QuicStream on the
@@ -2516,7 +2500,7 @@ class QuicClientSession extends QuicSession {
25162500
}
25172501

25182502
function streamOnResume() {
2519-
if (!this.destroyed)
2503+
if (!this.destroyed && this.readable)
25202504
this[kHandle].readStart();
25212505
}
25222506

@@ -2546,9 +2530,13 @@ class QuicStream extends Duplex {
25462530
const {
25472531
highWaterMark,
25482532
defaultEncoding,
2533+
readable = true,
2534+
writable = true,
25492535
} = options;
25502536

25512537
super({
2538+
readable,
2539+
writable,
25522540
highWaterMark,
25532541
defaultEncoding,
25542542
allowHalfOpen: true,
@@ -2996,10 +2984,6 @@ class QuicStream extends Duplex {
29962984
defaultEncoding,
29972985
}, this.session);
29982986

2999-
// TODO(@jasnell): The null push and subsequent read shouldn't be necessary
3000-
stream.push(null);
3001-
stream.read();
3002-
30032987
stream[kSetHandle](handle);
30042988
this.session[kAddStream](stream.id, stream);
30052989
return stream;

test/parallel/test-quic-client-server.js

+4-2
Original file line numberDiff line numberDiff line change
@@ -149,11 +149,13 @@ client.on('close', common.mustCall(onSocketClose.bind(client)));
149149
assert(uni.serverInitiated);
150150
assert(!uni.clientInitiated);
151151
assert(!uni.pending);
152+
// The data and end events will never emit because
153+
// the unidirectional stream is never readable.
154+
uni.on('end', common.mustNotCall());
155+
uni.on('data', common.mustNotCall());
152156
uni.write(unidata[0], common.mustCall());
153157
uni.end(unidata[1], common.mustCall());
154158
uni.on('finish', common.mustCall());
155-
uni.on('end', common.mustCall());
156-
uni.on('data', common.mustNotCall());
157159
uni.on('close', common.mustCall(() => {
158160
assert.strictEqual(uni.finalSize, 0);
159161
}));

test/parallel/test-quic-quicsession-send-fd.js

+4-1
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,13 @@ async function test({ variant, offset, length }) {
3434
session.on('secure', common.mustCall((servername, alpn, cipher) => {
3535
const stream = session.openStream({ halfOpen: true });
3636

37+
// The data and end events won't emit because
38+
// the stream is never readable.
3739
stream.on('data', common.mustNotCall());
40+
stream.on('end', common.mustNotCall());
41+
3842
stream.on('finish', common.mustCall());
3943
stream.on('close', common.mustCall());
40-
stream.on('end', common.mustCall());
4144

4245
if (variant === 'sendFD') {
4346
fd = fs.openSync(__filename, 'r');

0 commit comments

Comments
 (0)