http: improved timeout defaults handling

Co-authored-by: Luigi Pinca <[email protected]> PR-URL: #45778 Fixes: #43355 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Santiago Gimeno <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · Jan 31, 2023 · 3c0c5e0 · 3c0c5e0
1 parent 83875f4
commit 3c0c5e0
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 4 deletions.
diff --git a/doc/api/http.md b/doc/api/http.md
@@ -1502,9 +1502,13 @@ or waiting for a response.
 added:
  - v11.3.0
  - v10.14.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/45778
+    description: The default is now set to the minimum between 60000 (60 seconds) or `requestTimeout`.
 -->
 
-* {number} **Default:** `60000`
+* {number} **Default:** The minimum between [`server.requestTimeout`][] or `60000`.
 
 Limit the amount of time the parser will wait to receive the complete HTTP
 headers.

diff --git a/lib/_http_server.js b/lib/_http_server.js
@@ -24,6 +24,7 @@
 const {
   ArrayIsArray,
   Error,
+  MathMin,
   ObjectKeys,
   ObjectSetPrototypeOf,
   RegExpPrototypeExec,
@@ -451,11 +452,11 @@ function storeHTTPOptions(options) {
     validateInteger(headersTimeout, 'headersTimeout', 0);
     this.headersTimeout = headersTimeout;
   } else {
-    this.headersTimeout = 60_000; // 60 seconds
+    this.headersTimeout = MathMin(60_000, this.requestTimeout); // Minimum between 60 seconds or requestTimeout
   }
 
-  if (this.requestTimeout > 0 && this.headersTimeout > 0 && this.headersTimeout >= this.requestTimeout) {
-    throw new codes.ERR_OUT_OF_RANGE('headersTimeout', '< requestTimeout', headersTimeout);
+  if (this.requestTimeout > 0 && this.headersTimeout > 0 && this.headersTimeout > this.requestTimeout) {
+    throw new codes.ERR_OUT_OF_RANGE('headersTimeout', '<= requestTimeout', headersTimeout);
   }
 
   const keepAliveTimeout = options.keepAliveTimeout;

diff --git a/test/parallel/test-http-server-timeouts-validation.js b/test/parallel/test-http-server-timeouts-validation.js
@@ -0,0 +1,50 @@
+'use strict';
+
+require('../common');
+const assert = require('assert');
+const { createServer } = require('http');
+
+// This test validates that the HTTP server timeouts are properly validated and set.
+
+{
+  const server = createServer();
+  assert.strictEqual(server.headersTimeout, 60000);
+  assert.strictEqual(server.requestTimeout, 300000);
+}
+
+{
+  const server = createServer({ headersTimeout: 10000, requestTimeout: 20000 });
+  assert.strictEqual(server.headersTimeout, 10000);
+  assert.strictEqual(server.requestTimeout, 20000);
+}
+
+{
+  const server = createServer({ headersTimeout: 10000, requestTimeout: 10000 });
+  assert.strictEqual(server.headersTimeout, 10000);
+  assert.strictEqual(server.requestTimeout, 10000);
+}
+
+{
+  const server = createServer({ headersTimeout: 10000 });
+  assert.strictEqual(server.headersTimeout, 10000);
+  assert.strictEqual(server.requestTimeout, 300000);
+}
+
+{
+  const server = createServer({ requestTimeout: 20000 });
+  assert.strictEqual(server.headersTimeout, 20000);
+  assert.strictEqual(server.requestTimeout, 20000);
+}
+
+{
+  const server = createServer({ requestTimeout: 100000 });
+  assert.strictEqual(server.headersTimeout, 60000);
+  assert.strictEqual(server.requestTimeout, 100000);
+}
+
+{
+  assert.throws(
+    () => createServer({ headersTimeout: 10000, requestTimeout: 1000 }),
+    { code: 'ERR_OUT_OF_RANGE' }
+  );
+}