From 1f045f491a6a8d6ac193474f3856e2bdbd6847be Mon Sep 17 00:00:00 2001 From: Bryan English Date: Tue, 24 Oct 2017 23:34:43 -0700 Subject: [PATCH 1/2] http: use arrow fns for lexical `this` in Agent PR-URL: https://github.com/nodejs/node/pull/16475 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Gireesh Punathil Reviewed-By: Luigi Pinca Reviewed-By: Yuta Hiroto Reviewed-By: Colin Ihrig Reviewed-By: James M Snell --- lib/_http_agent.js | 80 ++++++++++++++++++++++------------------------ 1 file changed, 39 insertions(+), 41 deletions(-) diff --git a/lib/_http_agent.js b/lib/_http_agent.js index 564eab92543..b0e011d40c4 100644 --- a/lib/_http_agent.js +++ b/lib/_http_agent.js @@ -46,33 +46,31 @@ function Agent(options) { EventEmitter.call(this); - var self = this; + this.defaultPort = 80; + this.protocol = 'http:'; - self.defaultPort = 80; - self.protocol = 'http:'; - - self.options = util._extend({}, options); + this.options = util._extend({}, options); // don't confuse net and make it think that we're connecting to a pipe - self.options.path = null; - self.requests = {}; - self.sockets = {}; - self.freeSockets = {}; - self.keepAliveMsecs = self.options.keepAliveMsecs || 1000; - self.keepAlive = self.options.keepAlive || false; - self.maxSockets = self.options.maxSockets || Agent.defaultMaxSockets; - self.maxFreeSockets = self.options.maxFreeSockets || 256; - - self.on('free', function(socket, options) { - var name = self.getName(options); + this.options.path = null; + this.requests = {}; + this.sockets = {}; + this.freeSockets = {}; + this.keepAliveMsecs = this.options.keepAliveMsecs || 1000; + this.keepAlive = this.options.keepAlive || false; + this.maxSockets = this.options.maxSockets || Agent.defaultMaxSockets; + this.maxFreeSockets = this.options.maxFreeSockets || 256; + + this.on('free', (socket, options) => { + var name = this.getName(options); debug('agent.on(free)', name); if (socket.writable && - self.requests[name] && self.requests[name].length) { - self.requests[name].shift().onSocket(socket); - if (self.requests[name].length === 0) { + this.requests[name] && this.requests[name].length) { + this.requests[name].shift().onSocket(socket); + if (this.requests[name].length === 0) { // don't leak - delete self.requests[name]; + delete this.requests[name]; } } else { // If there are no pending requests, then put it in @@ -81,21 +79,21 @@ function Agent(options) { if (req && req.shouldKeepAlive && socket.writable && - self.keepAlive) { - var freeSockets = self.freeSockets[name]; + this.keepAlive) { + var freeSockets = this.freeSockets[name]; var freeLen = freeSockets ? freeSockets.length : 0; var count = freeLen; - if (self.sockets[name]) - count += self.sockets[name].length; + if (this.sockets[name]) + count += this.sockets[name].length; - if (count > self.maxSockets || freeLen >= self.maxFreeSockets) { + if (count > this.maxSockets || freeLen >= this.maxFreeSockets) { socket.destroy(); - } else if (self.keepSocketAlive(socket)) { + } else if (this.keepSocketAlive(socket)) { freeSockets = freeSockets || []; - self.freeSockets[name] = freeSockets; + this.freeSockets[name] = freeSockets; socket[async_id_symbol] = -1; socket._httpMessage = null; - self.removeSocket(socket, options); + this.removeSocket(socket, options); freeSockets.push(socket); } else { // Implementation doesn't want to keep socket alive @@ -196,39 +194,39 @@ Agent.prototype.addRequest = function addRequest(req, options, port/*legacy*/, }; Agent.prototype.createSocket = function createSocket(req, options, cb) { - var self = this; options = util._extend({}, options); - util._extend(options, self.options); + util._extend(options, this.options); if (options.socketPath) options.path = options.socketPath; if (!options.servername) options.servername = calculateServerName(options, req); - var name = self.getName(options); + var name = this.getName(options); options._agentKey = name; debug('createConnection', name, options); options.encoding = null; var called = false; - const newSocket = self.createConnection(options, oncreate); - if (newSocket) - oncreate(null, newSocket); - function oncreate(err, s) { + const oncreate = (err, s) => { if (called) return; called = true; if (err) return cb(err); - if (!self.sockets[name]) { - self.sockets[name] = []; + if (!this.sockets[name]) { + this.sockets[name] = []; } - self.sockets[name].push(s); - debug('sockets', name, self.sockets[name].length); - installListeners(self, s, options); + this.sockets[name].push(s); + debug('sockets', name, this.sockets[name].length); + installListeners(this, s, options); cb(null, s); - } + }; + + const newSocket = this.createConnection(options, oncreate); + if (newSocket) + oncreate(null, newSocket); }; function calculateServerName(options, req) { From 9f3d59eabb6564ad337a762d61ac767f9130e8a5 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Wed, 1 Nov 2017 11:48:11 -0700 Subject: [PATCH 2/2] http2: refactor multiple internals * eliminate pooling of Nghttp2Stream instances. After testing, the pooling is not having any tangible benefit and makes things more complicated. Simplify. Simplify. * refactor inbound headers * Enforce MAX_HEADERS_LIST setting and limit the number of header pairs accepted from the peer. Use the ENHANCE_YOUR_CALM error code when receiving either too many headers or too many octets. Use a vector to store the headers instead of a queue PR-URL: https://github.com/nodejs/node/pull/16676 Reviewed-By: Matteo Collina Reviewed-By: Anna Henningsen --- doc/api/http2.md | 33 ++++- lib/internal/http2/util.js | 8 +- src/node_http2.cc | 31 ++-- src/node_http2.h | 25 ++-- src/node_http2_core-inl.h | 132 +++++++++--------- src/node_http2_core.h | 104 ++++++++------ src/node_http2_state.h | 1 + test/parallel/test-http2-getpackedsettings.js | 8 +- test/parallel/test-http2-too-large-headers.js | 32 +++++ test/parallel/test-http2-too-many-headers.js | 34 +++++ .../test-http2-util-update-options-buffer.js | 13 +- 11 files changed, 275 insertions(+), 146 deletions(-) create mode 100644 test/parallel/test-http2-too-large-headers.js create mode 100644 test/parallel/test-http2-too-many-headers.js diff --git a/doc/api/http2.md b/doc/api/http2.md index 2a958c30c3f..623718b6788 100644 --- a/doc/api/http2.md +++ b/doc/api/http2.md @@ -1470,11 +1470,18 @@ not be emitted. ### http2.createServer(options[, onRequestHandler]) * `options` {Object} * `maxDeflateDynamicTableSize` {number} Sets the maximum dynamic table size for deflating header fields. **Default:** `4Kib` + * `maxHeaderListPairs` {number} Sets the maximum number of header entries. + **Default:** `128`. The minimum value is `4`. * `maxSendHeaderBlockLength` {number} Sets the maximum allowed size for a serialized, compressed block of headers. Attempts to send headers that exceed this limit will result in a `'frameError'` event being emitted @@ -1525,6 +1532,11 @@ server.listen(80); ### http2.createSecureServer(options[, onRequestHandler]) * `options` {Object} @@ -1533,6 +1545,8 @@ added: v8.4.0 `false`. See the [`'unknownProtocol'`][] event. See [ALPN negotiation][]. * `maxDeflateDynamicTableSize` {number} Sets the maximum dynamic table size for deflating header fields. **Default:** `4Kib` + * `maxHeaderListPairs` {number} Sets the maximum number of header entries. + **Default:** `128`. The minimum value is `4`. * `maxSendHeaderBlockLength` {number} Sets the maximum allowed size for a serialized, compressed block of headers. Attempts to send headers that exceed this limit will result in a `'frameError'` event being emitted @@ -1590,12 +1604,19 @@ server.listen(80); ### http2.connect(authority[, options][, listener]) * `authority` {string|URL} * `options` {Object} * `maxDeflateDynamicTableSize` {number} Sets the maximum dynamic table size for deflating header fields. **Default:** `4Kib` + * `maxHeaderListPairs` {number} Sets the maximum number of header entries. + **Default:** `128`. The minimum value is `1`. * `maxReservedRemoteStreams` {number} Sets the maximum number of reserved push streams the client will accept at any given time. Once the current number of currently reserved push streams exceeds reaches this limit, new push streams @@ -1747,7 +1768,13 @@ server.on('stream', (stream, headers) => { ``` ### Settings Object - + The `http2.getDefaultSettings()`, `http2.getPackedSettings()`, `http2.createServer()`, `http2.createSecureServer()`, `http2session.settings()`, `http2session.localSettings`, and @@ -1773,8 +1800,8 @@ properties. concurrently at any given time in an `Http2Session`. The minimum value is 0. The maximum allowed value is 231-1. * `maxHeaderListSize` {number} Specifies the maximum size (uncompressed octets) - of header list that will be accepted. There is no default value. The minimum - allowed value is 0. The maximum allowed value is 232-1. + of header list that will be accepted. The minimum allowed value is 0. The + maximum allowed value is 232-1. **Default:** 65535. All additional properties on the settings object are ignored. diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js index 03d06396240..b67b663f1ad 100644 --- a/lib/internal/http2/util.js +++ b/lib/internal/http2/util.js @@ -172,7 +172,8 @@ const IDX_OPTIONS_MAX_RESERVED_REMOTE_STREAMS = 1; const IDX_OPTIONS_MAX_SEND_HEADER_BLOCK_LENGTH = 2; const IDX_OPTIONS_PEER_MAX_CONCURRENT_STREAMS = 3; const IDX_OPTIONS_PADDING_STRATEGY = 4; -const IDX_OPTIONS_FLAGS = 5; +const IDX_OPTIONS_MAX_HEADER_LIST_PAIRS = 5; +const IDX_OPTIONS_FLAGS = 6; function updateOptionsBuffer(options) { var flags = 0; @@ -201,6 +202,11 @@ function updateOptionsBuffer(options) { optionsBuffer[IDX_OPTIONS_PADDING_STRATEGY] = options.paddingStrategy; } + if (typeof options.maxHeaderListPairs === 'number') { + flags |= (1 << IDX_OPTIONS_MAX_HEADER_LIST_PAIRS); + optionsBuffer[IDX_OPTIONS_MAX_HEADER_LIST_PAIRS] = + options.maxHeaderListPairs; + } optionsBuffer[IDX_OPTIONS_FLAGS] = flags; } diff --git a/src/node_http2.cc b/src/node_http2.cc index 8beaee05488..f9025d1fd31 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -5,6 +5,7 @@ #include "node_http2_state.h" #include +#include namespace node { @@ -20,8 +21,6 @@ using v8::Undefined; namespace http2 { -Freelist stream_free_list; - Nghttp2Session::Callbacks Nghttp2Session::callback_struct_saved[2] = { Callbacks(false), Callbacks(true)}; @@ -67,6 +66,10 @@ Http2Options::Http2Options(Environment* env) { buffer.GetValue(IDX_OPTIONS_PADDING_STRATEGY)); SetPaddingStrategy(strategy); } + + if (flags & (1 << IDX_OPTIONS_MAX_HEADER_LIST_PAIRS)) { + SetMaxHeaderPairs(buffer[IDX_OPTIONS_MAX_HEADER_LIST_PAIRS]); + } } Http2Settings::Http2Settings(Environment* env) : env_(env) { @@ -173,11 +176,14 @@ inline void Http2Settings::RefreshDefaults(Environment* env) { DEFAULT_SETTINGS_INITIAL_WINDOW_SIZE; buffer[IDX_SETTINGS_MAX_FRAME_SIZE] = DEFAULT_SETTINGS_MAX_FRAME_SIZE; + buffer[IDX_SETTINGS_MAX_HEADER_LIST_SIZE] = + DEFAULT_SETTINGS_MAX_HEADER_LIST_SIZE; buffer[IDX_SETTINGS_COUNT] = (1 << IDX_SETTINGS_HEADER_TABLE_SIZE) | (1 << IDX_SETTINGS_ENABLE_PUSH) | (1 << IDX_SETTINGS_INITIAL_WINDOW_SIZE) | - (1 << IDX_SETTINGS_MAX_FRAME_SIZE); + (1 << IDX_SETTINGS_MAX_FRAME_SIZE) | + (1 << IDX_SETTINGS_MAX_HEADER_LIST_SIZE); } @@ -192,7 +198,10 @@ Http2Session::Http2Session(Environment* env, padding_strategy_ = opts.GetPaddingStrategy(); - Init(type, *opts); + int32_t maxHeaderPairs = opts.GetMaxHeaderPairs(); + maxHeaderPairs = type == NGHTTP2_SESSION_SERVER ? + std::max(maxHeaderPairs, 4) : std::max(maxHeaderPairs, 1); + Init(type, *opts, nullptr, maxHeaderPairs); // For every node::Http2Session instance, there is a uv_prepare_t handle // whose callback is triggered on every tick of the event loop. When @@ -911,7 +920,8 @@ void Http2Session::OnTrailers(Nghttp2Stream* stream, void Http2Session::OnHeaders( Nghttp2Stream* stream, - std::queue* headers, + nghttp2_header* headers, + size_t count, nghttp2_headers_category cat, uint8_t flags) { Local context = env()->context(); @@ -936,10 +946,11 @@ void Http2Session::OnHeaders( // like {name1: value1, name2: value2, name3: [value3, value4]}. We do it // this way for performance reasons (it's faster to generate and pass an // array than it is to generate and pass the object). - do { + size_t n = 0; + while (count > 0) { size_t j = 0; - while (!headers->empty() && j < arraysize(argv) / 2) { - nghttp2_header item = headers->front(); + while (count > 0 && j < arraysize(argv) / 2) { + nghttp2_header item = headers[n++]; // The header name and value are passed as external one-byte strings name_str = ExternalHeader::New(env(), item.name).ToLocalChecked(); @@ -947,7 +958,7 @@ void Http2Session::OnHeaders( ExternalHeader::New(env(), item.value).ToLocalChecked(); argv[j * 2] = name_str; argv[j * 2 + 1] = value_str; - headers->pop(); + count--; j++; } // For performance, we pass name and value pairs to array.protototype.push @@ -956,7 +967,7 @@ void Http2Session::OnHeaders( if (j > 0) { fn->Call(env()->context(), holder, j * 2, argv).ToLocalChecked(); } - } while (!headers->empty()); + } Local args[4] = { Integer::New(isolate, stream->id()), diff --git a/src/node_http2.h b/src/node_http2.h index a19e032ccef..9454301ec46 100644 --- a/src/node_http2.h +++ b/src/node_http2.h @@ -292,14 +292,6 @@ const char* nghttp2_errname(int rv) { } } -#define DEFAULT_SETTINGS_HEADER_TABLE_SIZE 4096 -#define DEFAULT_SETTINGS_ENABLE_PUSH 1 -#define DEFAULT_SETTINGS_INITIAL_WINDOW_SIZE 65535 -#define DEFAULT_SETTINGS_MAX_FRAME_SIZE 16384 -#define MAX_MAX_FRAME_SIZE 16777215 -#define MIN_MAX_FRAME_SIZE DEFAULT_SETTINGS_MAX_FRAME_SIZE -#define MAX_INITIAL_WINDOW_SIZE 2147483647 - // This allows for 4 default-sized frames with their frame headers static const size_t kAllocBufferSize = 4 * (16384 + 9); @@ -324,19 +316,25 @@ class Http2Options { return options_; } + void SetMaxHeaderPairs(uint32_t max) { + max_header_pairs_ = max; + } + + uint32_t GetMaxHeaderPairs() const { + return max_header_pairs_; + } + void SetPaddingStrategy(padding_strategy_type val) { -#if DEBUG - CHECK_LE(val, PADDING_STRATEGY_CALLBACK); -#endif padding_strategy_ = static_cast(val); } - padding_strategy_type GetPaddingStrategy() { + padding_strategy_type GetPaddingStrategy() const { return padding_strategy_; } private: nghttp2_option* options_; + uint32_t max_header_pairs_ = DEFAULT_MAX_HEADER_LIST_PAIRS; padding_strategy_type padding_strategy_ = PADDING_STRATEGY_NONE; }; @@ -413,7 +411,8 @@ class Http2Session : public AsyncWrap, void OnHeaders( Nghttp2Stream* stream, - std::queue* headers, + nghttp2_header* headers, + size_t count, nghttp2_headers_category cat, uint8_t flags) override; void OnStreamClose(int32_t id, uint32_t code) override; diff --git a/src/node_http2_core-inl.h b/src/node_http2_core-inl.h index c78e5d673f4..a4c6ef9b0eb 100644 --- a/src/node_http2_core-inl.h +++ b/src/node_http2_core-inl.h @@ -5,17 +5,11 @@ #include "node_http2_core.h" #include "node_internals.h" // arraysize -#include "freelist.h" +#include namespace node { namespace http2 { -#define FREELIST_MAX 10240 - -// Instances of Nghttp2Stream are created and pooled in order to speed -// allocation under load. -extern Freelist stream_free_list; - #ifdef NODE_DEBUG_HTTP2 inline int Nghttp2Session::OnNghttpError(nghttp2_session* session, const char* message, @@ -45,13 +39,36 @@ inline int Nghttp2Session::OnBeginHeadersCallback(nghttp2_session* session, Nghttp2Stream* stream = handle->FindStream(id); if (stream == nullptr) { - Nghttp2Stream::Init(id, handle, frame->headers.cat); + new Nghttp2Stream(id, handle, frame->headers.cat); } else { stream->StartHeaders(frame->headers.cat); } return 0; } +inline size_t GetBufferLength(nghttp2_rcbuf* buf) { + return nghttp2_rcbuf_get_buf(buf).len; +} + +inline bool Nghttp2Stream::AddHeader(nghttp2_rcbuf* name, + nghttp2_rcbuf* value, + uint8_t flags) { + size_t length = GetBufferLength(name) + GetBufferLength(value) + 32; + if (current_headers_.size() == max_header_pairs_ || + current_headers_length_ + length > max_header_length_) { + return false; + } + nghttp2_header header; + header.name = name; + header.value = value; + header.flags = flags; + current_headers_.push_back(header); + nghttp2_rcbuf_incref(name); + nghttp2_rcbuf_incref(value); + current_headers_length_ += length; + return true; +} + // nghttp2 calls this once for every header name-value pair in a HEADERS // or PUSH_PROMISE block. CONTINUATION frames are handled automatically // and transparently so we do not need to worry about those at all. @@ -68,15 +85,12 @@ inline int Nghttp2Session::OnHeaderCallback(nghttp2_session* session, frame->push_promise.promised_stream_id : frame->hd.stream_id; Nghttp2Stream* stream = handle->FindStream(id); - // The header name and value are stored in a reference counted buffer - // provided to us by nghttp2. We need to increment the reference counter - // here, then decrement it when we're done using it later. - nghttp2_rcbuf_incref(name); - nghttp2_rcbuf_incref(value); - nghttp2_header header; - header.name = name; - header.value = value; - stream->headers()->emplace(header); + if (!stream->AddHeader(name, value, flags)) { + // This will only happen if the connected peer sends us more + // than the allowed number of header items at any given time + stream->SubmitRstStream(NGHTTP2_ENHANCE_YOUR_CALM); + return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; + } return 0; } @@ -447,6 +461,7 @@ inline void Nghttp2Session::HandleHeadersFrame(const nghttp2_frame* frame) { #endif OnHeaders(stream, stream->headers(), + stream->headers_count(), stream->headers_category(), frame->hd.flags); } @@ -551,12 +566,15 @@ inline void Nghttp2Session::SendPendingData() { // uv_loop_t. inline int Nghttp2Session::Init(const nghttp2_session_type type, nghttp2_option* options, - nghttp2_mem* mem) { + nghttp2_mem* mem, + uint32_t maxHeaderPairs) { session_type_ = type; DEBUG_HTTP2("Nghttp2Session %s: initializing session\n", TypeName()); destroying_ = false; int ret = 0; + max_header_pairs_ = maxHeaderPairs; + nghttp2_session_callbacks* callbacks = callback_struct_saved[HasGetPaddingCallback() ? 1 : 0].callbacks; @@ -637,44 +655,31 @@ inline void Nghttp2Session::RemoveStream(int32_t id) { // Implementation for Nghttp2Stream functions -inline Nghttp2Stream* Nghttp2Stream::Init( +Nghttp2Stream::Nghttp2Stream( int32_t id, Nghttp2Session* session, nghttp2_headers_category category, - int options) { - DEBUG_HTTP2("Nghttp2Stream %d: initializing stream\n", id); - Nghttp2Stream* stream = stream_free_list.pop(); - stream->ResetState(id, session, category, options); - session->AddStream(stream); - return stream; -} - + int options) : id_(id), + session_(session), + current_headers_category_(category) { + // Limit the number of header pairs + max_header_pairs_ = session->GetMaxHeaderPairs(); + if (max_header_pairs_ == 0) + max_header_pairs_ = DEFAULT_MAX_HEADER_LIST_PAIRS; + current_headers_.reserve(max_header_pairs_); + + // Limit the number of header octets + max_header_length_ = + std::min( + nghttp2_session_get_local_settings( + session->session(), + NGHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE), + MAX_MAX_HEADER_LIST_SIZE); -// Resets the state of the stream instance to defaults -inline void Nghttp2Stream::ResetState( - int32_t id, - Nghttp2Session* session, - nghttp2_headers_category category, - int options) { - DEBUG_HTTP2("Nghttp2Stream %d: resetting stream state\n", id); - session_ = session; - while (!queue_.empty()) { - nghttp2_stream_write* head = queue_.front(); - delete head; - queue_.pop(); - } - while (!data_chunks_.empty()) - data_chunks_.pop(); - while (!current_headers_.empty()) - current_headers_.pop(); - current_headers_category_ = category; - flags_ = NGHTTP2_STREAM_FLAG_NONE; - id_ = id; - code_ = NGHTTP2_NO_ERROR; - prev_local_window_size_ = 65535; - queue_index_ = 0; - queue_offset_ = 0; getTrailers_ = options & STREAM_OPTION_GET_TRAILERS; + if (options & STREAM_OPTION_EMPTY_PAYLOAD) + Shutdown(); + session->AddStream(this); } @@ -687,14 +692,16 @@ inline void Nghttp2Stream::Destroy() { Nghttp2Session* session = this->session_; if (session != nullptr) { - // Remove this stream from the associated session session_->RemoveStream(this->id()); session_ = nullptr; } // Free any remaining incoming data chunks. - while (!data_chunks_.empty()) + while (!data_chunks_.empty()) { + uv_buf_t buf = data_chunks_.front(); + free(buf.base); data_chunks_.pop(); + } // Free any remaining outgoing data chunks. while (!queue_.empty()) { @@ -704,12 +711,7 @@ inline void Nghttp2Stream::Destroy() { queue_.pop(); } - // Free any remaining headers - while (!current_headers_.empty()) - current_headers_.pop(); - - // Return this stream instance to the freelist - stream_free_list.push(this); + delete this; } // Submit informational headers for a stream. @@ -759,9 +761,9 @@ inline int32_t Nghttp2Stream::SubmitPushPromise( id_, nva, len, nullptr); if (ret > 0) { - auto stream = Nghttp2Stream::Init(ret, session_); - if (options & STREAM_OPTION_EMPTY_PAYLOAD) - stream->Shutdown(); + auto stream = new Nghttp2Stream(ret, session_, + NGHTTP2_HCAT_HEADERS, + options); if (assigned != nullptr) *assigned = stream; } return ret; @@ -837,11 +839,7 @@ inline int32_t Nghttp2Session::SubmitRequest( provider, nullptr); // Assign the Nghttp2Stream handle if (ret > 0) { - Nghttp2Stream* stream = Nghttp2Stream::Init(ret, this, - NGHTTP2_HCAT_HEADERS, - options); - if (options & STREAM_OPTION_EMPTY_PAYLOAD) - stream->Shutdown(); + auto stream = new Nghttp2Stream(ret, this, NGHTTP2_HCAT_HEADERS, options); if (assigned != nullptr) *assigned = stream; } return ret; diff --git a/src/node_http2_core.h b/src/node_http2_core.h index 1de018c676f..5fbb7fa9f2a 100644 --- a/src/node_http2_core.h +++ b/src/node_http2_core.h @@ -9,6 +9,7 @@ #include "nghttp2/nghttp2.h" #include +#include #include #include @@ -36,6 +37,19 @@ void inline debug_vfprintf(const char* format, ...) { } while (0) #endif +#define DEFAULT_SETTINGS_HEADER_TABLE_SIZE 4096 +#define DEFAULT_SETTINGS_ENABLE_PUSH 1 +#define DEFAULT_SETTINGS_INITIAL_WINDOW_SIZE 65535 +#define DEFAULT_SETTINGS_MAX_FRAME_SIZE 16384 +#define DEFAULT_SETTINGS_MAX_HEADER_LIST_SIZE 65535 +#define MAX_MAX_FRAME_SIZE 16777215 +#define MIN_MAX_FRAME_SIZE DEFAULT_SETTINGS_MAX_FRAME_SIZE +#define MAX_INITIAL_WINDOW_SIZE 2147483647 + +#define MAX_MAX_HEADER_LIST_SIZE 16777215u +#define DEFAULT_MAX_HEADER_LIST_PAIRS 128u + + class Nghttp2Session; class Nghttp2Stream; @@ -86,6 +100,7 @@ struct nghttp2_stream_write { struct nghttp2_header { nghttp2_rcbuf* name = nullptr; nghttp2_rcbuf* value = nullptr; + uint8_t flags = 0; }; // Handle Types @@ -95,7 +110,8 @@ class Nghttp2Session { inline int Init( const nghttp2_session_type type = NGHTTP2_SESSION_SERVER, nghttp2_option* options = nullptr, - nghttp2_mem* mem = nullptr); + nghttp2_mem* mem = nullptr, + uint32_t maxHeaderPairs = DEFAULT_MAX_HEADER_LIST_PAIRS); // Frees this session instance inline ~Nghttp2Session(); @@ -104,6 +120,10 @@ class Nghttp2Session { return destroying_; } + uint32_t GetMaxHeaderPairs() const { + return max_header_pairs_; + } + inline const char* TypeName() { switch (session_type_) { case NGHTTP2_SESSION_SERVER: return "server"; @@ -156,7 +176,8 @@ class Nghttp2Session { virtual void OnHeaders( Nghttp2Stream* stream, - std::queue* headers, + nghttp2_header* headers, + size_t count, nghttp2_headers_category cat, uint8_t flags) {} virtual void OnStreamClose(int32_t id, uint32_t code) {} @@ -288,6 +309,7 @@ class Nghttp2Session { nghttp2_session* session_; nghttp2_session_type session_type_; + uint32_t max_header_pairs_ = DEFAULT_MAX_HEADER_LIST_PAIRS; std::unordered_map streams_; bool destroying_ = false; @@ -298,37 +320,21 @@ class Nghttp2Session { class Nghttp2Stream { public: - static inline Nghttp2Stream* Init( - int32_t id, - Nghttp2Session* session, - nghttp2_headers_category category = NGHTTP2_HCAT_HEADERS, - int options = 0); + // Resets the state of the stream instance to defaults + Nghttp2Stream( + int32_t id, + Nghttp2Session* session, + nghttp2_headers_category category = NGHTTP2_HCAT_HEADERS, + int options = 0); - inline ~Nghttp2Stream() { -#if defined(DEBUG) && DEBUG - CHECK_EQ(session_, nullptr); -#endif - DEBUG_HTTP2("Nghttp2Stream %d: freed\n", id_); - } + inline ~Nghttp2Stream() {} inline void FlushDataChunks(); - // Resets the state of the stream instance to defaults - inline void ResetState( - int32_t id, - Nghttp2Session* session, - nghttp2_headers_category category = NGHTTP2_HCAT_HEADERS, - int options = 0); - // Destroy this stream instance and free all held memory. // Note that this will free queued outbound and inbound // data chunks and inbound headers, so it's important not // to call this until those are fully consumed. - // - // Also note: this does not actually destroy the instance. - // instead, it frees the held memory, removes the stream - // from the parent session, and returns the instance to - // the FreeList so that it can be reused. inline void Destroy(); // Returns true if this stream has been destroyed @@ -434,34 +440,44 @@ class Nghttp2Stream { return id_; } - inline std::queue* headers() { - return ¤t_headers_; + inline bool AddHeader(nghttp2_rcbuf* name, + nghttp2_rcbuf* value, + uint8_t flags); + + inline nghttp2_header* headers() { + return current_headers_.data(); } inline nghttp2_headers_category headers_category() const { return current_headers_category_; } + inline size_t headers_count() const { + return current_headers_.size(); + } + void StartHeaders(nghttp2_headers_category category) { DEBUG_HTTP2("Nghttp2Stream %d: starting headers, category: %d\n", id_, category); - // We shouldn't be in the middle of a headers block already. - // Something bad happened if this fails -#if defined(DEBUG) && DEBUG - CHECK(current_headers_.empty()); -#endif + current_headers_length_ = 0; + current_headers_.clear(); current_headers_category_ = category; } private: - // The Parent HTTP/2 Session - Nghttp2Session* session_ = nullptr; - // The Stream Identifier - int32_t id_ = 0; + int32_t id_; + + // The Parent HTTP/2 Session + Nghttp2Session* session_; // Internal state flags - int flags_ = 0; + int flags_ = NGHTTP2_STREAM_FLAG_NONE; + uint32_t max_header_pairs_ = DEFAULT_MAX_HEADER_LIST_PAIRS; + uint32_t max_header_length_ = DEFAULT_SETTINGS_MAX_HEADER_LIST_SIZE; + + // The RST_STREAM code used to close this stream + int32_t code_ = NGHTTP2_NO_ERROR; // Outbound Data... This is the data written by the JS layer that is // waiting to be written out to the socket. @@ -471,23 +487,19 @@ class Nghttp2Stream { int64_t fd_offset_ = 0; int64_t fd_length_ = -1; + // True if this stream will have outbound trailers + bool getTrailers_ = false; + // The Current Headers block... As headers are received for this stream, // they are temporarily stored here until the OnFrameReceived is called // signalling the end of the HEADERS frame nghttp2_headers_category current_headers_category_ = NGHTTP2_HCAT_HEADERS; - std::queue current_headers_; + uint32_t current_headers_length_ = 0; // total number of octets + std::vector current_headers_; // Inbound Data... This is the data received via DATA frames for this stream. std::queue data_chunks_; - // The RST_STREAM code used to close this stream - int32_t code_ = NGHTTP2_NO_ERROR; - - int32_t prev_local_window_size_ = 65535; - - // True if this stream will have outbound trailers - bool getTrailers_ = false; - friend class Nghttp2Session; }; diff --git a/src/node_http2_state.h b/src/node_http2_state.h index a945f07b686..dd8954de2ae 100755 --- a/src/node_http2_state.h +++ b/src/node_http2_state.h @@ -47,6 +47,7 @@ namespace http2 { IDX_OPTIONS_MAX_SEND_HEADER_BLOCK_LENGTH, IDX_OPTIONS_PEER_MAX_CONCURRENT_STREAMS, IDX_OPTIONS_PADDING_STRATEGY, + IDX_OPTIONS_MAX_HEADER_LIST_PAIRS, IDX_OPTIONS_FLAGS }; diff --git a/test/parallel/test-http2-getpackedsettings.js b/test/parallel/test-http2-getpackedsettings.js index 2f7c76e9b6a..4c7cf3d85ca 100644 --- a/test/parallel/test-http2-getpackedsettings.js +++ b/test/parallel/test-http2-getpackedsettings.js @@ -6,9 +6,11 @@ if (!common.hasCrypto) const assert = require('assert'); const http2 = require('http2'); -const check = Buffer.from([0x00, 0x01, 0x00, 0x00, 0x10, 0x00, 0x00, 0x05, - 0x00, 0x00, 0x40, 0x00, 0x00, 0x04, 0x00, 0x00, - 0xff, 0xff, 0x00, 0x02, 0x00, 0x00, 0x00, 0x01]); +const check = Buffer.from([0x00, 0x01, 0x00, 0x00, 0x10, 0x00, + 0x00, 0x05, 0x00, 0x00, 0x40, 0x00, + 0x00, 0x04, 0x00, 0x00, 0xff, 0xff, + 0x00, 0x06, 0x00, 0x00, 0xff, 0xff, + 0x00, 0x02, 0x00, 0x00, 0x00, 0x01]); const val = http2.getPackedSettings(http2.getDefaultSettings()); assert.deepStrictEqual(val, check); diff --git a/test/parallel/test-http2-too-large-headers.js b/test/parallel/test-http2-too-large-headers.js new file mode 100644 index 00000000000..fa15b6ea114 --- /dev/null +++ b/test/parallel/test-http2-too-large-headers.js @@ -0,0 +1,32 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +const http2 = require('http2'); +const assert = require('assert'); +const { + NGHTTP2_ENHANCE_YOUR_CALM +} = http2.constants; + +const server = http2.createServer({ settings: { maxHeaderListSize: 100 } }); +server.on('stream', common.mustNotCall()); + +server.listen(0, common.mustCall(() => { + const client = http2.connect(`http://localhost:${server.address().port}`); + + client.on('remoteSettings', () => { + const req = client.request({ 'foo': 'a'.repeat(1000) }); + req.on('error', common.expectsError({ + code: 'ERR_HTTP2_STREAM_ERROR', + type: Error, + message: 'Stream closed with error code 11' + })); + req.on('streamClosed', common.mustCall((code) => { + assert.strictEqual(code, NGHTTP2_ENHANCE_YOUR_CALM); + server.close(); + client.destroy(); + })); + }); + +})); diff --git a/test/parallel/test-http2-too-many-headers.js b/test/parallel/test-http2-too-many-headers.js new file mode 100644 index 00000000000..3d63727ed50 --- /dev/null +++ b/test/parallel/test-http2-too-many-headers.js @@ -0,0 +1,34 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +const http2 = require('http2'); +const assert = require('assert'); +const { + NGHTTP2_ENHANCE_YOUR_CALM +} = http2.constants; + +// By default, the maximum number of header fields allowed per +// block is 128, including the HTTP pseudo-header fields. The +// minimum value for servers is 4, setting this to any value +// less than 4 will still leave the minimum to 4. +const server = http2.createServer({ maxHeaderListPairs: 0 }); +server.on('stream', common.mustNotCall()); + +server.listen(0, common.mustCall(() => { + const client = http2.connect(`http://localhost:${server.address().port}`); + + const req = client.request({ foo: 'bar' }); + req.on('error', common.expectsError({ + code: 'ERR_HTTP2_STREAM_ERROR', + type: Error, + message: 'Stream closed with error code 11' + })); + req.on('streamClosed', common.mustCall((code) => { + assert.strictEqual(code, NGHTTP2_ENHANCE_YOUR_CALM); + server.close(); + client.destroy(); + })); + +})); diff --git a/test/parallel/test-http2-util-update-options-buffer.js b/test/parallel/test-http2-util-update-options-buffer.js index 952b9e2dd52..83c97c06b04 100644 --- a/test/parallel/test-http2-util-update-options-buffer.js +++ b/test/parallel/test-http2-util-update-options-buffer.js @@ -15,7 +15,8 @@ const IDX_OPTIONS_MAX_RESERVED_REMOTE_STREAMS = 1; const IDX_OPTIONS_MAX_SEND_HEADER_BLOCK_LENGTH = 2; const IDX_OPTIONS_PEER_MAX_CONCURRENT_STREAMS = 3; const IDX_OPTIONS_PADDING_STRATEGY = 4; -const IDX_OPTIONS_FLAGS = 5; +const IDX_OPTIONS_MAX_HEADER_LIST_PAIRS = 5; +const IDX_OPTIONS_FLAGS = 6; { updateOptionsBuffer({ @@ -23,7 +24,8 @@ const IDX_OPTIONS_FLAGS = 5; maxReservedRemoteStreams: 2, maxSendHeaderBlockLength: 3, peerMaxConcurrentStreams: 4, - paddingStrategy: 5 + paddingStrategy: 5, + maxHeaderListPairs: 6 }); strictEqual(optionsBuffer[IDX_OPTIONS_MAX_DEFLATE_DYNAMIC_TABLE_SIZE], 1); @@ -31,6 +33,7 @@ const IDX_OPTIONS_FLAGS = 5; strictEqual(optionsBuffer[IDX_OPTIONS_MAX_SEND_HEADER_BLOCK_LENGTH], 3); strictEqual(optionsBuffer[IDX_OPTIONS_PEER_MAX_CONCURRENT_STREAMS], 4); strictEqual(optionsBuffer[IDX_OPTIONS_PADDING_STRATEGY], 5); + strictEqual(optionsBuffer[IDX_OPTIONS_MAX_HEADER_LIST_PAIRS], 6); const flags = optionsBuffer[IDX_OPTIONS_FLAGS]; @@ -39,6 +42,7 @@ const IDX_OPTIONS_FLAGS = 5; ok(flags & (1 << IDX_OPTIONS_MAX_SEND_HEADER_BLOCK_LENGTH)); ok(flags & (1 << IDX_OPTIONS_PEER_MAX_CONCURRENT_STREAMS)); ok(flags & (1 << IDX_OPTIONS_PADDING_STRATEGY)); + ok(flags & (1 << IDX_OPTIONS_MAX_HEADER_LIST_PAIRS)); } { @@ -48,7 +52,8 @@ const IDX_OPTIONS_FLAGS = 5; maxDeflateDynamicTableSize: 1, maxReservedRemoteStreams: 2, peerMaxConcurrentStreams: 4, - paddingStrategy: 5 + paddingStrategy: 5, + maxHeaderListPairs: 6 }); strictEqual(optionsBuffer[IDX_OPTIONS_MAX_DEFLATE_DYNAMIC_TABLE_SIZE], 1); @@ -56,6 +61,7 @@ const IDX_OPTIONS_FLAGS = 5; strictEqual(optionsBuffer[IDX_OPTIONS_MAX_SEND_HEADER_BLOCK_LENGTH], 0); strictEqual(optionsBuffer[IDX_OPTIONS_PEER_MAX_CONCURRENT_STREAMS], 4); strictEqual(optionsBuffer[IDX_OPTIONS_PADDING_STRATEGY], 5); + strictEqual(optionsBuffer[IDX_OPTIONS_MAX_HEADER_LIST_PAIRS], 6); const flags = optionsBuffer[IDX_OPTIONS_FLAGS]; @@ -64,4 +70,5 @@ const IDX_OPTIONS_FLAGS = 5; ok(!(flags & (1 << IDX_OPTIONS_MAX_SEND_HEADER_BLOCK_LENGTH))); ok(flags & (1 << IDX_OPTIONS_PEER_MAX_CONCURRENT_STREAMS)); ok(flags & (1 << IDX_OPTIONS_PADDING_STRATEGY)); + ok(flags & (1 << IDX_OPTIONS_MAX_HEADER_LIST_PAIRS)); }