From 098edcb19645304285b9c903ec67a691eec4cee0 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Mon, 18 Dec 2017 14:55:16 -0800 Subject: [PATCH] http2: convert Http2Settings to an AsyncWrap Backport-PR-URL: https://github.com/nodejs/node/pull/18050 PR-URL: https://github.com/nodejs/node/pull/17763 Refs: https://github.com/nodejs/node/issues/17746 Reviewed-By: Matteo Collina --- lib/internal/http2/core.js | 57 +++--- lib/internal/http2/util.js | 8 +- src/async_wrap.h | 1 + src/env.h | 1 + src/node_http2.cc | 185 ++++++++++++------ src/node_http2.h | 91 +++++---- src/node_http2_state.h | 1 + test/parallel/test-http2-session-settings.js | 4 +- test/parallel/test-http2-too-many-settings.js | 76 +++---- .../test-http2-util-update-options-buffer.js | 8 +- test/sequential/test-async-wrap-getasyncid.js | 1 + 11 files changed, 269 insertions(+), 164 deletions(-) diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 7d8ec2e772002e..c4dd547178d90d 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -322,23 +322,14 @@ function onStreamRead(nread, buf, handle) { // Called when the remote peer settings have been updated. // Resets the cached settings. -function onSettings(ack) { +function onSettings() { const session = this[kOwner]; if (session.destroyed) return; session[kUpdateTimer](); - let event = 'remoteSettings'; - if (ack) { - debug(`Http2Session ${sessionName(session[kType])}: settings acknowledged`); - if (session[kState].pendingAck > 0) - session[kState].pendingAck--; - session[kLocalSettings] = undefined; - event = 'localSettings'; - } else { - debug(`Http2Session ${sessionName(session[kType])}: new settings received`); - session[kRemoteSettings] = undefined; - } - process.nextTick(emit, session, event, session[event]); + debug(`Http2Session ${sessionName(session[kType])}: new settings received`); + session[kRemoteSettings] = undefined; + process.nextTick(emit, session, 'remoteSettings', session.remoteSettings); } // If the stream exists, an attempt will be made to emit an event @@ -538,15 +529,32 @@ function onSessionInternalError(code) { this[kOwner].destroy(new NghttpError(code)); } +function settingsCallback(cb, ack, duration) { + this[kState].pendingAck--; + this[kLocalSettings] = undefined; + if (ack) { + debug(`Http2Session ${sessionName(this[kType])}: settings received`); + const settings = this.localSettings; + if (typeof cb === 'function') + cb(null, settings, duration); + this.emit('localSettings', settings); + } else { + debug(`Http2Session ${sessionName(this[kType])}: settings canceled`); + if (typeof cb === 'function') + cb(new errors.Error('ERR_HTTP2_SETTINGS_CANCEL')); + } +} + // Submits a SETTINGS frame to be sent to the remote peer. -function submitSettings(settings) { +function submitSettings(settings, callback) { if (this.destroyed) return; debug(`Http2Session ${sessionName(this[kType])}: submitting settings`); this[kUpdateTimer](); - this[kLocalSettings] = undefined; updateSettingsBuffer(settings); - this[kHandle].settings(); + if (!this[kHandle].settings(settingsCallback.bind(this, callback))) { + this.destroy(new errors.Error('ERR_HTTP2_MAX_PENDING_SETTINGS_ACK')); + } } // Submits a PRIORITY frame to be sent to the remote peer @@ -781,7 +789,6 @@ class Http2Session extends EventEmitter { streams: new Map(), pendingStreams: new Set(), pendingAck: 0, - maxPendingAck: Math.max(1, (options.maxPendingAck | 0) || 10), writeQueueSize: 0 }; @@ -948,21 +955,19 @@ class Http2Session extends EventEmitter { } // Submits a SETTINGS frame to be sent to the remote peer. - settings(settings) { + settings(settings, callback) { if (this.destroyed) throw new errors.Error('ERR_HTTP2_INVALID_SESSION'); - assertIsObject(settings, 'settings'); settings = validateSettings(settings); - const state = this[kState]; - if (state.pendingAck === state.maxPendingAck) { - throw new errors.Error('ERR_HTTP2_MAX_PENDING_SETTINGS_ACK', - this[kState].pendingAck); - } + + if (callback && typeof callback !== 'function') + throw new errors.TypeError('ERR_INVALID_CALLBACK'); debug(`Http2Session ${sessionName(this[kType])}: sending settings`); - state.pendingAck++; - const settingsFn = submitSettings.bind(this, settings); + this[kState].pendingAck++; + + const settingsFn = submitSettings.bind(this, settings, callback); if (this.connecting) { this.once('connect', settingsFn); return; diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js index 1f9853aa1fd128..fffea10ab6f819 100644 --- a/lib/internal/http2/util.js +++ b/lib/internal/http2/util.js @@ -174,7 +174,8 @@ const IDX_OPTIONS_PEER_MAX_CONCURRENT_STREAMS = 3; const IDX_OPTIONS_PADDING_STRATEGY = 4; const IDX_OPTIONS_MAX_HEADER_LIST_PAIRS = 5; const IDX_OPTIONS_MAX_OUTSTANDING_PINGS = 6; -const IDX_OPTIONS_FLAGS = 7; +const IDX_OPTIONS_MAX_OUTSTANDING_SETTINGS = 7; +const IDX_OPTIONS_FLAGS = 8; function updateOptionsBuffer(options) { var flags = 0; @@ -213,6 +214,11 @@ function updateOptionsBuffer(options) { optionsBuffer[IDX_OPTIONS_MAX_OUTSTANDING_PINGS] = options.maxOutstandingPings; } + if (typeof options.maxOutstandingSettings === 'number') { + flags |= (1 << IDX_OPTIONS_MAX_OUTSTANDING_SETTINGS); + optionsBuffer[IDX_OPTIONS_MAX_OUTSTANDING_SETTINGS] = + Math.max(1, options.maxOutstandingSettings); + } optionsBuffer[IDX_OPTIONS_FLAGS] = flags; } diff --git a/src/async_wrap.h b/src/async_wrap.h index ec9e162ca79bc4..c5dd4506886984 100644 --- a/src/async_wrap.h +++ b/src/async_wrap.h @@ -43,6 +43,7 @@ namespace node { V(HTTP2SESSION) \ V(HTTP2STREAM) \ V(HTTP2PING) \ + V(HTTP2SETTINGS) \ V(HTTPPARSER) \ V(JSSTREAM) \ V(PIPECONNECTWRAP) \ diff --git a/src/env.h b/src/env.h index 9414e357162d93..4a0e7a4b1295c9 100644 --- a/src/env.h +++ b/src/env.h @@ -287,6 +287,7 @@ class ModuleWrap; V(context, v8::Context) \ V(http2ping_constructor_template, v8::ObjectTemplate) \ V(http2stream_constructor_template, v8::ObjectTemplate) \ + V(http2settings_constructor_template, v8::ObjectTemplate) \ V(inspector_console_api_object, v8::Object) \ V(module_load_list_array, v8::Array) \ V(pbkdf2_constructor_template, v8::ObjectTemplate) \ diff --git a/src/node_http2.cc b/src/node_http2.cc index 56ef79f8a5da03..ba2a8a655fdac7 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -153,22 +153,28 @@ Http2Options::Http2Options(Environment* env) { if (flags & (1 << IDX_OPTIONS_MAX_OUTSTANDING_PINGS)) { SetMaxOutstandingPings(buffer[IDX_OPTIONS_MAX_OUTSTANDING_PINGS]); } + + // The HTTP2 specification places no limits on the number of HTTP2 + // SETTINGS frames that can be sent. In order to prevent PINGS from being + // abused as an attack vector, however, we place a strict upper limit + // on the number of unacknowledged SETTINGS that can be sent at any given + // time. + if (flags & (1 << IDX_OPTIONS_MAX_OUTSTANDING_SETTINGS)) { + SetMaxOutstandingSettings(buffer[IDX_OPTIONS_MAX_OUTSTANDING_SETTINGS]); + } } -// The Http2Settings class is used to configure a SETTINGS frame that is -// to be sent to the connected peer. The settings are set using a TypedArray -// that is shared with the JavaScript side. -Http2Settings::Http2Settings(Environment* env) : env_(env) { +void Http2Session::Http2Settings::Init() { entries_.AllocateSufficientStorage(IDX_SETTINGS_COUNT); AliasedBuffer& buffer = - env->http2_state()->settings_buffer; + env()->http2_state()->settings_buffer; uint32_t flags = buffer[IDX_SETTINGS_COUNT]; size_t n = 0; if (flags & (1 << IDX_SETTINGS_HEADER_TABLE_SIZE)) { uint32_t val = buffer[IDX_SETTINGS_HEADER_TABLE_SIZE]; - DEBUG_HTTP2("Http2Settings: setting header table size: %d\n", val); + DEBUG_HTTP2SESSION2(session, "setting header table size: %d\n", val); entries_[n].settings_id = NGHTTP2_SETTINGS_HEADER_TABLE_SIZE; entries_[n].value = val; n++; @@ -176,7 +182,7 @@ Http2Settings::Http2Settings(Environment* env) : env_(env) { if (flags & (1 << IDX_SETTINGS_MAX_CONCURRENT_STREAMS)) { uint32_t val = buffer[IDX_SETTINGS_MAX_CONCURRENT_STREAMS]; - DEBUG_HTTP2("Http2Settings: setting max concurrent streams: %d\n", val); + DEBUG_HTTP2SESSION2(session, "setting max concurrent streams: %d\n", val); entries_[n].settings_id = NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS; entries_[n].value = val; n++; @@ -184,7 +190,7 @@ Http2Settings::Http2Settings(Environment* env) : env_(env) { if (flags & (1 << IDX_SETTINGS_MAX_FRAME_SIZE)) { uint32_t val = buffer[IDX_SETTINGS_MAX_FRAME_SIZE]; - DEBUG_HTTP2("Http2Settings: setting max frame size: %d\n", val); + DEBUG_HTTP2SESSION2(session, "setting max frame size: %d\n", val); entries_[n].settings_id = NGHTTP2_SETTINGS_MAX_FRAME_SIZE; entries_[n].value = val; n++; @@ -192,7 +198,7 @@ Http2Settings::Http2Settings(Environment* env) : env_(env) { if (flags & (1 << IDX_SETTINGS_INITIAL_WINDOW_SIZE)) { uint32_t val = buffer[IDX_SETTINGS_INITIAL_WINDOW_SIZE]; - DEBUG_HTTP2("Http2Settings: setting initial window size: %d\n", val); + DEBUG_HTTP2SESSION2(session, "setting initial window size: %d\n", val); entries_[n].settings_id = NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE; entries_[n].value = val; n++; @@ -200,7 +206,7 @@ Http2Settings::Http2Settings(Environment* env) : env_(env) { if (flags & (1 << IDX_SETTINGS_MAX_HEADER_LIST_SIZE)) { uint32_t val = buffer[IDX_SETTINGS_MAX_HEADER_LIST_SIZE]; - DEBUG_HTTP2("Http2Settings: setting max header list size: %d\n", val); + DEBUG_HTTP2SESSION2(session, "setting max header list size: %d\n", val); entries_[n].settings_id = NGHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE; entries_[n].value = val; n++; @@ -208,7 +214,7 @@ Http2Settings::Http2Settings(Environment* env) : env_(env) { if (flags & (1 << IDX_SETTINGS_ENABLE_PUSH)) { uint32_t val = buffer[IDX_SETTINGS_ENABLE_PUSH]; - DEBUG_HTTP2("Http2Settings: setting enable push: %d\n", val); + DEBUG_HTTP2SESSION2(session, "setting enable push: %d\n", val); entries_[n].settings_id = NGHTTP2_SETTINGS_ENABLE_PUSH; entries_[n].value = val; n++; @@ -217,13 +223,46 @@ Http2Settings::Http2Settings(Environment* env) : env_(env) { count_ = n; } +Http2Session::Http2Settings::Http2Settings( + Environment* env) + : AsyncWrap(env, + env->http2settings_constructor_template() + ->NewInstance(env->context()) + .ToLocalChecked(), + AsyncWrap::PROVIDER_HTTP2SETTINGS), + session_(nullptr), + startTime_(0) { + Init(); +} + +// The Http2Settings class is used to configure a SETTINGS frame that is +// to be sent to the connected peer. The settings are set using a TypedArray +// that is shared with the JavaScript side. +Http2Session::Http2Settings::Http2Settings( + Http2Session* session) + : AsyncWrap(session->env(), + session->env()->http2settings_constructor_template() + ->NewInstance(session->env()->context()) + .ToLocalChecked(), + AsyncWrap::PROVIDER_HTTP2SETTINGS), + session_(session), + startTime_(uv_hrtime()) { + Init(); +} + +Http2Session::Http2Settings::~Http2Settings() { + if (!object().IsEmpty()) + ClearWrap(object()); + persistent().Reset(); + CHECK(persistent().IsEmpty()); +} // Generates a Buffer that contains the serialized payload of a SETTINGS // frame. This can be used, for instance, to create the Base64-encoded // content of an Http2-Settings header field. -inline Local Http2Settings::Pack() { +inline Local Http2Session::Http2Settings::Pack() { const size_t len = count_ * 6; - Local buf = Buffer::New(env_, len).ToLocalChecked(); + Local buf = Buffer::New(env(), len).ToLocalChecked(); ssize_t ret = nghttp2_pack_settings_payload( reinterpret_cast(Buffer::Data(buf)), len, @@ -231,14 +270,14 @@ inline Local Http2Settings::Pack() { if (ret >= 0) return buf; else - return Undefined(env_->isolate()); + return Undefined(env()->isolate()); } // Updates the shared TypedArray with the current remote or local settings for // the session. -inline void Http2Settings::Update(Environment* env, - Http2Session* session, - get_setting fn) { +inline void Http2Session::Http2Settings::Update(Environment* env, + Http2Session* session, + get_setting fn) { AliasedBuffer& buffer = env->http2_state()->settings_buffer; buffer[IDX_SETTINGS_HEADER_TABLE_SIZE] = @@ -256,7 +295,7 @@ inline void Http2Settings::Update(Environment* env, } // Initializes the shared TypedArray with the default settings values. -inline void Http2Settings::RefreshDefaults(Environment* env) { +inline void Http2Session::Http2Settings::RefreshDefaults(Environment* env) { AliasedBuffer& buffer = env->http2_state()->settings_buffer; @@ -279,6 +318,24 @@ inline void Http2Settings::RefreshDefaults(Environment* env) { } +void Http2Session::Http2Settings::Send() { + Http2Scope h2scope(session_); + CHECK_EQ(nghttp2_submit_settings(**session_, NGHTTP2_FLAG_NONE, + *entries_, length()), 0); +} + +void Http2Session::Http2Settings::Done(bool ack) { + uint64_t end = uv_hrtime(); + double duration = (end - startTime_) / 1e6; + + Local argv[2] = { + Boolean::New(env()->isolate(), ack), + Number::New(env()->isolate(), duration) + }; + MakeCallback(env()->ondone_string(), arraysize(argv), argv); + delete this; +} + // The Http2Priority class initializes an appropriate nghttp2_priority_spec // struct used when either creating a stream or updating its priority // settings. @@ -417,6 +474,7 @@ Http2Session::Http2Session(Environment* env, : std::max(maxHeaderPairs, 1); // minimum # of response headers max_outstanding_pings_ = opts.GetMaxOutstandingPings(); + max_outstanding_settings_ = opts.GetMaxOutstandingSettings(); padding_strategy_ = opts.GetPaddingStrategy(); @@ -554,22 +612,6 @@ inline ssize_t Http2Session::OnCallbackPadding(size_t frameLen, } -// Sends a SETTINGS frame to the connected peer. This has the side effect of -// changing the settings state within the nghttp2_session, but those will -// only be considered active once the connected peer acknowledges the SETTINGS -// frame. -// Note: This *must* send a SETTINGS frame even if niv == 0 -inline void Http2Session::Settings(const nghttp2_settings_entry iv[], - size_t niv) { - DEBUG_HTTP2SESSION2(this, "submitting %d settings", niv); - Http2Scope h2scope(this); - // This will fail either if the system is out of memory, or if the settings - // values are not within the appropriate range. We should be catching the - // latter before it gets this far so crash in either case. - CHECK_EQ(nghttp2_submit_settings(session_, NGHTTP2_FLAG_NONE, iv, niv), 0); -} - - // Write data received from the i/o stream to the underlying nghttp2_session. // On each call to nghttp2_session_mem_recv, nghttp2 will begin calling the // various callback functions. Each of these will typically result in a call @@ -1044,15 +1086,17 @@ inline void Http2Session::HandlePingFrame(const nghttp2_frame* frame) { // Called by OnFrameReceived when a complete SETTINGS frame has been received. inline void Http2Session::HandleSettingsFrame(const nghttp2_frame* frame) { - Isolate* isolate = env()->isolate(); - HandleScope scope(isolate); - Local context = env()->context(); - Context::Scope context_scope(context); - bool ack = frame->hd.flags & NGHTTP2_FLAG_ACK; - - Local argv[1] = { Boolean::New(isolate, ack) }; - MakeCallback(env()->onsettings_string(), arraysize(argv), argv); + if (ack) { + // If this is an acknowledgement, we should have an Http2Settings + // object for it. + Http2Settings* settings = PopSettings(); + if (settings != nullptr) + settings->Done(true); + } else { + // Otherwise, notify the session about a new settings + MakeCallback(env()->onsettings_string(), 0, nullptr); + } } // Callback used when data has been written to the stream. @@ -1954,7 +1998,7 @@ void HttpErrorString(const FunctionCallbackInfo& args) { // output for an HTTP2-Settings header field. void PackSettings(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - Http2Settings settings(env); + Http2Session::Http2Settings settings(env); args.GetReturnValue().Set(settings.Pack()); } @@ -1963,7 +2007,7 @@ void PackSettings(const FunctionCallbackInfo& args) { // default values. void RefreshDefaultSettings(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - Http2Settings::RefreshDefaults(env); + Http2Session::Http2Settings::RefreshDefaults(env); } // Sets the next stream ID the Http2Session. If successful, returns true. @@ -2061,17 +2105,6 @@ void Http2Session::Destroy(const FunctionCallbackInfo& args) { session->Close(code, socketDestroyed); } -// Submits a SETTINGS frame for the Http2Session -void Http2Session::Settings(const FunctionCallbackInfo& args) { - Http2Session* session; - ASSIGN_OR_RETURN_UNWRAP(&session, args.Holder()); - Environment* env = session->env(); - - Http2Settings settings(env); - session->Http2Session::Settings(*settings, settings.length()); - DEBUG_HTTP2SESSION(session, "settings submitted"); -} - // Submits a new request on the Http2Session and returns either an error code // or the Http2Stream object. void Http2Session::Request(const FunctionCallbackInfo& args) { @@ -2373,6 +2406,26 @@ void Http2Session::Ping(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(true); } +// Submits a SETTINGS frame for the Http2Session +void Http2Session::Settings(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + Http2Session* session; + ASSIGN_OR_RETURN_UNWRAP(&session, args.Holder()); + + Http2Session::Http2Settings* settings = new Http2Settings(session); + Local obj = settings->object(); + obj->Set(env->context(), env->ondone_string(), args[0]).FromJust(); + + if (!session->AddSettings(settings)) { + settings->Done(false); + return args.GetReturnValue().Set(false); + } + + settings->Send(); + args.GetReturnValue().Set(true); +} + + Http2Session::Http2Ping* Http2Session::PopPing() { Http2Ping* ping = nullptr; if (!outstanding_pings_.empty()) { @@ -2389,6 +2442,21 @@ bool Http2Session::AddPing(Http2Session::Http2Ping* ping) { return true; } +Http2Session::Http2Settings* Http2Session::PopSettings() { + Http2Settings* settings = nullptr; + if (!outstanding_settings_.empty()) { + settings = outstanding_settings_.front(); + outstanding_settings_.pop(); + } + return settings; +} + +bool Http2Session::AddSettings(Http2Session::Http2Settings* settings) { + if (outstanding_settings_.size() == max_outstanding_settings_) + return false; + outstanding_settings_.push(settings); + return true; +} Http2Session::Http2Ping::Http2Ping( Http2Session* session) @@ -2488,6 +2556,13 @@ void Initialize(Local target, pingt->SetInternalFieldCount(1); env->set_http2ping_constructor_template(pingt); + Local setting = FunctionTemplate::New(env->isolate()); + setting->SetClassName(FIXED_ONE_BYTE_STRING(env->isolate(), "Http2Setting")); + AsyncWrap::AddWrapMethods(env, setting); + Local settingt = setting->InstanceTemplate(); + settingt->SetInternalFieldCount(1); + env->set_http2settings_constructor_template(settingt); + Local stream = FunctionTemplate::New(env->isolate()); stream->SetClassName(FIXED_ONE_BYTE_STRING(env->isolate(), "Http2Stream")); env->SetProtoMethod(stream, "id", Http2Stream::GetID); diff --git a/src/node_http2.h b/src/node_http2.h index c61521f3ca46f2..abfa9652674e03 100644 --- a/src/node_http2.h +++ b/src/node_http2.h @@ -76,6 +76,9 @@ void inline debug_vfprintf(const char* format, ...) { // option. #define DEFAULT_MAX_PINGS 10 +// Also strictly limit the number of outstanding SETTINGS frames a user sends +#define DEFAULT_MAX_SETTINGS 10 + // These are the standard HTTP/2 defaults as specified by the RFC #define DEFAULT_SETTINGS_HEADER_TABLE_SIZE 4096 #define DEFAULT_SETTINGS_ENABLE_PUSH 1 @@ -484,41 +487,20 @@ class Http2Options { return max_outstanding_pings_; } + void SetMaxOutstandingSettings(size_t max) { + max_outstanding_settings_ = max; + } + + size_t GetMaxOutstandingSettings() { + return max_outstanding_settings_; + } + private: nghttp2_option* options_; uint32_t max_header_pairs_ = DEFAULT_MAX_HEADER_LIST_PAIRS; padding_strategy_type padding_strategy_ = PADDING_STRATEGY_NONE; size_t max_outstanding_pings_ = DEFAULT_MAX_PINGS; -}; - -// The Http2Settings class is used to parse the settings passed in for -// an Http2Session, converting those into an array of nghttp2_settings_entry -// structs. -class Http2Settings { - public: - explicit Http2Settings(Environment* env); - - size_t length() const { return count_; } - - nghttp2_settings_entry* operator*() { - return *entries_; - } - - // Returns a Buffer instance with the serialized SETTINGS payload - inline Local Pack(); - - // Resets the default values in the settings buffer - static inline void RefreshDefaults(Environment* env); - - // Update the local or remote settings for the given session - static inline void Update(Environment* env, - Http2Session* session, - get_setting fn); - - private: - Environment* env_; - size_t count_ = 0; - MaybeStackBuffer entries_; + size_t max_outstanding_settings_ = DEFAULT_MAX_SETTINGS; }; class Http2Priority { @@ -792,6 +774,7 @@ class Http2Session : public AsyncWrap { ~Http2Session() override; class Http2Ping; + class Http2Settings; void Start(); void Stop(); @@ -841,9 +824,6 @@ class Http2Session : public AsyncWrap { // Removes a stream instance from this session inline void RemoveStream(int32_t id); - // Submits a SETTINGS frame to the connected peer. - inline void Settings(const nghttp2_settings_entry iv[], size_t niv); - // Write data to the session inline ssize_t Write(const uv_buf_t* bufs, size_t nbufs); @@ -896,6 +876,9 @@ class Http2Session : public AsyncWrap { Http2Ping* PopPing(); bool AddPing(Http2Ping* ping); + Http2Settings* PopSettings(); + bool AddSettings(Http2Settings* settings); + private: // Frame Padding Strategies inline ssize_t OnMaxFrameSizePadding(size_t frameLength, @@ -1026,6 +1009,9 @@ class Http2Session : public AsyncWrap { size_t max_outstanding_pings_ = DEFAULT_MAX_PINGS; std::queue outstanding_pings_; + size_t max_outstanding_settings_ = DEFAULT_MAX_SETTINGS; + std::queue outstanding_settings_; + std::vector outgoing_buffers_; std::vector outgoing_storage_; @@ -1050,6 +1036,45 @@ class Http2Session::Http2Ping : public AsyncWrap { uint64_t startTime_; }; +// The Http2Settings class is used to parse the settings passed in for +// an Http2Session, converting those into an array of nghttp2_settings_entry +// structs. +class Http2Session::Http2Settings : public AsyncWrap { + public: + explicit Http2Settings(Environment* env); + explicit Http2Settings(Http2Session* session); + ~Http2Settings(); + + size_t self_size() const override { return sizeof(*this); } + + void Send(); + void Done(bool ack); + + size_t length() const { return count_; } + + nghttp2_settings_entry* operator*() { + return *entries_; + } + + // Returns a Buffer instance with the serialized SETTINGS payload + inline Local Pack(); + + // Resets the default values in the settings buffer + static inline void RefreshDefaults(Environment* env); + + // Update the local or remote settings for the given session + static inline void Update(Environment* env, + Http2Session* session, + get_setting fn); + + private: + void Init(); + Http2Session* session_; + uint64_t startTime_; + size_t count_ = 0; + MaybeStackBuffer entries_; +}; + class ExternalHeader : public String::ExternalOneByteStringResource { public: diff --git a/src/node_http2_state.h b/src/node_http2_state.h index a7ad23fb519886..ef8696ce60d8f8 100644 --- a/src/node_http2_state.h +++ b/src/node_http2_state.h @@ -49,6 +49,7 @@ namespace http2 { IDX_OPTIONS_PADDING_STRATEGY, IDX_OPTIONS_MAX_HEADER_LIST_PAIRS, IDX_OPTIONS_MAX_OUTSTANDING_PINGS, + IDX_OPTIONS_MAX_OUTSTANDING_SETTINGS, IDX_OPTIONS_FLAGS }; diff --git a/test/parallel/test-http2-session-settings.js b/test/parallel/test-http2-session-settings.js index 239e2fa76c9c02..75fcc1942104ac 100644 --- a/test/parallel/test-http2-session-settings.js +++ b/test/parallel/test-http2-session-settings.js @@ -77,9 +77,7 @@ server.listen( // State will only be valid after connect event is emitted req.on('ready', common.mustCall(() => { assert.doesNotThrow(() => { - client.settings({ - maxHeaderListSize: 1 - }); + client.settings({ maxHeaderListSize: 1 }, common.mustCall()); }); // Verify valid error ranges diff --git a/test/parallel/test-http2-too-many-settings.js b/test/parallel/test-http2-too-many-settings.js index 9ba8605be8b55b..0302fe623da07c 100644 --- a/test/parallel/test-http2-too-many-settings.js +++ b/test/parallel/test-http2-too-many-settings.js @@ -1,7 +1,7 @@ 'use strict'; // Tests that attempting to send too many non-acknowledged -// settings frames will result in a throw. +// settings frames will result in an error const common = require('../common'); if (!common.hasCrypto) @@ -9,53 +9,41 @@ if (!common.hasCrypto) const assert = require('assert'); const h2 = require('http2'); -const maxPendingAck = 2; -const server = h2.createServer({ maxPendingAck: maxPendingAck + 1 }); - -let clients = 2; +const maxOutstandingSettings = 2; function doTest(session) { - for (let n = 0; n < maxPendingAck; n++) - assert.doesNotThrow(() => session.settings({ enablePush: false })); - common.expectsError(() => session.settings({ enablePush: false }), - { - code: 'ERR_HTTP2_MAX_PENDING_SETTINGS_ACK', - type: Error - }); + session.on('error', common.expectsError({ + code: 'ERR_HTTP2_MAX_PENDING_SETTINGS_ACK', + type: Error + })); + for (let n = 0; n < maxOutstandingSettings; n++) { + session.settings({ enablePush: false }); + assert.strictEqual(session.pendingSettingsAck, true); + } } -server.on('stream', common.mustNotCall()); - -server.once('session', common.mustCall((session) => doTest(session))); - -server.listen(0); - -const closeServer = common.mustCall(() => { - if (--clients === 0) - server.close(); -}, clients); - -server.on('listening', common.mustCall(() => { - const client = h2.connect(`http://localhost:${server.address().port}`, - { maxPendingAck: maxPendingAck + 1 }); - let remaining = maxPendingAck + 1; - - client.on('close', closeServer); - client.on('localSettings', common.mustCall(() => { - if (--remaining <= 0) { - client.close(); - } - }, maxPendingAck + 1)); - client.on('connect', common.mustCall(() => doTest(client))); -})); +{ + const server = h2.createServer({ maxOutstandingSettings }); + server.on('stream', common.mustNotCall()); + server.once('session', common.mustCall((session) => doTest(session))); + + server.listen(0, common.mustCall(() => { + const client = h2.connect(`http://localhost:${server.address().port}`); + // On some operating systems, an ECONNRESET error may be emitted. + // On others it won't be. Do not make this a mustCall + client.on('error', () => {}); + client.on('close', common.mustCall(() => server.close())); + })); +} -// Setting maxPendingAck to 0, defaults it to 1 -server.on('listening', common.mustCall(() => { - const client = h2.connect(`http://localhost:${server.address().port}`, - { maxPendingAck: 0 }); +{ + const server = h2.createServer(); + server.on('stream', common.mustNotCall()); - client.on('close', closeServer); - client.on('localSettings', common.mustCall(() => { - client.close(); + server.listen(0, common.mustCall(() => { + const client = h2.connect(`http://localhost:${server.address().port}`, + { maxOutstandingSettings }); + client.on('connect', () => doTest(client)); + client.on('close', () => server.close()); })); -})); +} diff --git a/test/parallel/test-http2-util-update-options-buffer.js b/test/parallel/test-http2-util-update-options-buffer.js index 14e4ba23d8fdca..5768ce0204dc8c 100644 --- a/test/parallel/test-http2-util-update-options-buffer.js +++ b/test/parallel/test-http2-util-update-options-buffer.js @@ -19,7 +19,8 @@ const IDX_OPTIONS_PEER_MAX_CONCURRENT_STREAMS = 3; const IDX_OPTIONS_PADDING_STRATEGY = 4; const IDX_OPTIONS_MAX_HEADER_LIST_PAIRS = 5; const IDX_OPTIONS_MAX_OUTSTANDING_PINGS = 6; -const IDX_OPTIONS_FLAGS = 7; +const IDX_OPTIONS_MAX_OUTSTANDING_SETTINGS = 7; +const IDX_OPTIONS_FLAGS = 8; { updateOptionsBuffer({ @@ -29,7 +30,8 @@ const IDX_OPTIONS_FLAGS = 7; peerMaxConcurrentStreams: 4, paddingStrategy: 5, maxHeaderListPairs: 6, - maxOutstandingPings: 7 + maxOutstandingPings: 7, + maxOutstandingSettings: 8 }); strictEqual(optionsBuffer[IDX_OPTIONS_MAX_DEFLATE_DYNAMIC_TABLE_SIZE], 1); @@ -39,6 +41,7 @@ const IDX_OPTIONS_FLAGS = 7; strictEqual(optionsBuffer[IDX_OPTIONS_PADDING_STRATEGY], 5); strictEqual(optionsBuffer[IDX_OPTIONS_MAX_HEADER_LIST_PAIRS], 6); strictEqual(optionsBuffer[IDX_OPTIONS_MAX_OUTSTANDING_PINGS], 7); + strictEqual(optionsBuffer[IDX_OPTIONS_MAX_OUTSTANDING_SETTINGS], 8); const flags = optionsBuffer[IDX_OPTIONS_FLAGS]; @@ -49,6 +52,7 @@ const IDX_OPTIONS_FLAGS = 7; ok(flags & (1 << IDX_OPTIONS_PADDING_STRATEGY)); ok(flags & (1 << IDX_OPTIONS_MAX_HEADER_LIST_PAIRS)); ok(flags & (1 << IDX_OPTIONS_MAX_OUTSTANDING_PINGS)); + ok(flags & (1 << IDX_OPTIONS_MAX_OUTSTANDING_SETTINGS)); } { diff --git a/test/sequential/test-async-wrap-getasyncid.js b/test/sequential/test-async-wrap-getasyncid.js index bd2b3254f06594..5a6d4e0758adc1 100644 --- a/test/sequential/test-async-wrap-getasyncid.js +++ b/test/sequential/test-async-wrap-getasyncid.js @@ -25,6 +25,7 @@ const fixtures = require('../common/fixtures'); delete providers.HTTP2SESSION; delete providers.HTTP2STREAM; delete providers.HTTP2PING; + delete providers.HTTP2SETTINGS; const obj_keys = Object.keys(providers); if (obj_keys.length > 0)