diff options
author | Anna Henningsen <anna@addaleax.net> | 2019-09-05 21:01:18 +0300 |
---|---|---|
committer | Beth Griggs <Bethany.Griggs@uk.ibm.com> | 2019-10-01 21:41:37 +0300 |
commit | 36a0e9a06319210f7feff925840066e6bac7c3a4 (patch) | |
tree | b0e0f1bb91e21b0457c051ab70da3650189d4f08 | |
parent | c74c6a5ccf8a9c4b3775c7c8950dd16282f41c39 (diff) |
http2: do not crash on stream listener removal w/ destroyed session
Do not crash when the session is no longer available.
Fixes: https://github.com/nodejs/node/issues/29457
PR-URL: https://github.com/nodejs/node/pull/29459
Backport-PR-URL: https://github.com/nodejs/node/pull/29619
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
-rw-r--r-- | lib/internal/http2/core.js | 12 | ||||
-rw-r--r-- | test/parallel/test-http2-stream-removelisteners-after-close.js | 31 |
2 files changed, 39 insertions, 4 deletions
diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index d2f1c22ac70..5046539c570 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -422,23 +422,27 @@ function sessionListenerRemoved(name) { // Also keep track of listeners for the Http2Stream instances, as some events // are emitted on those objects. function streamListenerAdded(name) { + const session = this[kSession]; + if (!session) return; switch (name) { case 'priority': - this[kSession][kNativeFields][kSessionPriorityListenerCount]++; + session[kNativeFields][kSessionPriorityListenerCount]++; break; case 'frameError': - this[kSession][kNativeFields][kSessionFrameErrorListenerCount]++; + session[kNativeFields][kSessionFrameErrorListenerCount]++; break; } } function streamListenerRemoved(name) { + const session = this[kSession]; + if (!session) return; switch (name) { case 'priority': - this[kSession][kNativeFields][kSessionPriorityListenerCount]--; + session[kNativeFields][kSessionPriorityListenerCount]--; break; case 'frameError': - this[kSession][kNativeFields][kSessionFrameErrorListenerCount]--; + session[kNativeFields][kSessionFrameErrorListenerCount]--; break; } } diff --git a/test/parallel/test-http2-stream-removelisteners-after-close.js b/test/parallel/test-http2-stream-removelisteners-after-close.js new file mode 100644 index 00000000000..753467f0f88 --- /dev/null +++ b/test/parallel/test-http2-stream-removelisteners-after-close.js @@ -0,0 +1,31 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +const http2 = require('http2'); + +// Regression test for https://github.com/nodejs/node/issues/29457: +// HTTP/2 stream event listeners can be added and removed after the +// session has been destroyed. + +const server = http2.createServer((req, res) => { + res.end('Hi!\n'); +}); + +server.listen(0, common.mustCall(() => { + const client = http2.connect(`http://localhost:${server.address().port}`); + const headers = { ':path': '/' }; + const req = client.request(headers); + + req.on('close', common.mustCall(() => { + req.removeAllListeners(); + req.on('priority', common.mustNotCall()); + server.close(); + })); + + req.on('priority', common.mustNotCall()); + req.on('error', common.mustCall()); + + client.destroy(); +})); |