Skip to content

Commit 36a0e9a

Browse files
addaleaxBethGriggs
authored andcommitted
http2: do not crash on stream listener removal w/ destroyed session
Do not crash when the session is no longer available. Fixes: #29457 PR-URL: #29459 Backport-PR-URL: #29619 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
1 parent c74c6a5 commit 36a0e9a

File tree

2 files changed

+39
-4
lines changed

2 files changed

+39
-4
lines changed

lib/internal/http2/core.js

+8-4
Original file line numberDiff line numberDiff line change
@@ -422,23 +422,27 @@ function sessionListenerRemoved(name) {
422422
// Also keep track of listeners for the Http2Stream instances, as some events
423423
// are emitted on those objects.
424424
function streamListenerAdded(name) {
425+
const session = this[kSession];
426+
if (!session) return;
425427
switch (name) {
426428
case 'priority':
427-
this[kSession][kNativeFields][kSessionPriorityListenerCount]++;
429+
session[kNativeFields][kSessionPriorityListenerCount]++;
428430
break;
429431
case 'frameError':
430-
this[kSession][kNativeFields][kSessionFrameErrorListenerCount]++;
432+
session[kNativeFields][kSessionFrameErrorListenerCount]++;
431433
break;
432434
}
433435
}
434436

435437
function streamListenerRemoved(name) {
438+
const session = this[kSession];
439+
if (!session) return;
436440
switch (name) {
437441
case 'priority':
438-
this[kSession][kNativeFields][kSessionPriorityListenerCount]--;
442+
session[kNativeFields][kSessionPriorityListenerCount]--;
439443
break;
440444
case 'frameError':
441-
this[kSession][kNativeFields][kSessionFrameErrorListenerCount]--;
445+
session[kNativeFields][kSessionFrameErrorListenerCount]--;
442446
break;
443447
}
444448
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
if (!common.hasCrypto)
5+
common.skip('missing crypto');
6+
const http2 = require('http2');
7+
8+
// Regression test for https://github.com/nodejs/node/issues/29457:
9+
// HTTP/2 stream event listeners can be added and removed after the
10+
// session has been destroyed.
11+
12+
const server = http2.createServer((req, res) => {
13+
res.end('Hi!\n');
14+
});
15+
16+
server.listen(0, common.mustCall(() => {
17+
const client = http2.connect(`http://localhost:${server.address().port}`);
18+
const headers = { ':path': '/' };
19+
const req = client.request(headers);
20+
21+
req.on('close', common.mustCall(() => {
22+
req.removeAllListeners();
23+
req.on('priority', common.mustNotCall());
24+
server.close();
25+
}));
26+
27+
req.on('priority', common.mustNotCall());
28+
req.on('error', common.mustCall());
29+
30+
client.destroy();
31+
}));

0 commit comments

Comments
 (0)