Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

readline: fix .question still called after closed #42464

Merged
merged 9 commits into from
Apr 8, 2022
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