From 83190a808a582280caf21f0c6b31027dc3fb064a Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Thu, 21 Nov 2019 00:00:43 +0100
Subject: [PATCH] http: make maximum header size configurable per-stream or
 per-server

Make `maxHeaderSize` a.k.a. `--max-header-size` configurable now that
the legacy parser is gone (which only supported a single global value).

Refs: https://github.com/nodejs/node/pull/30567
---
 doc/api/http.md                               | 15 ++++
 lib/_http_client.js                           | 10 ++-
 lib/_http_server.js                           | 10 ++-
 src/node_http_parser.cc                       | 18 +++-
 src/node_options.cc                           |  8 +-
 src/node_options.h                            |  2 +-
 .../test-http-max-header-size-per-stream.js   | 82 +++++++++++++++++++
 7 files changed, 135 insertions(+), 10 deletions(-)
 create mode 100644 test/parallel/test-http-max-header-size-per-stream.js

diff --git a/doc/api/http.md b/doc/api/http.md
index c3ba806def480c..dacb443379b971 100644
--- a/doc/api/http.md
+++ b/doc/api/http.md
@@ -2047,6 +2047,9 @@ Found'`.
 <!-- YAML
 added: v0.1.13
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/?????
+    description: The `maxHeaderSize` option is supported now.
   - version: v9.6.0, v8.12.0
     pr-url: https://github.com/nodejs/node/pull/15752
     description: The `options` argument is supported now.
@@ -2059,6 +2062,9 @@ changes:
   * `ServerResponse` {http.ServerResponse} Specifies the `ServerResponse` class
     to be used. Useful for extending the original `ServerResponse`. **Default:**
     `ServerResponse`.
+  * `maxHeaderSize` {number} Optionally overrides the value of
+    [`--max-http-header-size][] for requests received by this server.
+    **Default:** 8KB.
 * `requestListener` {Function}
 
 * Returns: {http.Server}
@@ -2156,11 +2162,17 @@ added: v11.6.0
 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.
 
+This can be overridden for servers and client requests by passing the
+`maxHeaderSize` option.
+
 ## http.request(options\[, callback\])
 ## http.request(url\[, options\]\[, callback\])
 <!-- YAML
 added: v0.3.6
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/?????
+    description: The `maxHeaderSize` option is supported now.
   - version: v10.9.0
     pr-url: https://github.com/nodejs/node/pull/21616
     description: The `url` parameter can now be passed along with a separate
@@ -2196,6 +2208,9 @@ changes:
     `hostname` will be used if both `host` and `hostname` are specified.
   * `localAddress` {string} Local interface to bind for network connections.
   * `lookup` {Function} Custom lookup function. **Default:** [`dns.lookup()`][].
+  * `maxHeaderSize` {number} Optionally overrides the value of
+    [`--max-http-header-size][] for requests received by this server.
+    **Default:** 8KB.
   * `method` {string} A string specifying the HTTP request method. **Default:**
     `'GET'`.
   * `path` {string} Request path. Should include query string if any.
diff --git a/lib/_http_client.js b/lib/_http_client.js
index 30a6366f355708..73cf34129a20c3 100644
--- a/lib/_http_client.js
+++ b/lib/_http_client.js
@@ -172,6 +172,13 @@ function ClientRequest(input, options, cb) {
     method = this.method = 'GET';
   }
 
+  const maxHeaderSize = options.maxHeaderSize;
+  if (maxHeaderSize !== undefined && (typeof maxHeaderSize !== 'number' ||
+      maxHeaderSize < 0)) {
+    throw new ERR_INVALID_ARG_TYPE('maxHeaderSize', 'number', maxHeaderSize);
+  }
+  this.maxHeaderSize = maxHeaderSize;
+
   this.path = options.path || '/';
   if (cb) {
     this.once('response', cb);
@@ -662,7 +669,8 @@ function tickOnSocket(req, socket) {
   const parser = parsers.alloc();
   req.socket = socket;
   parser.initialize(HTTPParser.RESPONSE,
-                    new HTTPClientAsyncResource('HTTPINCOMINGMESSAGE', req));
+                    new HTTPClientAsyncResource('HTTPINCOMINGMESSAGE', req),
+                    req.maxHeaderSize || 0);
   parser.socket = socket;
   parser.outgoing = req;
   req.parser = parser;
diff --git a/lib/_http_server.js b/lib/_http_server.js
index bc956a01147ad1..6d9a003d41eeea 100644
--- a/lib/_http_server.js
+++ b/lib/_http_server.js
@@ -319,6 +319,13 @@ function Server(options, requestListener) {
   this[kIncomingMessage] = options.IncomingMessage || IncomingMessage;
   this[kServerResponse] = options.ServerResponse || ServerResponse;
 
+  const maxHeaderSize = options.maxHeaderSize;
+  if (maxHeaderSize !== undefined && (typeof maxHeaderSize !== 'number' ||
+      maxHeaderSize < 0)) {
+    throw new ERR_INVALID_ARG_TYPE('maxHeaderSize', 'number', maxHeaderSize);
+  }
+  this.maxHeaderSize = maxHeaderSize;
+
   net.Server.call(this, { allowHalfOpen: true });
 
   if (requestListener) {
@@ -377,7 +384,8 @@ function connectionListenerInternal(server, socket) {
   // https://github.com/nodejs/node/pull/21313
   parser.initialize(
     HTTPParser.REQUEST,
-    new HTTPServerAsyncResource('HTTPINCOMINGMESSAGE', socket)
+    new HTTPServerAsyncResource('HTTPINCOMINGMESSAGE', socket),
+    server.maxHeaderSize || 0
   );
   parser.socket = socket;
 
diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc
index c6136702c7cb43..0328dc7c0f684a 100644
--- a/src/node_http_parser.cc
+++ b/src/node_http_parser.cc
@@ -62,6 +62,7 @@ using v8::Int32;
 using v8::Integer;
 using v8::Local;
 using v8::MaybeLocal;
+using v8::Number;
 using v8::Object;
 using v8::String;
 using v8::Uint32;
@@ -486,8 +487,17 @@ class Parser : public AsyncWrap, public StreamListener {
   static void Initialize(const FunctionCallbackInfo<Value>& args) {
     Environment* env = Environment::GetCurrent(args);
 
+    uint64_t max_http_header_size = 0;
+
     CHECK(args[0]->IsInt32());
     CHECK(args[1]->IsObject());
+    if (args.Length() > 2) {
+      CHECK(args[2]->IsNumber());
+      max_http_header_size = args[2].As<Number>()->Value();
+    }
+    if (max_http_header_size == 0) {
+      max_http_header_size = env->options()->max_http_header_size;
+    }
 
     llhttp_type_t type =
         static_cast<llhttp_type_t>(args[0].As<Int32>()->Value());
@@ -505,7 +515,7 @@ class Parser : public AsyncWrap, public StreamListener {
 
     parser->set_provider_type(provider);
     parser->AsyncReset(args[1].As<Object>());
-    parser->Init(type);
+    parser->Init(type, max_http_header_size);
   }
 
   template <bool should_pause>
@@ -752,7 +762,7 @@ class Parser : public AsyncWrap, public StreamListener {
   }
 
 
-  void Init(llhttp_type_t type) {
+  void Init(llhttp_type_t type, uint64_t max_http_header_size) {
     llhttp_init(&parser_, type, &settings);
     header_nread_ = 0;
     url_.Reset();
@@ -761,12 +771,13 @@ class Parser : public AsyncWrap, public StreamListener {
     num_values_ = 0;
     have_flushed_ = false;
     got_exception_ = false;
+    max_http_header_size_ = max_http_header_size;
   }
 
 
   int TrackHeader(size_t len) {
     header_nread_ += len;
-    if (header_nread_ >= per_process::cli_options->max_http_header_size) {
+    if (header_nread_ >= max_http_header_size_) {
       llhttp_set_error_reason(&parser_, "HPE_HEADER_OVERFLOW:Header overflow");
       return HPE_USER;
     }
@@ -801,6 +812,7 @@ class Parser : public AsyncWrap, public StreamListener {
   unsigned int execute_depth_ = 0;
   bool pending_pause_ = false;
   uint64_t header_nread_ = 0;
+  uint64_t max_http_header_size_;
 
   // These are helper functions for filling `http_parser_settings`, which turn
   // a member function of Parser into a C-style HTTP parser callback.
diff --git a/src/node_options.cc b/src/node_options.cc
index 0bc6730156ce12..10833d2dac1877 100644
--- a/src/node_options.cc
+++ b/src/node_options.cc
@@ -435,6 +435,10 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
             "specified sampling interval in bytes for the V8 heap "
             "profile generated with --heap-prof. (default: 512 * 1024)",
             &EnvironmentOptions::heap_prof_interval);
+  AddOption("--max-http-header-size",
+            "set the maximum size of HTTP headers (default: 8KB)",
+            &EnvironmentOptions::max_http_header_size,
+            kAllowedInEnvironment);
 #endif  // HAVE_INSPECTOR
   AddOption("--redirect-warnings",
             "write warnings to file instead of stderr",
@@ -628,10 +632,6 @@ 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 ce0cee5fe56784..763e6ab9c87b57 100644
--- a/src/node_options.h
+++ b/src/node_options.h
@@ -115,6 +115,7 @@ class EnvironmentOptions : public Options {
   bool expose_internals = false;
   bool frozen_intrinsics = false;
   std::string heap_snapshot_signal;
+  uint64_t max_http_header_size = 8 * 1024;
   bool no_deprecation = false;
   bool no_force_async_hooks_checks = false;
   bool no_warnings = false;
@@ -200,7 +201,6 @@ 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;
   bool debug_arraybuffer_allocations = false;
diff --git a/test/parallel/test-http-max-header-size-per-stream.js b/test/parallel/test-http-max-header-size-per-stream.js
new file mode 100644
index 00000000000000..5edb8d3a954cef
--- /dev/null
+++ b/test/parallel/test-http-max-header-size-per-stream.js
@@ -0,0 +1,82 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const http = require('http');
+const MakeDuplexPair = require('../common/duplexpair');
+
+// Test that setting the `maxHeaderSize` option works on a per-stream-basis.
+
+// Test 1: The server sends larger headers than what would otherwise be allowed.
+{
+  const { clientSide, serverSide } = MakeDuplexPair();
+
+  const req = http.request({
+    createConnection: common.mustCall(() => clientSide),
+    maxHeaderSize: http.maxHeaderSize * 4
+  }, common.mustCall((res) => {
+    assert.strictEqual(res.headers.hello, 'A'.repeat(http.maxHeaderSize * 3));
+    res.resume();  // We don’t actually care about contents.
+    res.on('end', common.mustCall());
+  }));
+  req.end();
+
+  serverSide.resume();  // Dump the request
+  serverSide.end('HTTP/1.1 200 OK\r\n' +
+                 'Hello: ' + 'A'.repeat(http.maxHeaderSize * 3) + '\r\n' +
+                 'Content-Length: 0\r\n' +
+                 '\r\n\r\n');
+}
+
+// Test 2: The same as Test 1 except without the option, to make sure it fails.
+{
+  const { clientSide, serverSide } = MakeDuplexPair();
+
+  const req = http.request({
+    createConnection: common.mustCall(() => clientSide)
+  }, common.mustNotCall());
+  req.end();
+  req.on('error', common.mustCall());
+
+  serverSide.resume();  // Dump the request
+  serverSide.end('HTTP/1.1 200 OK\r\n' +
+                 'Hello: ' + 'A'.repeat(http.maxHeaderSize * 3) + '\r\n' +
+                 'Content-Length: 0\r\n' +
+                 '\r\n\r\n');
+}
+
+// Test 3: The client sends larger headers than what would otherwise be allowed.
+{
+  const testData = 'Hello, World!\n';
+  const server = http.createServer(
+    { maxHeaderSize: http.maxHeaderSize * 4 },
+    common.mustCall((req, res) => {
+      res.statusCode = 200;
+      res.setHeader('Content-Type', 'text/plain');
+      res.end(testData);
+    }));
+
+  server.on('clientError', common.mustNotCall());
+
+  const { clientSide, serverSide } = MakeDuplexPair();
+  serverSide.server = server;
+  server.emit('connection', serverSide);
+
+  clientSide.write('GET / HTTP/1.1\r\n' +
+                   'Hello: ' + 'A'.repeat(http.maxHeaderSize * 3) + '\r\n' +
+                   '\r\n\r\n');
+}
+
+// Test 4: The same as Test 3 except without the option, to make sure it fails.
+{
+  const server = http.createServer(common.mustNotCall());
+
+  server.on('clientError', common.mustCall());
+
+  const { clientSide, serverSide } = MakeDuplexPair();
+  serverSide.server = server;
+  server.emit('connection', serverSide);
+
+  clientSide.write('GET / HTTP/1.1\r\n' +
+                   'Hello: ' + 'A'.repeat(http.maxHeaderSize * 3) + '\r\n' +
+                   '\r\n\r\n');
+}