From 7e8a00a26df95f39fbacb4de9218f93de5d06989 Mon Sep 17 00:00:00 2001 From: Eugene Chapko Date: Sun, 12 Jun 2022 13:48:53 +0400 Subject: [PATCH] readline: fix question stack overflow 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: https://github.com/nodejs/node/pull/43320 Reviewed-By: Antoine du Hamel Reviewed-By: Colin Ihrig Reviewed-By: James M Snell --- lib/readline.js | 3 ++- test/parallel/test-readline-interface.js | 24 +++++++++++++++++++ .../test-readline-promises-interface.js | 11 +++++++++ 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/lib/readline.js b/lib/readline.js index 26e277c7abfd15..51ba11577bf813 100644 --- a/lib/readline.js +++ b/lib/readline.js @@ -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; } diff --git a/test/parallel/test-readline-interface.js b/test/parallel/test-readline-interface.js index 82e9d9c2791e48..ba72d282ea6d8e 100644 --- a/test/parallel/test-readline-interface.js +++ b/test/parallel/test-readline-interface.js @@ -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 }); @@ -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(); diff --git a/test/parallel/test-readline-promises-interface.js b/test/parallel/test-readline-promises-interface.js index 73f3d693b4f4d5..2a211025cdb27b 100644 --- a/test/parallel/test-readline-promises-interface.js +++ b/test/parallel/test-readline-promises-interface.js @@ -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();