From b560bfbb84cc56627ae8b85c58f84aa42b3f4864 Mon Sep 17 00:00:00 2001 From: xsbchen Date: Thu, 1 Feb 2024 22:45:38 +0800 Subject: [PATCH] http2: close idle connections when allowHTTP1 is true Fixes: https://github.com/nodejs/node/issues/51493 PR-URL: https://github.com/nodejs/node/pull/51569 Reviewed-By: Matteo Collina Reviewed-By: Stephen Belanger Reviewed-By: Marco Ippolito --- lib/internal/http2/core.js | 20 ++++++++- test/parallel/test-http2-allow-http1.js | 55 +++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-http2-allow-http1.js diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 4e67eba4ecac40..a82691a37344f9 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -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 { @@ -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); @@ -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 { diff --git a/test/parallel/test-http2-allow-http1.js b/test/parallel/test-http2-allow-http1.js new file mode 100644 index 00000000000000..b1acf8d8b91a0a --- /dev/null +++ b/test/parallel/test-http2-allow-http1.js @@ -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());