Skip to content

Commit

Permalink
readline: fix question still called after closed
Browse files Browse the repository at this point in the history
resolve: #42450

PR-URL: #42464
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
  • Loading branch information
meixg committed Apr 8, 2022
1 parent 0bac547 commit d36b60e
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 1 deletion.
8 changes: 8 additions & 0 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -2802,6 +2802,14 @@ import 'package-name'; // supported

`import` with URL schemes other than `file` and `data` is unsupported.

<a id="ERR_USE_AFTER_CLOSE"></a>

### `ERR_USE_AFTER_CLOSE`

> Stability: 1 - Experimental
An attempt was made to use something that was already closed.

<a id="ERR_VALID_PERFORMANCE_ENTRY_TYPE"></a>

### `ERR_VALID_PERFORMANCE_ENTRY_TYPE`
Expand Down
6 changes: 6 additions & 0 deletions doc/api/readline.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
8 changes: 7 additions & 1 deletion lib/internal/readline/interface.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down
34 changes: 34 additions & 0 deletions test/parallel/test-readline-interface.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand Down
17 changes: 17 additions & 0 deletions test/parallel/test-readline-promises-interface.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand Down

0 comments on commit d36b60e

Please sign in to comment.