Skip to content

Commit e585caa

Browse files
addaleaxTrott
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 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 eaa9f83 commit e585caa

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
@@ -426,23 +426,27 @@ function sessionListenerRemoved(name) {
426426
// Also keep track of listeners for the Http2Stream instances, as some events
427427
// are emitted on those objects.
428428
function streamListenerAdded(name) {
429+
const session = this[kSession];
430+
if (!session) return;
429431
switch (name) {
430432
case 'priority':
431-
this[kSession][kNativeFields][kSessionPriorityListenerCount]++;
433+
session[kNativeFields][kSessionPriorityListenerCount]++;
432434
break;
433435
case 'frameError':
434-
this[kSession][kNativeFields][kSessionFrameErrorListenerCount]++;
436+
session[kNativeFields][kSessionFrameErrorListenerCount]++;
435437
break;
436438
}
437439
}
438440

439441
function streamListenerRemoved(name) {
442+
const session = this[kSession];
443+
if (!session) return;
440444
switch (name) {
441445
case 'priority':
442-
this[kSession][kNativeFields][kSessionPriorityListenerCount]--;
446+
session[kNativeFields][kSessionPriorityListenerCount]--;
443447
break;
444448
case 'frameError':
445-
this[kSession][kNativeFields][kSessionFrameErrorListenerCount]--;
449+
session[kNativeFields][kSessionFrameErrorListenerCount]--;
446450
break;
447451
}
448452
}
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)