Skip to content

Commit

Permalink
http2: close idle connections when allowHTTP1 is true
Browse files Browse the repository at this point in the history
Fixes: #51493
PR-URL: #51569
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
  • Loading branch information
xsbchen authored and richardlau committed Mar 25, 2024
1 parent 62707a9 commit b560bfb
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 1 deletion.
20 changes: 19 additions & 1 deletion lib/internal/http2/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ const {
kIncomingMessage,
_checkIsHttpToken: checkIsHttpToken,
} = require('_http_common');
const { kServerResponse } = require('_http_server');
const { kServerResponse, Server: HttpServer, httpServerPreClose, setupConnectionsTracking } = require('_http_server');
const JSStreamSocket = require('internal/js_stream_socket');

const {
Expand Down Expand Up @@ -3180,6 +3180,11 @@ class Http2SecureServer extends TLSServer {
this[kOptions] = options;
this.timeout = 0;
this.on('newListener', setupCompat);
if (options.allowHTTP1 === true) {
this.headersTimeout = 60_000; // Minimum between 60 seconds or requestTimeout
this.requestTimeout = 300_000; // 5 minutes
this.on('listening', setupConnectionsTracking);
}
if (typeof requestListener === 'function')
this.on('request', requestListener);
this.on('tlsClientError', onErrorSecureServerSession);
Expand All @@ -3199,6 +3204,19 @@ class Http2SecureServer extends TLSServer {
validateSettings(settings);
this[kOptions].settings = { ...this[kOptions].settings, ...settings };
}

close() {
if (this[kOptions].allowHTTP1 === true) {
httpServerPreClose(this);
}
ReflectApply(TLSServer.prototype.close, this, arguments);
}

closeIdleConnections() {
if (this[kOptions].allowHTTP1 === true) {
ReflectApply(HttpServer.prototype.closeIdleConnections, this, arguments);
}
}
}

class Http2Server extends NETServer {
Expand Down
55 changes: 55 additions & 0 deletions test/parallel/test-http2-allow-http1.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
'use strict';

const common = require('../common');
const fixtures = require('../common/fixtures');
if (!common.hasCrypto) common.skip('missing crypto');

const assert = require('assert');
const https = require('https');
const http2 = require('http2');

(async function main() {
const server = http2.createSecureServer({
key: fixtures.readKey('agent1-key.pem'),
cert: fixtures.readKey('agent1-cert.pem'),
allowHTTP1: true,
});

server.on(
'request',
common.mustCall((req, res) => {
res.writeHead(200);
res.end();
})
);

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

await new Promise((resolve) => server.listen(0, resolve));

await new Promise((resolve) =>
https.get(
`https://localhost:${server.address().port}`,
{
rejectUnauthorized: false,
headers: { connection: 'keep-alive' },
},
resolve
)
);

let serverClosed = false;
setImmediate(
common.mustCall(() => {
assert.ok(serverClosed, 'server should been closed immediately');
})
);
server.close(
common.mustSucceed(() => {
serverClosed = true;
})
);
})().then(common.mustCall());

0 comments on commit b560bfb

Please sign in to comment.