From 632f2633249dc0f295e9d2abab33baaf0f75ce6f Mon Sep 17 00:00:00 2001 From: cjihrig Date: Mon, 3 Dec 2018 12:27:46 -0500 Subject: 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 PR-URL: https://github.com/nodejs/node/pull/24811 Fixes: https://github.com/nodejs/node/issues/24692 Reviewed-By: Anna Henningsen Reviewed-By: Myles Borins Reviewed-By: Michael Dawson Reviewed-By: Сковорода Никита Андреевич Reviewed-By: James M Snell Reviewed-By: Jeremiah Senkpiel --- test/sequential/test-http-max-http-headers.js | 69 +++++++++----- test/sequential/test-set-http-max-http-headers.js | 109 ++++++++++++++++++++++ 2 files changed, 154 insertions(+), 24 deletions(-) create mode 100644 test/sequential/test-set-http-max-http-headers.js (limited to 'test/sequential') 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(); -- cgit v1.2.3