From 6208a5f864f93f2faf897fdcb4c13d97e695b57f Mon Sep 17 00:00:00 2001 From: Feng Yu Date: Fri, 1 Jul 2022 07:11:38 +0800 Subject: [PATCH] Revert "http: use Keep-Alive by default in global agents" This reverts commit 4267b92604ad78584244488e7f7508a690cb80d0. --- doc/api/http.md | 16 +---------- doc/api/https.md | 5 ---- lib/_http_agent.js | 23 ++-------------- lib/_http_server.js | 1 - lib/https.js | 2 +- test/async-hooks/test-graph.http.js | 1 - test/parallel/test-http-agent-no-wait.js | 24 ----------------- test/parallel/test-http-client-agent.js | 12 ++++----- ...st-http-client-close-with-default-agent.js | 23 ---------------- .../test-http-client-headers-array.js | 3 +-- .../test-http-client-keep-alive-hint.js | 27 ------------------- .../test-http-client-spurious-aborted.js | 2 +- .../test-http-client-timeout-on-connect.js | 5 +--- test/parallel/test-http-content-length.js | 18 +++++-------- test/parallel/test-http-default-encoding.js | 2 +- test/parallel/test-http-max-headers-count.js | 2 +- ...http-outgoing-message-capture-rejection.js | 3 +-- test/parallel/test-http-raw-headers.js | 14 ++++------ test/parallel/test-http-request-end.js | 2 +- test/parallel/test-http-should-keep-alive.js | 4 --- .../test-http-unix-socket-keep-alive.js | 2 +- .../test-https-agent-session-eviction.js | 2 -- test/parallel/test-https-max-headers-count.js | 2 +- test/parallel/test-stream-destroy.js | 6 ++--- test/parallel/test-tls-over-http-tunnel.js | 2 +- test/parallel/test-tls-set-secure-context.js | 3 +-- test/sequential/test-http-econnrefused.js | 2 +- 27 files changed, 35 insertions(+), 173 deletions(-) delete mode 100644 test/parallel/test-http-agent-no-wait.js delete mode 100644 test/parallel/test-http-client-close-with-default-agent.js delete mode 100644 test/parallel/test-http-client-keep-alive-hint.js diff --git a/doc/api/http.md b/doc/api/http.md index 0c9117202498a8..88f2378421998e 100644 --- a/doc/api/http.md +++ b/doc/api/http.md @@ -1449,20 +1449,11 @@ type other than {net.Socket}. * `callback` {Function} -Stops the server from accepting new connections and closes all connections -connected to this server which are not sending a request or waiting for -a response. -See [`net.Server.close()`][]. +Stops the server from accepting new connections. See [`net.Server.close()`][]. ### `server.closeAllConnections()` @@ -3223,11 +3214,6 @@ server.listen(8000); * {http.Agent} diff --git a/doc/api/https.md b/doc/api/https.md index ce8a47fedded29..bc5c8a246dcdf4 100644 --- a/doc/api/https.md +++ b/doc/api/https.md @@ -309,11 +309,6 @@ https.get('https://encrypted.google.com/', (res) => { Global instance of [`https.Agent`][] for all HTTPS client requests. diff --git a/lib/_http_agent.js b/lib/_http_agent.js index 14096361760657..791a43c8d5d029 100644 --- a/lib/_http_agent.js +++ b/lib/_http_agent.js @@ -31,12 +31,10 @@ const { ArrayPrototypeSplice, FunctionPrototypeCall, NumberIsNaN, - NumberParseInt, ObjectCreate, ObjectKeys, ObjectSetPrototypeOf, ObjectValues, - RegExpPrototypeExec, StringPrototypeIndexOf, StringPrototypeSplit, StringPrototypeStartsWith, @@ -494,24 +492,7 @@ Agent.prototype.keepSocketAlive = function keepSocketAlive(socket) { socket.setKeepAlive(true, this.keepAliveMsecs); socket.unref(); - let agentTimeout = this.options.timeout || 0; - - if (socket._httpMessage?.res) { - const keepAliveHint = socket._httpMessage.res.headers['keep-alive']; - - if (keepAliveHint) { - const hint = RegExpPrototypeExec(/^timeout=(\d+)/, keepAliveHint)?.[1]; - - if (hint) { - const serverHintTimeout = NumberParseInt(hint) * 1000; - - if (serverHintTimeout < agentTimeout) { - agentTimeout = serverHintTimeout; - } - } - } - } - + const agentTimeout = this.options.timeout || 0; if (socket.timeout !== agentTimeout) { socket.setTimeout(agentTimeout); } @@ -561,5 +542,5 @@ function asyncResetHandle(socket) { module.exports = { Agent, - globalAgent: new Agent({ keepAlive: true, scheduling: 'lifo', timeout: 5000 }) + globalAgent: new Agent() }; diff --git a/lib/_http_server.js b/lib/_http_server.js index e908b1b1b3d5cf..c312267aa51e2e 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -478,7 +478,6 @@ ObjectSetPrototypeOf(Server.prototype, net.Server.prototype); ObjectSetPrototypeOf(Server, net.Server); Server.prototype.close = function() { - this.closeIdleConnections(); clearInterval(this[kConnectionsCheckingInterval]); ReflectApply(net.Server.prototype.close, this, arguments); }; diff --git a/lib/https.js b/lib/https.js index 0ecd13b1dd36ed..7ae8ab2987538b 100644 --- a/lib/https.js +++ b/lib/https.js @@ -331,7 +331,7 @@ Agent.prototype._evictSession = function _evictSession(key) { delete this._sessionCache.map[key]; }; -const globalAgent = new Agent({ keepAlive: true, scheduling: 'lifo', timeout: 5000 }); +const globalAgent = new Agent(); /** * Makes a request to a secure web server. diff --git a/test/async-hooks/test-graph.http.js b/test/async-hooks/test-graph.http.js index 62d27fe1b54431..0a003427f9e133 100644 --- a/test/async-hooks/test-graph.http.js +++ b/test/async-hooks/test-graph.http.js @@ -12,7 +12,6 @@ const hooks = initHooks(); hooks.enable(); const server = http.createServer(common.mustCall((req, res) => { - res.writeHead(200, { 'Connection': 'close' }); res.end(); server.close(common.mustCall()); })); diff --git a/test/parallel/test-http-agent-no-wait.js b/test/parallel/test-http-agent-no-wait.js deleted file mode 100644 index 62a381e85ce07c..00000000000000 --- a/test/parallel/test-http-agent-no-wait.js +++ /dev/null @@ -1,24 +0,0 @@ -'use strict'; - -const common = require('../common'); -const assert = require('assert'); -const http = require('http'); - -const server = http.createServer(function(req, res) { - res.writeHead(200); - res.end(); -}); - -server.listen(0, common.mustCall(() => { - const req = http.get({ port: server.address().port }, (res) => { - assert.strictEqual(res.statusCode, 200); - - res.resume(); - server.close(); - }); - - req.end(); -})); - -// This timer should never go off as the server will close the socket -setTimeout(common.mustNotCall(), 1000).unref(); diff --git a/test/parallel/test-http-client-agent.js b/test/parallel/test-http-client-agent.js index 0c348acc4bd68a..77616edd99d2fb 100644 --- a/test/parallel/test-http-client-agent.js +++ b/test/parallel/test-http-client-agent.js @@ -27,7 +27,6 @@ const Countdown = require('../common/countdown'); let name; const max = 3; -const agent = new http.Agent(); const server = http.Server(common.mustCall((req, res) => { if (req.url === '/0') { @@ -41,28 +40,27 @@ const server = http.Server(common.mustCall((req, res) => { } }, max)); server.listen(0, common.mustCall(() => { - name = agent.getName({ port: server.address().port }); + name = http.globalAgent.getName({ port: server.address().port }); for (let i = 0; i < max; ++i) request(i); })); const countdown = new Countdown(max, () => { - assert(!(name in agent.sockets)); - assert(!(name in agent.requests)); + assert(!(name in http.globalAgent.sockets)); + assert(!(name in http.globalAgent.requests)); server.close(); }); function request(i) { const req = http.get({ port: server.address().port, - path: `/${i}`, - agent + path: `/${i}` }, function(res) { const socket = req.socket; socket.on('close', common.mustCall(() => { countdown.dec(); if (countdown.remaining > 0) { - assert.strictEqual(agent.sockets[name].includes(socket), + assert.strictEqual(http.globalAgent.sockets[name].includes(socket), false); } })); diff --git a/test/parallel/test-http-client-close-with-default-agent.js b/test/parallel/test-http-client-close-with-default-agent.js deleted file mode 100644 index 8973f454e37728..00000000000000 --- a/test/parallel/test-http-client-close-with-default-agent.js +++ /dev/null @@ -1,23 +0,0 @@ -'use strict'; - -const common = require('../common'); -const assert = require('assert'); -const http = require('http'); - -const server = http.createServer(function(req, res) { - res.writeHead(200); - res.end(); -}); - -server.listen(0, common.mustCall(() => { - const req = http.get({ port: server.address().port }, (res) => { - assert.strictEqual(res.statusCode, 200); - res.resume(); - server.close(); - }); - - req.end(); -})); - -// This timer should never go off as the server will close the socket -setTimeout(common.mustNotCall(), common.platformTimeout(10000)).unref(); diff --git a/test/parallel/test-http-client-headers-array.js b/test/parallel/test-http-client-headers-array.js index d7e8d39c3c2560..2665c3a97bcf77 100644 --- a/test/parallel/test-http-client-headers-array.js +++ b/test/parallel/test-http-client-headers-array.js @@ -10,7 +10,7 @@ function execute(options) { const expectHeaders = { 'x-foo': 'boom', 'cookie': 'a=1; b=2; c=3', - 'connection': 'keep-alive' + 'connection': 'close' }; // no Host header when you set headers an array @@ -28,7 +28,6 @@ function execute(options) { assert.deepStrictEqual(req.headers, expectHeaders); - res.writeHead(200, { 'Connection': 'close' }); res.end(); }).listen(0, function() { options = Object.assign(options, { diff --git a/test/parallel/test-http-client-keep-alive-hint.js b/test/parallel/test-http-client-keep-alive-hint.js deleted file mode 100644 index 2618dfd552f8ab..00000000000000 --- a/test/parallel/test-http-client-keep-alive-hint.js +++ /dev/null @@ -1,27 +0,0 @@ -'use strict'; - -const common = require('../common'); -const assert = require('assert'); -const http = require('http'); - -const server = http.createServer( - { keepAliveTimeout: common.platformTimeout(60000) }, - function(req, res) { - req.resume(); - res.writeHead(200, { 'Connection': 'keep-alive', 'Keep-Alive': 'timeout=1' }); - res.end('FOO'); - } -); - -server.listen(0, common.mustCall(() => { - http.get({ port: server.address().port }, (res) => { - assert.strictEqual(res.statusCode, 200); - - res.resume(); - server.close(); - }); -})); - - -// This timer should never go off as the agent will parse the hint and terminate earlier -setTimeout(common.mustNotCall(), common.platformTimeout(3000)).unref(); diff --git a/test/parallel/test-http-client-spurious-aborted.js b/test/parallel/test-http-client-spurious-aborted.js index e5f0d41d2096ac..850462dadc76f8 100644 --- a/test/parallel/test-http-client-spurious-aborted.js +++ b/test/parallel/test-http-client-spurious-aborted.js @@ -10,7 +10,7 @@ const N = 2; let abortRequest = true; const server = http.Server(common.mustCall((req, res) => { - const headers = { 'Content-Type': 'text/plain', 'Connection': 'close' }; + const headers = { 'Content-Type': 'text/plain' }; headers['Content-Length'] = 50; const socket = res.socket; res.writeHead(200, headers); diff --git a/test/parallel/test-http-client-timeout-on-connect.js b/test/parallel/test-http-client-timeout-on-connect.js index 47b9fa800be527..3a0098229d9af8 100644 --- a/test/parallel/test-http-client-timeout-on-connect.js +++ b/test/parallel/test-http-client-timeout-on-connect.js @@ -13,10 +13,7 @@ const server = http.createServer((req, res) => { server.listen(0, common.localhostIPv4, common.mustCall(() => { const port = server.address().port; - const req = http.get( - `http://${common.localhostIPv4}:${port}`, - { agent: new http.Agent() } - ); + const req = http.get(`http://${common.localhostIPv4}:${port}`); req.setTimeout(1); req.on('socket', common.mustCall((socket) => { diff --git a/test/parallel/test-http-content-length.js b/test/parallel/test-http-content-length.js index 193ee5a8831e60..e6ba3719f95ba6 100644 --- a/test/parallel/test-http-content-length.js +++ b/test/parallel/test-http-content-length.js @@ -5,17 +5,17 @@ const http = require('http'); const Countdown = require('../common/countdown'); const expectedHeadersMultipleWrites = { - 'connection': 'keep-alive', + 'connection': 'close', 'transfer-encoding': 'chunked', }; const expectedHeadersEndWithData = { - 'connection': 'keep-alive', - 'content-length': String('hello world'.length), + 'connection': 'close', + 'content-length': String('hello world'.length) }; const expectedHeadersEndNoData = { - 'connection': 'keep-alive', + 'connection': 'close', 'content-length': '0', }; @@ -24,7 +24,6 @@ const countdown = new Countdown(3, () => server.close()); const server = http.createServer(function(req, res) { res.removeHeader('Date'); - res.setHeader('Keep-Alive', 'timeout=1'); switch (req.url.substr(1)) { case 'multiple-writes': @@ -60,8 +59,7 @@ server.listen(0, function() { req.write('hello '); req.end('world'); req.on('response', function(res) { - assert.deepStrictEqual(res.headers, { ...expectedHeadersMultipleWrites, 'keep-alive': 'timeout=1' }); - res.resume(); + assert.deepStrictEqual(res.headers, expectedHeadersMultipleWrites); }); req = http.request({ @@ -73,8 +71,7 @@ server.listen(0, function() { req.removeHeader('Host'); req.end('hello world'); req.on('response', function(res) { - assert.deepStrictEqual(res.headers, { ...expectedHeadersEndWithData, 'keep-alive': 'timeout=1' }); - res.resume(); + assert.deepStrictEqual(res.headers, expectedHeadersEndWithData); }); req = http.request({ @@ -86,8 +83,7 @@ server.listen(0, function() { req.removeHeader('Host'); req.end(); req.on('response', function(res) { - assert.deepStrictEqual(res.headers, { ...expectedHeadersEndNoData, 'keep-alive': 'timeout=1' }); - res.resume(); + assert.deepStrictEqual(res.headers, expectedHeadersEndNoData); }); }); diff --git a/test/parallel/test-http-default-encoding.js b/test/parallel/test-http-default-encoding.js index 0c0de0808ae8c2..8fcbf1a07f26ff 100644 --- a/test/parallel/test-http-default-encoding.js +++ b/test/parallel/test-http-default-encoding.js @@ -32,9 +32,9 @@ const server = http.Server((req, res) => { req.on('data', (chunk) => { result += chunk; }).on('end', () => { + server.close(); res.writeHead(200); res.end('hello world\n'); - server.close(); }); }); diff --git a/test/parallel/test-http-max-headers-count.js b/test/parallel/test-http-max-headers-count.js index 94685b228845aa..b707c62bfba8c7 100644 --- a/test/parallel/test-http-max-headers-count.js +++ b/test/parallel/test-http-max-headers-count.js @@ -48,7 +48,7 @@ const server = http.createServer(function(req, res) { expected = maxAndExpected[requests][1]; server.maxHeadersCount = max; } - res.writeHead(200, { ...headers, 'Connection': 'close' }); + res.writeHead(200, headers); res.end(); }); server.maxHeadersCount = max; diff --git a/test/parallel/test-http-outgoing-message-capture-rejection.js b/test/parallel/test-http-outgoing-message-capture-rejection.js index a89ef8baf70605..a19176df415263 100644 --- a/test/parallel/test-http-outgoing-message-capture-rejection.js +++ b/test/parallel/test-http-outgoing-message-capture-rejection.js @@ -16,11 +16,9 @@ events.captureRejections = true; res.socket.on('error', common.mustCall((err) => { assert.strictEqual(err, _err); - server.close(); })); // Write until there is space in the buffer - res.writeHead(200, { 'Connection': 'close' }); while (res.write('hello')); })); @@ -39,6 +37,7 @@ events.captureRejections = true; code: 'ECONNRESET' })); res.resume(); + server.close(); })); })); } diff --git a/test/parallel/test-http-raw-headers.js b/test/parallel/test-http-raw-headers.js index adc5bec1712201..d77a7cacc64c29 100644 --- a/test/parallel/test-http-raw-headers.js +++ b/test/parallel/test-http-raw-headers.js @@ -34,13 +34,13 @@ http.createServer(function(req, res) { 'x-BaR', 'yoyoyo', 'Connection', - 'keep-alive', + 'close', ]; const expectHeaders = { 'host': `localhost:${this.address().port}`, 'transfer-encoding': 'CHUNKED', 'x-bar': 'yoyoyo', - 'connection': 'keep-alive' + 'connection': 'close' }; const expectRawTrailers = [ 'x-bAr', @@ -65,7 +65,6 @@ http.createServer(function(req, res) { }); req.resume(); - res.setHeader('Keep-Alive', 'timeout=1'); res.setHeader('Trailer', 'x-foo'); res.addTrailers([ ['x-fOo', 'xOxOxOx'], @@ -87,25 +86,22 @@ http.createServer(function(req, res) { req.end('y b a r'); req.on('response', function(res) { const expectRawHeaders = [ - 'Keep-Alive', - 'timeout=1', 'Trailer', 'x-foo', 'Date', null, 'Connection', - 'keep-alive', + 'close', 'Transfer-Encoding', 'chunked', ]; const expectHeaders = { - 'keep-alive': 'timeout=1', 'trailer': 'x-foo', 'date': null, - 'connection': 'keep-alive', + 'connection': 'close', 'transfer-encoding': 'chunked' }; - res.rawHeaders[5] = null; + res.rawHeaders[3] = null; res.headers.date = null; assert.deepStrictEqual(res.rawHeaders, expectRawHeaders); assert.deepStrictEqual(res.headers, expectHeaders); diff --git a/test/parallel/test-http-request-end.js b/test/parallel/test-http-request-end.js index d762f459db27f0..c9278945b7993b 100644 --- a/test/parallel/test-http-request-end.js +++ b/test/parallel/test-http-request-end.js @@ -36,9 +36,9 @@ const server = http.Server(function(req, res) { req.on('end', function() { assert.strictEqual(result, expected); + server.close(); res.writeHead(200); res.end('hello world\n'); - server.close(); }); }); diff --git a/test/parallel/test-http-should-keep-alive.js b/test/parallel/test-http-should-keep-alive.js index 5cff29adbc54db..dfc99cf4f3b3a1 100644 --- a/test/parallel/test-http-should-keep-alive.js +++ b/test/parallel/test-http-should-keep-alive.js @@ -50,10 +50,6 @@ const getCountdownIndex = () => SERVER_RESPONSES.length - countdown.remaining; const server = net.createServer(function(socket) { socket.write(SERVER_RESPONSES[getCountdownIndex()]); - - if (SHOULD_KEEP_ALIVE[getCountdownIndex()]) { - socket.end(); - } }).listen(0, function() { function makeRequest() { const req = http.get({ port: server.address().port }, function(res) { diff --git a/test/parallel/test-http-unix-socket-keep-alive.js b/test/parallel/test-http-unix-socket-keep-alive.js index 36a3a4f535f07b..e1a2267e5c9689 100644 --- a/test/parallel/test-http-unix-socket-keep-alive.js +++ b/test/parallel/test-http-unix-socket-keep-alive.js @@ -20,7 +20,7 @@ server.listen(common.PIPE, common.mustCall(() => function asyncLoop(fn, times, cb) { fn(function handler() { if (--times) { - setTimeout(() => fn(handler), common.platformTimeout(10)); + fn(handler); } else { cb(); } diff --git a/test/parallel/test-https-agent-session-eviction.js b/test/parallel/test-https-agent-session-eviction.js index 20cdb870a09ddb..940c43cc40bf15 100644 --- a/test/parallel/test-https-agent-session-eviction.js +++ b/test/parallel/test-https-agent-session-eviction.js @@ -19,7 +19,6 @@ const options = { // Create TLS1.2 server https.createServer(options, function(req, res) { - res.writeHead(200, { 'Connection': 'close' }); res.end('ohai'); }).listen(0, function() { first(this); @@ -45,7 +44,6 @@ function first(server) { function faultyServer(port) { options.secureProtocol = 'TLSv1_method'; https.createServer(options, function(req, res) { - res.writeHead(200, { 'Connection': 'close' }); res.end('hello faulty'); }).listen(port, function() { second(this); diff --git a/test/parallel/test-https-max-headers-count.js b/test/parallel/test-https-max-headers-count.js index 0fdd38c73ed6f1..f8e4e64bfb94dd 100644 --- a/test/parallel/test-https-max-headers-count.js +++ b/test/parallel/test-https-max-headers-count.js @@ -37,7 +37,7 @@ const server = https.createServer(serverOptions, common.mustCall((req, res) => { expected = maxAndExpected[requests][1]; server.maxHeadersCount = max; } - res.writeHead(200, { ...headers, 'Connection': 'close' }); + res.writeHead(200, headers); res.end(); }, 3)); server.maxHeadersCount = max; diff --git a/test/parallel/test-stream-destroy.js b/test/parallel/test-stream-destroy.js index 77c3a0aaa3fea5..f99c29c811932d 100644 --- a/test/parallel/test-stream-destroy.js +++ b/test/parallel/test-stream-destroy.js @@ -62,8 +62,7 @@ const http = require('http'); server.listen(0, () => { const req = http.request({ - port: server.address().port, - agent: new http.Agent() + port: server.address().port }); req.write('asd'); @@ -97,8 +96,7 @@ const http = require('http'); server.listen(0, () => { const req = http.request({ - port: server.address().port, - agent: new http.Agent() + port: server.address().port }); req.write('asd'); diff --git a/test/parallel/test-tls-over-http-tunnel.js b/test/parallel/test-tls-over-http-tunnel.js index 503ec1f6600f5f..b26cf7872f6582 100644 --- a/test/parallel/test-tls-over-http-tunnel.js +++ b/test/parallel/test-tls-over-http-tunnel.js @@ -62,7 +62,7 @@ const proxy = net.createServer((clientSocket) => { 'HTTP/1.1\r\n' + 'Proxy-Connections: keep-alive\r\n' + `Host: localhost:${proxy.address().port}\r\n` + - 'Connection: keep-alive\r\n\r\n'); + 'Connection: close\r\n\r\n'); console.log('PROXY: got CONNECT request'); console.log('PROXY: creating a tunnel'); diff --git a/test/parallel/test-tls-set-secure-context.js b/test/parallel/test-tls-set-secure-context.js index f41a94cbf229b8..d62b6b9f94f601 100644 --- a/test/parallel/test-tls-set-secure-context.js +++ b/test/parallel/test-tls-set-secure-context.js @@ -82,8 +82,7 @@ function makeRequest(port, id) { rejectUnauthorized: true, ca: credentialOptions[0].ca, servername: 'agent1', - headers: { id }, - agent: new https.Agent() + headers: { id } }; let errored = false; diff --git a/test/sequential/test-http-econnrefused.js b/test/sequential/test-http-econnrefused.js index 7f8c2cee56c4c8..e1abde32bd81b1 100644 --- a/test/sequential/test-http-econnrefused.js +++ b/test/sequential/test-http-econnrefused.js @@ -43,7 +43,7 @@ const server = http.createServer(function(req, res) { req.on('end', function() { assert.strictEqual(body, 'PING'); - res.writeHead(200, { 'Connection': 'close' }); + res.writeHead(200); res.end('PONG'); }); });