diff --git a/doc/api/cli.md b/doc/api/cli.md index 5a943b3428c6ca..0765091106b12c 100644 --- 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` + + +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 index b36f176983787f..b5cdd08117c703 100644 --- 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 index 1076157f807fbb..4041196d81d597 100644 --- 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 index 4a397d1dcc6c2d..4746c775e12dfa 100644 --- 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 target, Local unused, Local context) { @@ -46,6 +54,11 @@ void InitConfig(Local 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 name = OneByteString(env->isolate(), "warningFile"); Local value = String::NewFromUtf8(env->isolate(), diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc index d634cba4d3af03..633e66aaef28a6 100644 --- 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 target, Local unused, @@ -775,6 +778,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); } } // namespace node diff --git a/src/node_internals.h b/src/node_internals.h index bfb9bf296d7285..9e6495028a0b41 100644 --- 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 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();