Skip to content

Commit

Permalink
http: added connection closing methods
Browse files Browse the repository at this point in the history
Fixes: #41578

PR-URL: #42812
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
  • Loading branch information
ShogunPanda authored and RafaelGSS committed May 10, 2022
1 parent 3ab3086 commit 2f192c4
Show file tree
Hide file tree
Showing 9 changed files with 360 additions and 13 deletions.
17 changes: 17 additions & 0 deletions doc/api/http.md
Original file line number Diff line number Diff line change
Expand Up @@ -1453,6 +1453,23 @@ added: v0.1.90

Stops the server from accepting new connections. See [`net.Server.close()`][].

### `server.closeAllConnections()`

<!-- YAML
added: REPLACEME
-->

Closes all connections connected to this server.

### `server.closeIdleConnections()`

<!-- YAML
added: REPLACEME
-->

Closes all connections connected to this server which are not sending a request
or waiting for a response.

### `server.headersTimeout`

<!-- YAML
Expand Down
22 changes: 20 additions & 2 deletions doc/api/https.md
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,23 @@ added: v0.1.90
* `callback` {Function}
* Returns: {https.Server}

See [`server.close()`][`http.close()`] from the HTTP module for details.
See [`http.Server.close()`][].

### `server.closeAllConnections()`

<!-- YAML
added: REPLACEME
-->

See [`http.Server.closeAllConnections()`][].

### `server.closeIdleConnections()`

<!-- YAML
added: REPLACEME
-->

See [`http.Server.closeIdleConnections()`][].

### `server.headersTimeout`

Expand Down Expand Up @@ -529,8 +545,10 @@ headers: max-age=0; pin-sha256="WoiWRyIOVNa9ihaBciRSC7XHjliYS9VwUGOIud4PB18="; p
[`http.Server#requestTimeout`]: http.md#serverrequesttimeout
[`http.Server#setTimeout()`]: http.md#serversettimeoutmsecs-callback
[`http.Server#timeout`]: http.md#servertimeout
[`http.Server.close()`]: http.md#serverclosecallback
[`http.Server.closeAllConnections()`]: http.md#servercloseallconnections
[`http.Server.closeIdleConnections()`]: http.md#servercloseidleconnections
[`http.Server`]: http.md#class-httpserver
[`http.close()`]: http.md#serverclosecallback
[`http.createServer()`]: http.md#httpcreateserveroptions-requestlistener
[`http.get()`]: http.md#httpgetoptions-callback
[`http.request()`]: http.md#httprequestoptions-callback
Expand Down
29 changes: 25 additions & 4 deletions lib/_http_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -409,10 +409,11 @@ function storeHTTPOptions(options) {
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();
}

// This checker is started without checking whether any headersTimeout or requestTimeout is non zero
// otherwise it would not be started if such timeouts are modified after createServer.
server[kConnectionsCheckingInterval] =
setInterval(checkConnections.bind(server), server.connectionsCheckingInterval).unref();
}

function Server(options, requestListener) {
Expand Down Expand Up @@ -458,6 +459,22 @@ Server.prototype.close = function() {
ReflectApply(net.Server.prototype.close, this, arguments);
};

Server.prototype.closeAllConnections = function() {
const connections = this[kConnections].all();

for (let i = 0, l = connections.length; i < l; i++) {
connections[i].socket.destroy();
}
};

Server.prototype.closeIdleConnections = function() {
const connections = this[kConnections].idle();

for (let i = 0, l = connections.length; i < l; i++) {
connections[i].socket.destroy();
}
};

Server.prototype.setTimeout = function setTimeout(msecs, callback) {
this.timeout = msecs;
if (callback)
Expand Down Expand Up @@ -489,6 +506,10 @@ Server.prototype[EE.captureRejectionSymbol] = function(err, event, ...args) {
};

function checkConnections() {
if (this.headersTimeout === 0 && this.requestTimeout === 0) {
return;
}

const expired = this[kConnections].expired(this.headersTimeout, this.requestTimeout);

for (let i = 0; i < expired.length; i++) {
Expand Down
4 changes: 4 additions & 0 deletions lib/https.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ function Server(opts, requestListener) {
ObjectSetPrototypeOf(Server.prototype, tls.Server.prototype);
ObjectSetPrototypeOf(Server, tls.Server);

Server.prototype.closeAllConnections = HttpServer.prototype.closeAllConnections;

Server.prototype.closeIdleConnections = HttpServer.prototype.closeIdleConnections;

Server.prototype.setTimeout = HttpServer.prototype.setTimeout;

/**
Expand Down
27 changes: 20 additions & 7 deletions src/node_http_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -257,9 +257,10 @@ class Parser : public AsyncWrap, public StreamListener {
SET_SELF_SIZE(Parser)

int on_message_begin() {
// Important: Pop from the list BEFORE resetting the last_message_start_
// Important: Pop from the lists BEFORE resetting the last_message_start_
// otherwise std::set.erase will fail.
if (connectionsList_ != nullptr) {
connectionsList_->Pop(this);
connectionsList_->PopActive(this);
}

Expand All @@ -270,6 +271,7 @@ class Parser : public AsyncWrap, public StreamListener {
status_message_.Reset();

if (connectionsList_ != nullptr) {
connectionsList_->Push(this);
connectionsList_->PushActive(this);
}

Expand Down Expand Up @@ -492,14 +494,19 @@ class Parser : public AsyncWrap, public StreamListener {
int on_message_complete() {
HandleScope scope(env()->isolate());

// Important: Pop from the list BEFORE resetting the last_message_start_
// Important: Pop from the lists BEFORE resetting the last_message_start_
// otherwise std::set.erase will fail.
if (connectionsList_ != nullptr) {
connectionsList_->Pop(this);
connectionsList_->PopActive(this);
}

last_message_start_ = 0;

if (connectionsList_ != nullptr) {
connectionsList_->Push(this);
}

if (num_fields_)
Flush(); // Flush trailing HTTP headers.

Expand Down Expand Up @@ -666,12 +673,14 @@ class Parser : public AsyncWrap, public StreamListener {
if (connectionsList != nullptr) {
parser->connectionsList_ = connectionsList;

parser->connectionsList_->Push(parser);

// This protects from a DoS attack where an attacker establishes
// the connection without sending any data on applications where
// server.timeout is left to the default value of zero.
parser->last_message_start_ = uv_hrtime();

// Important: Push into the lists AFTER setting the last_message_start_
// otherwise std::set.erase will fail later.
parser->connectionsList_->Push(parser);
parser->connectionsList_->PushActive(parser);
} else {
parser->connectionsList_ = nullptr;
Expand Down Expand Up @@ -1044,10 +1053,14 @@ class Parser : public AsyncWrap, public StreamListener {
};

bool ParserComparator::operator()(const Parser* lhs, const Parser* rhs) const {
if (lhs->last_message_start_ == 0) {
return false;
} else if (rhs->last_message_start_ == 0) {
if (lhs->last_message_start_ == 0 && rhs->last_message_start_ == 0) {
// When both parsers are idle, guarantee strict order by
// comparing pointers as ints.
return lhs < rhs;
} else if (lhs->last_message_start_ == 0) {
return true;
} else if (rhs->last_message_start_ == 0) {
return false;
}

return lhs->last_message_start_ < rhs->last_message_start_;
Expand Down
57 changes: 57 additions & 0 deletions test/parallel/test-http-server-close-all.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
'use strict';
const common = require('../common');
const assert = require('assert');

const { createServer } = require('http');
const { connect } = require('net');

let connections = 0;

const server = createServer(common.mustCall(function(req, res) {
res.writeHead(200, { Connection: 'keep-alive' });
res.end();
}), {
headersTimeout: 0,
keepAliveTimeout: 0,
requestTimeout: common.platformTimeout(60000),
});

server.on('connection', function() {
connections++;
});

server.listen(0, function() {
const port = server.address().port;

// Create a first request but never finish it
const client1 = connect(port);

client1.on('close', common.mustCall());

client1.on('error', () => {});

client1.write('GET / HTTP/1.1');

// Create a second request, let it finish but leave the connection opened using HTTP keep-alive
const client2 = connect(port);
let response = '';

client2.on('data', common.mustCall((chunk) => {
response += chunk.toString('utf-8');

if (response.endsWith('0\r\n\r\n')) {
assert(response.startsWith('HTTP/1.1 200 OK\r\nConnection: keep-alive'));
assert.strictEqual(connections, 2);

server.closeAllConnections();
server.close(common.mustCall());

// This timer should never go off as the server.close should shut everything down
setTimeout(common.mustNotCall(), common.platformTimeout(1500)).unref();
}
}));

client2.on('close', common.mustCall());

client2.write('GET / HTTP/1.1\r\n\r\n');
});
69 changes: 69 additions & 0 deletions test/parallel/test-http-server-close-idle.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
'use strict';
const common = require('../common');
const assert = require('assert');

const { createServer } = require('http');
const { connect } = require('net');

let connections = 0;

const server = createServer(common.mustCall(function(req, res) {
res.writeHead(200, { Connection: 'keep-alive' });
res.end();
}), {
headersTimeout: 0,
keepAliveTimeout: 0,
requestTimeout: common.platformTimeout(60000),
});

server.on('connection', function() {
connections++;
});

server.listen(0, function() {
const port = server.address().port;
let client1Closed = false;
let client2Closed = false;

// Create a first request but never finish it
const client1 = connect(port);

client1.on('close', common.mustCall(() => {
client1Closed = true;
}));

client1.on('error', () => {});

client1.write('GET / HTTP/1.1');

// Create a second request, let it finish but leave the connection opened using HTTP keep-alive
const client2 = connect(port);
let response = '';

client2.on('data', common.mustCall((chunk) => {
response += chunk.toString('utf-8');

if (response.endsWith('0\r\n\r\n')) {
assert(response.startsWith('HTTP/1.1 200 OK\r\nConnection: keep-alive'));
assert.strictEqual(connections, 2);

server.closeIdleConnections();
server.close(common.mustCall());

// Check that only the idle connection got closed
setTimeout(common.mustCall(() => {
assert(!client1Closed);
assert(client2Closed);

server.closeAllConnections();
server.close(common.mustCall());
}), common.platformTimeout(500)).unref();
}
}));

client2.on('close', common.mustCall(() => {
client2Closed = true;
}));

client2.write('GET / HTTP/1.1\r\n\r\n');
});
68 changes: 68 additions & 0 deletions test/parallel/test-https-server-close-all.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
'use strict';
const common = require('../common');
const assert = require('assert');

if (!common.hasCrypto) {
common.skip('missing crypto');
}

const { createServer } = require('https');
const { connect } = require('tls');

const fixtures = require('../common/fixtures');

const options = {
key: fixtures.readKey('agent1-key.pem'),
cert: fixtures.readKey('agent1-cert.pem')
};

let connections = 0;

const server = createServer(options, common.mustCall(function(req, res) {
res.writeHead(200, { Connection: 'keep-alive' });
res.end();
}), {
headersTimeout: 0,
keepAliveTimeout: 0,
requestTimeout: common.platformTimeout(60000),
});

server.on('connection', function() {
connections++;
});

server.listen(0, function() {
const port = server.address().port;

// Create a first request but never finish it
const client1 = connect({ port, rejectUnauthorized: false });

client1.on('close', common.mustCall());

client1.on('error', () => {});

client1.write('GET / HTTP/1.1');

// Create a second request, let it finish but leave the connection opened using HTTP keep-alive
const client2 = connect({ port, rejectUnauthorized: false });
let response = '';

client2.on('data', common.mustCall((chunk) => {
response += chunk.toString('utf-8');

if (response.endsWith('0\r\n\r\n')) {
assert(response.startsWith('HTTP/1.1 200 OK\r\nConnection: keep-alive'));
assert.strictEqual(connections, 2);

server.closeAllConnections();
server.close(common.mustCall());

// This timer should never go off as the server.close should shut everything down
setTimeout(common.mustNotCall(), common.platformTimeout(1500)).unref();
}
}));

client2.on('close', common.mustCall());

client2.write('GET / HTTP/1.1\r\n\r\n');
});
Loading

0 comments on commit 2f192c4

Please sign in to comment.