Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

http: make maximum header size configurable per-stream or per-server #30570

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -53,6 +53,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 @@ -177,6 +178,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 @@ -667,7 +673,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 @@ -436,6 +436,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 @@ -628,10 +632,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 @@ -200,7 +201,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));
addaleax marked this conversation as resolved.
Show resolved Hide resolved
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');
}