Skip to content

Commit

Permalink
readline: fix question stack overflow
Browse files Browse the repository at this point in the history
This commit fixes readline interface's question callback wrapping when
it is being called with `signal` option. Previous version completely
overwrites passed callback and throws "Maximum call stack size exceeded"
error.

PR-URL: nodejs#43320
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
chapko authored and italojs committed Jun 12, 2022
1 parent da234e8 commit 526d411
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 1 deletion.
3 changes: 2 additions & 1 deletion lib/readline.js
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,10 @@ Interface.prototype.question = function(query, options, cb) {
const cleanup = () => {
options.signal.removeEventListener(onAbort);
};
const originalCb = cb;
cb = typeof cb === 'function' ? (answer) => {
cleanup();
return cb(answer);
return originalCb(answer);
} : cleanup;
}

Expand Down
24 changes: 24 additions & 0 deletions test/parallel/test-readline-interface.js
Original file line number Diff line number Diff line change
Expand Up @@ -1006,6 +1006,17 @@ for (let i = 0; i < 12; i++) {
rli.close();
}

// Calling the question callback with abort signal
{
const [rli] = getInterface({ terminal });
const { signal } = new AbortController();
rli.question('foo?', { signal }, common.mustCall((answer) => {
assert.strictEqual(answer, 'bar');
}));
rli.write('bar\n');
rli.close();
}

// Calling the question multiple times
{
const [rli] = getInterface({ terminal });
Expand All @@ -1030,6 +1041,19 @@ for (let i = 0; i < 12; i++) {
rli.close();
}

// Calling the promisified question with abort signal
{
const [rli] = getInterface({ terminal });
const question = util.promisify(rli.question).bind(rli);
const { signal } = new AbortController();
question('foo?', { signal })
.then(common.mustCall((answer) => {
assert.strictEqual(answer, 'bar');
}));
rli.write('bar\n');
rli.close();
}

// Aborting a question
{
const ac = new AbortController();
Expand Down
11 changes: 11 additions & 0 deletions test/parallel/test-readline-promises-interface.js
Original file line number Diff line number Diff line change
Expand Up @@ -909,6 +909,17 @@ for (let i = 0; i < 12; i++) {
rli.close();
}

// Calling the question callback with abort signal
{
const [rli] = getInterface({ terminal });
const { signal } = new AbortController();
rli.question('foo?', { signal }).then(common.mustCall((answer) => {
assert.strictEqual(answer, 'bar');
}));
rli.write('bar\n');
rli.close();
}

// Aborting a question
{
const ac = new AbortController();
Expand Down

0 comments on commit 526d411

Please sign in to comment.