From 3c0c5e0567d3c2a1efea8ed79e37c48855dd0768 Mon Sep 17 00:00:00 2001 From: Paolo Insogna Date: Sat, 24 Dec 2022 09:28:46 +0100 Subject: [PATCH] http: improved timeout defaults handling Co-authored-by: Luigi Pinca PR-URL: https://github.com/nodejs/node/pull/45778 Fixes: https://github.com/nodejs/node/issues/43355 Reviewed-By: Matteo Collina Reviewed-By: Santiago Gimeno Reviewed-By: Antoine du Hamel Reviewed-By: Yagiz Nizipli Reviewed-By: Luigi Pinca Reviewed-By: James M Snell --- doc/api/http.md | 6 ++- lib/_http_server.js | 7 +-- .../test-http-server-timeouts-validation.js | 50 +++++++++++++++++++ 3 files changed, 59 insertions(+), 4 deletions(-) create mode 100644 test/parallel/test-http-server-timeouts-validation.js diff --git a/doc/api/http.md b/doc/api/http.md index fc56b0625f2dea..69bafb0a799768 100644 --- 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 index 506369114b7aca..dffce58d6b4016 100644 --- 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 new file mode 100644 index 00000000000000..681a8bc3210fba --- /dev/null +++ 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' } + ); +}