Skip to content

Commit

Permalink
cli: add --max-http-header-size flag
Browse files Browse the repository at this point in the history
Allow the maximum size of HTTP headers to be overridden from
the command line.

Backport-PR-URL: #25171
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]>
  • Loading branch information
2 people authored and MylesBorins committed Dec 22, 2018
1 parent 4fb5a1b commit 693e362
Show file tree
Hide file tree
Showing 8 changed files with 183 additions and 24 deletions.
8 changes: 8 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
<!-- YAML
added: REPLACEME
-->

Specify the maximum size, in bytes, of HTTP headers. Defaults to 8KB.

## Environment Variables

### `NODE_DEBUG=module[,…]`
Expand Down Expand Up @@ -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`
Expand Down
4 changes: 4 additions & 0 deletions doc/node.1
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
7 changes: 7 additions & 0 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions src/node_config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ static void InitConfig(Local<Object> 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)));
Expand Down
5 changes: 5 additions & 0 deletions src/node_http_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<Object> target,
Local<Value> unused,
Expand Down Expand Up @@ -801,6 +804,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);
}

} // anonymous namespace
Expand Down
3 changes: 3 additions & 0 deletions src/node_internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
72 changes: 48 additions & 24 deletions test/sequential/test-http-max-http-headers.js
Original file line number Diff line number Diff line change
@@ -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.

Expand All @@ -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++) {
Expand All @@ -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' +
Expand All @@ -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(() => {
Expand All @@ -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...
Expand All @@ -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');
}));

Expand All @@ -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);
}));
});

Expand Down
104 changes: 104 additions & 0 deletions test/sequential/test-set-http-max-http-headers.js
Original file line number Diff line number Diff line change
@@ -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();

0 comments on commit 693e362

Please sign in to comment.