From d7640a37b22ad2c10a0742620500f8c3ef84204e Mon Sep 17 00:00:00 2001
From: Mattias Runge-Broberg <mattias@runge.se>
Date: Sat, 30 May 2020 22:17:09 +0200
Subject: [PATCH] readline: add history event and option to set initial history

Add a history event which is emitted when the history has
been changed. This enables persisting of the history in
some way but also to allows a listener to alter the
history. One use-case could be to prevent passwords from
ending up in the history.

A constructor option is also added to allow for setting
an initial history list when creating a Readline interface.

PR-URL: https://github.com/nodejs/node/pull/33662
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 doc/api/readline.md                      | 35 +++++++++-
 lib/readline.js                          | 22 ++++++-
 test/parallel/test-readline-interface.js | 81 ++++++++++++++++--------
 3 files changed, 106 insertions(+), 32 deletions(-)

diff --git a/doc/api/readline.md b/doc/api/readline.md
index 38c4cb2f10656a..2a2a0018a3dfc8 100644
--- a/doc/api/readline.md
+++ b/doc/api/readline.md
@@ -88,6 +88,28 @@ rl.on('line', (input) => {
 });
 ```
 
+### Event: `'history'`
+<!-- YAML
+added: REPLACEME
+-->
+
+The `'history'` event is emitted whenever the history array has changed.
+
+The listener function is called with an array containing the history array.
+It will reflect all changes, added lines and removed lines due to
+`historySize` and `removeHistoryDuplicates`.
+
+The primary purpose is to allow a listener to persist the history.
+It is also possible for the listener to change the history object. This
+could be useful to prevent certain lines to be added to the history, like
+a password.
+
+```js
+rl.on('history', (history) => {
+  console.log(`Received: ${history}`);
+});
+```
+
 ### Event: `'pause'`
 <!-- YAML
 added: v0.7.5
@@ -522,6 +544,9 @@ the current position of the cursor down.
 <!-- YAML
 added: v0.1.98
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/33662
+    description: The `history` option is supported now.
   - version: v13.9.0
     pr-url: https://github.com/nodejs/node/pull/31318
     description: The `tabSize` option is supported now.
@@ -550,11 +575,18 @@ changes:
   * `terminal` {boolean} `true` if the `input` and `output` streams should be
     treated like a TTY, and have ANSI/VT100 escape codes written to it.
     **Default:** checking `isTTY` on the `output` stream upon instantiation.
+  * `history` {string[]} Initial list of history lines. This option makes sense
+    only if `terminal` is set to `true` by the user or by an internal `output`
+    check, otherwise the history caching mechanism is not initialized at all.
+    **Default:** `[]`.
   * `historySize` {number} Maximum number of history lines retained. To disable
     the history set this value to `0`. This option makes sense only if
     `terminal` is set to `true` by the user or by an internal `output` check,
     otherwise the history caching mechanism is not initialized at all.
     **Default:** `30`.
+  * `removeHistoryDuplicates` {boolean} If `true`, when a new input line added
+    to the history list duplicates an older one, this removes the older line
+    from the list. **Default:** `false`.
   * `prompt` {string} The prompt string to use. **Default:** `'> '`.
   * `crlfDelay` {number} If the delay between `\r` and `\n` exceeds
     `crlfDelay` milliseconds, both `\r` and `\n` will be treated as separate
@@ -562,9 +594,6 @@ changes:
     `100`. It can be set to `Infinity`, in which case `\r` followed by `\n`
     will always be considered a single newline (which may be reasonable for
     [reading files][] with `\r\n` line delimiter). **Default:** `100`.
-  * `removeHistoryDuplicates` {boolean} If `true`, when a new input line added
-    to the history list duplicates an older one, this removes the older line
-    from the list. **Default:** `false`.
   * `escapeCodeTimeout` {number} The duration `readline` will wait for a
     character (when reading an ambiguous key sequence in milliseconds one that
     can both form a complete key sequence using the input read so far and can
diff --git a/lib/readline.js b/lib/readline.js
index dc6ca288bc29fd..a284cc8afcf0d0 100644
--- a/lib/readline.js
+++ b/lib/readline.js
@@ -73,6 +73,7 @@ const {
   ERR_INVALID_CURSOR_POS,
 } = codes;
 const {
+  validateArray,
   validateCallback,
   validateString,
   validateUint32,
@@ -142,6 +143,7 @@ function Interface(input, output, completer, terminal) {
   this.tabSize = 8;
 
   FunctionPrototypeCall(EventEmitter, this,);
+  let history;
   let historySize;
   let removeHistoryDuplicates = false;
   let crlfDelay;
@@ -152,6 +154,7 @@ function Interface(input, output, completer, terminal) {
     output = input.output;
     completer = input.completer;
     terminal = input.terminal;
+    history = input.history;
     historySize = input.historySize;
     if (input.tabSize !== undefined) {
       validateUint32(input.tabSize, 'tabSize', true);
@@ -179,6 +182,12 @@ function Interface(input, output, completer, terminal) {
     throw new ERR_INVALID_ARG_VALUE('completer', completer);
   }
 
+  if (history === undefined) {
+    history = [];
+  } else {
+    validateArray(history, 'history');
+  }
+
   if (historySize === undefined) {
     historySize = kHistorySize;
   }
@@ -201,6 +210,7 @@ function Interface(input, output, completer, terminal) {
   this[kSubstringSearch] = null;
   this.output = output;
   this.input = input;
+  this.history = history;
   this.historySize = historySize;
   this.removeHistoryDuplicates = !!removeHistoryDuplicates;
   this.crlfDelay = crlfDelay ?
@@ -300,7 +310,6 @@ function Interface(input, output, completer, terminal) {
     // Cursor position on the line.
     this.cursor = 0;
 
-    this.history = [];
     this.historyIndex = -1;
 
     if (output !== null && output !== undefined)
@@ -447,7 +456,16 @@ Interface.prototype._addHistory = function() {
   }
 
   this.historyIndex = -1;
-  return this.history[0];
+
+  // The listener could change the history object, possibly
+  // to remove the last added entry if it is sensitive and should
+  // not be persisted in the history, like a password
+  const line = this.history[0];
+
+  // Emit history event to notify listeners of update
+  this.emit('history', this.history);
+
+  return line;
 };
 
 
diff --git a/test/parallel/test-readline-interface.js b/test/parallel/test-readline-interface.js
index db844e7ce122a1..1a7e0440a2bb81 100644
--- a/test/parallel/test-readline-interface.js
+++ b/test/parallel/test-readline-interface.js
@@ -116,35 +116,30 @@ function assertCursorRowsAndCols(rli, rows, cols) {
     code: 'ERR_INVALID_ARG_VALUE'
   });
 
-  // Constructor throws if historySize is not a positive number
-  assert.throws(() => {
-    readline.createInterface({
-      input,
-      historySize: 'not a number'
-    });
-  }, {
-    name: 'RangeError',
-    code: 'ERR_INVALID_ARG_VALUE'
-  });
-
-  assert.throws(() => {
-    readline.createInterface({
-      input,
-      historySize: -1
+  // Constructor throws if history is not an array
+  ['not an array', 123, 123n, {}, true, Symbol(), null].forEach((history) => {
+    assert.throws(() => {
+      readline.createInterface({
+        input,
+        history,
+      });
+    }, {
+      name: 'TypeError',
+      code: 'ERR_INVALID_ARG_TYPE'
     });
-  }, {
-    name: 'RangeError',
-    code: 'ERR_INVALID_ARG_VALUE'
   });
 
-  assert.throws(() => {
-    readline.createInterface({
-      input,
-      historySize: NaN
+  // Constructor throws if historySize is not a positive number
+  ['not a number', -1, NaN, {}, true, Symbol(), null].forEach((historySize) => {
+    assert.throws(() => {
+      readline.createInterface({
+        input,
+        historySize,
+      });
+    }, {
+      name: 'RangeError',
+      code: 'ERR_INVALID_ARG_VALUE'
     });
-  }, {
-    name: 'RangeError',
-    code: 'ERR_INVALID_ARG_VALUE'
   });
 
   // Check for invalid tab sizes.
@@ -239,6 +234,38 @@ function assertCursorRowsAndCols(rli, rows, cols) {
   rli.close();
 }
 
+// Adding history lines should emit the history event with
+// the history array
+{
+  const [rli, fi] = getInterface({ terminal: true });
+  const expectedLines = ['foo', 'bar', 'baz', 'bat'];
+  rli.on('history', common.mustCall((history) => {
+    const expectedHistory = expectedLines.slice(0, history.length).reverse();
+    assert.deepStrictEqual(history, expectedHistory);
+  }, expectedLines.length));
+  for (const line of expectedLines) {
+    fi.emit('data', `${line}\n`);
+  }
+  rli.close();
+}
+
+// Altering the history array in the listener should not alter
+// the line being processed
+{
+  const [rli, fi] = getInterface({ terminal: true });
+  const expectedLine = 'foo';
+  rli.on('history', common.mustCall((history) => {
+    assert.strictEqual(history[0], expectedLine);
+    history.shift();
+  }));
+  rli.on('line', common.mustCall((line) => {
+    assert.strictEqual(line, expectedLine);
+    assert.strictEqual(rli.history.length, 0);
+  }));
+  fi.emit('data', `${expectedLine}\n`);
+  rli.close();
+}
+
 // Duplicate lines are removed from history when
 // `options.removeHistoryDuplicates` is `true`
 {
@@ -774,7 +801,7 @@ for (let i = 0; i < 12; i++) {
     assert.strictEqual(rli.historySize, 0);
 
     fi.emit('data', 'asdf\n');
-    assert.deepStrictEqual(rli.history, terminal ? [] : undefined);
+    assert.deepStrictEqual(rli.history, []);
     rli.close();
   }
 
@@ -784,7 +811,7 @@ for (let i = 0; i < 12; i++) {
     assert.strictEqual(rli.historySize, 30);
 
     fi.emit('data', 'asdf\n');
-    assert.deepStrictEqual(rli.history, terminal ? ['asdf'] : undefined);
+    assert.deepStrictEqual(rli.history, terminal ? ['asdf'] : []);
     rli.close();
   }