Skip to content

Commit 0082f62

Browse files
addaleaxsam-github
authored andcommitted
http: make --insecure-http-parser configurable per-stream or per-server
From the issue: > Some servers deviate from HTTP spec enougth that Node.js can't > communicate with them, but "work" when `--insecure-http-parser` > is enabled globally. It would be useful to be able to use this > mode, as a client, only when connecting to known bad servers. This is largely equivalent to #31446 in terms of code changes. Fixes: #31440 Refs: #31446 Backport-PR-URL: #30471 PR-URL: #31448 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
1 parent d616722 commit 0082f62

File tree

4 files changed

+118
-2
lines changed

4 files changed

+118
-2
lines changed

doc/api/http.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1830,6 +1830,9 @@ Found'`.
18301830
<!-- YAML
18311831
added: v0.1.13
18321832
changes:
1833+
- version: REPLACEME
1834+
pr-url: https://github.com/nodejs/node/pull/31448
1835+
description: The `insecureHTTPParser` option is supported now.
18331836
- version: v9.6.0, v8.12.0
18341837
pr-url: https://github.com/nodejs/node/pull/15752
18351838
description: The `options` argument is supported now.
@@ -1841,6 +1844,10 @@ changes:
18411844
* `ServerResponse` {http.ServerResponse} Specifies the `ServerResponse` class
18421845
to be used. Useful for extending the original `ServerResponse`. **Default:**
18431846
`ServerResponse`.
1847+
* `insecureHTTPParser` {boolean} Use an insecure HTTP parser that accepts
1848+
invalid HTTP headers when `true`. Using the insecure parser should be
1849+
avoided. See [`--insecure-http-parser`][] for more information.
1850+
**Default:** `false`
18441851
* `requestListener` {Function}
18451852

18461853
* Returns: {http.Server}
@@ -1943,6 +1950,9 @@ Defaults to 8KB. Configurable using the [`--max-http-header-size`][] CLI option.
19431950
<!-- YAML
19441951
added: v0.3.6
19451952
changes:
1953+
- version: REPLACEME
1954+
pr-url: https://github.com/nodejs/node/pull/31448
1955+
description: The `insecureHTTPParser` option is supported now.
19461956
- version: v10.9.0
19471957
pr-url: https://github.com/nodejs/node/pull/21616
19481958
description: The `url` parameter can now be passed along with a separate
@@ -1962,6 +1972,10 @@ changes:
19621972
* `family` {number} IP address family to use when resolving `host` or
19631973
`hostname`. Valid values are `4` or `6`. When unspecified, both IP v4 and
19641974
v6 will be used.
1975+
* `insecureHTTPParser` {boolean} Use an insecure HTTP parser that accepts
1976+
invalid HTTP headers when `true`. Using the insecure parser should be
1977+
avoided. See [`--insecure-http-parser`][] for more information.
1978+
**Default:** `false`
19651979
* `port` {number} Port of remote server. **Default:** `80`.
19661980
* `localAddress` {string} Local interface to bind for network connections.
19671981
* `socketPath` {string} Unix Domain Socket (cannot be used if one of `host`
@@ -2123,6 +2137,7 @@ will be emitted in the following order:
21232137
Note that setting the `timeout` option or using the `setTimeout()` function will
21242138
not abort the request or do anything besides add a `'timeout'` event.
21252139

2140+
[`--insecure-http-parser`]: cli.html#cli_insecure_http_parser
21262141
[`--max-http-header-size`]: cli.html#cli_max_http_header_size_size
21272142
[`'checkContinue'`]: #http_event_checkcontinue
21282143
[`'request'`]: #http_event_request

lib/_http_client.js

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,14 @@ function ClientRequest(input, options, cb) {
147147
method = this.method = 'GET';
148148
}
149149

150+
const insecureHTTPParser = options.insecureHTTPParser;
151+
if (insecureHTTPParser !== undefined &&
152+
typeof insecureHTTPParser !== 'boolean') {
153+
throw new ERR_INVALID_ARG_TYPE(
154+
'insecureHTTPParser', 'boolean', insecureHTTPParser);
155+
}
156+
this.insecureHTTPParser = insecureHTTPParser;
157+
150158
this.path = options.path || '/';
151159
if (cb) {
152160
this.once('response', cb);
@@ -628,7 +636,8 @@ function tickOnSocket(req, socket) {
628636
req.socket = socket;
629637
req.connection = socket;
630638
parser.reinitialize(HTTPParser.RESPONSE, parser[is_reused_symbol],
631-
isLenient());
639+
req.insecureHTTPParser === undefined ?
640+
isLenient() : req.insecureHTTPParser);
632641
parser.socket = socket;
633642
parser.outgoing = req;
634643
req.parser = parser;

lib/_http_server.js

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ const { IncomingMessage } = require('_http_incoming');
4848
const {
4949
ERR_HTTP_HEADERS_SENT,
5050
ERR_HTTP_INVALID_STATUS_CODE,
51+
ERR_INVALID_ARG_TYPE,
5152
ERR_INVALID_CHAR
5253
} = require('internal/errors').codes;
5354
const Buffer = require('buffer').Buffer;
@@ -290,6 +291,14 @@ function Server(options, requestListener) {
290291
this[kIncomingMessage] = options.IncomingMessage || IncomingMessage;
291292
this[kServerResponse] = options.ServerResponse || ServerResponse;
292293

294+
const insecureHTTPParser = options.insecureHTTPParser;
295+
if (insecureHTTPParser !== undefined &&
296+
typeof insecureHTTPParser !== 'boolean') {
297+
throw new ERR_INVALID_ARG_TYPE(
298+
'insecureHTTPParser', 'boolean', insecureHTTPParser);
299+
}
300+
this.insecureHTTPParser = insecureHTTPParser;
301+
293302
net.Server.call(this, { allowHalfOpen: true });
294303

295304
if (requestListener) {
@@ -344,7 +353,8 @@ function connectionListenerInternal(server, socket) {
344353

345354
var parser = parsers.alloc();
346355
parser.reinitialize(HTTPParser.REQUEST, parser[is_reused_symbol],
347-
isLenient());
356+
server.insecureHTTPParser === undefined ?
357+
isLenient() : server.insecureHTTPParser);
348358
parser.socket = socket;
349359

350360
// We are starting to wait for our headers.
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
'use strict';
2+
const common = require('../common');
3+
const assert = require('assert');
4+
const http = require('http');
5+
const MakeDuplexPair = require('../common/duplexpair');
6+
7+
// Test that setting the `maxHeaderSize` option works on a per-stream-basis.
8+
9+
// Test 1: The server sends an invalid header.
10+
{
11+
const { clientSide, serverSide } = MakeDuplexPair();
12+
13+
const req = http.request({
14+
createConnection: common.mustCall(() => clientSide),
15+
insecureHTTPParser: true
16+
}, common.mustCall((res) => {
17+
assert.strictEqual(res.headers.hello, 'foo\x08foo');
18+
res.resume(); // We don’t actually care about contents.
19+
res.on('end', common.mustCall());
20+
}));
21+
req.end();
22+
23+
serverSide.resume(); // Dump the request
24+
serverSide.end('HTTP/1.1 200 OK\r\n' +
25+
'Hello: foo\x08foo\r\n' +
26+
'Content-Length: 0\r\n' +
27+
'\r\n\r\n');
28+
}
29+
30+
// Test 2: The same as Test 1 except without the option, to make sure it fails.
31+
{
32+
const { clientSide, serverSide } = MakeDuplexPair();
33+
34+
const req = http.request({
35+
createConnection: common.mustCall(() => clientSide)
36+
}, common.mustNotCall());
37+
req.end();
38+
req.on('error', common.mustCall());
39+
40+
serverSide.resume(); // Dump the request
41+
serverSide.end('HTTP/1.1 200 OK\r\n' +
42+
'Hello: foo\x08foo\r\n' +
43+
'Content-Length: 0\r\n' +
44+
'\r\n\r\n');
45+
}
46+
47+
// Test 3: The client sends an invalid header.
48+
{
49+
const testData = 'Hello, World!\n';
50+
const server = http.createServer(
51+
{ insecureHTTPParser: true },
52+
common.mustCall((req, res) => {
53+
res.statusCode = 200;
54+
res.setHeader('Content-Type', 'text/plain');
55+
res.end(testData);
56+
}));
57+
58+
server.on('clientError', common.mustNotCall());
59+
60+
const { clientSide, serverSide } = MakeDuplexPair();
61+
serverSide.server = server;
62+
server.emit('connection', serverSide);
63+
64+
clientSide.write('GET / HTTP/1.1\r\n' +
65+
'Hello: foo\x08foo\r\n' +
66+
'\r\n\r\n');
67+
}
68+
69+
// Test 4: The same as Test 3 except without the option, to make sure it fails.
70+
{
71+
const server = http.createServer(common.mustNotCall());
72+
73+
server.on('clientError', common.mustCall());
74+
75+
const { clientSide, serverSide } = MakeDuplexPair();
76+
serverSide.server = server;
77+
server.emit('connection', serverSide);
78+
79+
clientSide.write('GET / HTTP/1.1\r\n' +
80+
'Hello: foo\x08foo\r\n' +
81+
'\r\n\r\n');
82+
}

0 commit comments

Comments
 (0)