From 6bf5a1d691291cdfcc4941e68f00d0003e565476 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Thu, 21 Nov 2019 00:00:43 +0100
Subject: 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

PR-URL: https://github.com/nodejs/node/pull/30570
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
---
 doc/api/http.md                                    | 17 +++++
 lib/_http_client.js                                |  9 ++-
 lib/_http_server.js                                |  9 ++-
 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 9762abd27e..89031331d2 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/30570
+    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,10 @@ 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, i.e.
+    the maximum length of request headers in bytes.
+    **Default:** 8192 (8KB).
 * `requestListener` {Function}
 
 * Returns: {http.Server}
@@ -2156,11 +2163,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/30570
+    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 +2209,10 @@ 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 from the server, i.e.
+    the maximum length of response headers in bytes.
+    **Default:** 8192 (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 6d8cb8fc8e..ece93d14e0 100644
--- a/lib/_http_client.js
+++ b/lib/_http_client.js
@@ -55,6 +55,7 @@ const {
   ERR_INVALID_PROTOCOL,
   ERR_UNESCAPED_CHARACTERS
 } = codes;
+const { validateInteger } = require('internal/validators');
 const { getTimerDuration } = require('internal/timers');
 const {
   DTRACE_HTTP_CLIENT_REQUEST,
@@ -179,6 +180,11 @@ function ClientRequest(input, options, cb) {
     method = this.method = 'GET';
   }
 
+  const maxHeaderSize = options.maxHeaderSize;
+  if (maxHeaderSize !== undefined)
+    validateInteger(maxHeaderSize, 'maxHeaderSize', 0);
+  this.maxHeaderSize = maxHeaderSize;
+
   this.path = options.path || '/';
   if (cb) {
     this.once('response', cb);
@@ -669,7 +675,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 3d114d8711..6c52f7adbc 100644
--- a/lib/_http_server.js
+++ b/lib/_http_server.js
@@ -58,6 +58,7 @@ const {
   ERR_INVALID_ARG_TYPE,
   ERR_INVALID_CHAR
 } = require('internal/errors').codes;
+const { validateInteger } = require('internal/validators');
 const Buffer = require('buffer').Buffer;
 const {
   DTRACE_HTTP_SERVER_REQUEST,
@@ -322,6 +323,11 @@ function Server(options, requestListener) {
   this[kIncomingMessage] = options.IncomingMessage || IncomingMessage;
   this[kServerResponse] = options.ServerResponse || ServerResponse;
 
+  const maxHeaderSize = options.maxHeaderSize;
+  if (maxHeaderSize !== undefined)
+    validateInteger(maxHeaderSize, 'maxHeaderSize', 0);
+  this.maxHeaderSize = maxHeaderSize;
+
   net.Server.call(this, { allowHalfOpen: true });
 
   if (requestListener) {
@@ -379,7 +385,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 c6136702c7..0328dc7c0f 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 44b125775f..498bedd1e5 100644
--- a/src/node_options.cc
+++ b/src/node_options.cc
@@ -440,6 +440,10 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
             "profile generated with --heap-prof. (default: 512 * 1024)",
             &EnvironmentOptions::heap_prof_interval);
 #endif  // HAVE_INSPECTOR
+  AddOption("--max-http-header-size",
+            "set the maximum size of HTTP headers (default: 8192 (8KB))",
+            &EnvironmentOptions::max_http_header_size,
+            kAllowedInEnvironment);
   AddOption("--redirect-warnings",
             "write warnings to file instead of stderr",
             &EnvironmentOptions::redirect_warnings,
@@ -632,10 +636,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 adc0ef783f..fea912da44 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;
@@ -201,7 +202,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 0000000000..5edb8d3a95
--- /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');
+}
-- 
cgit v1.2.3