Commit df271f4
http2: setting shuttingDown=true after validation
In shutdown(), shuttingDown was set to true before validating options.
If invalid options are passed, error was thrown and server remained in
shuttingDown state. This code change fixes it.
PR-URL: #15676
Fixes: #15666
Refs: #14985
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>1 parent ae91ffe commit df271f4
2 files changed
Lines changed: 65 additions & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -982,9 +982,6 @@ class Http2Session extends EventEmitter { | |||
| 982 | 982 | if (this[kState].shutdown || this[kState].shuttingDown) | |
| 983 | 983 | return; | |
| 984 | 984 | ||
| 985 | - debug(`[${sessionName(this[kType])}] initiating shutdown`); | ||
| 986 | - this[kState].shuttingDown = true; | ||
| 987 | - | ||
| 988 | 985 | const type = this[kType]; | |
| 989 | 986 | ||
| 990 | 987 | if (typeof options === 'function') { | |
@@ -1022,6 +1019,9 @@ class Http2Session extends EventEmitter { | |||
| 1022 | 1019 | options.lastStreamID); | |
| 1023 | 1020 | } | |
| 1024 | 1021 | ||
| 1022 | + debug(`[${sessionName(this[kType])}] initiating shutdown`); | ||
| 1023 | + this[kState].shuttingDown = true; | ||
| 1024 | + | ||
| 1025 | 1025 | if (callback) { | |
| 1026 | 1026 | this.on('shutdown', callback); | |
| 1027 | 1027 | } | |
Lines changed: 62 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,62 @@ | |||
| 1 | + // Flags: --expose-http2 | ||
| 2 | + 'use strict'; | ||
| 3 | + | ||
| 4 | + const common = require('../common'); | ||
| 5 | + if (!common.hasCrypto) | ||
| 6 | + common.skip('missing crypto'); | ||
| 7 | + const http2 = require('http2'); | ||
| 8 | + | ||
| 9 | + const server = http2.createServer(); | ||
| 10 | + | ||
| 11 | + const optionsToTest = { | ||
| 12 | + opaqueData: 'Uint8Array', | ||
| 13 | + graceful: 'boolean', | ||
| 14 | + errorCode: 'number', | ||
| 15 | + lastStreamID: 'number' | ||
| 16 | + }; | ||
| 17 | + | ||
| 18 | + const types = { | ||
| 19 | + boolean: true, | ||
| 20 | + number: 1, | ||
| 21 | + object: {}, | ||
| 22 | + array: [], | ||
| 23 | + null: null, | ||
| 24 | + Uint8Array: Buffer.from([0x1, 0x2, 0x3, 0x4, 0x5]) | ||
| 25 | + }; | ||
| 26 | + | ||
| 27 | + server.on( | ||
| 28 | + 'stream', | ||
| 29 | + common.mustCall((stream) => { | ||
| 30 | + Object.keys(optionsToTest).forEach((option) => { | ||
| 31 | + Object.keys(types).forEach((type) => { | ||
| 32 | + if (type === optionsToTest[option]) { | ||
| 33 | + return; | ||
| 34 | + } | ||
| 35 | + common.expectsError( | ||
| 36 | + () => | ||
| 37 | + stream.session.shutdown( | ||
| 38 | + { [option]: types[type] }, | ||
| 39 | + common.mustNotCall() | ||
| 40 | + ), | ||
| 41 | + { | ||
| 42 | + type: TypeError, | ||
| 43 | + code: 'ERR_INVALID_OPT_VALUE', | ||
| 44 | + message: `The value "${String(types[type])}" is invalid ` + | ||
| 45 | + `for option "${option}"` | ||
| 46 | + } | ||
| 47 | + ); | ||
| 48 | + }); | ||
| 49 | + }); | ||
| 50 | + stream.session.destroy(); | ||
| 51 | + }) | ||
| 52 | + ); | ||
| 53 | + | ||
| 54 | + server.listen( | ||
| 55 | + 0, | ||
| 56 | + common.mustCall(() => { | ||
| 57 | + const client = http2.connect(`http://localhost:${server.address().port}`); | ||
| 58 | + const req = client.request(); | ||
| 59 | + req.resume(); | ||
| 60 | + req.on('end', common.mustCall(() => server.close())); | ||
| 61 | + }) | ||
| 62 | + ); | ||
0 commit comments