Skip to content

Commit 6a7d24b

Browse files
addaleaxBridgeAR
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 67aa5ef commit 6a7d24b

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
@@ -434,23 +434,27 @@ function sessionListenerRemoved(name) {
434434
// Also keep track of listeners for the Http2Stream instances, as some events
435435
// are emitted on those objects.
436436
function streamListenerAdded(name) {
437+
const session = this[kSession];
438+
if (!session) return;
437439
switch (name) {
438440
case 'priority':
439-
this[kSession][kNativeFields][kSessionPriorityListenerCount]++;
441+
session[kNativeFields][kSessionPriorityListenerCount]++;
440442
break;
441443
case 'frameError':
442-
this[kSession][kNativeFields][kSessionFrameErrorListenerCount]++;
444+
session[kNativeFields][kSessionFrameErrorListenerCount]++;
443445
break;
444446
}
445447
}
446448

447449
function streamListenerRemoved(name) {
450+
const session = this[kSession];
451+
if (!session) return;
448452
switch (name) {
449453
case 'priority':
450-
this[kSession][kNativeFields][kSessionPriorityListenerCount]--;
454+
session[kNativeFields][kSessionPriorityListenerCount]--;
451455
break;
452456
case 'frameError':
453-
this[kSession][kNativeFields][kSessionFrameErrorListenerCount]--;
457+
session[kNativeFields][kSessionFrameErrorListenerCount]--;
454458
break;
455459
}
456460
}
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)