diff options
author | Paolo Insogna <paolo@cowtech.it> | 2022-04-13 17:47:59 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-04-13 17:47:59 +0300 |
commit | 3caa2c1a005652fdb3e896ef940cd5ffe5fdff10 (patch) | |
tree | f8e9e9bfe7c95d5633c86e021518f487a9eadb8f /lib | |
parent | 9d6af7d1fe66afdcb781fb5bad37b4cb4d396f0e (diff) |
http: refactor headersTimeout and requestTimeout logic
PR-URL: https://github.com/nodejs/node/pull/41263
Fixes: https://github.com/nodejs/node/issues/33440
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Diffstat (limited to 'lib')
-rw-r--r-- | lib/_http_client.js | 3 | ||||
-rw-r--r-- | lib/_http_common.js | 12 | ||||
-rw-r--r-- | lib/_http_server.js | 132 | ||||
-rw-r--r-- | lib/https.js | 5 |
4 files changed, 72 insertions, 80 deletions
diff --git a/lib/_http_client.js b/lib/_http_client.js index 42b4683d141..a4f7a255a99 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -735,8 +735,7 @@ function tickOnSocket(req, socket) { parser.initialize(HTTPParser.RESPONSE, new HTTPClientAsyncResource('HTTPINCOMINGMESSAGE', req), req.maxHeaderSize || 0, - lenient ? kLenientAll : kLenientNone, - 0); + lenient ? kLenientAll : kLenientNone); parser.socket = socket; parser.outgoing = req; req.parser = parser; diff --git a/lib/_http_common.js b/lib/_http_common.js index 796deeff055..2055ca205b8 100644 --- a/lib/_http_common.js +++ b/lib/_http_common.js @@ -40,12 +40,7 @@ const { readStop } = incoming; -let debug = require('internal/util/debuglog').debuglog('http', (fn) => { - debug = fn; -}); - const kIncomingMessage = Symbol('IncomingMessage'); -const kRequestTimeout = Symbol('RequestTimeout'); const kOnMessageBegin = HTTPParser.kOnMessageBegin | 0; const kOnHeaders = HTTPParser.kOnHeaders | 0; const kOnHeadersComplete = HTTPParser.kOnHeadersComplete | 0; @@ -102,12 +97,6 @@ function parserOnHeadersComplete(versionMajor, versionMinor, headers, method, incoming.url = url; incoming.upgrade = upgrade; - if (socket) { - debug('requestTimeout timer moved to req'); - incoming[kRequestTimeout] = incoming.socket[kRequestTimeout]; - incoming.socket[kRequestTimeout] = undefined; - } - let n = headers.length; // If parser.maxHeaderPairs <= 0 assume that there's no limit. @@ -273,7 +262,6 @@ module.exports = { methods, parsers, kIncomingMessage, - kRequestTimeout, HTTPParser, isLenient, prepareError, diff --git a/lib/_http_server.js b/lib/_http_server.js index 00c810e51df..84be32c78c4 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -27,6 +27,7 @@ const { ObjectKeys, ObjectSetPrototypeOf, RegExpPrototypeTest, + ReflectApply, Symbol, SymbolFor, } = primordials; @@ -40,12 +41,12 @@ const { continueExpression, chunkExpression, kIncomingMessage, - kRequestTimeout, HTTPParser, isLenient, _checkInvalidHeaderChar: checkInvalidHeaderChar, prepareError, } = require('_http_common'); +const { ConnectionsList } = internalBinding('http_parser'); const { OutgoingMessage } = require('_http_outgoing'); const { kOutHeaders, @@ -79,8 +80,7 @@ const { DTRACE_HTTP_SERVER_REQUEST, DTRACE_HTTP_SERVER_RESPONSE } = require('internal/dtrace'); -const { setTimeout, clearTimeout } = require('timers'); - +const { setInterval, clearInterval } = require('timers'); let debug = require('internal/util/debuglog').debuglog('http', (fn) => { debug = fn; }); @@ -162,11 +162,12 @@ const STATUS_CODES = { 511: 'Network Authentication Required' // RFC 6585 6 }; -const kOnMessageBegin = HTTPParser.kOnMessageBegin | 0; const kOnExecute = HTTPParser.kOnExecute | 0; const kOnTimeout = HTTPParser.kOnTimeout | 0; const kLenientAll = HTTPParser.kLenientAll | 0; const kLenientNone = HTTPParser.kLenientNone | 0; +const kConnections = Symbol('http.server.connections'); +const kConnectionsCheckingInterval = Symbol('http.server.connectionsCheckingInterval'); class HTTPServerAsyncResource { constructor(type, socket) { @@ -367,6 +368,51 @@ function storeHTTPOptions(options) { if (options.noDelay === undefined) options.noDelay = true; + + const requestTimeout = options.requestTimeout; + if (requestTimeout !== undefined) { + validateInteger(requestTimeout, 'requestTimeout', 0); + this.requestTimeout = requestTimeout; + } else { + this.requestTimeout = 300_000; // 5 minutes + } + + const headersTimeout = options.headersTimeout; + if (headersTimeout !== undefined) { + validateInteger(headersTimeout, 'headersTimeout', 0); + this.headersTimeout = headersTimeout; + } else { + this.headersTimeout = 60_000; // 60 seconds + } + + 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; + if (keepAliveTimeout !== undefined) { + validateInteger(keepAliveTimeout, 'keepAliveTimeout', 0); + this.keepAliveTimeout = keepAliveTimeout; + } else { + this.keepAliveTimeout = 5_000; // 5 seconds; + } + + const connectionsCheckingInterval = options.connectionsCheckingInterval; + if (connectionsCheckingInterval !== undefined) { + validateInteger(connectionsCheckingInterval, 'connectionsCheckingInterval', 0); + this.connectionsCheckingInterval = connectionsCheckingInterval; + } else { + this.connectionsCheckingInterval = 30_000; // 30 seconds + } +} + +function setupConnectionsTracking(server) { + // Start connection handling + server[kConnections] = new ConnectionsList(); + if (server.headersTimeout > 0 || server.requestTimeout > 0) { + server[kConnectionsCheckingInterval] = + setInterval(checkConnections.bind(server), server.connectionsCheckingInterval).unref(); + } } function Server(options, requestListener) { @@ -400,15 +446,17 @@ function Server(options, requestListener) { this.on('connection', connectionListener); this.timeout = 0; - this.keepAliveTimeout = 5000; this.maxHeadersCount = null; this.maxRequestsPerSocket = 0; - this.headersTimeout = 60 * 1000; // 60 seconds - this.requestTimeout = 0; + setupConnectionsTracking(this); } ObjectSetPrototypeOf(Server.prototype, net.Server.prototype); ObjectSetPrototypeOf(Server, net.Server); +Server.prototype.close = function() { + clearInterval(this[kConnectionsCheckingInterval]); + ReflectApply(net.Server.prototype.close, this, arguments); +}; Server.prototype.setTimeout = function setTimeout(msecs, callback) { this.timeout = msecs; @@ -440,6 +488,18 @@ Server.prototype[EE.captureRejectionSymbol] = function(err, event, ...args) { } }; +function checkConnections() { + const expired = this[kConnections].expired(this.headersTimeout, this.requestTimeout); + + for (let i = 0; i < expired.length; i++) { + const socket = expired[i].socket; + + if (socket) { + onRequestTimeout(socket); + } + } +} + function connectionListener(socket) { defaultTriggerAsyncIdScope( getOrSetAsyncId(socket), connectionListenerInternal, this, socket @@ -473,7 +533,7 @@ function connectionListenerInternal(server, socket) { new HTTPServerAsyncResource('HTTPINCOMINGMESSAGE', socket), server.maxHeaderSize || 0, lenient ? kLenientAll : kLenientNone, - server.headersTimeout || 0, + server[kConnections], ); parser.socket = socket; socket.parser = parser; @@ -539,17 +599,6 @@ function connectionListenerInternal(server, socket) { onParserTimeout.bind(undefined, server, socket); - // When receiving new requests on the same socket (pipelining or keep alive) - // make sure the requestTimeout is active. - parser[kOnMessageBegin] = - setRequestTimeout.bind(undefined, - server, socket); - - // This protects from DOS attack where an attacker establish the connection - // without sending any data on applications where server.timeout is left to - // the default value of zero. - setRequestTimeout(server, socket); - socket._paused = false; } @@ -632,7 +681,6 @@ function socketOnData(server, socket, parser, state, d) { } function onRequestTimeout(socket) { - socket[kRequestTimeout] = undefined; // socketOnError has additional logic and will call socket.destroy(err). socketOnError.call(socket, new ERR_HTTP_REQUEST_TIMEOUT()); } @@ -727,9 +775,6 @@ function onParserExecuteCommon(server, socket, parser, state, ret, d) { socket.readableFlowing = null; - // Clear the requestTimeout after upgrading the connection. - clearRequestTimeout(req); - server.emit(eventName, req, socket, bodyHead); } else { // Got CONNECT method, but have no handler. @@ -738,11 +783,6 @@ function onParserExecuteCommon(server, socket, parser, state, ret, d) { } else if (parser.incoming && parser.incoming.method === 'PRI') { debug('SERVER got PRI request'); socket.destroy(); - } else { - // When receiving new requests on the same socket (pipelining or keep alive) - // make sure the requestTimeout is active. - parser[kOnMessageBegin] = - setRequestTimeout.bind(undefined, server, socket); } if (socket._paused && socket.parser) { @@ -765,32 +805,6 @@ function clearIncoming(req) { } } -function setRequestTimeout(server, socket) { - // Set the request timeout handler. - if ( - !socket[kRequestTimeout] && - server.requestTimeout && server.requestTimeout > 0 - ) { - debug('requestTimeout timer set'); - socket[kRequestTimeout] = - setTimeout(onRequestTimeout, server.requestTimeout, socket).unref(); - } -} - -function clearRequestTimeout(req) { - if (!req) { - req = this; - } - - if (!req[kRequestTimeout]) { - return; - } - - debug('requestTimeout timer cleared'); - clearTimeout(req[kRequestTimeout]); - req[kRequestTimeout] = undefined; -} - function resOnFinish(req, res, socket, state, server) { if (onResponseFinishChannel.hasSubscribers) { onResponseFinishChannel.publish({ @@ -814,14 +828,6 @@ function resOnFinish(req, res, socket, state, server) { if (!req._consuming && !req._readableState.resumeScheduled) req._dump(); - // Make sure the requestTimeout is cleared before finishing. - // This might occur if the application has sent a response - // without consuming the request body, which would have already - // cleared the timer. - // clearRequestTimeout can be executed even if the timer is not active, - // so this is safe. - clearRequestTimeout(req); - res.detachSocket(socket); clearIncoming(req); process.nextTick(emitCloseNT, res); @@ -955,7 +961,6 @@ function parserOnIncoming(server, socket, state, req, keepAlive) { } if (!handled) { - req.on('end', clearRequestTimeout); server.emit('request', req, res); } @@ -1027,6 +1032,7 @@ module.exports = { STATUS_CODES, Server, ServerResponse, + setupConnectionsTracking, storeHTTPOptions, _connectionListener: connectionListener, kServerResponse diff --git a/lib/https.js b/lib/https.js index 3834a881775..74ce93baaaf 100644 --- a/lib/https.js +++ b/lib/https.js @@ -40,6 +40,7 @@ const tls = require('tls'); const { Agent: HttpAgent } = require('_http_agent'); const { Server: HttpServer, + setupConnectionsTracking, storeHTTPOptions, _connectionListener, } = require('_http_server'); @@ -80,10 +81,8 @@ function Server(opts, requestListener) { }); this.timeout = 0; - this.keepAliveTimeout = 5000; this.maxHeadersCount = null; - this.headersTimeout = 60 * 1000; // 60 seconds - this.requestTimeout = 0; + setupConnectionsTracking(this); } ObjectSetPrototypeOf(Server.prototype, tls.Server.prototype); ObjectSetPrototypeOf(Server, tls.Server); |