diff --git a/src/js/node/_http_client.ts b/src/js/node/_http_client.ts index 29f3abf068..7402d1d428 100644 --- a/src/js/node/_http_client.ts +++ b/src/js/node/_http_client.ts @@ -4,6 +4,7 @@ const { checkIsHttpToken, validateFunction, validateInteger, validateBoolean } = const { urlToHttpOptions } = require("internal/url"); const { isValidTLSArray } = require("internal/tls"); const { validateHeaderName } = require("node:_http_common"); +const { getTimerDuration } = require("internal/timers"); const { kBodyChunks, abortedSymbol, @@ -40,7 +41,6 @@ const { reqSymbol, callCloseCallback, emitCloseNTAndComplete, - validateMsecs, ConnResetException, } = require("internal/http"); @@ -799,8 +799,9 @@ function ClientRequest(input, options, cb) { this[kHost] = host; this[kProtocol] = protocol; - const timeout = options.timeout; - if (timeout !== undefined && timeout !== 0) { + if (options.timeout !== undefined) { + const timeout = getTimerDuration(options.timeout, "timeout"); + this.timeout = timeout; this.setTimeout(timeout, undefined); } @@ -918,17 +919,18 @@ function ClientRequest(input, options, cb) { this.removeAllListeners("timeout"); } }; +} - const onTimeout = () => { - this[kTimeoutTimer] = undefined; - this[kAbortController]?.abort(); - this.emit("timeout"); - }; +const ClientRequestPrototype = { + constructor: ClientRequest, + __proto__: OutgoingMessage.prototype, - this.setTimeout = (msecs, callback) => { - if (this.destroyed) return this; + setTimeout(msecs, callback) { + if (this.destroyed) { + return this; + } - this.timeout = msecs = validateMsecs(msecs, "timeout"); + this.timeout = msecs = getTimerDuration(msecs, "msecs"); // Attempt to clear an existing timer in both cases - // even if it will be rescheduled we don't want to leak an existing timer. @@ -942,7 +944,11 @@ function ClientRequest(input, options, cb) { this[kTimeoutTimer] = undefined; } else { - this[kTimeoutTimer] = setTimeout(onTimeout, msecs).unref(); + this[kTimeoutTimer] = setTimeout(() => { + this[kTimeoutTimer] = undefined; + this[kAbortController]?.abort(); + this.emit("timeout"); + }, msecs).unref(); if (callback !== undefined) { validateFunction(callback, "callback"); @@ -951,12 +957,11 @@ function ClientRequest(input, options, cb) { } return this; - }; -} + }, -const ClientRequestPrototype = { - constructor: ClientRequest, - __proto__: OutgoingMessage.prototype, + clearTimeout(cb) { + this.setTimeout(0, cb); + }, get path() { return this[kPath]; diff --git a/src/js/node/_http_outgoing.ts b/src/js/node/_http_outgoing.ts index 8f2c8c8277..093598bcf1 100644 --- a/src/js/node/_http_outgoing.ts +++ b/src/js/node/_http_outgoing.ts @@ -1,5 +1,5 @@ const { Stream } = require("internal/stream"); -const { validateFunction, isUint8Array, validateString } = require("internal/validators"); +const { isUint8Array, validateString } = require("internal/validators"); const { deprecate } = require("node:util"); const ObjectDefineProperty = Object.defineProperty; const ObjectKeys = Object.keys; @@ -13,8 +13,6 @@ const { kEmitState, ClientRequestEmitState, kEmptyObject, - validateMsecs, - timeoutTimerSymbol, kHandle, getHeader, setHeader, @@ -367,28 +365,16 @@ const OutgoingMessagePrototype = { }, setTimeout(msecs, callback) { - if (this.destroyed) return this; + if (this.callback) { + this.emit("timeout", callback); + } - this.timeout = msecs = validateMsecs(msecs, "timeout"); - - // Attempt to clear an existing timer in both cases - - // even if it will be rescheduled we don't want to leak an existing timer. - clearTimeout(this[timeoutTimerSymbol]); - - if (msecs === 0) { - if (callback != null) { - if (!$isCallable(callback)) validateFunction(callback, "callback"); - this.removeListener("timeout", callback); - } - - this[timeoutTimerSymbol] = undefined; + if (!this[fakeSocketSymbol]) { + this.once("socket", function socketSetTimeoutOnConnect(socket) { + socket.setTimeout(msecs, callback); + }); } else { - this[timeoutTimerSymbol] = setTimeout(onTimeout.bind(this), msecs).unref(); - - if (callback != null) { - if (!$isCallable(callback)) validateFunction(callback, "callback"); - this.once("timeout", callback); - } + this.socket.setTimeout(msecs); } return this; @@ -592,17 +578,6 @@ ObjectDefineProperty(OutgoingMessage.prototype, "_headers", { }); $setPrototypeDirect.$call(OutgoingMessage, Stream); -function onTimeout() { - this[timeoutTimerSymbol] = undefined; - this[kAbortController]?.abort(); - const handle = this[kHandle]; - - this.emit("timeout"); - if (handle) { - handle.abort(); - } -} - export default { OutgoingMessage, FakeSocket, diff --git a/test/js/node/test/parallel/test-http-outgoing-settimeout.js b/test/js/node/test/parallel/test-http-outgoing-settimeout.js new file mode 100644 index 0000000000..3065c53f11 --- /dev/null +++ b/test/js/node/test/parallel/test-http-outgoing-settimeout.js @@ -0,0 +1,30 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); + +const { OutgoingMessage } = require('http'); + +{ + // Tests for settimeout method with socket + const expectedMsecs = 42; + const outgoingMessage = new OutgoingMessage(); + outgoingMessage.socket = { + setTimeout: common.mustCall((msecs) => { + assert.strictEqual(msecs, expectedMsecs); + }) + }; + outgoingMessage.setTimeout(expectedMsecs); +} + +{ + // Tests for settimeout method without socket + const expectedMsecs = 23; + const outgoingMessage = new OutgoingMessage(); + outgoingMessage.setTimeout(expectedMsecs); + + outgoingMessage.emit('socket', { + setTimeout: common.mustCall((msecs) => { + assert.strictEqual(msecs, expectedMsecs); + }) + }); +} diff --git a/test/js/node/test/parallel/test-http-timeout-client-warning.js b/test/js/node/test/parallel/test-http-timeout-client-warning.js new file mode 100644 index 0000000000..f11515b95f --- /dev/null +++ b/test/js/node/test/parallel/test-http-timeout-client-warning.js @@ -0,0 +1,21 @@ +'use strict'; +const common = require('../common'); +const http = require('http'); +const assert = require('assert'); + +// Checks that the setTimeout duration overflow warning is emitted +// synchronously and therefore contains a meaningful stacktrace. + +process.on('warning', common.mustCall((warning) => { + assert(warning.stack.includes(__filename)); +})); + +const server = http.createServer((req, resp) => resp.end()); +server.listen(common.mustCall(() => { + http.request(`http://localhost:${server.address().port}`) + .setTimeout(2 ** 40) + .on('response', common.mustCall(() => { + server.close(); + })) + .end(); +})); diff --git a/test/js/node/test/parallel/test-http-timeout-overflow.js b/test/js/node/test/parallel/test-http-timeout-overflow.js new file mode 100644 index 0000000000..e95405bc8b --- /dev/null +++ b/test/js/node/test/parallel/test-http-timeout-overflow.js @@ -0,0 +1,51 @@ +// Copyright Joyent, Inc. and other Node contributors. +// +// Permission is hereby granted, free of charge, to any person obtaining a +// copy of this software and associated documentation files (the +// "Software"), to deal in the Software without restriction, including +// without limitation the rights to use, copy, modify, merge, publish, +// distribute, sublicense, and/or sell copies of the Software, and to permit +// persons to whom the Software is furnished to do so, subject to the +// following conditions: +// +// The above copyright notice and this permission notice shall be included +// in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN +// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE +// USE OR OTHER DEALINGS IN THE SOFTWARE. + +'use strict'; +const common = require('../common'); +const http = require('http'); + +const server = http.createServer(common.mustCall(function(req, res) { + res.writeHead(200, { 'Content-Type': 'text/plain' }); + res.end('OK'); +})); + +server.listen(0, function() { + function callback() {} + + const req = http.request({ + port: this.address().port, + path: '/', + agent: false + }, function(res) { + req.clearTimeout(callback); + + res.on('end', common.mustCall(function() { + server.close(); + })); + + res.resume(); + }); + + // Overflow signed int32 + req.setTimeout(0xffffffff, callback); + req.end(); +}); diff --git a/test/js/node/test/parallel/test-http-timeout.js b/test/js/node/test/parallel/test-http-timeout.js new file mode 100644 index 0000000000..c3f0e542d2 --- /dev/null +++ b/test/js/node/test/parallel/test-http-timeout.js @@ -0,0 +1,62 @@ +// Copyright Joyent, Inc. and other Node contributors. +// +// Permission is hereby granted, free of charge, to any person obtaining a +// copy of this software and associated documentation files (the +// "Software"), to deal in the Software without restriction, including +// without limitation the rights to use, copy, modify, merge, publish, +// distribute, sublicense, and/or sell copies of the Software, and to permit +// persons to whom the Software is furnished to do so, subject to the +// following conditions: +// +// The above copyright notice and this permission notice shall be included +// in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN +// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE +// USE OR OTHER DEALINGS IN THE SOFTWARE. + +'use strict'; +const common = require('../common'); + +const http = require('http'); +const Countdown = require('../common/countdown'); +const MAX_COUNT = 11; + +const server = http.createServer(common.mustCall(function(req, res) { + res.writeHead(200, { 'Content-Type': 'text/plain' }); + res.end('OK'); +}, MAX_COUNT)); + +const agent = new http.Agent({ maxSockets: 1 }); +const countdown = new Countdown(MAX_COUNT, () => server.close()); + +server.listen(0, function() { + + for (let i = 0; i < MAX_COUNT; ++i) { + createRequest().end(); + } + + function callback() {} + + function createRequest() { + const req = http.request( + { port: server.address().port, path: '/', agent: agent }, + function(res) { + req.clearTimeout(callback); + + res.on('end', function() { + countdown.dec(); + }); + + res.resume(); + } + ); + + req.setTimeout(1000, callback); + return req; + } +});