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

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>

2 files changed, 154 insertions, 24 deletions

diff --git a/test/sequential/test-http-max-http-headers.js b/test/sequential/test-http-max-http-headers.js
index f1be923e22..b91135a61a 100644
--- a/test/sequential/test-http-max-http-headers.js
+++ b/test/sequential/test-http-max-http-headers.js
@@ -4,10 +4,14 @@ const common = require('../common');
 const assert = require('assert');
 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'));
+console.log('current http parser is', getOptionValue('--http-parser'));
+
 // Verify that we cannot receive more than 8KB of headers.
 
 function once(cb) {
@@ -38,19 +42,15 @@ function fillHeaders(headers, currentSize, valid = false) {
 
   // 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++) {
@@ -65,19 +65,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' +
@@ -92,6 +98,9 @@ function test1() {
       writeHeaders(sock, headers);
       sock.resume();
     });
+
+    // The socket might error but that's ok
+    sock.on('error', () => {});
   });
 
   server.listen(0, common.mustCall(() => {
@@ -100,17 +109,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...
@@ -119,7 +128,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');
   }));
 
@@ -131,34 +140,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 0000000000..657f9948be
--- /dev/null
+++ b/test/sequential/test-set-http-max-http-headers.js
@@ -0,0 +1,109 @@
+'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 parsers = ['legacy', 'llhttp'];
+
+const timeout = common.platformTimeout(100);
+
+const tests = [];
+
+function test(fn) {
+  tests.push(fn);
+}
+
+parsers.forEach((parser) => {
+  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',
+                  `--http-parser=${parser}`,
+                  '--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',
+                  `--http-parser=${parser}`,
+                  '--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: `--http-parser=${parser} --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();