http,https: align server option of https with http

Fixes: #38954 PR-URL: #38992 Refs: #30570 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
nodejs · Jun 26, 2021 · 8a8c7f4 · 8a8c7f4
1 parent 8954e03
commit 8a8c7f4
Show file tree

Hide file tree

Showing 4 changed files with 268 additions and 18 deletions.
diff --git a/lib/_http_server.js b/lib/_http_server.js
@@ -351,18 +351,7 @@ function writeHead(statusCode, reason, obj) {
 // Docs-only deprecated: DEP0063
 ServerResponse.prototype.writeHeader = ServerResponse.prototype.writeHead;
 
-function Server(options, requestListener) {
-  if (!(this instanceof Server)) return new Server(options, requestListener);
-
-  if (typeof options === 'function') {
-    requestListener = options;
-    options = {};
-  } else if (options == null || typeof options === 'object') {
-    options = { ...options };
-  } else {
-    throw new ERR_INVALID_ARG_TYPE('options', 'object', options);
-  }
-
+function storeHTTPOptions(options) {
   this[kIncomingMessage] = options.IncomingMessage || IncomingMessage;
   this[kServerResponse] = options.ServerResponse || ServerResponse;
 
@@ -375,7 +364,21 @@ function Server(options, requestListener) {
   if (insecureHTTPParser !== undefined)
     validateBoolean(insecureHTTPParser, 'options.insecureHTTPParser');
   this.insecureHTTPParser = insecureHTTPParser;
+}
+
+function Server(options, requestListener) {
+  if (!(this instanceof Server)) return new Server(options, requestListener);
+
+  if (typeof options === 'function') {
+    requestListener = options;
+    options = {};
+  } else if (options == null || typeof options === 'object') {
+    options = { ...options };
+  } else {
+    throw new ERR_INVALID_ARG_TYPE('options', 'object', options);
+  }
 
+  storeHTTPOptions.call(this, options);
   net.Server.call(this, { allowHalfOpen: true });
 
   if (requestListener) {
@@ -991,6 +994,7 @@ module.exports = {
   STATUS_CODES,
   Server,
   ServerResponse,
+  storeHTTPOptions,
   _connectionListener: connectionListener,
   kServerResponse
 };
diff --git a/lib/https.js b/lib/https.js
@@ -40,16 +40,14 @@ const tls = require('tls');
 const { Agent: HttpAgent } = require('_http_agent');
 const {
   Server: HttpServer,
+  storeHTTPOptions,
   _connectionListener,
-  kServerResponse
 } = require('_http_server');
 const { ClientRequest } = require('_http_client');
 let debug = require('internal/util/debuglog').debuglog('https', (fn) => {
   debug = fn;
 });
 const { URL, urlToHttpOptions, searchParamsSymbol } = require('internal/url');
-const { IncomingMessage, ServerResponse } = require('http');
-const { kIncomingMessage } = require('_http_common');
 
 function Server(opts, requestListener) {
   if (!(this instanceof Server)) return new Server(opts, requestListener);
@@ -67,9 +65,7 @@ function Server(opts, requestListener) {
     opts.ALPNProtocols = ['http/1.1'];
   }
 
-  this[kIncomingMessage] = opts.IncomingMessage || IncomingMessage;
-  this[kServerResponse] = opts.ServerResponse || ServerResponse;
-
+  FunctionPrototypeCall(storeHTTPOptions, this, opts);
   FunctionPrototypeCall(tls.Server, this, opts, _connectionListener);
 
   this.httpAllowHalfOpen = false;

diff --git a/test/parallel/test-https-insecure-parse-per-stream.js b/test/parallel/test-https-insecure-parse-per-stream.js
@@ -0,0 +1,131 @@
+'use strict';
+const common = require('../common');
+if (!common.hasCrypto) {
+  common.skip('missing crypto');
+}
+
+const fixtures = require('../common/fixtures');
+const assert = require('assert');
+const https = require('https');
+const MakeDuplexPair = require('../common/duplexpair');
+const tls = require('tls');
+const { finished } = require('stream');
+
+const certFixture = {
+  key: fixtures.readKey('agent1-key.pem'),
+  cert: fixtures.readKey('agent1-cert.pem'),
+  ca: fixtures.readKey('ca1-cert.pem'),
+};
+
+
+// Test that setting the `insecureHTTPParse` option works on a per-stream-basis.
+
+// Test 1: The server sends an invalid header.
+{
+  const { clientSide, serverSide } = MakeDuplexPair();
+
+  const req = https.request({
+    rejectUnauthorized: false,
+    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 = https.request({
+    rejectUnauthorized: false,
+    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 = https.createServer(
+    { insecureHTTPParser: true,
+      ...certFixture },
+    common.mustCall((req, res) => {
+      res.statusCode = 200;
+      res.setHeader('Content-Type', 'text/plain');
+      res.end(testData);
+    }));
+
+  server.on('clientError', common.mustNotCall());
+
+  server.listen(0, common.mustCall(() => {
+    const client = tls.connect({
+      port: server.address().port,
+      rejectUnauthorized: false
+    });
+    client.write(
+      'GET / HTTP/1.1\r\n' +
+      'Hello: foo\x08foo\r\n' +
+      '\r\n\r\n');
+    client.end();
+
+    client.on('data', () => {});
+    finished(client, common.mustCall(() => {
+      server.close();
+    }));
+  }));
+}
+
+// Test 4: The same as Test 3 except without the option, to make sure it fails.
+{
+  const server = https.createServer(
+    { ...certFixture },
+    common.mustNotCall());
+
+  server.on('clientError', common.mustCall());
+
+  server.listen(0, common.mustCall(() => {
+    const client = tls.connect({
+      port: server.address().port,
+      rejectUnauthorized: false
+    });
+    client.write(
+      'GET / HTTP/1.1\r\n' +
+      'Hello: foo\x08foo\r\n' +
+      '\r\n\r\n');
+    client.end();
+
+    client.on('data', () => {});
+    finished(client, common.mustCall(() => {
+      server.close();
+    }));
+  }));
+}
+
+// Test 5: Invalid argument type
+{
+  assert.throws(
+    () => https.request({ insecureHTTPParser: 0 }, common.mustNotCall()),
+    common.expectsError({
+      code: 'ERR_INVALID_ARG_TYPE',
+      message: 'The "options.insecureHTTPParser" property must be of' +
+      ' type boolean. Received type number (0)'
+    })
+  );
+}
diff --git a/test/parallel/test-https-max-header-size-per-stream.js b/test/parallel/test-https-max-header-size-per-stream.js
@@ -0,0 +1,119 @@
+'use strict';
+const common = require('../common');
+
+if (!common.hasCrypto) {
+  common.skip('missing crypto');
+}
+
+const fixtures = require('../common/fixtures');
+const assert = require('assert');
+const https = require('https');
+const http = require('http');
+const tls = require('tls');
+const MakeDuplexPair = require('../common/duplexpair');
+const { finished } = require('stream');
+
+const certFixture = {
+  key: fixtures.readKey('agent1-key.pem'),
+  cert: fixtures.readKey('agent1-cert.pem'),
+  ca: fixtures.readKey('ca1-cert.pem'),
+};
+
+
+// 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 = https.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 = https.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 = https.createServer(
+    { maxHeaderSize: http.maxHeaderSize * 4,
+      ...certFixture },
+    common.mustCall((req, res) => {
+      res.statusCode = 200;
+      res.setHeader('Content-Type', 'text/plain');
+      res.end(testData);
+    }));
+
+  server.on('clientError', common.mustNotCall());
+
+  server.listen(0, common.mustCall(() => {
+    const client = tls.connect({
+      port: server.address().port,
+      rejectUnauthorized: false
+    });
+    client.write(
+      'GET / HTTP/1.1\r\n' +
+      'Hello: ' + 'A'.repeat(http.maxHeaderSize * 3) + '\r\n' +
+      '\r\n\r\n');
+    client.end();
+
+    client.on('data', () => {});
+    finished(client, common.mustCall(() => {
+      server.close();
+    }));
+  }));
+}
+
+// Test 4: The same as Test 3 except without the option, to make sure it fails.
+{
+  const server = https.createServer({ ...certFixture }, common.mustNotCall());
+
+  // clientError may be emitted multiple times when header is larger than
+  // maxHeaderSize.
+  server.on('clientError', common.mustCallAtLeast(() => {}, 1));
+
+  server.listen(0, common.mustCall(() => {
+    const client = tls.connect({
+      port: server.address().port,
+      rejectUnauthorized: false
+    });
+    client.write(
+      'GET / HTTP/1.1\r\n' +
+      'Hello: ' + 'A'.repeat(http.maxHeaderSize * 3) + '\r\n' +
+      '\r\n\r\n');
+    client.end();
+
+    client.on('data', () => {});
+    finished(client, common.mustCall(() => {
+      server.close();
+    }));
+  }));
+}