From 3e14212f0e86bd5a8b5916d8a3acbbc0f42a1b1c Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Mon, 10 Dec 2018 12:11:47 -0800 Subject: [PATCH 01/10] test: remove magic numbers in test-gc-http-client-onerror Remove magic numbers (500, 10, 100) from the test. Instead, detect when GC has started and stop sending requests at that point. On my laptop, this results in 16 or 20 requests per run instead of 500. Fixes: https://github.com/nodejs/node/issues/23089 PR-URL: https://github.com/nodejs/node/pull/24943 Reviewed-By: Colin Ihrig --- test/parallel/test-gc-http-client-onerror.js | 53 +++++++++++--------- 1 file changed, 30 insertions(+), 23 deletions(-) diff --git a/test/parallel/test-gc-http-client-onerror.js b/test/parallel/test-gc-http-client-onerror.js index 28a8aecd27e7..30b272ed94aa 100644 --- a/test/parallel/test-gc-http-client-onerror.js +++ b/test/parallel/test-gc-http-client-onerror.js @@ -6,6 +6,8 @@ const common = require('../common'); const onGC = require('../common/ongc'); +const cpus = require('os').cpus().length; + function serverHandler(req, res) { req.resume(); res.writeHead(200, { 'Content-Type': 'text/plain' }); @@ -13,38 +15,35 @@ function serverHandler(req, res) { } const http = require('http'); -const todo = 500; +let createClients = true; let done = 0; let count = 0; let countGC = 0; -console.log(`We should do ${todo} requests`); - const server = http.createServer(serverHandler); server.listen(0, common.mustCall(() => { - for (let i = 0; i < 10; i++) - getall(); + for (let i = 0; i < cpus; i++) + getAll(); })); -function getall() { - if (count >= todo) - return; - - const req = http.get({ - hostname: 'localhost', - pathname: '/', - port: server.address().port - }, cb).on('error', onerror); +function getAll() { + if (createClients) { + const req = http.get({ + hostname: 'localhost', + pathname: '/', + port: server.address().port + }, cb).on('error', onerror); - count++; - onGC(req, { ongc }); + count++; + onGC(req, { ongc }); - setImmediate(getall); + setImmediate(getAll); + } } function cb(res) { res.resume(); - done += 1; + done++; } function onerror(err) { @@ -55,11 +54,19 @@ function ongc() { countGC++; } -setInterval(status, 100).unref(); +setImmediate(status); function status() { - global.gc(); - console.log('Done: %d/%d', done, todo); - console.log('Collected: %d/%d', countGC, count); - if (countGC === todo) server.close(); + if (done > 0) { + createClients = false; + global.gc(); + console.log(`done/collected/total: ${done}/${countGC}/${count}`); + if (countGC === count) { + server.close(); + } else { + setImmediate(status); + } + } else { + setImmediate(status); + } } From 527407c49fe37599a6d8daec70d2ff1e30d2a4f8 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Mon, 5 Nov 2018 04:52:50 +0800 Subject: [PATCH 02/10] src: cache the result of GetOptions() in JS land Instead of calling into C++ each time we need to check the value of a command line option, cache the option map in a new `internal/options` module for faster access to the values in JS land. PR-URL: https://github.com/nodejs/node/pull/24091 Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: Gus Caplan Reviewed-By: Refael Ackermann --- lib/crypto.js | 2 +- lib/internal/bash_completion.js | 3 +-- lib/internal/bootstrap/loaders.js | 3 +-- lib/internal/bootstrap/node.js | 16 ++++++++-------- lib/internal/modules/cjs/helpers.js | 4 ++-- lib/internal/modules/cjs/loader.js | 8 ++++---- lib/internal/modules/esm/default_resolve.js | 6 +++--- lib/internal/options.js | 18 ++++++++++++++++++ lib/internal/print_help.js | 5 +++-- lib/internal/process/esm_loader.js | 2 +- lib/repl.js | 2 +- lib/vm.js | 2 +- node.gyp | 1 + src/node_options.cc | 17 ++--------------- 14 files changed, 47 insertions(+), 42 deletions(-) create mode 100644 lib/internal/options.js diff --git a/lib/crypto.js b/lib/crypto.js index f23f1f9ae749..9ab08ab4b2a5 100644 --- a/lib/crypto.js +++ b/lib/crypto.js @@ -34,7 +34,7 @@ const { ERR_CRYPTO_FIPS_FORCED, ERR_CRYPTO_FIPS_UNAVAILABLE } = require('internal/errors').codes; -const constants = process.binding('constants').crypto; +const constants = internalBinding('constants').crypto; const { fipsMode, fipsForced diff --git a/lib/internal/bash_completion.js b/lib/internal/bash_completion.js index cb8eab9ceec8..13363e8c4b8c 100644 --- a/lib/internal/bash_completion.js +++ b/lib/internal/bash_completion.js @@ -1,8 +1,7 @@ 'use strict'; -const { getOptions } = internalBinding('options'); +const { options, aliases } = require('internal/options'); function print(stream) { - const { options, aliases } = getOptions(); const all_opts = [...options.keys(), ...aliases.keys()]; stream.write(`_node_complete() { diff --git a/lib/internal/bootstrap/loaders.js b/lib/internal/bootstrap/loaders.js index a90da4861b7b..e2cfdbf83d38 100644 --- a/lib/internal/bootstrap/loaders.js +++ b/lib/internal/bootstrap/loaders.js @@ -217,8 +217,7 @@ NativeModule.isInternal = function(id) { return id.startsWith('internal/') || - (id === 'worker_threads' && - !internalBinding('options').getOptions('--experimental-worker')); + (id === 'worker_threads' && !config.experimentalWorker); }; } diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index 067998f115cc..0ac055900073 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -113,12 +113,13 @@ NativeModule.require('internal/inspector_async_hook').setup(); } - const { getOptions } = internalBinding('options'); - const helpOption = getOptions('--help'); - const completionBashOption = getOptions('--completion-bash'); - const experimentalModulesOption = getOptions('--experimental-modules'); - const experimentalVMModulesOption = getOptions('--experimental-vm-modules'); - const experimentalWorkerOption = getOptions('--experimental-worker'); + const { getOptionValue } = NativeModule.require('internal/options'); + const helpOption = getOptionValue('--help'); + const completionBashOption = getOptionValue('--completion-bash'); + const experimentalModulesOption = getOptionValue('--experimental-modules'); + const experimentalVMModulesOption = + getOptionValue('--experimental-vm-modules'); + const experimentalWorkerOption = getOptionValue('--experimental-worker'); if (helpOption) { NativeModule.require('internal/print_help').print(process.stdout); return; @@ -631,10 +632,9 @@ const get = () => { const { - getOptions, envSettings: { kAllowedInEnvironment } } = internalBinding('options'); - const { options, aliases } = getOptions(); + const { options, aliases } = NativeModule.require('internal/options'); const allowedNodeEnvironmentFlags = []; for (const [name, info] of options) { diff --git a/lib/internal/modules/cjs/helpers.js b/lib/internal/modules/cjs/helpers.js index cda94987fad2..b08e97b29c5c 100644 --- a/lib/internal/modules/cjs/helpers.js +++ b/lib/internal/modules/cjs/helpers.js @@ -9,7 +9,7 @@ const { CHAR_HASH, } = require('internal/constants'); -const { getOptions } = internalBinding('options'); +const { getOptionValue } = require('internal/options'); // Invoke with makeRequireFunction(module) where |module| is the Module object // to use as the context for the require() function. @@ -107,7 +107,7 @@ const builtinLibs = [ 'v8', 'vm', 'zlib' ]; -if (getOptions('--experimental-worker')) { +if (getOptionValue('--experimental-worker')) { builtinLibs.push('worker_threads'); builtinLibs.sort(); } diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 4e48d5493770..fb3770b7299d 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -40,10 +40,10 @@ const { stripBOM, stripShebang } = require('internal/modules/cjs/helpers'); -const options = internalBinding('options'); -const preserveSymlinks = options.getOptions('--preserve-symlinks'); -const preserveSymlinksMain = options.getOptions('--preserve-symlinks-main'); -const experimentalModules = options.getOptions('--experimental-modules'); +const { getOptionValue } = require('internal/options'); +const preserveSymlinks = getOptionValue('--preserve-symlinks'); +const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main'); +const experimentalModules = getOptionValue('--experimental-modules'); const { ERR_INVALID_ARG_TYPE, diff --git a/lib/internal/modules/esm/default_resolve.js b/lib/internal/modules/esm/default_resolve.js index 7654ca911293..9aa54d09a1b0 100644 --- a/lib/internal/modules/esm/default_resolve.js +++ b/lib/internal/modules/esm/default_resolve.js @@ -6,9 +6,9 @@ const internalFS = require('internal/fs/utils'); const { NativeModule } = require('internal/bootstrap/loaders'); const { extname } = require('path'); const { realpathSync } = require('fs'); -const { getOptions } = internalBinding('options'); -const preserveSymlinks = getOptions('--preserve-symlinks'); -const preserveSymlinksMain = getOptions('--preserve-symlinks-main'); +const { getOptionValue } = require('internal/options'); +const preserveSymlinks = getOptionValue('--preserve-symlinks'); +const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main'); const { ERR_MISSING_MODULE, ERR_MODULE_RESOLUTION_LEGACY, diff --git a/lib/internal/options.js b/lib/internal/options.js new file mode 100644 index 000000000000..e494787b96c0 --- /dev/null +++ b/lib/internal/options.js @@ -0,0 +1,18 @@ +'use strict'; + +const { getOptions } = internalBinding('options'); +const { options, aliases } = getOptions(); + +function getOptionValue(option) { + const result = options.get(option); + if (!result) { + return undefined; + } + return result.value; +} + +module.exports = { + options, + aliases, + getOptionValue +}; diff --git a/lib/internal/print_help.js b/lib/internal/print_help.js index 8acc9271b191..d9ab3c1ad84e 100644 --- a/lib/internal/print_help.js +++ b/lib/internal/print_help.js @@ -1,5 +1,6 @@ 'use strict'; -const { getOptions, types } = internalBinding('options'); + +const { types } = internalBinding('options'); const typeLookup = []; for (const key of Object.keys(types)) @@ -132,7 +133,7 @@ function format({ options, aliases = new Map(), firstColumn, secondColumn }) { } function print(stream) { - const { options, aliases } = getOptions(); + const { options, aliases } = require('internal/options'); // Use 75 % of the available width, and at least 70 characters. const width = Math.max(70, (stream.columns || 0) * 0.75); diff --git a/lib/internal/process/esm_loader.js b/lib/internal/process/esm_loader.js index d775e685d13e..be08a6364781 100644 --- a/lib/internal/process/esm_loader.js +++ b/lib/internal/process/esm_loader.js @@ -48,7 +48,7 @@ exports.setup = function() { let ESMLoader = new Loader(); const loaderPromise = (async () => { - const userLoader = internalBinding('options').getOptions('--loader'); + const userLoader = require('internal/options').getOptionValue('--loader'); if (userLoader) { const hooks = await ESMLoader.import( userLoader, pathToFileURL(`${process.cwd()}/`).href); diff --git a/lib/repl.js b/lib/repl.js index b00bca41cbda..448308a44c00 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -71,7 +71,7 @@ const { ERR_SCRIPT_EXECUTION_INTERRUPTED } = require('internal/errors').codes; const { sendInspectorCommand } = require('internal/util/inspector'); -const experimentalREPLAwait = internalBinding('options').getOptions( +const experimentalREPLAwait = require('internal/options').getOptionValue( '--experimental-repl-await' ); const { isRecoverableError } = require('internal/repl/recoverable'); diff --git a/lib/vm.js b/lib/vm.js index 720d46ea1abd..dbe48b7b15c1 100644 --- a/lib/vm.js +++ b/lib/vm.js @@ -402,7 +402,7 @@ module.exports = { compileFunction, }; -if (internalBinding('options').getOptions('--experimental-vm-modules')) { +if (require('internal/options').getOptionValue('--experimental-vm-modules')) { const { SourceTextModule } = require('internal/vm/source_text_module'); module.exports.SourceTextModule = SourceTextModule; } diff --git a/node.gyp b/node.gyp index f59b071b1ebb..55f028636fd2 100644 --- a/node.gyp +++ b/node.gyp @@ -134,6 +134,7 @@ 'lib/internal/modules/esm/translators.js', 'lib/internal/safe_globals.js', 'lib/internal/net.js', + 'lib/internal/options.js', 'lib/internal/print_help.js', 'lib/internal/process/esm_loader.js', 'lib/internal/process/main_thread_only.js', diff --git a/src/node_options.cc b/src/node_options.cc index 2b95b3f811b4..ec819538d629 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -367,9 +367,8 @@ HostPort SplitHostPort(const std::string& arg, ParseAndValidatePort(arg.substr(colon + 1), errors) }; } -// Usage: Either: -// - getOptions() to get all options + metadata or -// - getOptions(string) to get the value of a particular option +// Return a map containing all the options and their metadata as well +// as the aliases void GetOptions(const FunctionCallbackInfo& args) { Mutex::ScopedLock lock(per_process_opts_mutex); Environment* env = Environment::GetCurrent(args); @@ -390,13 +389,8 @@ void GetOptions(const FunctionCallbackInfo& args) { const auto& parser = PerProcessOptionsParser::instance; - std::string filter; - if (args[0]->IsString()) filter = *node::Utf8Value(isolate, args[0]); - Local options = Map::New(isolate); for (const auto& item : parser.options_) { - if (!filter.empty() && item.first != filter) continue; - Local value; const auto& option_info = item.second; auto field = option_info.field; @@ -445,11 +439,6 @@ void GetOptions(const FunctionCallbackInfo& args) { } CHECK(!value.IsEmpty()); - if (!filter.empty()) { - args.GetReturnValue().Set(value); - return; - } - Local name = ToV8Value(context, item.first).ToLocalChecked(); Local info = Object::New(isolate); Local help_text; @@ -471,8 +460,6 @@ void GetOptions(const FunctionCallbackInfo& args) { } } - if (!filter.empty()) return; - Local aliases; if (!ToV8Value(context, parser.aliases_).ToLocal(&aliases)) return; From 728bc631e512e0cb325a071ee83c8384c75661ff Mon Sep 17 00:00:00 2001 From: Ali Ijaz Sheikh Date: Tue, 18 Dec 2018 11:04:03 -0800 Subject: [PATCH 03/10] test: fix expectation in test-bootstrap-modules PR-URL: https://github.com/nodejs/node/pull/25112 Reviewed-By: Myles Borins Reviewed-By: Shelley Vohr --- test/parallel/test-bootstrap-modules.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/parallel/test-bootstrap-modules.js b/test/parallel/test-bootstrap-modules.js index 70011637e08a..a047d50b57e2 100644 --- a/test/parallel/test-bootstrap-modules.js +++ b/test/parallel/test-bootstrap-modules.js @@ -11,4 +11,5 @@ const list = process.moduleLoadList.slice(); const assert = require('assert'); -assert(list.length <= 78, list); +assert(list.length <= 81, + `Expected <= 81 elements in moduleLoadLists, got ${list.length}`); From 1aea1e3634de7331f670f0594c023ca7c64a759f Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Fri, 14 Dec 2018 12:44:39 +0100 Subject: [PATCH 04/10] http: fix regression of binary upgrade response body PR-URL: https://github.com/nodejs/node/pull/25039 Fixes: https://github.com/nodejs/node/issues/24958 Reviewed-By: Myles Borins --- src/node_http_parser.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc index 5ad8afbdbf76..58bae3323c04 100644 --- a/src/node_http_parser.cc +++ b/src/node_http_parser.cc @@ -605,8 +605,6 @@ class Parser : public AsyncWrap, public StreamListener { size_t nparsed = http_parser_execute(&parser_, &settings, data, len); - enum http_errno err = HTTP_PARSER_ERRNO(&parser_); - Save(); // Unassign the 'buffer_' variable @@ -621,7 +619,9 @@ class Parser : public AsyncWrap, public StreamListener { Local nparsed_obj = Integer::New(env()->isolate(), nparsed); // If there was a parse error in one of the callbacks // TODO(bnoordhuis) What if there is an error on EOF? - if ((!parser_.upgrade && nparsed != len) || err != HPE_OK) { + if (!parser_.upgrade && nparsed != len) { + enum http_errno err = HTTP_PARSER_ERRNO(&parser_); + Local e = Exception::Error(env()->parse_error_string()); Local obj = e->ToObject(env()->isolate()->GetCurrentContext()) .ToLocalChecked(); From 6183c7107de539a2169ddd4bfe283026d986dd36 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Thu, 29 Nov 2018 17:29:53 -0500 Subject: [PATCH 05/10] deps: cherry-pick http_parser_set_max_header_size MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit adds http_parser_set_max_header_size() to the http-parser for overriding the compile time maximum HTTP header size. Backport-PR-URL: https://github.com/nodejs/node/pull/25168 PR-URL: https://github.com/nodejs/node/pull/24811 Fixes: https://github.com/nodejs/node/issues/24692 Refs: https://github.com/nodejs/http-parser/pull/453 Reviewed-By: Anna Henningsen Reviewed-By: Matteo Collina Reviewed-By: Myles Borins Reviewed-By: Michael Dawson Reviewed-By: Сковорода Никита Андреевич Reviewed-By: James M Snell Reviewed-By: Jeremiah Senkpiel --- deps/http_parser/http_parser.c | 15 +++++++++++---- deps/http_parser/http_parser.h | 3 +++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/deps/http_parser/http_parser.c b/deps/http_parser/http_parser.c index 6522618671d0..46764bced094 100644 --- a/deps/http_parser/http_parser.c +++ b/deps/http_parser/http_parser.c @@ -25,6 +25,8 @@ #include #include +static uint32_t max_header_size = HTTP_MAX_HEADER_SIZE; + #ifndef ULLONG_MAX # define ULLONG_MAX ((uint64_t) -1) /* 2^64-1 */ #endif @@ -137,20 +139,20 @@ do { \ } while (0) /* Don't allow the total size of the HTTP headers (including the status - * line) to exceed HTTP_MAX_HEADER_SIZE. This check is here to protect + * line) to exceed max_header_size. This check is here to protect * embedders against denial-of-service attacks where the attacker feeds * us a never-ending header that the embedder keeps buffering. * * This check is arguably the responsibility of embedders but we're doing * it on the embedder's behalf because most won't bother and this way we - * make the web a little safer. HTTP_MAX_HEADER_SIZE is still far bigger + * make the web a little safer. max_header_size is still far bigger * than any reasonable request or response so this should never affect * day-to-day operation. */ #define COUNT_HEADER_SIZE(V) \ do { \ parser->nread += (V); \ - if (UNLIKELY(parser->nread > (HTTP_MAX_HEADER_SIZE))) { \ + if (UNLIKELY(parser->nread > max_header_size)) { \ SET_ERRNO(HPE_HEADER_OVERFLOW); \ goto error; \ } \ @@ -1471,7 +1473,7 @@ size_t http_parser_execute (http_parser *parser, const char* p_lf; size_t limit = data + len - p; - limit = MIN(limit, HTTP_MAX_HEADER_SIZE); + limit = MIN(limit, max_header_size); p_cr = (const char*) memchr(p, CR, limit); p_lf = (const char*) memchr(p, LF, limit); @@ -2437,3 +2439,8 @@ http_parser_version(void) { HTTP_PARSER_VERSION_MINOR * 0x00100 | HTTP_PARSER_VERSION_PATCH * 0x00001; } + +void +http_parser_set_max_header_size(uint32_t size) { + max_header_size = size; +} diff --git a/deps/http_parser/http_parser.h b/deps/http_parser/http_parser.h index 1fbf30e2b474..ea7bafef2c31 100644 --- a/deps/http_parser/http_parser.h +++ b/deps/http_parser/http_parser.h @@ -427,6 +427,9 @@ void http_parser_pause(http_parser *parser, int paused); /* Checks if this is the final chunk of the body. */ int http_body_is_final(const http_parser *parser); +/* Change the maximum header size provided at compile time. */ +void http_parser_set_max_header_size(uint32_t size); + #ifdef __cplusplus } #endif From a57aed144a60b5ee0c73b969f5b9ffc71fd93296 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Tue, 18 Dec 2018 12:12:21 +0100 Subject: [PATCH 06/10] src: add kUInteger parsing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit adds support for uint64_t option parsing. Backport-PR-URL: https://github.com/nodejs/node/pull/25168 PR-URL: https://github.com/nodejs/node/pull/24811 Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: Myles Borins Reviewed-By: Michael Dawson Reviewed-By: Сковорода Никита Андреевич Reviewed-By: James M Snell Reviewed-By: Jeremiah Senkpiel --- lib/internal/print_help.js | 1 + src/node_options-inl.h | 16 ++++++++++++++++ src/node_options.cc | 4 ++++ src/node_options.h | 5 +++++ 4 files changed, 26 insertions(+) diff --git a/lib/internal/print_help.js b/lib/internal/print_help.js index d9ab3c1ad84e..c453562c5c40 100644 --- a/lib/internal/print_help.js +++ b/lib/internal/print_help.js @@ -59,6 +59,7 @@ function getArgDescription(type) { case 'kHostPort': return '[host:]port'; case 'kInteger': + case 'kUInteger': case 'kString': case 'kStringList': return '...'; diff --git a/src/node_options-inl.h b/src/node_options-inl.h index 277121036e51..69b8ee0e7877 100644 --- a/src/node_options-inl.h +++ b/src/node_options-inl.h @@ -35,6 +35,19 @@ void OptionsParser::AddOption(const std::string& name, help_text}); } +template +void OptionsParser::AddOption(const std::string& name, + const std::string& help_text, + uint64_t Options::* field, + OptionEnvvarSettings env_setting) { + options_.emplace( + name, + OptionInfo{kUInteger, + std::make_shared>(field), + env_setting, + help_text}); +} + template void OptionsParser::AddOption(const std::string& name, const std::string& help_text, @@ -397,6 +410,9 @@ void OptionsParser::Parse( case kInteger: *Lookup(info.field, options) = std::atoll(value.c_str()); break; + case kUInteger: + *Lookup(info.field, options) = std::stoull(value.c_str()); + break; case kString: *Lookup(info.field, options) = value; break; diff --git a/src/node_options.cc b/src/node_options.cc index ec819538d629..1c63fa286f0f 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -406,6 +406,9 @@ void GetOptions(const FunctionCallbackInfo& args) { case kInteger: value = Number::New(isolate, *parser.Lookup(field, opts)); break; + case kUInteger: + value = Number::New(isolate, *parser.Lookup(field, opts)); + break; case kString: if (!ToV8Value(context, *parser.Lookup(field, opts)) .ToLocal(&value)) { @@ -492,6 +495,7 @@ void Initialize(Local target, NODE_DEFINE_CONSTANT(types, kV8Option); NODE_DEFINE_CONSTANT(types, kBoolean); NODE_DEFINE_CONSTANT(types, kInteger); + NODE_DEFINE_CONSTANT(types, kUInteger); NODE_DEFINE_CONSTANT(types, kString); NODE_DEFINE_CONSTANT(types, kHostPort); NODE_DEFINE_CONSTANT(types, kStringList); diff --git a/src/node_options.h b/src/node_options.h index 8c71881e6431..32e01760f859 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -168,6 +168,7 @@ enum OptionType { kV8Option, kBoolean, kInteger, + kUInteger, kString, kHostPort, kStringList, @@ -194,6 +195,10 @@ class OptionsParser { const std::string& help_text, bool Options::* field, OptionEnvvarSettings env_setting = kDisallowedInEnvironment); + void AddOption(const std::string& name, + const std::string& help_text, + uint64_t Options::* field, + OptionEnvvarSettings env_setting = kDisallowedInEnvironment); void AddOption(const std::string& name, const std::string& help_text, int64_t Options::* field, From 9b2ffc81c0e4b29c0a8ace8b2a7526029f526a7e Mon Sep 17 00:00:00 2001 From: cjihrig Date: Mon, 3 Dec 2018 12:27:46 -0500 Subject: [PATCH 07/10] cli: add --max-http-header-size flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Allow the maximum size of HTTP headers to be overridden from the command line. Backport-PR-URL: https://github.com/nodejs/node/pull/25168 co-authored-by: Matteo Collina PR-URL: https://github.com/nodejs/node/pull/24811 Fixes: https://github.com/nodejs/node/issues/24692 Reviewed-By: Anna Henningsen Reviewed-By: Myles Borins Reviewed-By: Michael Dawson Reviewed-By: Сковорода Никита Андреевич Reviewed-By: James M Snell Reviewed-By: Jeremiah Senkpiel --- doc/api/cli.md | 8 ++ doc/node.1 | 3 + src/node_http_parser.cc | 7 ++ src/node_options.cc | 4 + src/node_options.h | 1 + test/sequential/test-http-max-http-headers.js | 72 +++++++----- .../test-set-http-max-http-headers.js | 104 ++++++++++++++++++ 7 files changed, 174 insertions(+), 25 deletions(-) create mode 100644 test/sequential/test-set-http-max-http-headers.js diff --git a/doc/api/cli.md b/doc/api/cli.md index 29f3360dda1d..d1fbf9432eb3 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -181,6 +181,13 @@ added: v9.0.0 Specify the `file` of the custom [experimental ECMAScript Module][] loader. +### `--max-http-header-size=size` + + +Specify the maximum size, in bytes, of HTTP headers. Defaults to 8KB. + ### `--napi-modules` Too much HTTP header data was received. In order to protect against malicious or -malconfigured clients, if more than 80KB of HTTP header data is received then +malconfigured clients, if more than 8KB of HTTP header data is received then HTTP parsing will abort without a request or response object being created, and an `Error` with this code will be emitted. From b6d3afb25734b81b20859d2363cfa91e6ff0a725 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Wed, 5 Dec 2018 19:59:12 -0500 Subject: [PATCH 09/10] http: add maxHeaderSize property This commit exposes the value of --max-http-header-size as a property of the http module. PR-URL: https://github.com/nodejs/node/pull/24860 Reviewed-By: Richard Lau Reviewed-By: Matteo Collina Reviewed-By: Michael Dawson Reviewed-By: Shelley Vohr Reviewed-By: James M Snell --- doc/api/http.md | 11 +++++++++++ lib/http.js | 14 ++++++++++++++ test/parallel/test-http-max-header-size.js | 11 +++++++++++ 3 files changed, 36 insertions(+) create mode 100644 test/parallel/test-http-max-header-size.js diff --git a/doc/api/http.md b/doc/api/http.md index 9a9e5c939ddf..18c92aa38585 100644 --- a/doc/api/http.md +++ b/doc/api/http.md @@ -1894,6 +1894,16 @@ added: v0.5.9 Global instance of `Agent` which is used as the default for all HTTP client requests. +## http.maxHeaderSize + + +* {number} + +Read-only property specifying the maximum allowed size of HTTP headers in bytes. +Defaults to 8KB. Configurable using the [`--max-http-header-size`][] CLI option. + ## http.request(options[, callback]) ## http.request(url[, options][, callback]) Specify the maximum size, in bytes, of HTTP headers. Defaults to 8KB. diff --git a/doc/api/errors.md b/doc/api/errors.md index 4a39f8c4b7be..f51ba038d9cb 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1858,7 +1858,7 @@ Creation of a [`zlib`][] object failed due to incorrect configuration. ### HPE_HEADER_OVERFLOW diff --git a/doc/api/http.md b/doc/api/http.md index 18c92aa38585..af4c2c65f53b 100644 --- a/doc/api/http.md +++ b/doc/api/http.md @@ -1896,7 +1896,7 @@ requests. ## http.maxHeaderSize * {number} diff --git a/doc/changelogs/CHANGELOG_V10.md b/doc/changelogs/CHANGELOG_V10.md index 965ee40ac8bb..9a3c2920a356 100644 --- a/doc/changelogs/CHANGELOG_V10.md +++ b/doc/changelogs/CHANGELOG_V10.md @@ -10,6 +10,7 @@ +10.15.0
10.14.2
10.14.1
10.14.0
@@ -47,6 +48,32 @@ * [io.js](CHANGELOG_IOJS.md) * [Archive](CHANGELOG_ARCHIVE.md) + +## 2018-12-26, Version 10.15.0 'Dubnium' (LTS), @MylesBorins + +The 10.14.0 security release introduced some unexpected breakages on the 10.x release line. +This is a special release to fix a regression in the HTTP binary upgrade response body and add +a missing CLI flag to adjust the max header size of the http parser. + +### Notable Changes + +* **cli**: + - add --max-http-header-size flag (cjihrig) [#24811](https://github.com/nodejs/node/pull/24811) +* **http**: + - add maxHeaderSize property (cjihrig) [#24860](https://github.com/nodejs/node/pull/24860) + +### Commits + +* [[`9b2ffc81c0`](https://github.com/nodejs/node/commit/9b2ffc81c0)] - **(SEMVER-MINOR)** **cli**: add --max-http-header-size flag (cjihrig) [#24811](https://github.com/nodejs/node/pull/24811) +* [[`6183c7107d`](https://github.com/nodejs/node/commit/6183c7107d)] - **(SEMVER-MINOR)** **deps**: cherry-pick http\_parser\_set\_max\_header\_size (cjihrig) [#24811](https://github.com/nodejs/node/pull/24811) +* [[`e669733595`](https://github.com/nodejs/node/commit/e669733595)] - **doc**: describe current HTTP header size limit (Sam Roberts) [#24700](https://github.com/nodejs/node/pull/24700) +* [[`b6d3afb257`](https://github.com/nodejs/node/commit/b6d3afb257)] - **(SEMVER-MINOR)** **http**: add maxHeaderSize property (cjihrig) [#24860](https://github.com/nodejs/node/pull/24860) +* [[`1aea1e3634`](https://github.com/nodejs/node/commit/1aea1e3634)] - **http**: fix regression of binary upgrade response body (Matteo Collina) [#25039](https://github.com/nodejs/node/pull/25039) +* [[`a57aed144a`](https://github.com/nodejs/node/commit/a57aed144a)] - **(SEMVER-MINOR)** **src**: add kUInteger parsing (Matteo Collina) [#24811](https://github.com/nodejs/node/pull/24811) +* [[`527407c49f`](https://github.com/nodejs/node/commit/527407c49f)] - **src**: cache the result of GetOptions() in JS land (Joyee Cheung) [#24091](https://github.com/nodejs/node/pull/24091) +* [[`728bc631e5`](https://github.com/nodejs/node/commit/728bc631e5)] - **test**: fix expectation in test-bootstrap-modules (Ali Ijaz Sheikh) [#25112](https://github.com/nodejs/node/pull/25112) +* [[`3e14212f0e`](https://github.com/nodejs/node/commit/3e14212f0e)] - **test**: remove magic numbers in test-gc-http-client-onerror (Rich Trott) [#24943](https://github.com/nodejs/node/pull/24943) + ## 2018-12-11, Version 10.14.2 'Dubnium' (LTS), @MylesBorins prepared by @codebytere diff --git a/src/node_version.h b/src/node_version.h index c9d6f49b40ba..b463eb6763a1 100644 --- a/src/node_version.h +++ b/src/node_version.h @@ -23,13 +23,13 @@ #define SRC_NODE_VERSION_H_ #define NODE_MAJOR_VERSION 10 -#define NODE_MINOR_VERSION 14 -#define NODE_PATCH_VERSION 3 +#define NODE_MINOR_VERSION 15 +#define NODE_PATCH_VERSION 0 #define NODE_VERSION_IS_LTS 1 #define NODE_VERSION_LTS_CODENAME "Dubnium" -#define NODE_VERSION_IS_RELEASE 0 +#define NODE_VERSION_IS_RELEASE 1 #ifndef NODE_STRINGIFY #define NODE_STRINGIFY(n) NODE_STRINGIFY_HELPER(n)