diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index 407d84e7fc93..2eedfd39010a 100644 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -162,9 +162,9 @@ function onStreamEnd() { function onStreamError(error) { // Mirror HTTP/1's IncomingMessage._destroy: forward 'error' to req - // only when a listener is attached. + // only when a listener is attached. Pushed streams have no request. const request = this[kRequest]; - if (request !== undefined && request.listenerCount('error') > 0) + if (request != null && request.listenerCount('error') > 0) request.emit('error', error); } @@ -928,6 +928,10 @@ class Http2ServerResponse extends Stream { callback(err); return; } + // Pushed streams have no Http2ServerRequest to install the stream + // 'error' handler, so attach it here. Otherwise a peer refusing the + // push (e.g. RST_STREAM with REFUSED_STREAM) would crash the process. + stream.on('error', onStreamError); callback(null, new Http2ServerResponse(stream)); }); } diff --git a/test/parallel/test-http2-compat-serverresponse-createpushresponse-refused.js b/test/parallel/test-http2-compat-serverresponse-createpushresponse-refused.js new file mode 100644 index 000000000000..3b909992ef78 --- /dev/null +++ b/test/parallel/test-http2-compat-serverresponse-createpushresponse-refused.js @@ -0,0 +1,48 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +// A client refusing a push created through the compat API with +// RST_STREAM(REFUSED_STREAM) must not crash the server with an unhandled +// 'error' event on the pushed stream. + +const assert = require('assert'); +const http2 = require('http2'); + +const server = http2.createServer(common.mustCall((req, res) => { + res.createPushResponse({ ':path': '/style.css' }, common.mustSucceed((pushRes) => { + pushRes.on('close', common.mustCall()); + pushRes.setHeader('content-type', 'text/css'); + pushRes.end('body { color: red }'); + })); + res.end('hello'); +})); + +server.listen(0, common.mustCall(() => { + // A zero initial window keeps the pushed body from completing before the + // client refuses it, so the server sees a peer reset with its writable + // side still pending. + const client = http2.connect(`http://localhost:${server.address().port}`, { + settings: { initialWindowSize: 0 }, + }); + + client.on('stream', common.mustCall((pushed) => { + pushed.on('error', common.expectsError({ + code: 'ERR_HTTP2_STREAM_ERROR', + })); + pushed.close(http2.constants.NGHTTP2_REFUSED_STREAM); + // Reopen the window so the main response can complete. + client.settings({ initialWindowSize: 65535 }); + })); + + const req = client.request({ ':path': '/' }); + req.resume(); + req.on('close', common.mustCall(() => { + assert.strictEqual(req.rstCode, http2.constants.NGHTTP2_NO_ERROR); + client.close(); + server.close(); + })); + req.end(); +}));