Skip to content

Commit 9c0de7d

Browse files
pimterryruyadorno
authored andcommitted
http: keep HTTP/1.1 conns alive even if the Connection header is removed
Previously persistence was completed disabled if you removed this header, which is not correct for modern HTTP, where the header is optional and all connections should persist by default regardless. PR-URL: #46331 Backport-PR-URL: #49016 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Minwoo Jung <nodecorelab@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ricky Zhou <0x19951125@gmail.com>
1 parent d3637cd commit 9c0de7d

File tree

2 files changed

+75
-2
lines changed

2 files changed

+75
-2
lines changed

lib/_http_outgoing.js

+3-2
Original file line numberDiff line numberDiff line change
@@ -483,8 +483,9 @@ function _storeHeader(firstLine, headers) {
483483

484484
// keep-alive logic
485485
if (this._removedConnection) {
486-
this._last = true;
487-
this.shouldKeepAlive = false;
486+
// shouldKeepAlive is generally true for HTTP/1.1. In that common case,
487+
// even if the connection header isn't sent, we still persist by default.
488+
this._last = !this.shouldKeepAlive;
488489
} else if (!state.connection) {
489490
const shouldSendKeepAlive = this.shouldKeepAlive &&
490491
(state.contLen || this.useChunkedEncodingByDefault || this.agent);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
'use strict';
2+
require('../common');
3+
const assert = require('assert');
4+
5+
const net = require('net');
6+
const http = require('http');
7+
8+
const server = http.createServer(function(request, response) {
9+
// When the connection header is removed, for HTTP/1.1 the connection should still persist.
10+
// For HTTP/1.0, the connection should be closed after the response automatically.
11+
response.removeHeader('connection');
12+
13+
response.end('beep boop\n');
14+
});
15+
16+
const agent = new http.Agent({ keepAlive: true });
17+
18+
function makeHttp11Request(cb) {
19+
http.get({
20+
port: server.address().port,
21+
agent
22+
}, function(res) {
23+
const socket = res.socket;
24+
25+
assert.strictEqual(res.statusCode, 200);
26+
assert.strictEqual(res.headers.connection, undefined);
27+
28+
res.setEncoding('ascii');
29+
let response = '';
30+
res.on('data', function(chunk) {
31+
response += chunk;
32+
});
33+
res.on('end', function() {
34+
assert.strictEqual(response, 'beep boop\n');
35+
36+
// Wait till next tick to ensure that the socket is returned to the agent before
37+
// we continue to the next request
38+
process.nextTick(function() {
39+
cb(socket);
40+
});
41+
});
42+
});
43+
}
44+
45+
function makeHttp10Request(cb) {
46+
// We have to manually make HTTP/1.0 requests since Node does not allow sending them:
47+
const socket = net.connect({ port: server.address().port }, function() {
48+
socket.write('GET / HTTP/1.0\r\n' +
49+
'Host: localhost:' + server.address().port + '\r\n' +
50+
'\r\n');
51+
socket.resume(); // Ignore the response itself
52+
53+
setTimeout(function() {
54+
cb(socket);
55+
}, 10);
56+
});
57+
}
58+
59+
server.listen(0, function() {
60+
makeHttp11Request(function(firstSocket) {
61+
makeHttp11Request(function(secondSocket) {
62+
// Both HTTP/1.1 requests should have used the same socket:
63+
assert.strictEqual(firstSocket, secondSocket);
64+
65+
makeHttp10Request(function(socket) {
66+
// The server should have immediately closed the HTTP/1.0 socket:
67+
assert.strictEqual(socket.closed, true);
68+
server.close();
69+
});
70+
});
71+
});
72+
});

0 commit comments

Comments
 (0)