diff --git a/doc/api/cli.md b/doc/api/cli.md index 28668703f0672d..e906c848b3cb8a 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -405,6 +405,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` + + +Specify the maximum size, in bytes, of HTTP headers. Defaults to 8KB. + ## Environment Variables ### `NODE_DEBUG=module[,…]` @@ -472,6 +479,7 @@ Node.js options that are allowed are: - `--inspect-brk` - `--inspect-port` - `--inspect` +- `--max-http-header-size` - `--no-deprecation` - `--no-warnings` - `--openssl-config` diff --git a/doc/node.1 b/doc/node.1 index 76b7455953d5af..9447bacbe0a40b 100644 --- a/doc/node.1 +++ b/doc/node.1 @@ -110,6 +110,10 @@ Activate inspector on host:port and break at start of user script. .BR \-\-inspect-port \fI=[host:]port\fR Set the host:port to be used when the inspector is activated. +.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 index f4d27093d02ccf..3d791f04ada8af 100644 --- a/src/node.cc +++ b/src/node.cc @@ -201,6 +201,8 @@ unsigned int reverted = 0; 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; @@ -3120,6 +3122,8 @@ static void PrintHelp() { " set host:port for inspector\n" #endif " --no-deprecation silence deprecation warnings\n" + " --max-http-header-size Specify the maximum size of HTTP\n" + " headers in bytes. Defaults to 8KB.\n" " --trace-deprecation show stack traces on deprecations\n" " --throw-deprecation throw an exception on deprecations\n" " --pending-deprecation emit pending deprecation warnings\n" @@ -3264,6 +3268,7 @@ static void CheckIfAllowedInEnv(const char* exe, bool is_env, "--expose-http2", "--experimental-modules", "--loader", + "--max-http-header-size", "--trace-warnings", "--redirect-warnings", "--trace-sync-io", @@ -3468,6 +3473,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 index 0a78da7198b6b0..756cdfaaa82baa 100644 --- a/src/node_config.cc +++ b/src/node_config.cc @@ -88,6 +88,10 @@ static void InitConfig(Local target, if (env->abort_on_uncaught_exception()) READONLY_BOOLEAN_PROPERTY("shouldAbortOnUncaughtException"); + READONLY_PROPERTY(target, + "maxHTTPHeaderSize", + Number::New(env->isolate(), max_http_header_size)); + READONLY_PROPERTY(target, "bits", Number::New(env->isolate(), 8 * sizeof(intptr_t))); diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc index 94abe9e390857b..504130e5d7c3f1 100644 --- a/src/node_http_parser.cc +++ b/src/node_http_parser.cc @@ -755,6 +755,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 target, Local unused, @@ -801,6 +804,8 @@ void InitHttpParser(Local 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); } } // anonymous namespace diff --git a/src/node_internals.h b/src/node_internals.h index a891b1b846678c..a27ac6f2350adf 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -176,6 +176,9 @@ extern std::string config_warning_file; // NOLINT(runtime/string) // NODE_PENDING_DEPRECATION is used extern bool config_pending_deprecation; +// Set in node.cc by ParseArgs when --max-http-header-size is used +extern uint64_t max_http_header_size; + // Tells whether it is safe to call v8::Isolate::GetCurrent(). extern bool v8_initialized; diff --git a/test/sequential/test-http-max-http-headers.js b/test/sequential/test-http-max-http-headers.js index ae76142a4fd077..51d071f95a2e81 100644 --- 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); })); }); 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 00000000000000..7ec13f370784f8 --- /dev/null +++ b/test/sequential/test-set-http-max-http-headers.js @@ -0,0 +1,104 @@ +'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 timeout = common.platformTimeout(100); + +const tests = []; + +function test(fn) { + tests.push(fn); +} + +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', + '--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', + '--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: '--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();