readline: add stricter validation for functions called after closed

PR-URL: #57680
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Edy Silva <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>

Author: dario-piotrowicz

lib/internal/readline/interface.js

@@ -599,6 +599,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;
@@ -611,6 +614,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;
@@ -631,6 +637,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);

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-import-referrer.js

@@ -8,20 +8,24 @@ const args = ['--interactive'];
 const opts = { cwd: fixtures.path('es-modules') };
 const child = cp.spawn(process.execPath, args, opts);
 
-let output = '';
+const outputs = [];
 child.stdout.setEncoding('utf8');
 child.stdout.on('data', (data) => {
-  output += data;
+  outputs.push(data);
+  if (outputs.length === 3) {
+    // All the expected outputs have been received
+    // so we can close the child process's stdin
+    child.stdin.end();
+  }
 });
 
 child.on('exit', common.mustCall(() => {
-  const results = output.replace(/^> /mg, '').split('\n').slice(2);
-  assert.deepStrictEqual(
+  const results = outputs[2].split('\n')[0];
+  assert.strictEqual(
     results,
-    ['[Module: null prototype] { message: \'A message\' }', '']
+    '[Module: null prototype] { message: \'A message\' }'
   );
 }));
 
 child.stdin.write('await import(\'./message.mjs\');\n');
 child.stdin.write('.exit');
-child.stdin.end();

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

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