From 9762ece0f24fa234a9b8a5bda558fac37c1f899c Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Sat, 29 Mar 2025 22:20:33 +0000 Subject: [PATCH] readline: add stricter validation for functions called after closed --- lib/internal/main/repl.js | 1 - lib/internal/readline/interface.js | 9 ++++ lib/internal/repl/history.js | 6 ++- lib/repl.js | 6 +-- test/parallel/test-readline-interface.js | 41 +++++++++++++++++++ .../test-readline-promises-interface.js | 2 +- .../test-readline-promises-tab-complete.js | 4 +- test/parallel/test-repl-close.js | 18 ++++++++ test/parallel/test-repl-import-referrer.js | 10 ++--- test/parallel/test-repl-no-terminal.js | 13 ++++-- .../test-repl-uncaught-exception-async.js | 2 +- 11 files changed, 93 insertions(+), 19 deletions(-) create mode 100644 test/parallel/test-repl-close.js diff --git a/lib/internal/main/repl.js b/lib/internal/main/repl.js index 823040321262f6..7bf0eeec90fac9 100644 --- a/lib/internal/main/repl.js +++ b/lib/internal/main/repl.js @@ -45,7 +45,6 @@ if (process.env.NODE_REPL_EXTERNAL_MODULE) { } repl.on('exit', () => { if (repl._flushing) { - repl.pause(); return repl.once('flushHistory', () => { process.exit(); }); diff --git a/lib/internal/readline/interface.js b/lib/internal/readline/interface.js index 89f1a8d9df565e..6c9e20ac6c6f56 100644 --- a/lib/internal/readline/interface.js +++ b/lib/internal/readline/interface.js @@ -611,6 +611,9 @@ class Interface extends InterfaceConstructor { * @returns {void | Interface} */ pause() { + if (this.closed) { + throw new ERR_USE_AFTER_CLOSE('readline'); + } if (this.paused) return; this.input.pause(); this.paused = true; @@ -623,6 +626,9 @@ class Interface extends InterfaceConstructor { * @returns {void | Interface} */ resume() { + if (this.closed) { + throw new ERR_USE_AFTER_CLOSE('readline'); + } if (!this.paused) return; this.input.resume(); this.paused = false; @@ -643,6 +649,9 @@ class Interface extends InterfaceConstructor { * @returns {void} */ write(d, key) { + if (this.closed) { + throw new ERR_USE_AFTER_CLOSE('readline'); + } if (this.paused) this.resume(); if (this.terminal) { this[kTtyWrite](d, key); diff --git a/lib/internal/repl/history.js b/lib/internal/repl/history.js index 09647493895707..a4fc4e376e990d 100644 --- a/lib/internal/repl/history.js +++ b/lib/internal/repl/history.js @@ -116,8 +116,10 @@ function setupHistory(repl, historyPath, ready) { // Reading the file data out erases it repl.once('flushHistory', function() { - repl.resume(); - ready(null, repl); + if (!repl.closed) { + repl.resume(); + ready(null, repl); + } }); flushHistory(); }); diff --git a/lib/repl.js b/lib/repl.js index 42fa0dc5ae640c..88fd10e3fbc9c9 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -974,9 +974,9 @@ function REPLServer(prompt, self.output.write(self.writer(ret) + '\n'); } - // Display prompt again (unless we already did by emitting the 'error' - // event on the domain instance). - if (!e) { + // If the REPL sever hasn't closed display prompt again (unless we already + // did by emitting the 'error' event on the domain instance). + if (!self.closed && !e) { self[kLastCommandErrored] = false; self.displayPrompt(); } diff --git a/test/parallel/test-readline-interface.js b/test/parallel/test-readline-interface.js index 12ba0c709622e9..c640654a7c742d 100644 --- a/test/parallel/test-readline-interface.js +++ b/test/parallel/test-readline-interface.js @@ -1202,6 +1202,47 @@ for (let i = 0; i < 12; i++) { fi.emit('data', 'Node.js\n'); } + // Call write 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.write('I said Node.js'); + }, { + name: 'Error', + code: 'ERR_USE_AFTER_CLOSE' + }); + })); + fi.emit('data', 'Node.js\n'); + } + + // Call pause/resume after close + { + const [rli, fi] = getInterface({ terminal }); + rli.question('What\'s your name?', common.mustCall((name) => { + assert.strictEqual(name, 'Node.js'); + rli.close(); + // No 'resume' nor 'pause' event should be emitted after close + rli.on('resume', common.mustNotCall()); + rli.on('pause', common.mustNotCall()); + assert.throws(() => { + rli.pause(); + }, { + name: 'Error', + code: 'ERR_USE_AFTER_CLOSE' + }); + assert.throws(() => { + rli.resume(); + }, { + name: 'Error', + code: 'ERR_USE_AFTER_CLOSE' + }); + })); + 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 32aab1b60c2ee5..12d72f49735401 100644 --- a/test/parallel/test-readline-promises-interface.js +++ b/test/parallel/test-readline-promises-interface.js @@ -204,7 +204,7 @@ function assertCursorRowsAndCols(rli, rows, cols) { fi.emit('data', character); } fi.emit('data', '\n'); - rli.close(); + fi.end(); } // \t when there is no completer function should behave like an ordinary diff --git a/test/parallel/test-readline-promises-tab-complete.js b/test/parallel/test-readline-promises-tab-complete.js index d8b0ac30ee779d..602bdd9e7965bf 100644 --- a/test/parallel/test-readline-promises-tab-complete.js +++ b/test/parallel/test-readline-promises-tab-complete.js @@ -80,7 +80,7 @@ if (process.env.TERM === 'dumb') { output = ''; }); } - rli.close(); + fi.end(); }); }); }); @@ -114,5 +114,5 @@ if (process.env.TERM === 'dumb') { assert.match(output, /^Tab completion error: Error: message/); output = ''; }); - rli.close(); + fi.end(); } diff --git a/test/parallel/test-repl-close.js b/test/parallel/test-repl-close.js new file mode 100644 index 00000000000000..0e20ddfb0517fc --- /dev/null +++ b/test/parallel/test-repl-close.js @@ -0,0 +1,18 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const cp = require('child_process'); + +const child = cp.spawn(process.execPath, ['--interactive']); + +let output = ''; +child.stdout.on('data', (data) => { + output += data; +}); + +child.on('exit', common.mustCall(() => { + assert.doesNotMatch(output, /Uncaught Error/); +})); + +child.stdin.write('await null;\n'); +child.stdin.write('.exit\n'); diff --git a/test/parallel/test-repl-import-referrer.js b/test/parallel/test-repl-import-referrer.js index bc2cda49b496fb..8e242f01922d58 100644 --- a/test/parallel/test-repl-import-referrer.js +++ b/test/parallel/test-repl-import-referrer.js @@ -15,11 +15,11 @@ child.stdout.on('data', (data) => { }); child.on('exit', common.mustCall(() => { - const results = output.replace(/^> /mg, '').split('\n').slice(2); - assert.deepStrictEqual( - results, - ['[Module: null prototype] { message: \'A message\' }', ''] - ); + const result = output.replace(/^> /mg, '').split('\n').slice(2); + assert.deepStrictEqual(result, [ + '[Module: null prototype] { message: \'A message\' }', + '', + ]); })); child.stdin.write('await import(\'./message.mjs\');\n'); diff --git a/test/parallel/test-repl-no-terminal.js b/test/parallel/test-repl-no-terminal.js index 60f97b52e26942..f569adcc6322cf 100644 --- a/test/parallel/test-repl-no-terminal.js +++ b/test/parallel/test-repl-no-terminal.js @@ -1,7 +1,12 @@ 'use strict'; const common = require('../common'); - +const ArrayStream = require('../common/arraystream'); const repl = require('repl'); -const r = repl.start({ terminal: false }); -r.setupHistory('/nonexistent/file', common.mustSucceed()); -process.stdin.unref?.(); + +const stream = new ArrayStream(); + +const replServer = repl.start({ terminal: false, input: stream, output: stream }); + +replServer.setupHistory('/nonexistent/file', common.mustSucceed(() => { + replServer.close(); +})); diff --git a/test/parallel/test-repl-uncaught-exception-async.js b/test/parallel/test-repl-uncaught-exception-async.js index 366a4e6f2968af..24710e062e0b75 100644 --- a/test/parallel/test-repl-uncaught-exception-async.js +++ b/test/parallel/test-repl-uncaught-exception-async.js @@ -34,9 +34,9 @@ r.write( ' throw new RangeError("abc");\n' + '}, 1);console.log()\n' ); -r.close(); setTimeout(() => { + r.close(); const len = process.listenerCount('uncaughtException'); process.removeAllListeners('uncaughtException'); assert.strictEqual(len, 0);