Skip to content

Commit

Permalink
http: make maximum header size configurable per-stream or per-server
Browse files Browse the repository at this point in the history
Make `maxHeaderSize` a.k.a. `--max-header-size` configurable now that
the legacy parser is gone (which only supported a single global value).

Refs: nodejs#30567

PR-URL: nodejs#30570
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
  • Loading branch information
addaleax committed Dec 1, 2019
1 parent 2205f85 commit 6bf5a1d
Show file tree
Hide file tree
Showing 7 changed files with 135 additions and 10 deletions.
17 changes: 17 additions & 0 deletions doc/api/http.md
Original file line number Diff line number Diff line change
Expand Up @@ -2047,6 +2047,9 @@ Found'`.
<!-- YAML
added: v0.1.13
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/30570
description: The `maxHeaderSize` option is supported now.
- version: v9.6.0, v8.12.0
pr-url: https://github.com/nodejs/node/pull/15752
description: The `options` argument is supported now.
Expand All @@ -2059,6 +2062,10 @@ changes:
* `ServerResponse` {http.ServerResponse} Specifies the `ServerResponse` class
to be used. Useful for extending the original `ServerResponse`. **Default:**
`ServerResponse`.
* `maxHeaderSize` {number} Optionally overrides the value of
[`--max-http-header-size`][] for requests received by this server, i.e.
the maximum length of request headers in bytes.
**Default:** 8192 (8KB).
* `requestListener` {Function}

* Returns: {http.Server}
Expand Down Expand Up @@ -2156,11 +2163,17 @@ added: v11.6.0
Read-only property specifying the maximum allowed size of HTTP headers in bytes.
Defaults to 8KB. Configurable using the [`--max-http-header-size`][] CLI option.

This can be overridden for servers and client requests by passing the
`maxHeaderSize` option.

## http.request(options\[, callback\])
## http.request(url\[, options\]\[, callback\])
<!-- YAML
added: v0.3.6
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/30570
description: The `maxHeaderSize` option is supported now.
- version: v10.9.0
pr-url: https://github.com/nodejs/node/pull/21616
description: The `url` parameter can now be passed along with a separate
Expand Down Expand Up @@ -2196,6 +2209,10 @@ changes:
`hostname` will be used if both `host` and `hostname` are specified.
* `localAddress` {string} Local interface to bind for network connections.
* `lookup` {Function} Custom lookup function. **Default:** [`dns.lookup()`][].
* `maxHeaderSize` {number} Optionally overrides the value of
[`--max-http-header-size`][] for requests received from the server, i.e.
the maximum length of response headers in bytes.
**Default:** 8192 (8KB).
* `method` {string} A string specifying the HTTP request method. **Default:**
`'GET'`.
* `path` {string} Request path. Should include query string if any.
Expand Down
9 changes: 8 additions & 1 deletion lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ const {
ERR_INVALID_PROTOCOL,
ERR_UNESCAPED_CHARACTERS
} = codes;
const { validateInteger } = require('internal/validators');
const { getTimerDuration } = require('internal/timers');
const {
DTRACE_HTTP_CLIENT_REQUEST,
Expand Down Expand Up @@ -179,6 +180,11 @@ function ClientRequest(input, options, cb) {
method = this.method = 'GET';
}

const maxHeaderSize = options.maxHeaderSize;
if (maxHeaderSize !== undefined)
validateInteger(maxHeaderSize, 'maxHeaderSize', 0);
this.maxHeaderSize = maxHeaderSize;

this.path = options.path || '/';
if (cb) {
this.once('response', cb);
Expand Down Expand Up @@ -669,7 +675,8 @@ function tickOnSocket(req, socket) {
const parser = parsers.alloc();
req.socket = socket;
parser.initialize(HTTPParser.RESPONSE,
new HTTPClientAsyncResource('HTTPINCOMINGMESSAGE', req));
new HTTPClientAsyncResource('HTTPINCOMINGMESSAGE', req),
req.maxHeaderSize || 0);
parser.socket = socket;
parser.outgoing = req;
req.parser = parser;
Expand Down
9 changes: 8 additions & 1 deletion lib/_http_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ const {
ERR_INVALID_ARG_TYPE,
ERR_INVALID_CHAR
} = require('internal/errors').codes;
const { validateInteger } = require('internal/validators');
const Buffer = require('buffer').Buffer;
const {
DTRACE_HTTP_SERVER_REQUEST,
Expand Down Expand Up @@ -322,6 +323,11 @@ function Server(options, requestListener) {
this[kIncomingMessage] = options.IncomingMessage || IncomingMessage;
this[kServerResponse] = options.ServerResponse || ServerResponse;

const maxHeaderSize = options.maxHeaderSize;
if (maxHeaderSize !== undefined)
validateInteger(maxHeaderSize, 'maxHeaderSize', 0);
this.maxHeaderSize = maxHeaderSize;

net.Server.call(this, { allowHalfOpen: true });

if (requestListener) {
Expand Down Expand Up @@ -379,7 +385,8 @@ function connectionListenerInternal(server, socket) {
// https://github.com/nodejs/node/pull/21313
parser.initialize(
HTTPParser.REQUEST,
new HTTPServerAsyncResource('HTTPINCOMINGMESSAGE', socket)
new HTTPServerAsyncResource('HTTPINCOMINGMESSAGE', socket),
server.maxHeaderSize || 0
);
parser.socket = socket;

Expand Down
18 changes: 15 additions & 3 deletions src/node_http_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ using v8::Int32;
using v8::Integer;
using v8::Local;
using v8::MaybeLocal;
using v8::Number;
using v8::Object;
using v8::String;
using v8::Uint32;
Expand Down Expand Up @@ -486,8 +487,17 @@ class Parser : public AsyncWrap, public StreamListener {
static void Initialize(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

uint64_t max_http_header_size = 0;

CHECK(args[0]->IsInt32());
CHECK(args[1]->IsObject());
if (args.Length() > 2) {
CHECK(args[2]->IsNumber());
max_http_header_size = args[2].As<Number>()->Value();
}
if (max_http_header_size == 0) {
max_http_header_size = env->options()->max_http_header_size;
}

llhttp_type_t type =
static_cast<llhttp_type_t>(args[0].As<Int32>()->Value());
Expand All @@ -505,7 +515,7 @@ class Parser : public AsyncWrap, public StreamListener {

parser->set_provider_type(provider);
parser->AsyncReset(args[1].As<Object>());
parser->Init(type);
parser->Init(type, max_http_header_size);
}

template <bool should_pause>
Expand Down Expand Up @@ -752,7 +762,7 @@ class Parser : public AsyncWrap, public StreamListener {
}


void Init(llhttp_type_t type) {
void Init(llhttp_type_t type, uint64_t max_http_header_size) {
llhttp_init(&parser_, type, &settings);
header_nread_ = 0;
url_.Reset();
Expand All @@ -761,12 +771,13 @@ class Parser : public AsyncWrap, public StreamListener {
num_values_ = 0;
have_flushed_ = false;
got_exception_ = false;
max_http_header_size_ = max_http_header_size;
}


int TrackHeader(size_t len) {
header_nread_ += len;
if (header_nread_ >= per_process::cli_options->max_http_header_size) {
if (header_nread_ >= max_http_header_size_) {
llhttp_set_error_reason(&parser_, "HPE_HEADER_OVERFLOW:Header overflow");
return HPE_USER;
}
Expand Down Expand Up @@ -801,6 +812,7 @@ class Parser : public AsyncWrap, public StreamListener {
unsigned int execute_depth_ = 0;
bool pending_pause_ = false;
uint64_t header_nread_ = 0;
uint64_t max_http_header_size_;

// These are helper functions for filling `http_parser_settings`, which turn
// a member function of Parser into a C-style HTTP parser callback.
Expand Down
8 changes: 4 additions & 4 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,10 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
"profile generated with --heap-prof. (default: 512 * 1024)",
&EnvironmentOptions::heap_prof_interval);
#endif // HAVE_INSPECTOR
AddOption("--max-http-header-size",
"set the maximum size of HTTP headers (default: 8192 (8KB))",
&EnvironmentOptions::max_http_header_size,
kAllowedInEnvironment);
AddOption("--redirect-warnings",
"write warnings to file instead of stderr",
&EnvironmentOptions::redirect_warnings,
Expand Down Expand Up @@ -632,10 +636,6 @@ PerProcessOptionsParser::PerProcessOptionsParser(
kAllowedInEnvironment);
AddAlias("--trace-events-enabled", {
"--trace-event-categories", "v8,node,node.async_hooks" });
AddOption("--max-http-header-size",
"set the maximum size of HTTP headers (default: 8KB)",
&PerProcessOptions::max_http_header_size,
kAllowedInEnvironment);
AddOption("--v8-pool-size",
"set V8's thread pool size",
&PerProcessOptions::v8_thread_pool_size,
Expand Down
2 changes: 1 addition & 1 deletion src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ class EnvironmentOptions : public Options {
bool expose_internals = false;
bool frozen_intrinsics = false;
std::string heap_snapshot_signal;
uint64_t max_http_header_size = 8 * 1024;
bool no_deprecation = false;
bool no_force_async_hooks_checks = false;
bool no_warnings = false;
Expand Down Expand Up @@ -201,7 +202,6 @@ class PerProcessOptions : public Options {
std::string title;
std::string trace_event_categories;
std::string trace_event_file_pattern = "node_trace.${rotation}.log";
uint64_t max_http_header_size = 8 * 1024;
int64_t v8_thread_pool_size = 4;
bool zero_fill_all_buffers = false;
bool debug_arraybuffer_allocations = false;
Expand Down
82 changes: 82 additions & 0 deletions test/parallel/test-http-max-header-size-per-stream.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const http = require('http');
const MakeDuplexPair = require('../common/duplexpair');

// Test that setting the `maxHeaderSize` option works on a per-stream-basis.

// Test 1: The server sends larger headers than what would otherwise be allowed.
{
const { clientSide, serverSide } = MakeDuplexPair();

const req = http.request({
createConnection: common.mustCall(() => clientSide),
maxHeaderSize: http.maxHeaderSize * 4
}, common.mustCall((res) => {
assert.strictEqual(res.headers.hello, 'A'.repeat(http.maxHeaderSize * 3));
res.resume(); // We don’t actually care about contents.
res.on('end', common.mustCall());
}));
req.end();

serverSide.resume(); // Dump the request
serverSide.end('HTTP/1.1 200 OK\r\n' +
'Hello: ' + 'A'.repeat(http.maxHeaderSize * 3) + '\r\n' +
'Content-Length: 0\r\n' +
'\r\n\r\n');
}

// Test 2: The same as Test 1 except without the option, to make sure it fails.
{
const { clientSide, serverSide } = MakeDuplexPair();

const req = http.request({
createConnection: common.mustCall(() => clientSide)
}, common.mustNotCall());
req.end();
req.on('error', common.mustCall());

serverSide.resume(); // Dump the request
serverSide.end('HTTP/1.1 200 OK\r\n' +
'Hello: ' + 'A'.repeat(http.maxHeaderSize * 3) + '\r\n' +
'Content-Length: 0\r\n' +
'\r\n\r\n');
}

// Test 3: The client sends larger headers than what would otherwise be allowed.
{
const testData = 'Hello, World!\n';
const server = http.createServer(
{ maxHeaderSize: http.maxHeaderSize * 4 },
common.mustCall((req, res) => {
res.statusCode = 200;
res.setHeader('Content-Type', 'text/plain');
res.end(testData);
}));

server.on('clientError', common.mustNotCall());

const { clientSide, serverSide } = MakeDuplexPair();
serverSide.server = server;
server.emit('connection', serverSide);

clientSide.write('GET / HTTP/1.1\r\n' +
'Hello: ' + 'A'.repeat(http.maxHeaderSize * 3) + '\r\n' +
'\r\n\r\n');
}

// Test 4: The same as Test 3 except without the option, to make sure it fails.
{
const server = http.createServer(common.mustNotCall());

server.on('clientError', common.mustCall());

const { clientSide, serverSide } = MakeDuplexPair();
serverSide.server = server;
server.emit('connection', serverSide);

clientSide.write('GET / HTTP/1.1\r\n' +
'Hello: ' + 'A'.repeat(http.maxHeaderSize * 3) + '\r\n' +
'\r\n\r\n');
}

0 comments on commit 6bf5a1d

Please sign in to comment.