readline: add stricter validation for functions called after closed

PR-URL: #58283
Fixes: #57678
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>

Author: dario-piotrowicz

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();
           });

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);

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();
     });

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();
       }

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 });

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

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();
 }

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');

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');

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();
+}));