From 0c334f87bf9235032d3b0ad8de66c89a5bed1cd8 Mon Sep 17 00:00:00 2001
From: cjihrig <cjihrig@gmail.com>
Date: Thu, 29 Nov 2018 17:29:53 -0500
Subject: [PATCH 1/3] 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.

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 <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
---
 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 6522618671d09c..46764bced09478 100644
--- a/deps/http_parser/http_parser.c
+++ b/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;
+}
diff --git a/deps/http_parser/http_parser.h b/deps/http_parser/http_parser.h
index 1fbf30e2b4740b..ea7bafef2c3178 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 25c5d39623602f8d407124340eb9fddfc7a7386f Mon Sep 17 00:00:00 2001
From: Matteo Collina <hello@matteocollina.com>
Date: Tue, 18 Dec 2018 12:12:21 +0100
Subject: [PATCH 2/3] 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.

PR-URL: https://github.com/nodejs/node/pull/24811
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
---
 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 d9ab3c1ad84e90..c453562c5c40bf 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 277121036e519d..69b8ee0e787747 100644
--- a/src/node_options-inl.h
+++ b/src/node_options-inl.h
@@ -35,6 +35,19 @@ void OptionsParser<Options>::AddOption(const std::string& name,
                               help_text});
 }
 
+template <typename Options>
+void OptionsParser<Options>::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<SimpleOptionField<uint64_t>>(field),
+                 env_setting,
+                 help_text});
+}
+
 template <typename Options>
 void OptionsParser<Options>::AddOption(const std::string& name,
                                        const std::string& help_text,
@@ -397,6 +410,9 @@ void OptionsParser<Options>::Parse(
       case kInteger:
         *Lookup<int64_t>(info.field, options) = std::atoll(value.c_str());
         break;
+      case kUInteger:
+        *Lookup<uint64_t>(info.field, options) = std::stoull(value.c_str());
+        break;
       case kString:
         *Lookup<std::string>(info.field, options) = value;
         break;
diff --git a/src/node_options.cc b/src/node_options.cc
index ec819538d62977..1c63fa286f0f89 100644
--- a/src/node_options.cc
+++ b/src/node_options.cc
@@ -406,6 +406,9 @@ void GetOptions(const FunctionCallbackInfo<Value>& args) {
       case kInteger:
         value = Number::New(isolate, *parser.Lookup<int64_t>(field, opts));
         break;
+      case kUInteger:
+        value = Number::New(isolate, *parser.Lookup<uint64_t>(field, opts));
+        break;
       case kString:
         if (!ToV8Value(context, *parser.Lookup<std::string>(field, opts))
                  .ToLocal(&value)) {
@@ -492,6 +495,7 @@ void Initialize(Local<Object> 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 8c71881e64311d..32e01760f859bf 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 0f632482e10bf321b64cc9a8e9f7106110cdbb8f Mon Sep 17 00:00:00 2001
From: cjihrig <cjihrig@gmail.com>
Date: Mon, 3 Dec 2018 12:27:46 -0500
Subject: [PATCH 3/3] 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.

co-authored-by: Matteo Collina <hello@matteocollina.com>
PR-URL: https://github.com/nodejs/node/pull/24811
Fixes: https://github.com/nodejs/node/issues/24692
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
---
 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 |  71 ++++++++----
 .../test-set-http-max-http-headers.js         | 104 ++++++++++++++++++
 7 files changed, 174 insertions(+), 24 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 29f3360dda1d78..d1fbf9432eb376 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`
+<!-- YAML
+added: REPLACEME
+-->
+
+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`
diff --git a/doc/node.1 b/doc/node.1
index b8144f4ebb5f6c..3d2344822df261 100644
--- a/doc/node.1
+++ b/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.
diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc
index 58bae3323c0484..4411b6b32f8e2b 100644
--- a/src/node_http_parser.cc
+++ b/src/node_http_parser.cc
@@ -738,6 +738,10 @@ const struct http_parser_settings Parser::settings = {
   nullptr   // on_chunk_complete
 };
 
+void InitMaxHttpHeaderSizeOnce() {
+  const uint32_t max_http_header_size = per_process_opts->max_http_header_size;
+  http_parser_set_max_header_size(max_http_header_size);
+}
 
 void Initialize(Local<Object> target,
                 Local<Value> unused,
@@ -784,6 +788,9 @@ void Initialize(Local<Object> target,
 
   target->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "HTTPParser"),
               t->GetFunction(env->context()).ToLocalChecked());
+
+  static uv_once_t init_once = UV_ONCE_INIT;
+  uv_once(&init_once, InitMaxHttpHeaderSizeOnce);
 }
 
 }  // anonymous namespace
diff --git a/src/node_options.cc b/src/node_options.cc
index 1c63fa286f0f89..79dff7adbf9d63 100644
--- a/src/node_options.cc
+++ b/src/node_options.cc
@@ -236,6 +236,10 @@ PerProcessOptionsParser::PerProcessOptionsParser() {
             kAllowedInEnvironment);
   AddAlias("--trace-events-enabled", {
     "--trace-event-categories", "v8,node,node.async_hooks" });
+  AddOption("--max-http-header-size",
+            "set the maximum size of HTTP headers (default: 8KB)",
+            &PerProcessOptions::max_http_header_size,
+            kAllowedInEnvironment);
   AddOption("--v8-pool-size",
             "set V8's thread pool size",
             &PerProcessOptions::v8_thread_pool_size,
diff --git a/src/node_options.h b/src/node_options.h
index 32e01760f859bf..4edec114d6da1f 100644
--- a/src/node_options.h
+++ b/src/node_options.h
@@ -116,6 +116,7 @@ class PerProcessOptions : public Options {
   std::string title;
   std::string trace_event_categories;
   std::string trace_event_file_pattern = "node_trace.${rotation}.log";
+  uint64_t max_http_header_size = 8 * 1024;
   int64_t v8_thread_pool_size = 4;
   bool zero_fill_all_buffers = false;
 
diff --git a/test/sequential/test-http-max-http-headers.js b/test/sequential/test-http-max-http-headers.js
index ae76142a4fd077..c7bd0ad3e9be27 100644
--- a/test/sequential/test-http-max-http-headers.js
+++ b/test/sequential/test-http-max-http-headers.js
@@ -1,10 +1,16 @@
 'use strict';
+// Flags: --expose_internals
 
 const assert = require('assert');
 const common = require('../common');
 const http = require('http');
 const net = require('net');
-const MAX = 8 * 1024; // 8KB
+const MAX = +(process.argv[2] || 8 * 1024); // Command line option, or 8KB.
+
+const { getOptionValue } = require('internal/options');
+
+console.log('pid is', process.pid);
+console.log('max header size is', getOptionValue('--max-http-header-size'));
 
 // Verify that we cannot receive more than 8KB of headers.
 
@@ -28,19 +34,15 @@ function fillHeaders(headers, currentSize, valid = false) {
   headers += 'a'.repeat(MAX - headers.length - 3);
   // Generate valid headers
   if (valid) {
-    // TODO(mcollina): understand why -9 is needed instead of -1
-    headers = headers.slice(0, -9);
+    // TODO(mcollina): understand why -32 is needed instead of -1
+    headers = headers.slice(0, -32);
   }
   return headers + '\r\n\r\n';
 }
 
-const timeout = common.platformTimeout(10);
-
 function writeHeaders(socket, headers) {
   const array = [];
-
-  // this is off from 1024 so that \r\n does not get split
-  const chunkSize = 1000;
+  const chunkSize = 100;
   let last = 0;
 
   for (let i = 0; i < headers.length / chunkSize; i++) {
@@ -55,19 +57,25 @@ function writeHeaders(socket, headers) {
   next();
 
   function next() {
-    if (socket.write(array.shift())) {
-      if (array.length === 0) {
-        socket.end();
-      } else {
-        setTimeout(next, timeout);
-      }
+    if (socket.destroyed) {
+      console.log('socket was destroyed early, data left to write:',
+                  array.join('').length);
+      return;
+    }
+
+    const chunk = array.shift();
+
+    if (chunk) {
+      console.log('writing chunk of size', chunk.length);
+      socket.write(chunk, next);
     } else {
-      socket.once('drain', next);
+      socket.end();
     }
   }
 }
 
 function test1() {
+  console.log('test1');
   let headers =
     'HTTP/1.1 200 OK\r\n' +
     'Content-Length: 0\r\n' +
@@ -82,6 +90,9 @@ function test1() {
       writeHeaders(sock, headers);
       sock.resume();
     });
+
+    // The socket might error but that's ok
+    sock.on('error', () => {});
   });
 
   server.listen(0, common.mustCall(() => {
@@ -90,17 +101,17 @@ function test1() {
 
     client.on('error', common.mustCall((err) => {
       assert.strictEqual(err.code, 'HPE_HEADER_OVERFLOW');
-      server.close();
-      setImmediate(test2);
+      server.close(test2);
     }));
   }));
 }
 
 const test2 = common.mustCall(() => {
+  console.log('test2');
   let headers =
     'GET / HTTP/1.1\r\n' +
     'Host: localhost\r\n' +
-    'Agent: node\r\n' +
+    'Agent: nod2\r\n' +
     'X-CRASH: ';
 
   // /, Host, localhost, Agent, node, X-CRASH, a...
@@ -109,7 +120,7 @@ const test2 = common.mustCall(() => {
 
   const server = http.createServer(common.mustNotCall());
 
-  server.on('clientError', common.mustCall((err) => {
+  server.once('clientError', common.mustCall((err) => {
     assert.strictEqual(err.code, 'HPE_HEADER_OVERFLOW');
   }));
 
@@ -121,34 +132,46 @@ const test2 = common.mustCall(() => {
     });
 
     finished(client, common.mustCall((err) => {
-      server.close();
-      setImmediate(test3);
+      server.close(test3);
     }));
   }));
 });
 
 const test3 = common.mustCall(() => {
+  console.log('test3');
   let headers =
     'GET / HTTP/1.1\r\n' +
     'Host: localhost\r\n' +
-    'Agent: node\r\n' +
+    'Agent: nod3\r\n' +
     'X-CRASH: ';
 
   // /, Host, localhost, Agent, node, X-CRASH, a...
   const currentSize = 1 + 4 + 9 + 5 + 4 + 7;
   headers = fillHeaders(headers, currentSize, true);
 
+  console.log('writing', headers.length);
+
   const server = http.createServer(common.mustCall((req, res) => {
-    res.end('hello world');
-    setImmediate(server.close.bind(server));
+    res.end('hello from test3 server');
+    server.close();
   }));
 
+  server.on('clientError', (err) => {
+    console.log(err.code);
+    if (err.code === 'HPE_HEADER_OVERFLOW') {
+      console.log(err.rawPacket.toString('hex'));
+    }
+  });
+  server.on('clientError', common.mustNotCall());
+
   server.listen(0, common.mustCall(() => {
     const client = net.connect(server.address().port);
     client.on('connect', () => {
       writeHeaders(client, headers);
       client.resume();
     });
+
+    client.pipe(process.stdout);
   }));
 });
 
diff --git a/test/sequential/test-set-http-max-http-headers.js b/test/sequential/test-set-http-max-http-headers.js
new file mode 100644
index 00000000000000..7ec13f370784f8
--- /dev/null
+++ b/test/sequential/test-set-http-max-http-headers.js
@@ -0,0 +1,104 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const { spawn } = require('child_process');
+const path = require('path');
+const testName = path.join(__dirname, 'test-http-max-http-headers.js');
+
+const timeout = common.platformTimeout(100);
+
+const tests = [];
+
+function test(fn) {
+  tests.push(fn);
+}
+
+test(function(cb) {
+  console.log('running subtest expecting failure');
+
+  // Validate that the test fails if the max header size is too small.
+  const args = ['--expose-internals',
+                '--max-http-header-size=1024',
+                testName];
+  const cp = spawn(process.execPath, args, { stdio: 'inherit' });
+
+  cp.on('close', common.mustCall((code, signal) => {
+    assert.strictEqual(code, 1);
+    assert.strictEqual(signal, null);
+    cb();
+  }));
+});
+
+test(function(cb) {
+  console.log('running subtest expecting success');
+
+  const env = Object.assign({}, process.env, {
+    NODE_DEBUG: 'http'
+  });
+
+  // Validate that the test fails if the max header size is too small.
+  // Validate that the test now passes if the same limit becomes large enough.
+  const args = ['--expose-internals',
+                '--max-http-header-size=1024',
+                testName,
+                '1024'];
+  const cp = spawn(process.execPath, args, {
+    env,
+    stdio: 'inherit'
+  });
+
+  cp.on('close', common.mustCall((code, signal) => {
+    assert.strictEqual(code, 0);
+    assert.strictEqual(signal, null);
+    cb();
+  }));
+});
+
+// Next, repeat the same checks using NODE_OPTIONS if it is supported.
+if (process.config.variables.node_without_node_options) {
+  const env = Object.assign({}, process.env, {
+    NODE_OPTIONS: '--max-http-header-size=1024'
+  });
+
+  test(function(cb) {
+    console.log('running subtest expecting failure');
+
+    // Validate that the test fails if the max header size is too small.
+    const args = ['--expose-internals', testName];
+    const cp = spawn(process.execPath, args, { env, stdio: 'inherit' });
+
+    cp.on('close', common.mustCall((code, signal) => {
+      assert.strictEqual(code, 1);
+      assert.strictEqual(signal, null);
+      cb();
+    }));
+  });
+
+  test(function(cb) {
+    // Validate that the test now passes if the same limit
+    // becomes large enough.
+    const args = ['--expose-internals', testName, '1024'];
+    const cp = spawn(process.execPath, args, { env, stdio: 'inherit' });
+
+    cp.on('close', common.mustCall((code, signal) => {
+      assert.strictEqual(code, 0);
+      assert.strictEqual(signal, null);
+      cb();
+    }));
+  });
+}
+
+function runTest() {
+  const fn = tests.shift();
+
+  if (!fn) {
+    return;
+  }
+
+  fn(() => {
+    setTimeout(runTest, timeout);
+  });
+}
+
+runTest();