From 391dc74a10426034c99ca0e2cd995864e9bc36c3 Mon Sep 17 00:00:00 2001
From: Deokjin Kim <deokjin81.kim@gmail.com>
Date: Wed, 25 Jan 2023 17:52:33 +0900
Subject: [PATCH] http: throw error if options of http.Server is array

If options of http.Server is array, throw ERR_INVALID_ARG_TYPE.

Refs: https://github.com/nodejs/node/pull/24176
PR-URL: https://github.com/nodejs/node/pull/46283
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
---
 lib/_http_server.js                | 17 ++++++++---------
 lib/https.js                       | 26 ++++++++++++++++----------
 test/parallel/test-http-server.js  |  5 +----
 test/parallel/test-https-simple.js |  9 +++++++++
 4 files changed, 34 insertions(+), 23 deletions(-)

diff --git a/lib/_http_server.js b/lib/_http_server.js
index 4f16ab7b403..7e96a0b2bb3 100644
--- a/lib/_http_server.js
+++ b/lib/_http_server.js
@@ -75,10 +75,12 @@ const {
   ERR_HTTP_HEADERS_SENT,
   ERR_HTTP_INVALID_STATUS_CODE,
   ERR_HTTP_SOCKET_ENCODING,
-  ERR_INVALID_ARG_TYPE,
   ERR_INVALID_ARG_VALUE,
   ERR_INVALID_CHAR
 } = codes;
+const {
+  kEmptyObject,
+} = require('internal/util');
 const {
   validateInteger,
   validateBoolean,
@@ -433,9 +435,6 @@ function storeHTTPOptions(options) {
     validateBoolean(insecureHTTPParser, 'options.insecureHTTPParser');
   this.insecureHTTPParser = insecureHTTPParser;
 
-  if (options.noDelay === undefined)
-    options.noDelay = true;
-
   const requestTimeout = options.requestTimeout;
   if (requestTimeout !== undefined) {
     validateInteger(requestTimeout, 'requestTimeout', 0);
@@ -502,17 +501,17 @@ function Server(options, requestListener) {
 
   if (typeof options === 'function') {
     requestListener = options;
-    options = {};
-  } else if (options == null || typeof options === 'object') {
-    options = { ...options };
+    options = kEmptyObject;
+  } else if (options == null) {
+    options = kEmptyObject;
   } else {
-    throw new ERR_INVALID_ARG_TYPE('options', 'object', options);
+    validateObject(options, 'options');
   }
 
   storeHTTPOptions.call(this, options);
   net.Server.call(
     this,
-    { allowHalfOpen: true, noDelay: options.noDelay,
+    { allowHalfOpen: true, noDelay: options.noDelay ?? true,
       keepAlive: options.keepAlive,
       keepAliveInitialDelay: options.keepAliveInitialDelay });
 
diff --git a/lib/https.js b/lib/https.js
index e871b35bcac..c3ecbc45ee4 100644
--- a/lib/https.js
+++ b/lib/https.js
@@ -53,25 +53,31 @@ let debug = require('internal/util/debuglog').debuglog('https', (fn) => {
   debug = fn;
 });
 const { URL, urlToHttpOptions, searchParamsSymbol } = require('internal/url');
+const { validateObject } = require('internal/validators');
 
 function Server(opts, requestListener) {
   if (!(this instanceof Server)) return new Server(opts, requestListener);
 
   if (typeof opts === 'function') {
     requestListener = opts;
-    opts = undefined;
-  }
-  opts = { ...opts };
-
-  if (!opts.ALPNProtocols) {
-    // http/1.0 is not defined as Protocol IDs in IANA
-    // https://www.iana.org/assignments/tls-extensiontype-values
-    //       /tls-extensiontype-values.xhtml#alpn-protocol-ids
-    opts.ALPNProtocols = ['http/1.1'];
+    opts = kEmptyObject;
+  } else if (opts == null) {
+    opts = kEmptyObject;
+  } else {
+    validateObject(opts, 'options');
   }
 
   FunctionPrototypeCall(storeHTTPOptions, this, opts);
-  FunctionPrototypeCall(tls.Server, this, opts, _connectionListener);
+  FunctionPrototypeCall(tls.Server, this,
+                        {
+                          noDelay: true,
+                          // http/1.0 is not defined as Protocol IDs in IANA
+                          // https://www.iana.org/assignments/tls-extensiontype-values
+                          //       /tls-extensiontype-values.xhtml#alpn-protocol-ids
+                          ALPNProtocols: ['http/1.1'],
+                          ...opts,
+                        },
+                        _connectionListener);
 
   this.httpAllowHalfOpen = false;
 
diff --git a/test/parallel/test-http-server.js b/test/parallel/test-http-server.js
index a26912c409a..b93258b198a 100644
--- a/test/parallel/test-http-server.js
+++ b/test/parallel/test-http-server.js
@@ -27,10 +27,7 @@ const http = require('http');
 const url = require('url');
 const qs = require('querystring');
 
-// TODO: documentation does not allow Array as an option, so testing that
-// should fail, but currently http.Server does not typecheck further than
-// if `option` is `typeof object` - so we don't test that here right now
-const invalid_options = [ 'foo', 42, true ];
+const invalid_options = [ 'foo', 42, true, [] ];
 
 invalid_options.forEach((option) => {
   assert.throws(() => {
diff --git a/test/parallel/test-https-simple.js b/test/parallel/test-https-simple.js
index 89707cc6468..a65883162f6 100644
--- a/test/parallel/test-https-simple.js
+++ b/test/parallel/test-https-simple.js
@@ -44,6 +44,15 @@ const serverCallback = common.mustCall(function(req, res) {
   res.end(body);
 });
 
+const invalid_options = [ 'foo', 42, true, [] ];
+invalid_options.forEach((option) => {
+  assert.throws(() => {
+    new https.Server(option);
+  }, {
+    code: 'ERR_INVALID_ARG_TYPE',
+  });
+});
+
 const server = https.createServer(options, serverCallback);
 
 server.listen(0, common.mustCall(() => {