v6.16.0 proposal

CHANGELOG.md

@@ -26,7 +26,8 @@ release.
 </tr>
 <tr>
   <td valign="top">
-<b><a href="doc/changelogs/CHANGELOG_V6.md#6.15.1">6.15.1</a></b><br/>
+<b><a href="doc/changelogs/CHANGELOG_V6.md#6.16.0">6.16.0</a></b><br/>
+<a href="doc/changelogs/CHANGELOG_V6.md#6.15.1">6.15.1</a><br/>
 <a href="doc/changelogs/CHANGELOG_V6.md#6.15.0">6.15.0</a><br/>
 <a href="doc/changelogs/CHANGELOG_V6.md#6.14.4">6.14.4</a><br/>
 <a href="doc/changelogs/CHANGELOG_V6.md#6.14.3">6.14.3</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

@@ -293,6 +293,13 @@ Indicate the end of node options. Pass the rest of the arguments to the script.
 If no script filename or eval/print script is supplied prior to this, then
 the next argument will be used as a script filename.
 
+### `--max-http-header-size=size`
+<!-- YAML
+added: v6.16.0
+-->
+
+Specify the maximum size, in bytes, of HTTP headers. Defaults to 8KB.
+
 ## Environment Variables
 
 ### `NODE_DEBUG=module[,…]`
@@ -353,6 +360,7 @@ Node options that are allowed are:
 - `--debug-brk`
 - `--debug-port`
 - `--debug`
+- `--max-http-header-size`
 - `--no-deprecation`
 - `--no-warnings`
 - `--openssl-config`

doc/api/http.md

@@ -1581,6 +1581,16 @@ added: v0.5.9
 Global instance of `Agent` which is used as the default for all HTTP client
 requests.
 
+## http.maxHeaderSize
+<!-- YAML
+added: v6.16.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])
 <!-- YAML
 added: v0.3.6
@@ -1698,6 +1708,7 @@ There are a few special headers that should be noted.
 * Sending an Authorization header will override using the `auth` option
   to compute basic authentication.
 
+[`--max-http-header-size`]: cli.html#cli_max_http_header_size_size
 [`'checkContinue'`]: #http_event_checkcontinue
 [`'listening'`]: net.html#net_event_listening
 [`'request'`]: #http_event_request

doc/changelogs/CHANGELOG_V6.md

@@ -7,6 +7,7 @@
 </tr>
 <tr>
 <td valign="top">
+<a href="#6.16.0">6.16.0</a><br/>
 <a href="#6.15.1">6.15.1</a><br/>
 <a href="#6.15.0">6.15.0</a><br/>
 <a href="#6.14.4">6.14.4</a><br/>
@@ -67,6 +68,27 @@
 [Node.js Long Term Support Plan](https://github.com/nodejs/LTS) and
 will be supported actively until April 2018 and maintained until April 2019.
 
+<a id="6.16.0"></a>
+## 2018-12-26, Version 6.16.0 'Boron' (LTS), @MylesBorins
+
+The 6.15.0 security release introduced some unexpected breakages on the 6.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
+
+* [[`f233b160c9`](https://github.com/nodejs/node/commit/f233b160c9)] - **(SEMVER-MINOR)** **cli**: add --max-http-header-size flag (cjihrig) [#24811](https://github.com/nodejs/node/pull/24811)
+* [[`59f83d6896`](https://github.com/nodejs/node/commit/59f83d6896)] - **(SEMVER-MINOR)** **deps**: cherry-pick http\_parser\_set\_max\_header\_size (cjihrig) [#24811](https://github.com/nodejs/node/pull/24811)
+* [[`c0c4de71f0`](https://github.com/nodejs/node/commit/c0c4de71f0)] - **(SEMVER-MINOR)** **http**: add maxHeaderSize property (cjihrig) [#24860](https://github.com/nodejs/node/pull/24860)
+* [[`8a3e0c0697`](https://github.com/nodejs/node/commit/8a3e0c0697)] - **http**: fix regression of binary upgrade response body (Matteo Collina) [#25036](https://github.com/nodejs/node/pull/25036)
+
 <a id="6.15.1"></a>
 ## 2018-12-03, Version 6.15.1 'Boron' (LTS), @rvagg
 

doc/node.1

@@ -92,6 +92,10 @@ Open the REPL even if stdin does not appear to be a terminal.
 Preload the specified module at startup. Follows `require()`'s module resolution
 rules. \fImodule\fR may be either a path to a file, or a node module name.
 
+.TP
+.BR \-\-max\-http\-header-size \fI=size\fR
+Specify the maximum size of HTTP headers in bytes. Defaults to 8KB.
+
 .TP
 .BR \-\-no\-deprecation
 Silence deprecation warnings.

lib/http.js

@@ -19,6 +19,8 @@ const server = require('_http_server');
 exports.ServerResponse = server.ServerResponse;
 exports.STATUS_CODES = server.STATUS_CODES;
 
+let maxHeaderSize;
+
 
 const agent = require('_http_agent');
 const Agent = exports.Agent = agent.Agent;
@@ -92,8 +94,21 @@ Client.prototype.request = function(method, path, headers) {
   return c;
 };
 
+
 exports.Client = internalUtil.deprecate(Client, 'http.Client is deprecated.');
 
 exports.createClient = internalUtil.deprecate(function(port, host) {
   return new Client(port, host);
 }, 'http.createClient is deprecated. Use http.request instead.');
+
+Object.defineProperty(module.exports, 'maxHeaderSize', {
+  configurable: true,
+  enumerable: true,
+  get() {
+    if (maxHeaderSize === undefined) {
+      maxHeaderSize = process.binding('config').maxHTTPHeaderSize;
+    }
+
+    return maxHeaderSize;
+  }
+});

src/node.cc

@@ -170,6 +170,8 @@ unsigned int reverted = 0;
 static std::string icu_data_dir;  // NOLINT(runtime/string)
 #endif
 
+uint64_t max_http_header_size = 8 * 1024;
+
 // used by C++ modules as well
 bool no_deprecation = false;
 
@@ -3731,6 +3733,8 @@ static void PrintHelp() {
          "  --trace-deprecation   show stack traces on deprecations\n"
          "  --throw-deprecation   throw an exception anytime a deprecated "
          "function is used\n"
+         "  --max-http-header-size     Specify the maximum size of HTTP\n"
+         "                             headers in bytes. Defaults to 8KB.\n"
          "  --no-warnings         silence all process warnings\n"
          "  --napi-modules        load N-API modules (no-op - option kept for "
          "                        compatibility)\n"
@@ -3852,6 +3856,7 @@ static void CheckIfAllowedInEnv(const char* exe, bool is_env,
     "--pending-deprecation",
     "--no-warnings",
     "--napi-modules",
+    "--max-http-header-size",
     "--trace-warnings",
     "--redirect-warnings",
     "--trace-sync-io",
@@ -4010,6 +4015,8 @@ static void ParseArgs(int* argc,
       new_v8_argc += 1;
     } else if (strncmp(arg, "--v8-pool-size=", 15) == 0) {
       v8_thread_pool_size = atoi(arg + 15);
+    } else if (strncmp(arg, "--max-http-header-size=", 23) == 0) {
+      max_http_header_size = atoi(arg + 23);
 #if HAVE_OPENSSL
     } else if (strncmp(arg, "--tls-cipher-list=", 18) == 0) {
       default_cipher_list = arg + 18;

src/node_config.cc

@@ -7,6 +7,7 @@ namespace node {
 
 using v8::Context;
 using v8::Local;
+using v8::Number;
 using v8::Object;
 using v8::ReadOnly;
 using v8::String;
@@ -24,6 +25,13 @@ using v8::Value;
                               True(env->isolate()), ReadOnly).FromJust();     \
   } while (0)
 
+#define READONLY_PROPERTY(obj, name, value)                                   \
+  do {                                                                        \
+    obj->DefineOwnProperty(env->context(),                                    \
+                           FIXED_ONE_BYTE_STRING(env->isolate(), name),       \
+                           value, ReadOnly).FromJust();                       \
+  } while (0)
+
 void InitConfig(Local<Object> target,
                 Local<Value> unused,
                 Local<Context> context) {
@@ -46,6 +54,11 @@ void InitConfig(Local<Object> target,
   if (config_expose_internals)
     READONLY_BOOLEAN_PROPERTY("exposeInternals");
 
+
+  READONLY_PROPERTY(target,
+                    "maxHTTPHeaderSize",
+                    Number::New(env->isolate(), max_http_header_size));
+
   if (!config_warning_file.empty()) {
     Local<String> name = OneByteString(env->isolate(), "warningFile");
     Local<String> value = String::NewFromUtf8(env->isolate(),

src/node_http_parser.cc

@@ -610,8 +610,6 @@ class Parser : public AsyncWrap {
     size_t nparsed =
       http_parser_execute(&parser_, &settings, data, len);
 
-    enum http_errno err = HTTP_PARSER_ERRNO(&parser_);
-
     Save();
 
     // Unassign the 'buffer_' variable
@@ -626,7 +624,9 @@ class Parser : public AsyncWrap {
     Local<Integer> 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<Value> e = Exception::Error(env()->parse_error_string());
       Local<Object> obj = e->ToObject(env()->isolate());
       obj->Set(env()->bytes_parsed_string(), nparsed_obj);
@@ -731,6 +731,9 @@ const struct http_parser_settings Parser::settings = {
   nullptr   // on_chunk_complete
 };
 
+void InitMaxHttpHeaderSizeOnce() {
+  http_parser_set_max_header_size(max_http_header_size);
+}
 
 void InitHttpParser(Local<Object> target,
                     Local<Value> unused,
@@ -775,6 +778,8 @@ void InitHttpParser(Local<Object> target,
 
   target->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "HTTPParser"),
               t->GetFunction());
+  static uv_once_t init_once = UV_ONCE_INIT;
+  uv_once(&init_once, InitMaxHttpHeaderSizeOnce);
 }
 
 }  // namespace node

src/node_internals.h

@@ -56,6 +56,9 @@ extern bool config_expose_internals;
 // it to stderr.
 extern std::string config_warning_file;
 
+// Set in node.cc by ParseArgs when --max-http-header-size is used
+extern uint64_t max_http_header_size;
+
 // Forward declaration
 class Environment;
 

src/node_version.h

@@ -2,13 +2,13 @@
 #define SRC_NODE_VERSION_H_
 
 #define NODE_MAJOR_VERSION 6
-#define NODE_MINOR_VERSION 15
-#define NODE_PATCH_VERSION 2
+#define NODE_MINOR_VERSION 16
+#define NODE_PATCH_VERSION 0
 
 #define NODE_VERSION_IS_LTS 1
 #define NODE_VERSION_LTS_CODENAME "Boron"
 
-#define NODE_VERSION_IS_RELEASE 0
+#define NODE_VERSION_IS_RELEASE 1
 
 #ifndef NODE_STRINGIFY
 #define NODE_STRINGIFY(n) NODE_STRINGIFY_HELPER(n)

test/parallel/test-http-max-header-size.js

@@ -0,0 +1,11 @@
+'use strict';
+
+require('../common');
+const assert = require('assert');
+const { spawnSync } = require('child_process');
+const http = require('http');
+
+assert.strictEqual(http.maxHeaderSize, 8 * 1024);
+const child = spawnSync(process.execPath, ['--max-http-header-size=10', '-p',
+                                           'http.maxHeaderSize']);
+assert.strictEqual(+child.stdout.toString().trim(), 10);