http: make TCP keep-alive and TCP noDelay enabled by default

ShogunPanda · ShogunPanda · commit 61517606ba71 · 2022-03-01T11:39:19.000+01:00
diff --git a/doc/api/http.md b/doc/api/http.md
@@ -117,6 +117,10 @@ http.get({
 <!-- YAML
 added: v0.3.4
 changes:
+  - version:
+      - REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/42163
+    description: Changed the default value of `keepAlive` to `true`.
   - version:
       - v15.6.0
       - v14.17.0
@@ -144,11 +148,11 @@ changes:
     header is always sent when using an agent except when the `Connection`
     header is explicitly specified or when the `keepAlive` and `maxSockets`
     options are respectively set to `false` and `Infinity`, in which case
-    `Connection: close` will be used. **Default:** `false`.
+    `Connection: close` will be used. **Default:** `true`.
   * `keepAliveMsecs` {number} When using the `keepAlive` option, specifies
     the [initial delay][]
     for TCP Keep-Alive packets. Ignored when the
-    `keepAlive` option is `false` or `undefined`. **Default:** `1000`.
+    `keepAlive` option is `false`. **Default:** `1000`.
   * `maxSockets` {number} Maximum number of sockets to allow per host.
     If the same host opens multiple concurrent connections, each request
     will use new socket until the `maxSockets` value is reached.
@@ -2838,6 +2842,12 @@ Found'`.
 <!-- YAML
 added: v0.1.13
 changes:
+  - version:
+      - REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/42163
+    description: Changed the default value of `noDelay` and `keepAlive`
+                 to `true`. Changed the default value of `keepAliveInitialDelay`
+                 to `1000`.
   - version:
      - v13.8.0
      - v12.15.0
@@ -2871,14 +2881,14 @@ changes:
     **Default:** 16384 (16 KB).
   * `noDelay` {boolean} If set to `true`, it disables the use of Nagle's
     algorithm immediately after a new incoming connection is received.
-    **Default:** `false`.
+    **Default:** `true`.
   * `keepAlive` {boolean} If set to `true`, it enables keep-alive functionality
     on the socket immediately after a new incoming connection is received,
     similarly on what is done in \[`socket.setKeepAlive([enable][, initialDelay])`]\[`socket.setKeepAlive(enable, initialDelay)`].
-    **Default:** `false`.
+    **Default:** `true`.
   * `keepAliveInitialDelay` {number} If set to a positive number, it sets the
     initial delay before the first keepalive probe is sent on an idle socket.
-    **Default:** `0`.
+    **Default:** `1000`.
 
 * `requestListener` {Function}
 
diff --git a/lib/_http_agent.js b/lib/_http_agent.js
@@ -101,13 +101,21 @@ function Agent(options) {
 
   this.options = { __proto__: null, ...options };
 
+  if (this.options.noDelay === undefined)
+    this.options.noDelay = true;
+  if (this.options.keepAlive === undefined)
+    this.options.keepAlive = true;
+  if (this.options.keepAliveInitialDelay === undefined)
+    this.options.keepAliveInitialDelay = 1000;
+
   // Don't confuse net and make it think that we're connecting to a pipe
   this.options.path = null;
   this.requests = ObjectCreate(null);
   this.sockets = ObjectCreate(null);
   this.freeSockets = ObjectCreate(null);
-  this.keepAliveMsecs = this.options.keepAliveMsecs || 1000;
-  this.keepAlive = this.options.keepAlive || false;
+  this.keepAliveMsecs =
+    this.options.keepAliveMsecs || this.options.keepAliveInitialDelay || 1000;
+  this.keepAlive = this.options.keepAlive;
   this.maxSockets = this.options.maxSockets || Agent.defaultMaxSockets;
   this.maxFreeSockets = this.options.maxFreeSockets || 256;
   this.scheduling = this.options.scheduling || 'lifo';
diff --git a/lib/_http_server.js b/lib/_http_server.js
@@ -363,6 +363,13 @@ function storeHTTPOptions(options) {
   if (insecureHTTPParser !== undefined)
     validateBoolean(insecureHTTPParser, 'options.insecureHTTPParser');
   this.insecureHTTPParser = insecureHTTPParser;
+
+  if (options.noDelay === undefined)
+    options.noDelay = true;
+  if (options.keepAlive === undefined)
+    options.keepAlive = true;
+  if (options.keepAliveInitialDelay === undefined)
+    options.keepAliveInitialDelay = 1000;
 }
 
 function Server(options, requestListener) {
diff --git a/test/async-hooks/test-graph.http.js b/test/async-hooks/test-graph.http.js
@@ -19,8 +19,9 @@ server.listen(0, common.mustCall(() => {
   http.get({
     host: '::1',
     family: 6,
-    port: server.address().port
-  }, common.mustCall());
+    port: server.address().port,
+    headers: { connection: 'close' }
+  }, common.mustCall(() => {}));
 }));
 
 process.on('exit', () => {
diff --git a/test/parallel/test-child-process-http-socket-leak.js b/test/parallel/test-child-process-http-socket-leak.js
@@ -49,6 +49,7 @@ server.listen(0, common.mustCall(() => {
         assert.strictEqual(socket[kTimeout], null);
         assert.strictEqual(socket.parser, null);
         assert.strictEqual(socket._httpMessage, null);
+        res.req.socket.end();
       }));
     }));
 
diff --git a/test/parallel/test-http-client-agent.js b/test/parallel/test-http-client-agent.js
@@ -31,11 +31,11 @@ const max = 3;
 const server = http.Server(common.mustCall((req, res) => {
   if (req.url === '/0') {
     setTimeout(common.mustCall(() => {
-      res.writeHead(200);
+      res.writeHead(200, { connection: 'close' });
       res.end('Hello, World!');
     }), 100);
   } else {
-    res.writeHead(200);
+    res.writeHead(200, { connection: 'close' });
     res.end('Hello, World!');
   }
 }, max));
diff --git 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': 'close'
+      'connection': 'keep-alive'
     };
 
     // no Host header when you set headers an array
diff --git a/test/parallel/test-http-client-readable.js b/test/parallel/test-http-client-readable.js
@@ -53,6 +53,8 @@ class FakeAgent extends http.Agent {
 
     return s;
   }
+
+  keepSocketAlive() {}
 }
 
 let received = '';
diff --git a/test/parallel/test-http-client-spurious-aborted.js b/test/parallel/test-http-client-spurious-aborted.js
@@ -40,7 +40,7 @@ function download() {
   req.on('error', common.mustNotCall());
   req.on('response', (res) => {
     assert.strictEqual(res.statusCode, 200);
-    assert.strictEqual(res.headers.connection, 'close');
+    assert.strictEqual(res.headers.connection, 'keep-alive');
     let aborted = false;
     const writable = new Writable({
       write(chunk, encoding, callback) {
diff --git a/test/parallel/test-http-content-length.js b/test/parallel/test-http-content-length.js
@@ -24,19 +24,29 @@ const countdown = new Countdown(3, () => server.close());
 
 const server = http.createServer(function(req, res) {
   res.removeHeader('Date');
+  res.setHeader('connection', 'close');
 
   switch (req.url.substr(1)) {
     case 'multiple-writes':
-      assert.deepStrictEqual(req.headers, expectedHeadersMultipleWrites);
+      assert.deepStrictEqual(
+        req.headers,
+        { ...expectedHeadersMultipleWrites, 'connection': 'keep-alive' }
+      );
       res.write('hello');
       res.end('world');
       break;
     case 'end-with-data':
-      assert.deepStrictEqual(req.headers, expectedHeadersEndWithData);
+      assert.deepStrictEqual(
+        req.headers,
+        { ...expectedHeadersEndWithData, 'connection': 'keep-alive' }
+      );
       res.end('hello world');
       break;
     case 'empty':
-      assert.deepStrictEqual(req.headers, expectedHeadersEndNoData);
+      assert.deepStrictEqual(
+        req.headers,
+        { ...expectedHeadersEndNoData, 'connection': 'keep-alive' }
+      );
       res.end();
       break;
     default:
diff --git a/test/parallel/test-http-dump-req-when-res-ends.js b/test/parallel/test-http-dump-req-when-res-ends.js
@@ -55,6 +55,7 @@ server.listen(0, mustCall(function() {
     // invalidate the test.
     res.on('close', function() {
       server.close();
+      res.req.socket.end();
     });
   }));
 
diff --git a/test/parallel/test-http-max-headers-count.js b/test/parallel/test-http-max-headers-count.js
@@ -66,7 +66,7 @@ server.listen(0, function() {
     const expected = maxAndExpected[responses][1];
     const req = http.request({
       port: server.address().port,
-      headers: headers
+      headers: { connection: 'close', ...headers }
     }, function(res) {
       assert.strictEqual(Object.keys(res.headers).length, expected);
       res.on('end', function() {
diff --git a/test/parallel/test-http-nodelay.js b/test/parallel/test-http-nodelay.js
@@ -0,0 +1,42 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const http = require('http');
+const net = require('net');
+
+const originalConnect = net.Socket.prototype.connect;
+
+net.Socket.prototype.connect = common.mustCall(function(args) {
+  assert.strictEqual(args[0].noDelay, true);
+  assert.strictEqual(args[0].keepAlive, true);
+  assert.strictEqual(args[0].keepAliveInitialDelay, 1000);
+  return originalConnect.call(this, args);
+});
+
+const server = http.createServer(common.mustCall((req, res) => {
+  res.writeHead(200);
+  res.end();
+  server.close();
+}));
+
+server.listen(0, common.mustCall(() => {
+  assert.strictEqual(server.noDelay, true);
+  assert.strictEqual(server.keepAlive, true);
+  // Value is converted to second from net.Server
+  assert.strictEqual(server.keepAliveInitialDelay, 1);
+
+  const req = http.request({
+    method: 'GET',
+    port: server.address().port
+  }, common.mustCall((res) => {
+    res.on('end', () => {
+      server.close();
+      res.req.socket.end();
+    });
+
+    res.resume();
+  }));
+
+  req.end();
+}));
diff --git a/test/parallel/test-http-pause-no-dump.js b/test/parallel/test-http-pause-no-dump.js
@@ -26,6 +26,7 @@ server.on('listening', common.mustCall(function() {
     res.resume();
     res.on('end', common.mustCall(() => {
       server.close();
+      res.req.socket.end();
     }));
   }));
 
diff --git 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',
-    'close',
+    'keep-alive',
   ];
   const expectHeaders = {
     'host': `localhost:${this.address().port}`,
     'transfer-encoding': 'CHUNKED',
     'x-bar': 'yoyoyo',
-    'connection': 'close'
+    'connection': 'keep-alive'
   };
   const expectRawTrailers = [
     'x-bAr',
@@ -91,14 +91,17 @@ http.createServer(function(req, res) {
       'Date',
       null,
       'Connection',
-      'close',
+      'keep-alive',
+      'Keep-Alive',
+      'timeout=5',
       'Transfer-Encoding',
       'chunked',
     ];
     const expectHeaders = {
       'trailer': 'x-foo',
       'date': null,
-      'connection': 'close',
+      'connection': 'keep-alive',
+      'keep-alive': 'timeout=5',
       'transfer-encoding': 'chunked'
     };
     res.rawHeaders[3] = null;
@@ -121,6 +124,7 @@ http.createServer(function(req, res) {
       assert.deepStrictEqual(res.rawTrailers, expectRawTrailers);
       assert.deepStrictEqual(res.trailers, expectTrailers);
       console.log('ok');
+      req.socket.end();
     });
     res.resume();
   });
diff --git a/test/parallel/test-http-request-large-payload.js b/test/parallel/test-http-request-large-payload.js
@@ -8,7 +8,7 @@ require('../common');
 const http = require('http');
 
 const server = http.createServer(function(req, res) {
-  res.writeHead(200);
+  res.writeHead(200, { connection: 'close' });
   res.end();
 
   server.close();
diff --git a/test/parallel/test-http-should-keep-alive.js b/test/parallel/test-http-should-keep-alive.js
@@ -50,6 +50,7 @@ const getCountdownIndex = () => SERVER_RESPONSES.length - countdown.remaining;
 
 const server = net.createServer(function(socket) {
   socket.write(SERVER_RESPONSES[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
@@ -11,7 +11,7 @@ tmpdir.refresh();
 server.listen(common.PIPE, common.mustCall(() =>
   asyncLoop(makeKeepAliveRequest, 10, common.mustCall(() =>
     server.getConnections(common.mustSucceed((conns) => {
-      assert.strictEqual(conns, 1);
+      assert.strictEqual(conns, 2);
       server.close();
     }))
   ))
diff --git a/test/parallel/test-http2-https-fallback-http-server-options.js b/test/parallel/test-http2-https-fallback-http-server-options.js
@@ -82,6 +82,7 @@ class MyServerResponse extends http.ServerResponse {
           assert.strictEqual(userAgent, 'node-test');
 
           server.close();
+          response.req.socket.end();
         }));
       })
     );
diff --git a/test/parallel/test-http2-https-fallback.js b/test/parallel/test-http2-https-fallback.js
@@ -100,6 +100,7 @@ function onSession(session, next) {
           strictEqual(alpnProtocol, false);
           strictEqual(httpVersion, '1.1');
 
+          response.req.socket.end();
           cleanup();
         }));
       })
diff --git a/test/parallel/test-https-agent-session-eviction.js b/test/parallel/test-https-agent-session-eviction.js
@@ -19,6 +19,7 @@ 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);
diff --git a/test/parallel/test-https-agent-session-reuse.js b/test/parallel/test-https-agent-session-reuse.js
@@ -96,6 +96,8 @@ const server = https.createServer(options, function(req, res) {
   function request() {
     const options = queue.shift();
     options.agent = agent;
+    options.headers = { ...options.headers, connection: 'close' };
+
     https.request(options, function(res) {
       clientSessions[options.name] = res.socket.getSession();
 
diff --git a/test/parallel/test-https-agent-sockets-leak.js b/test/parallel/test-https-agent-sockets-leak.js
@@ -27,7 +27,7 @@ server.listen(0, common.mustCall(() => {
   https.get({
     host: server.address().host,
     port: server.address().port,
-    headers: { host: 'agent1' },
+    headers: { host: 'agent1', connection: 'close' },
     rejectUnauthorized: true,
     ca: options.ca,
     agent: agent
diff --git a/test/parallel/test-https-max-headers-count.js b/test/parallel/test-https-max-headers-count.js
@@ -53,7 +53,7 @@ server.listen(0, common.mustCall(() => {
     const expected = maxAndExpected[responses][1];
     const req = https.request({
       port: server.address().port,
-      headers: headers,
+      headers: { connection: 'close', ...headers },
       rejectUnauthorized: false
     }, (res) => {
       assert.strictEqual(Object.keys(res.headers).length, expected);
diff --git a/test/parallel/test-stream-destroy.js b/test/parallel/test-stream-destroy.js
diff --git a/test/parallel/test-tls-over-http-tunnel.js b/test/parallel/test-tls-over-http-tunnel.js
diff --git a/test/parallel/test-tls-set-secure-context.js b/test/parallel/test-tls-set-secure-context.js
diff --git a/test/sequential/test-http-econnrefused.js b/test/sequential/test-http-econnrefused.js
diff --git a/test/sequential/test-http-keep-alive-large-write.js b/test/sequential/test-http-keep-alive-large-write.js

Original file line number	Diff line number	Diff line change
`@@ -53,6 +53,8 @@ class FakeAgent extends http.Agent {`
`53`	`53`
`54`	`54`	`return s;`
`55`	`55`	`}`
	`56`	`+`
	`57`	`+ keepSocketAlive() {}`
`56`	`58`	`}`
`57`	`59`
`58`	`60`	`let received = '';`