From 324866de072053bf2ed3bae81172f350c0827702 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Tue, 21 Jan 2020 22:35:27 +0100
Subject: [PATCH 1/4] 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 https://github.com/nodejs/node/pull/31446
in terms of code changes.

Fixes: https://github.com/nodejs/node/issues/31440
Refs: https://github.com/nodejs/node/pull/31446
---
 doc/api/http.md                               | 14 ++++
 lib/_http_client.js                           | 11 ++-
 lib/_http_server.js                           | 11 ++-
 .../test-http-insecure-parser-per-stream.js   | 82 +++++++++++++++++++
 4 files changed, 116 insertions(+), 2 deletions(-)
 create mode 100644 test/parallel/test-http-insecure-parser-per-stream.js

diff --git a/doc/api/http.md b/doc/api/http.md
index 14b900ac55f035..bd6d6ed2323862 100644
--- a/doc/api/http.md
+++ b/doc/api/http.md
@@ -2031,6 +2031,9 @@ Found'`.
 <!-- YAML
 added: v0.1.13
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/?????
+    description: The `insecureHTTPParser` option is supported now.
   - version: v13.3.0
     pr-url: https://github.com/nodejs/node/pull/30570
     description: The `maxHeaderSize` option is supported now.
@@ -2046,6 +2049,10 @@ changes:
   * `ServerResponse` {http.ServerResponse} Specifies the `ServerResponse` class
     to be used. Useful for extending the original `ServerResponse`. **Default:**
     `ServerResponse`.
+  * `insecureHTTPParser` {boolean} Optionally overrides the value of
+    [`--insecure-http-parser`][] for requests received by this server, i.e.
+    whether to accept invalid HTTP headers that must manually be sanitized.
+    **Default:** false.
   * `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.
@@ -2155,6 +2162,9 @@ This can be overridden for servers and client requests by passing the
 <!-- YAML
 added: v0.3.6
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/?????
+    description: The `insecureHTTPParser` option is supported now.
   - version: v13.3.0
     pr-url: https://github.com/nodejs/node/pull/30570
     description: The `maxHeaderSize` option is supported now.
@@ -2191,6 +2201,9 @@ changes:
     request to. **Default:** `'localhost'`.
   * `hostname` {string} Alias for `host`. To support [`url.parse()`][],
     `hostname` will be used if both `host` and `hostname` are specified.
+  * `insecureHTTPParser` {boolean} Optionally overrides the value of
+    [`--insecure-http-parser`][] for requests received from the server, i.e.
+    whether to accept invalid HTTP headers that must manually be sanitized.
   * `localAddress` {string} Local interface to bind for network connections.
   * `lookup` {Function} Custom lookup function. **Default:** [`dns.lookup()`][].
   * `maxHeaderSize` {number} Optionally overrides the value of
@@ -2364,6 +2377,7 @@ will be emitted in the following order:
 Setting the `timeout` option or using the `setTimeout()` function will
 not abort the request or do anything besides add a `'timeout'` event.
 
+[`--insecure-http-parser`]: cli.html#cli_insecure_http_parser
 [`--max-http-header-size`]: cli.html#cli_max_http_header_size_size
 [`'checkContinue'`]: #http_event_checkcontinue
 [`'request'`]: #http_event_request
diff --git a/lib/_http_client.js b/lib/_http_client.js
index 20fa1e4f2e0c98..fef4b635a00166 100644
--- a/lib/_http_client.js
+++ b/lib/_http_client.js
@@ -187,6 +187,14 @@ function ClientRequest(input, options, cb) {
     validateInteger(maxHeaderSize, 'maxHeaderSize', 0);
   this.maxHeaderSize = maxHeaderSize;
 
+  const insecureHTTPParser = options.insecureHTTPParser;
+  if (insecureHTTPParser !== undefined &&
+      typeof insecureHTTPParser !== 'boolean') {
+    throw new ERR_INVALID_ARG_TYPE(
+      'insecureHTTPParser', 'boolean', insecureHTTPParser);
+  }
+  this.insecureHTTPParser = insecureHTTPParser;
+
   this.path = options.path || '/';
   if (cb) {
     this.once('response', cb);
@@ -683,7 +691,8 @@ function tickOnSocket(req, socket) {
   parser.initialize(HTTPParser.RESPONSE,
                     new HTTPClientAsyncResource('HTTPINCOMINGMESSAGE', req),
                     req.maxHeaderSize || 0,
-                    isLenient());
+                    req.insecureHTTPParser === undefined ?
+                      isLenient() : req.insecureHTTPParser);
   parser.socket = socket;
   parser.outgoing = req;
   req.parser = parser;
diff --git a/lib/_http_server.js b/lib/_http_server.js
index 64190969e5f402..3f9c98e8380363 100644
--- a/lib/_http_server.js
+++ b/lib/_http_server.js
@@ -335,6 +335,14 @@ function Server(options, requestListener) {
     validateInteger(maxHeaderSize, 'maxHeaderSize', 0);
   this.maxHeaderSize = maxHeaderSize;
 
+  const insecureHTTPParser = options.insecureHTTPParser;
+  if (insecureHTTPParser !== undefined &&
+      typeof insecureHTTPParser !== 'boolean') {
+    throw new ERR_INVALID_ARG_TYPE(
+      'insecureHTTPParser', 'boolean', insecureHTTPParser);
+  }
+  this.insecureHTTPParser = insecureHTTPParser;
+
   net.Server.call(this, { allowHalfOpen: true });
 
   if (requestListener) {
@@ -416,7 +424,8 @@ function connectionListenerInternal(server, socket) {
     HTTPParser.REQUEST,
     new HTTPServerAsyncResource('HTTPINCOMINGMESSAGE', socket),
     server.maxHeaderSize || 0,
-    isLenient(),
+    server.insecureHTTPParser === undefined ?
+      isLenient() : server.insecureHTTPParser,
   );
   parser.socket = socket;
 
diff --git a/test/parallel/test-http-insecure-parser-per-stream.js b/test/parallel/test-http-insecure-parser-per-stream.js
new file mode 100644
index 00000000000000..d2b8ffe708770c
--- /dev/null
+++ b/test/parallel/test-http-insecure-parser-per-stream.js
@@ -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 an invalid header.
+{
+  const { clientSide, serverSide } = MakeDuplexPair();
+
+  const req = http.request({
+    createConnection: common.mustCall(() => clientSide),
+    insecureHTTPParser: true
+  }, common.mustCall((res) => {
+    assert.strictEqual(res.headers.hello, 'foo\x08foo');
+    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: foo\x08foo\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: foo\x08foo\r\n' +
+                 'Content-Length: 0\r\n' +
+                 '\r\n\r\n');
+}
+
+// Test 3: The client sends an invalid header.
+{
+  const testData = 'Hello, World!\n';
+  const server = http.createServer(
+    { insecureHTTPParser: true },
+    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: foo\x08foo\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: foo\x08foo\r\n' +
+                   '\r\n\r\n');
+}

From 79c6d9bde742721594f9d07bf578fd4836918132 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Tue, 21 Jan 2020 23:55:30 +0100
Subject: [PATCH 2/4] fixup! http: make --insecure-http-parser configurable
 per-stream or per-server

---
 doc/api/http.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/api/http.md b/doc/api/http.md
index bd6d6ed2323862..656f4ccb99b95b 100644
--- a/doc/api/http.md
+++ b/doc/api/http.md
@@ -2032,7 +2032,7 @@ Found'`.
 added: v0.1.13
 changes:
   - version: REPLACEME
-    pr-url: https://github.com/nodejs/node/pull/?????
+    pr-url: https://github.com/nodejs/node/pull/31448
     description: The `insecureHTTPParser` option is supported now.
   - version: v13.3.0
     pr-url: https://github.com/nodejs/node/pull/30570
@@ -2163,7 +2163,7 @@ This can be overridden for servers and client requests by passing the
 added: v0.3.6
 changes:
   - version: REPLACEME
-    pr-url: https://github.com/nodejs/node/pull/?????
+    pr-url: https://github.com/nodejs/node/pull/31448
     description: The `insecureHTTPParser` option is supported now.
   - version: v13.3.0
     pr-url: https://github.com/nodejs/node/pull/30570

From 946e815d81f07bcf355f21156d14ec5ab4d2fee1 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Wed, 22 Jan 2020 17:43:36 +0100
Subject: [PATCH 3/4] fixup! http: make --insecure-http-parser configurable
 per-stream or per-server

---
 doc/api/http.md | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/doc/api/http.md b/doc/api/http.md
index 656f4ccb99b95b..13016f69e68a2b 100644
--- a/doc/api/http.md
+++ b/doc/api/http.md
@@ -2049,10 +2049,10 @@ changes:
   * `ServerResponse` {http.ServerResponse} Specifies the `ServerResponse` class
     to be used. Useful for extending the original `ServerResponse`. **Default:**
     `ServerResponse`.
-  * `insecureHTTPParser` {boolean} Optionally overrides the value of
-    [`--insecure-http-parser`][] for requests received by this server, i.e.
-    whether to accept invalid HTTP headers that must manually be sanitized.
-    **Default:** false.
+  * `insecureHTTPParser` {boolean} Use an insecure HTTP parser that accepts
+    invalid HTTP headers when `true`. Using the insecure parser should be
+    avoided, see [`--insecure-http-parser`][] for more information.
+    **Default:** `false`
   * `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.
@@ -2201,9 +2201,10 @@ changes:
     request to. **Default:** `'localhost'`.
   * `hostname` {string} Alias for `host`. To support [`url.parse()`][],
     `hostname` will be used if both `host` and `hostname` are specified.
-  * `insecureHTTPParser` {boolean} Optionally overrides the value of
-    [`--insecure-http-parser`][] for requests received from the server, i.e.
-    whether to accept invalid HTTP headers that must manually be sanitized.
+  * `insecureHTTPParser` {boolean} Use an insecure HTTP parser that accepts
+    invalid HTTP headers when `true`. Using the insecure parser should be
+    avoided, see [`--insecure-http-parser`][] for more information.
+    **Default:** `false`
   * `localAddress` {string} Local interface to bind for network connections.
   * `lookup` {Function} Custom lookup function. **Default:** [`dns.lookup()`][].
   * `maxHeaderSize` {number} Optionally overrides the value of

From 819468e142445ace10e965d5842433884adeb6ec Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Thu, 23 Jan 2020 21:50:49 +0100
Subject: [PATCH 4/4] fixup! http: make --insecure-http-parser configurable
 per-stream or per-server

Co-Authored-By: Rich Trott <rtrott@gmail.com>
---
 doc/api/http.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/api/http.md b/doc/api/http.md
index 13016f69e68a2b..dd90b54d3e11cb 100644
--- a/doc/api/http.md
+++ b/doc/api/http.md
@@ -2051,7 +2051,7 @@ changes:
     `ServerResponse`.
   * `insecureHTTPParser` {boolean} Use an insecure HTTP parser that accepts
     invalid HTTP headers when `true`. Using the insecure parser should be
-    avoided, see [`--insecure-http-parser`][] for more information.
+    avoided. See [`--insecure-http-parser`][] for more information.
     **Default:** `false`
   * `maxHeaderSize` {number} Optionally overrides the value of
     [`--max-http-header-size`][] for requests received by this server, i.e.
@@ -2203,7 +2203,7 @@ changes:
     `hostname` will be used if both `host` and `hostname` are specified.
   * `insecureHTTPParser` {boolean} Use an insecure HTTP parser that accepts
     invalid HTTP headers when `true`. Using the insecure parser should be
-    avoided, see [`--insecure-http-parser`][] for more information.
+    avoided. See [`--insecure-http-parser`][] for more information.
     **Default:** `false`
   * `localAddress` {string} Local interface to bind for network connections.
   * `lookup` {Function} Custom lookup function. **Default:** [`dns.lookup()`][].