v10.15.0 proposal

CHANGELOG.md

@@ -26,7 +26,8 @@ release.
 </tr>
 <tr>
     <td valign="top">
-<b><a href="doc/changelogs/CHANGELOG_V10.md#10.14.2">10.14.2</a></b><br/>
+<b><a href="doc/changelogs/CHANGELOG_V10.md#10.15.0">10.15.0</a></b><br/>
+<a href="doc/changelogs/CHANGELOG_V10.md#10.14.2">10.14.2</a><br/>
 <a href="doc/changelogs/CHANGELOG_V10.md#10.14.1">10.14.1</a><br/>
 <a href="doc/changelogs/CHANGELOG_V10.md#10.14.0">10.14.0</a><br/>
 <a href="doc/changelogs/CHANGELOG_V10.md#10.13.0">10.13.0</a><br/>

deps/http_parser/http_parser.c

@@ -25,6 +25,8 @@
 #include <string.h>
 #include <limits.h>
 
+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;
+}

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

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`
+<!-- YAML
+added: v10.15.0
+-->
+
+Specify the maximum size, in bytes, of HTTP headers. Defaults to 8KB.
+
 ### `--napi-modules`
 <!-- YAML
 added: v7.10.0
@@ -584,6 +591,7 @@ Node.js options that are allowed are:
 - `--inspect-brk`
 - `--inspect-port`
 - `--loader`
+- `--max-http-header-size`
 - `--napi-modules`
 - `--no-deprecation`
 - `--no-force-async-hooks-checks`

doc/api/errors.md

@@ -1856,9 +1856,15 @@ Creation of a [`zlib`][] object failed due to incorrect configuration.
 
 <a id="HPE_HEADER_OVERFLOW"></a>
 ### HPE_HEADER_OVERFLOW
+<!-- YAML
+changes:
+  - version: v10.15.0
+    pr-url: https://github.com/nodejs/node/commit/186035243fad247e3955f
+    description: Max header size in `http_parser` was set to 8KB.
+-->
 
 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.
 

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
+<!-- YAML
+added: v10.15.0
+-->
+
+* {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])
 <!-- YAML
@@ -2079,6 +2089,7 @@ will be emitted in the following order:
 Note that setting the `timeout` option or using the `setTimeout()` function will
 not abort the request or do anything besides add a `'timeout'` event.
 
+[`--max-http-header-size`]: cli.html#cli_max_http_header_size_size
 [`'checkContinue'`]: #http_event_checkcontinue
 [`'request'`]: #http_event_request
 [`'response'`]: #http_event_response

doc/changelogs/CHANGELOG_V10.md

@@ -10,6 +10,7 @@
 </tr>
 <tr>
 <td valign="top">
+<a href="#10.15.0">10.15.0</a><br/>
 <a href="#10.14.2">10.14.2</a><br/>
 <a href="#10.14.1">10.14.1</a><br/>
 <a href="#10.14.0">10.14.0</a><br/>
@@ -47,6 +48,32 @@
   * [io.js](CHANGELOG_IOJS.md)
   * [Archive](CHANGELOG_ARCHIVE.md)
 
+<a id="10.15.0"></a>
+## 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)
+
 <a id="10.14.2"></a>
 ## 2018-12-11, Version 10.14.2 'Dubnium' (LTS), @MylesBorins prepared by @codebytere
 

doc/node.1

@@ -133,6 +133,9 @@ Specify the
 as a custom loader, to load
 .Fl -experimental-modules .
 .
+.It Fl -max-http-header-size Ns = Ns Ar size
+Specify the maximum size of HTTP headers in bytes. Defaults to 8KB.
+.
 .It Fl -napi-modules
 This option is a no-op.
 It is kept for compatibility.

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

lib/http.js

@@ -32,6 +32,7 @@ const {
   Server,
   ServerResponse
 } = require('_http_server');
+let maxHeaderSize;
 
 function createServer(opts, requestListener) {
   return new Server(opts, requestListener);
@@ -62,3 +63,16 @@ module.exports = {
   get,
   request
 };
+
+Object.defineProperty(module.exports, 'maxHeaderSize', {
+  configurable: true,
+  enumerable: true,
+  get() {
+    if (maxHeaderSize === undefined) {
+      const { getOptionValue } = require('internal/options');
+      maxHeaderSize = getOptionValue('--max-http-header-size');
+    }
+
+    return maxHeaderSize;
+  }
+});

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() {

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);
     };
   }
 

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) {

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();
 }

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,

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,

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
+};

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))
@@ -58,6 +59,7 @@ function getArgDescription(type) {
     case 'kHostPort':
       return '[host:]port';
     case 'kInteger':
+    case 'kUInteger':
     case 'kString':
     case 'kStringList':
       return '...';
@@ -132,7 +134,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);

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);

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');

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;
 }

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',