Skip to content

Commit 7ab7790

Browse files
committed
fixup! quic: do not destroy incoming streams that have a consumer
Signed-off-by: Naman Trivedi <trivenay@amazon.com>
1 parent e432622 commit 7ab7790

1 file changed

Lines changed: 77 additions & 6 deletions

File tree

‎test/parallel/test-quic-h3-stream-without-onstream.mjs‎

Lines changed: 77 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -130,12 +130,83 @@ function failOnConsumerWarning(warning) {
130130
await serverEndpoint.close();
131131
}
132132

133-
// --- Stream callbacks that do not expose the stream are not a consumer ---
134-
// Only `onheaders` is invoked for every incoming h3 request stream.
135-
// `ontrailers` and `oninfo` are conditional and `onwanttrailers` is
136-
// outbound-only, so a session registering only those callbacks has no way
137-
// to observe an incoming stream and it must be destroyed with the warning.
138-
for (const callbackName of ['ontrailers', 'oninfo', 'onwanttrailers']) {
133+
// Session-level stream callbacks, classified by whether the application is
134+
// guaranteed to hand every incoming stream to that callback. `onheaders` is
135+
// invoked for every incoming h3 request stream; the others are conditional
136+
// (`ontrailers`, `oninfo`) or outbound-only (`onwanttrailers`) and do not
137+
// expose the stream, so registering only those is not a consumer.
138+
const kConsumerCallbacks = ['onheaders'];
139+
const kNonConsumerCallbacks = ['oninfo', 'ontrailers', 'onwanttrailers'];
140+
141+
// --- Every session-level stream callback is classified ---
142+
// Guard: discover the callbacks the session attaches to an incoming stream
143+
// and assert each one is classified above, so a newly added stream callback
144+
// trips this test until someone puts it in the right list (and, if it is a
145+
// consumer, accepts it in QuicSession#hasStreamConsumer).
146+
{
147+
// QuicStream is not exported; obtain its prototype from a stream instance,
148+
// then offer every `on*` accessor to listen() and see which ones the
149+
// session actually attaches to a received stream.
150+
const bootstrap = await listen(mustCall((session) => {
151+
session.onerror = () => {};
152+
}), { sni: { '*': { keys: [key], certs: [cert] } }, onstream: () => {} });
153+
const bootSession = await connect(bootstrap.address, {
154+
servername: 'localhost',
155+
verifyPeer: 'manual',
156+
});
157+
await bootSession.opened;
158+
const probeStream = await bootSession.createBidirectionalStream();
159+
probeStream.onerror = () => {};
160+
const candidates = Object.getOwnPropertyNames(Object.getPrototypeOf(probeStream))
161+
.filter((name) => name.startsWith('on'));
162+
await bootSession.close();
163+
await bootstrap.close();
164+
165+
const probes = { __proto__: null };
166+
for (const name of candidates) probes[name] = () => {};
167+
168+
const applied = Promise.withResolvers();
169+
const serverEndpoint = await listen(mustCall((session) => {
170+
session.onerror = () => {};
171+
}), {
172+
__proto__: null,
173+
...probes,
174+
sni: { '*': { keys: [key], certs: [cert] } },
175+
onstream: mustCall((stream) => {
176+
applied.resolve(candidates.filter((n) => typeof stream[n] === 'function'));
177+
}),
178+
});
179+
180+
const clientSession = await connect(serverEndpoint.address, {
181+
servername: 'localhost',
182+
verifyPeer: 'manual',
183+
});
184+
await clientSession.opened;
185+
const stream = await clientSession.createBidirectionalStream({
186+
headers: {
187+
':method': 'GET',
188+
':path': '/test',
189+
':scheme': 'https',
190+
':authority': 'localhost',
191+
},
192+
});
193+
stream.onerror = () => {};
194+
195+
assert.deepStrictEqual(
196+
(await applied.promise).sort(),
197+
[...kConsumerCallbacks, ...kNonConsumerCallbacks].sort(),
198+
'A session-level stream callback was added: classify it in ' +
199+
'kConsumerCallbacks (and accept it in QuicSession#hasStreamConsumer) ' +
200+
'or kNonConsumerCallbacks.');
201+
202+
await clientSession.close();
203+
await serverEndpoint.close();
204+
}
205+
206+
// --- Callbacks that do not expose the stream are not a consumer ---
207+
// A session registering only a non-consumer callback has no way to observe
208+
// an incoming stream, so it must be destroyed with the warning.
209+
for (const callbackName of kNonConsumerCallbacks) {
139210
const warned = Promise.withResolvers();
140211
process.on('warning', function onWarning(warning) {
141212
if (warning.message === kWarning) {

0 commit comments

Comments
 (0)