FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

cli: add --max-http-header-size flag · nodejs/node@f233b16 · GitHub

/ node Public

Commit f233b16

Browse files
authored and
MylesBorins
committed
cli: add --max-http-header-size flag
Allow the maximum size of HTTP headers to be overridden from the command line. Backport-PR-URL: #25173 co-authored-by: Matteo Collina <hello@matteocollina.com> PR-URL: #24811 Fixes: #24692 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
1 parent 59f83d6 commit f233b16

8 files changed

Lines changed: 192 additions & 24 deletions

File tree

‎doc/api/cli.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,13 @@ Indicate the end of node options. Pass the rest of the arguments to the script.
293293
If no script filename or eval/print script is supplied prior to this, then
294294
the next argument will be used as a script filename.
295295

296+
### `--max-http-header-size=size`
297+
<!-- YAML
298+
added: REPLACEME
299+
-->
300+
301+
Specify the maximum size, in bytes, of HTTP headers. Defaults to 8KB.
302+
296303
## Environment Variables
297304

298305
### `NODE_DEBUG=module[,…]`
@@ -353,6 +360,7 @@ Node options that are allowed are:
353360
- `--debug-brk`
354361
- `--debug-port`
355362
- `--debug`
363+
- `--max-http-header-size`
356364
- `--no-deprecation`
357365
- `--no-warnings`
358366
- `--openssl-config`

‎doc/node.1‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,10 @@ Open the REPL even if stdin does not appear to be a terminal.
9292
Preload the specified module at startup. Follows `require()`'s module resolution
9393
rules. \fImodule\fR may be either a path to a file, or a node module name.
9494

95+
.TP
96+
.BR \-\-max\-http\-header-size \fI=size\fR
97+
Specify the maximum size of HTTP headers in bytes. Defaults to 8KB.
98+
9599
.TP
96100
.BR \-\-no\-deprecation
97101
Silence deprecation warnings.

‎src/node.cc‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,8 @@ unsigned int reverted = 0;
170170
static std::string icu_data_dir; // NOLINT(runtime/string)
171171
#endif
172172

173+
uint64_t max_http_header_size = 8 * 1024;
174+
173175
// used by C++ modules as well
174176
bool no_deprecation = false;
175177

@@ -3731,6 +3733,8 @@ static void PrintHelp() {
37313733
" --trace-deprecation show stack traces on deprecations\n"
37323734
" --throw-deprecation throw an exception anytime a deprecated "
37333735
"function is used\n"
3736+
" --max-http-header-size Specify the maximum size of HTTP\n"
3737+
" headers in bytes. Defaults to 8KB.\n"
37343738
" --no-warnings silence all process warnings\n"
37353739
" --napi-modules load N-API modules (no-op - option kept for "
37363740
" compatibility)\n"
@@ -3852,6 +3856,7 @@ static void CheckIfAllowedInEnv(const char* exe, bool is_env,
38523856
"--pending-deprecation",
38533857
"--no-warnings",
38543858
"--napi-modules",
3859+
"--max-http-header-size",
38553860
"--trace-warnings",
38563861
"--redirect-warnings",
38573862
"--trace-sync-io",
@@ -4010,6 +4015,8 @@ static void ParseArgs(int* argc,
40104015
new_v8_argc += 1;
40114016
} else if (strncmp(arg, "--v8-pool-size=", 15) == 0) {
40124017
v8_thread_pool_size = atoi(arg + 15);
4018+
} else if (strncmp(arg, "--max-http-header-size=", 23) == 0) {
4019+
max_http_header_size = atoi(arg + 23);
40134020
#if HAVE_OPENSSL
40144021
} else if (strncmp(arg, "--tls-cipher-list=", 18) == 0) {
40154022
default_cipher_list = arg + 18;

‎src/node_config.cc‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ namespace node {
77

88
using v8::Context;
99
using v8::Local;
10+
using v8::Number;
1011
using v8::Object;
1112
using v8::ReadOnly;
1213
using v8::String;
@@ -24,6 +25,13 @@ using v8::Value;
2425
True(env->isolate()), ReadOnly).FromJust(); \
2526
} while (0)
2627

28+
#define READONLY_PROPERTY(obj, name, value) \
29+
do { \
30+
obj->DefineOwnProperty(env->context(), \
31+
FIXED_ONE_BYTE_STRING(env->isolate(), name), \
32+
value, ReadOnly).FromJust(); \
33+
} while (0)
34+
2735
void InitConfig(Local<Object> target,
2836
Local<Value> unused,
2937
Local<Context> context) {
@@ -46,6 +54,11 @@ void InitConfig(Local<Object> target,
4654
if (config_expose_internals)
4755
READONLY_BOOLEAN_PROPERTY("exposeInternals");
4856

57+
58+
READONLY_PROPERTY(target,
59+
"maxHTTPHeaderSize",
60+
Number::New(env->isolate(), max_http_header_size));
61+
4962
if (!config_warning_file.empty()) {
5063
Local<String> name = OneByteString(env->isolate(), "warningFile");
5164
Local<String> value = String::NewFromUtf8(env->isolate(),

‎src/node_http_parser.cc‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -731,6 +731,9 @@ const struct http_parser_settings Parser::settings = {
731731
nullptr // on_chunk_complete
732732
};
733733

734+
void InitMaxHttpHeaderSizeOnce() {
735+
http_parser_set_max_header_size(max_http_header_size);
736+
}
734737

735738
void InitHttpParser(Local<Object> target,
736739
Local<Value> unused,
@@ -775,6 +778,8 @@ void InitHttpParser(Local<Object> target,
775778

776779
target->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "HTTPParser"),
777780
t->GetFunction());
781+
static uv_once_t init_once = UV_ONCE_INIT;
782+
uv_once(&init_once, InitMaxHttpHeaderSizeOnce);
778783
}
779784

780785
} // namespace node

‎src/node_internals.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,9 @@ extern bool config_expose_internals;
5656
// it to stderr.
5757
extern std::string config_warning_file;
5858

59+
// Set in node.cc by ParseArgs when --max-http-header-size is used
60+
extern uint64_t max_http_header_size;
61+
5962
// Forward declaration
6063
class Environment;
6164

‎test/sequential/test-http-max-http-headers.js‎

Lines changed: 48 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,17 @@
11
'use strict';
2+
// Flags: --expose_internals
23

34
const assert = require('assert');
45
const common = require('../common');
56
const http = require('http');
67
const net = require('net');
7-
const MAX = 8 * 1024; // 8KB
8+
const MAX = +(process.argv[2] || 8 * 1024); // Command line option, or 8KB.
9+
10+
assert(process.binding('config').maxHTTPHeaderSize,
11+
'The option should exist on process.binding(\'config\')');
12+
13+
console.log('pid is', process.pid);
14+
console.log('max header size is', process.binding('config').maxHTTPHeaderSize);
815

916
// Verify that we cannot receive more than 8KB of headers.
1017

@@ -28,19 +35,15 @@ function fillHeaders(headers, currentSize, valid = false) {
2835
headers += 'a'.repeat(MAX - headers.length - 3);
2936
// Generate valid headers
3037
if (valid) {
31-
// TODO(mcollina): understand why -9 is needed instead of -1
32-
headers = headers.slice(0, -9);
38+
// TODO(mcollina): understand why -32 is needed instead of -1
39+
headers = headers.slice(0, -32);
3340
}
3441
return headers + '\r\n\r\n';
3542
}
3643

37-
const timeout = common.platformTimeout(10);
38-
3944
function writeHeaders(socket, headers) {
4045
const array = [];
41-
42-
// this is off from 1024 so that \r\n does not get split
43-
const chunkSize = 1000;
46+
const chunkSize = 100;
4447
let last = 0;
4548

4649
for (let i = 0; i < headers.length / chunkSize; i++) {
@@ -55,19 +58,25 @@ function writeHeaders(socket, headers) {
5558
next();
5659

5760
function next() {
58-
if (socket.write(array.shift())) {
59-
if (array.length === 0) {
60-
socket.end();
61-
} else {
62-
setTimeout(next, timeout);
63-
}
61+
if (socket.destroyed) {
62+
console.log('socket was destroyed early, data left to write:',
63+
array.join('').length);
64+
return;
65+
}
66+
67+
const chunk = array.shift();
68+
69+
if (chunk) {
70+
console.log('writing chunk of size', chunk.length);
71+
socket.write(chunk, next);
6472
} else {
65-
socket.once('drain', next);
73+
socket.end();
6674
}
6775
}
6876
}
6977

7078
function test1() {
79+
console.log('test1');
7180
let headers =
7281
'HTTP/1.1 200 OK\r\n' +
7382
'Content-Length: 0\r\n' +
@@ -82,6 +91,9 @@ function test1() {
8291
writeHeaders(sock, headers);
8392
sock.resume();
8493
});
94+
95+
// The socket might error but that's ok
96+
sock.on('error', () => {});
8597
});
8698

8799
server.listen(0, common.mustCall(() => {
@@ -90,17 +102,17 @@ function test1() {
90102

91103
client.on('error', common.mustCall((err) => {
92104
assert.strictEqual(err.code, 'HPE_HEADER_OVERFLOW');
93-
server.close();
94-
setImmediate(test2);
105+
server.close(test2);
95106
}));
96107
}));
97108
}
98109

99110
const test2 = common.mustCall(() => {
111+
console.log('test2');
100112
let headers =
101113
'GET / HTTP/1.1\r\n' +
102114
'Host: localhost\r\n' +
103-
'Agent: node\r\n' +
115+
'Agent: nod2\r\n' +
104116
'X-CRASH: ';
105117

106118
// /, Host, localhost, Agent, node, X-CRASH, a...
@@ -109,7 +121,7 @@ const test2 = common.mustCall(() => {
109121

110122
const server = http.createServer(common.mustNotCall());
111123

112-
server.on('clientError', common.mustCall((err) => {
124+
server.once('clientError', common.mustCall((err) => {
113125
assert.strictEqual(err.code, 'HPE_HEADER_OVERFLOW');
114126
}));
115127

@@ -121,34 +133,46 @@ const test2 = common.mustCall(() => {
121133
});
122134

123135
finished(client, common.mustCall((err) => {
124-
server.close();
125-
setImmediate(test3);
136+
server.close(test3);
126137
}));
127138
}));
128139
});
129140

130141
const test3 = common.mustCall(() => {
142+
console.log('test3');
131143
let headers =
132144
'GET / HTTP/1.1\r\n' +
133145
'Host: localhost\r\n' +
134-
'Agent: node\r\n' +
146+
'Agent: nod3\r\n' +
135147
'X-CRASH: ';
136148

137149
// /, Host, localhost, Agent, node, X-CRASH, a...
138150
const currentSize = 1 + 4 + 9 + 5 + 4 + 7;
139151
headers = fillHeaders(headers, currentSize, true);
140152

153+
console.log('writing', headers.length);
154+
141155
const server = http.createServer(common.mustCall((req, res) => {
142-
res.end('hello world');
143-
setImmediate(server.close.bind(server));
156+
res.end('hello from test3 server');
157+
server.close();
144158
}));
145159

160+
server.on('clientError', (err) => {
161+
console.log(err.code);
162+
if (err.code === 'HPE_HEADER_OVERFLOW') {
163+
console.log(err.rawPacket.toString('hex'));
164+
}
165+
});
166+
server.on('clientError', common.mustNotCall());
167+
146168
server.listen(0, common.mustCall(() => {
147169
const client = net.connect(server.address().port);
148170
client.on('connect', () => {
149171
writeHeaders(client, headers);
150172
client.resume();
151173
});
174+
175+
client.pipe(process.stdout);
152176
}));
153177
});
154178

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL