cli: add --max-http-header-size flag

Allow the maximum size of HTTP headers to be overridden from the command line. Backport-PR-URL: #25173 co-authored-by: Matteo Collina <[email protected]> PR-URL: #24811 Fixes: #24692 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Сковорода Никита Андреевич <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
nodejs · Dec 21, 2018 · f233b16 · f233b16
1 parent 59f83d6
commit f233b16
Show file tree

Hide file tree

Showing 8 changed files with 192 additions and 24 deletions.
diff --git a/doc/api/cli.md b/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: REPLACEME
+-->
+
+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`

diff --git a/doc/node.1 b/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.

diff --git a/src/node.cc b/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;

diff --git a/src/node_config.cc b/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(),

diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc
@@ -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

diff --git a/src/node_internals.h b/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;
 

diff --git a/test/sequential/test-http-max-http-headers.js b/test/sequential/test-http-max-http-headers.js
@@ -1,10 +1,17 @@
 '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.
+
+assert(process.binding('config').maxHTTPHeaderSize,
+       'The option should exist on process.binding(\'config\')');
+
+console.log('pid is', process.pid);
+console.log('max header size is', process.binding('config').maxHTTPHeaderSize);
 
 // Verify that we cannot receive more than 8KB of headers.
 
@@ -28,19 +35,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 +58,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 +91,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 +102,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 +121,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 +133,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);
   }));
 });