diff --git a/src/node_http2.cc b/src/node_http2.cc index f5a1db0d022a18..eb33da6eae785e 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -1009,6 +1009,21 @@ int Http2Session::OnInvalidFrame(nghttp2_session* handle, return 0; } +// Remove the headers reference. +// Implicitly calls nghttp2_rcbuf_decref +void Http2Session::DecrefHeaders(const nghttp2_frame* frame) { + int32_t id = GetFrameID(frame); + BaseObjectPtr stream = FindStream(id); + + if (stream && !stream->is_destroyed() && stream->headers_count() > 0) { + Debug(this, "freeing headers for stream %d", id); + stream->ClearHeaders(); + CHECK_EQ(stream->headers_count(), 0); + DecrementCurrentSessionMemory(stream->current_headers_length_); + stream->current_headers_length_ = 0; + } +} + // 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 @@ -1029,6 +1044,11 @@ int Http2Session::OnFrameNotSent(nghttp2_session* handle, error_code == NGHTTP2_ERR_STREAM_CLOSED || error_code == NGHTTP2_ERR_STREAM_CLOSING || session->js_fields_->frame_error_listener_count == 0) { + // Nghttp2 contains header limit of 65536. When this value is exceeded the + // pipeline is stopped and we should remove the current headers reference + // to destroy the session completely. + // Further information see: https://github.com/nodejs/node/issues/35233 + session->DecrefHeaders(frame); return 0; } diff --git a/src/node_http2.h b/src/node_http2.h index e3d4d70b2d1fd5..5bd715da8a2697 100644 --- a/src/node_http2.h +++ b/src/node_http2.h @@ -401,6 +401,10 @@ class Http2Stream : public AsyncWrap, size_t i = 0; for (const auto& header : current_headers_ ) fn(header, i++); + ClearHeaders(); + } + + void ClearHeaders() { current_headers_.clear(); } @@ -787,6 +791,8 @@ class Http2Session : public AsyncWrap, void HandleAltSvcFrame(const nghttp2_frame* frame); void HandleOriginFrame(const nghttp2_frame* frame); + void DecrefHeaders(const nghttp2_frame* frame); + // nghttp2 callbacks static int OnBeginHeadersCallback( nghttp2_session* session, diff --git a/test/parallel/test-http2-options-max-headers-exceeds-nghttp2.js b/test/parallel/test-http2-options-max-headers-exceeds-nghttp2.js new file mode 100644 index 00000000000000..e503e226ef2da1 --- /dev/null +++ b/test/parallel/test-http2-options-max-headers-exceeds-nghttp2.js @@ -0,0 +1,46 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +const h2 = require('http2'); + +const server = h2.createServer(); + +server.on('stream', common.mustNotCall()); +server.on('error', common.mustNotCall()); + +server.listen(0, common.mustCall(() => { + + // Setting the maxSendHeaderBlockLength > nghttp2 threshold + // cause a 'sessionError' and no memory leak when session destroy + const options = { + maxSendHeaderBlockLength: 100000 + }; + + const client = h2.connect(`http://localhost:${server.address().port}`, + options); + client.on('error', common.expectsError({ + code: 'ERR_HTTP2_SESSION_ERROR', + name: 'Error', + message: 'Session closed with error code 9' + })); + + const req = client.request({ + // Greater than 65536 bytes + 'test-header': 'A'.repeat(90000) + }); + req.on('response', common.mustNotCall()); + + req.on('close', common.mustCall(() => { + client.close(); + server.close(); + })); + + req.on('error', common.expectsError({ + code: 'ERR_HTTP2_SESSION_ERROR', + name: 'Error', + message: 'Session closed with error code 9' + })); + req.end(); +}));