From 9bc7a954fd5f9e01e9b1a1daabfbf387edcc2931 Mon Sep 17 00:00:00 2001 From: Rafael Silva Date: Wed, 2 Mar 2022 09:53:11 -0300 Subject: [PATCH] http2: close stream and session on frameError PR-URL: https://github.com/nodejs/node/pull/42147 Reviewed-By: Matteo Collina Reviewed-By: Benjamin Gruenbaum --- lib/internal/http2/core.js | 2 + src/node_http2.cc | 23 ++++++++- .../test-http2-exceeds-server-trailer-size.js | 51 +++++++++++++++++++ ...-http2-options-max-headers-block-length.js | 4 +- 4 files changed, 77 insertions(+), 3 deletions(-) create mode 100644 test/parallel/test-http2-exceeds-server-trailer-size.js diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 0960bcc5e63e40..7aebd96e09c955 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -607,6 +607,8 @@ function onFrameError(id, type, code) { const emitter = session[kState].streams.get(id) || session; emitter[kUpdateTimer](); emitter.emit('frameError', type, code, id); + session[kState].streams.get(id).close(code); + session.close(); } function onAltSvc(stream, origin, alt) { diff --git a/src/node_http2.cc b/src/node_http2.cc index 9c609f802b8695..c498d9ce12265d 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -1024,6 +1024,27 @@ void Http2Session::DecrefHeaders(const nghttp2_frame* frame) { } } +uint32_t TranslateNghttp2ErrorCode(const int libErrorCode) { + switch (libErrorCode) { + case NGHTTP2_ERR_STREAM_CLOSED: + return NGHTTP2_STREAM_CLOSED; + case NGHTTP2_ERR_HEADER_COMP: + return NGHTTP2_COMPRESSION_ERROR; + case NGHTTP2_ERR_FRAME_SIZE_ERROR: + return NGHTTP2_FRAME_SIZE_ERROR; + case NGHTTP2_ERR_FLOW_CONTROL: + return NGHTTP2_FLOW_CONTROL_ERROR; + case NGHTTP2_ERR_REFUSED_STREAM: + return NGHTTP2_REFUSED_STREAM; + case NGHTTP2_ERR_PROTO: + case NGHTTP2_ERR_HTTP_HEADER: + case NGHTTP2_ERR_HTTP_MESSAGING: + return NGHTTP2_PROTOCOL_ERROR; + default: + return NGHTTP2_INTERNAL_ERROR; + } +} + // If nghttp2 is unable to send a queued up frame, it will call this callback // to let us know. If the failure occurred because we are in the process of // closing down the session or stream, we go ahead and ignore it. We don't @@ -1060,7 +1081,7 @@ int Http2Session::OnFrameNotSent(nghttp2_session* handle, Local argv[3] = { Integer::New(isolate, frame->hd.stream_id), Integer::New(isolate, frame->hd.type), - Integer::New(isolate, error_code) + Integer::New(isolate, TranslateNghttp2ErrorCode(error_code)) }; session->MakeCallback( env->http2session_on_frame_error_function(), diff --git a/test/parallel/test-http2-exceeds-server-trailer-size.js b/test/parallel/test-http2-exceeds-server-trailer-size.js new file mode 100644 index 00000000000000..87c1070afbb7a4 --- /dev/null +++ b/test/parallel/test-http2-exceeds-server-trailer-size.js @@ -0,0 +1,51 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('assert'); +const { createServer, constants, connect } = require('http2'); + +const server = createServer(); + +server.on('stream', (stream, headers) => { + stream.respond(undefined, { waitForTrailers: true }); + + stream.on('data', common.mustNotCall()); + + stream.on('wantTrailers', common.mustCall(() => { + // Trigger a frame error by sending a trailer that is too large + stream.sendTrailers({ 'test-trailer': 'X'.repeat(64 * 1024) }); + })); + + stream.on('frameError', common.mustCall((frameType, errorCode) => { + assert.strictEqual(errorCode, constants.NGHTTP2_FRAME_SIZE_ERROR); + })); + + stream.on('error', common.expectsError({ + code: 'ERR_HTTP2_STREAM_ERROR', + })); + + stream.on('close', common.mustCall()); + + stream.end(); +}); + +server.listen(0, () => { + const clientSession = connect(`http://localhost:${server.address().port}`); + + clientSession.on('frameError', common.mustNotCall()); + clientSession.on('close', common.mustCall(() => { + server.close(); + })); + + const clientStream = clientSession.request(); + + clientStream.on('close', common.mustCall()); + // These events mustn't be called once the frame size error is from the server + clientStream.on('frameError', common.mustNotCall()); + clientStream.on('error', common.mustNotCall()); + + clientStream.end(); +}); diff --git a/test/parallel/test-http2-options-max-headers-block-length.js b/test/parallel/test-http2-options-max-headers-block-length.js index 11632c6e825c53..af1cc6f9bc4860 100644 --- a/test/parallel/test-http2-options-max-headers-block-length.js +++ b/test/parallel/test-http2-options-max-headers-block-length.js @@ -32,12 +32,12 @@ server.listen(0, common.mustCall(() => { })); req.on('frameError', common.mustCall((type, code) => { - assert.strictEqual(code, h2.constants.NGHTTP2_ERR_FRAME_SIZE_ERROR); + assert.strictEqual(code, h2.constants.NGHTTP2_FRAME_SIZE_ERROR); })); req.on('error', common.expectsError({ code: 'ERR_HTTP2_STREAM_ERROR', name: 'Error', - message: 'Stream closed with error code NGHTTP2_REFUSED_STREAM' + message: 'Stream closed with error code NGHTTP2_FRAME_SIZE_ERROR' })); }));