From d36b60e69a00b60b8f2c260b8c376c9a16a152df Mon Sep 17 00:00:00 2001 From: Xuguang Mei Date: Fri, 8 Apr 2022 18:17:03 +0800 Subject: [PATCH] readline: fix question still called after closed resolve: https://github.com/nodejs/node/issues/42450 PR-URL: https://github.com/nodejs/node/pull/42464 Reviewed-By: Antoine du Hamel Reviewed-By: Matteo Collina Reviewed-By: Darshan Sen --- doc/api/errors.md | 8 +++++ doc/api/readline.md | 6 ++++ lib/internal/errors.js | 1 + lib/internal/readline/interface.js | 8 ++++- test/parallel/test-readline-interface.js | 34 +++++++++++++++++++ .../test-readline-promises-interface.js | 17 ++++++++++ 6 files changed, 73 insertions(+), 1 deletion(-) diff --git a/doc/api/errors.md b/doc/api/errors.md index a11a94981fefa6..71cae6300b26ed 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -2802,6 +2802,14 @@ import 'package-name'; // supported `import` with URL schemes other than `file` and `data` is unsupported. + + +### `ERR_USE_AFTER_CLOSE` + +> Stability: 1 - Experimental + +An attempt was made to use something that was already closed. + ### `ERR_VALID_PERFORMANCE_ENTRY_TYPE` diff --git a/doc/api/readline.md b/doc/api/readline.md index 58a67256104b24..d3a01755fa0f57 100644 --- a/doc/api/readline.md +++ b/doc/api/readline.md @@ -330,6 +330,8 @@ The `callback` function passed to `rl.question()` does not follow the typical pattern of accepting an `Error` object or `null` as the first argument. The `callback` is called with the provided answer as the only argument. +An error will be thrown if calling `rl.question()` after `rl.close()`. + Example usage: ```js @@ -586,6 +588,8 @@ paused. If the `readlinePromises.Interface` was created with `output` set to `null` or `undefined` the `query` is not written. +If the question is called after `rl.close()`, it returns a rejected promise. + Example usage: ```mjs @@ -855,6 +859,8 @@ The `callback` function passed to `rl.question()` does not follow the typical pattern of accepting an `Error` object or `null` as the first argument. The `callback` is called with the provided answer as the only argument. +An error will be thrown if calling `rl.question()` after `rl.close()`. + Example usage: ```js diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 5f75c0290b33d9..a5c64080a59aae 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1627,6 +1627,7 @@ E('ERR_UNSUPPORTED_ESM_URL_SCHEME', (url, supported) => { msg += `. Received protocol '${url.protocol}'`; return msg; }, Error); +E('ERR_USE_AFTER_CLOSE', '%s was closed', Error); // This should probably be a `TypeError`. E('ERR_VALID_PERFORMANCE_ENTRY_TYPE', diff --git a/lib/internal/readline/interface.js b/lib/internal/readline/interface.js index b5a5455ed3da85..47e5ca580ab317 100644 --- a/lib/internal/readline/interface.js +++ b/lib/internal/readline/interface.js @@ -38,7 +38,10 @@ const { const { codes } = require('internal/errors'); -const { ERR_INVALID_ARG_VALUE } = codes; +const { + ERR_INVALID_ARG_VALUE, + ERR_USE_AFTER_CLOSE, +} = codes; const { validateAbortSignal, validateArray, @@ -398,6 +401,9 @@ class Interface extends InterfaceConstructor { } question(query, cb) { + if (this.closed) { + throw new ERR_USE_AFTER_CLOSE('readline'); + } if (this[kQuestionCallback]) { this.prompt(); } else { diff --git a/test/parallel/test-readline-interface.js b/test/parallel/test-readline-interface.js index ad7eee7c42e485..52cfdac2b8f88a 100644 --- a/test/parallel/test-readline-interface.js +++ b/test/parallel/test-readline-interface.js @@ -1092,6 +1092,40 @@ for (let i = 0; i < 12; i++) { rli.close(); } + // Call question after close + { + const [rli, fi] = getInterface({ terminal }); + rli.question('What\'s your name?', common.mustCall((name) => { + assert.strictEqual(name, 'Node.js'); + rli.close(); + assert.throws(() => { + rli.question('How are you?', common.mustNotCall()); + }, { + name: 'Error', + code: 'ERR_USE_AFTER_CLOSE' + }); + assert.notStrictEqual(rli.getPrompt(), 'How are you?'); + })); + fi.emit('data', 'Node.js\n'); + } + + // Call promisified question after close + { + const [rli, fi] = getInterface({ terminal }); + const question = util.promisify(rli.question).bind(rli); + question('What\'s your name?').then(common.mustCall((name) => { + assert.strictEqual(name, 'Node.js'); + rli.close(); + question('How are you?') + .then(common.mustNotCall(), common.expectsError({ + code: 'ERR_USE_AFTER_CLOSE', + name: 'Error' + })); + assert.notStrictEqual(rli.getPrompt(), 'How are you?'); + })); + fi.emit('data', 'Node.js\n'); + } + // Can create a new readline Interface with a null output argument { const [rli, fi] = getInterface({ output: null, terminal }); diff --git a/test/parallel/test-readline-promises-interface.js b/test/parallel/test-readline-promises-interface.js index cf0543d7d255f7..455dabe6749bd5 100644 --- a/test/parallel/test-readline-promises-interface.js +++ b/test/parallel/test-readline-promises-interface.js @@ -952,6 +952,23 @@ for (let i = 0; i < 12; i++) { rli.close(); } + // Call question after close + { + const [rli, fi] = getInterface({ terminal }); + rli.question('What\'s your name?').then(common.mustCall((name) => { + assert.strictEqual(name, 'Node.js'); + rli.close(); + rli.question('How are you?') + .then(common.mustNotCall(), common.expectsError({ + code: 'ERR_USE_AFTER_CLOSE', + name: 'Error' + })); + assert.notStrictEqual(rli.getPrompt(), 'How are you?'); + })); + fi.emit('data', 'Node.js\n'); + } + + // Can create a new readline Interface with a null output argument { const [rli, fi] = getInterface({ output: null, terminal });