Commit 682951a
http2: close idle connections when allowHTTP1 is true
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>1 parent 2ad665e commit 682951a
2 files changed
Lines changed: 74 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -59,7 +59,7 @@ const { | |||
| 59 | 59 | kIncomingMessage, | |
| 60 | 60 | _checkIsHttpToken: checkIsHttpToken, | |
| 61 | 61 | } = require('_http_common'); | |
| 62 | - const { kServerResponse } = require('_http_server'); | ||
| 62 | + const { kServerResponse, Server: HttpServer, httpServerPreClose, setupConnectionsTracking } = require('_http_server'); | ||
| 63 | 63 | const JSStreamSocket = require('internal/js_stream_socket'); | |
| 64 | 64 | ||
| 65 | 65 | const { | |
@@ -3180,6 +3180,11 @@ class Http2SecureServer extends TLSServer { | |||
| 3180 | 3180 | this[kOptions] = options; | |
| 3181 | 3181 | this.timeout = 0; | |
| 3182 | 3182 | this.on('newListener', setupCompat); | |
| 3183 | + if (options.allowHTTP1 === true) { | ||
| 3184 | + this.headersTimeout = 60_000; // Minimum between 60 seconds or requestTimeout | ||
| 3185 | + this.requestTimeout = 300_000; // 5 minutes | ||
| 3186 | + this.on('listening', setupConnectionsTracking); | ||
| 3187 | + } | ||
| 3183 | 3188 | if (typeof requestListener === 'function') | |
| 3184 | 3189 | this.on('request', requestListener); | |
| 3185 | 3190 | this.on('tlsClientError', onErrorSecureServerSession); | |
@@ -3199,6 +3204,19 @@ class Http2SecureServer extends TLSServer { | |||
| 3199 | 3204 | validateSettings(settings); | |
| 3200 | 3205 | this[kOptions].settings = { ...this[kOptions].settings, ...settings }; | |
| 3201 | 3206 | } | |
| 3207 | + | ||
| 3208 | + close() { | ||
| 3209 | + if (this[kOptions].allowHTTP1 === true) { | ||
| 3210 | + httpServerPreClose(this); | ||
| 3211 | + } | ||
| 3212 | + ReflectApply(TLSServer.prototype.close, this, arguments); | ||
| 3213 | + } | ||
| 3214 | + | ||
| 3215 | + closeIdleConnections() { | ||
| 3216 | + if (this[kOptions].allowHTTP1 === true) { | ||
| 3217 | + ReflectApply(HttpServer.prototype.closeIdleConnections, this, arguments); | ||
| 3218 | + } | ||
| 3219 | + } | ||
| 3202 | 3220 | } | |
| 3203 | 3221 | ||
| 3204 | 3222 | class Http2Server extends NETServer { | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,55 @@ | |||
| 1 | + 'use strict'; | ||
| 2 | + | ||
| 3 | + const common = require('../common'); | ||
| 4 | + const fixtures = require('../common/fixtures'); | ||
| 5 | + if (!common.hasCrypto) common.skip('missing crypto'); | ||
| 6 | + | ||
| 7 | + const assert = require('assert'); | ||
| 8 | + const https = require('https'); | ||
| 9 | + const http2 = require('http2'); | ||
| 10 | + | ||
| 11 | + (async function main() { | ||
| 12 | + const server = http2.createSecureServer({ | ||
| 13 | + key: fixtures.readKey('agent1-key.pem'), | ||
| 14 | + cert: fixtures.readKey('agent1-cert.pem'), | ||
| 15 | + allowHTTP1: true, | ||
| 16 | + }); | ||
| 17 | + | ||
| 18 | + server.on( | ||
| 19 | + 'request', | ||
| 20 | + common.mustCall((req, res) => { | ||
| 21 | + res.writeHead(200); | ||
| 22 | + res.end(); | ||
| 23 | + }) | ||
| 24 | + ); | ||
| 25 | + | ||
| 26 | + server.on( | ||
| 27 | + 'close', | ||
| 28 | + common.mustCall() | ||
| 29 | + ); | ||
| 30 | + | ||
| 31 | + await new Promise((resolve) => server.listen(0, resolve)); | ||
| 32 | + | ||
| 33 | + await new Promise((resolve) => | ||
| 34 | + https.get( | ||
| 35 | + `https://localhost:${server.address().port}`, | ||
| 36 | + { | ||
| 37 | + rejectUnauthorized: false, | ||
| 38 | + headers: { connection: 'keep-alive' }, | ||
| 39 | + }, | ||
| 40 | + resolve | ||
| 41 | + ) | ||
| 42 | + ); | ||
| 43 | + | ||
| 44 | + let serverClosed = false; | ||
| 45 | + setImmediate( | ||
| 46 | + common.mustCall(() => { | ||
| 47 | + assert.ok(serverClosed, 'server should been closed immediately'); | ||
| 48 | + }) | ||
| 49 | + ); | ||
| 50 | + server.close( | ||
| 51 | + common.mustSucceed(() => { | ||
| 52 | + serverClosed = true; | ||
| 53 | + }) | ||
| 54 | + ); | ||
| 55 | + })().then(common.mustCall()); | ||
0 commit comments