repl: Fixed node repl history edge case.

If the deprecated NODE_REPL_HISTORY_FILE is set to default node history file path ($HOME/.node_repl_history) and the file doesn't exist, then node creates the file and then crashes when it tries to parse that file as JSON thinking that it's an older JSON formatted history file. This fixes that bug. This patch also prevents node repl from throwing if the old history file is empty or if $HOME/.node_repl_history is empty. Fixes: #4102 PR-URL: #4108 Reviewed-By: Jeremiah Senkpiel <[email protected]>
nodejs · Jan 19, 2016 · db0e906 · db0e906
1 parent 4856420
commit db0e906
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 3 deletions.
diff --git a/lib/internal/repl.js b/lib/internal/repl.js
@@ -120,15 +120,29 @@ function setupHistory(repl, historyPath, oldHistoryPath, ready) {
 
     if (data) {
       repl.history = data.split(/[\n\r]+/, repl.historySize);
-    } else if (oldHistoryPath) {
+    } else if (oldHistoryPath === historyPath) {
+      // If pre-v3.0, the user had set NODE_REPL_HISTORY_FILE to
+      // ~/.node_repl_history, warn the user about it and proceed.
+      repl._writeToOutput(
+          '\nThe old repl history file has the same name and location as ' +
+          `the new one i.e., ${historyPath} and is empty.\nUsing it as is.\n`);
+      repl._refreshLine();
+
+    }  else if (oldHistoryPath) {
       // Grab data from the older pre-v3.0 JSON NODE_REPL_HISTORY_FILE format.
       repl._writeToOutput(
           '\nConverting old JSON repl history to line-separated history.\n' +
           `The new repl history file can be found at ${historyPath}.\n`);
       repl._refreshLine();
 
       try {
-        repl.history = JSON.parse(fs.readFileSync(oldHistoryPath, 'utf8'));
+        // Pre-v3.0, repl history was stored as JSON.
+        // Try and convert it to line separated history.
+        const oldReplJSONHistory = fs.readFileSync(oldHistoryPath, 'utf8');
+
+        // Only attempt to use the history if there was any.
+        if (oldReplJSONHistory) repl.history = JSON.parse(oldReplJSONHistory);
+
         if (!Array.isArray(repl.history)) {
           throw new Error('Expected array, got ' + typeof repl.history);
         }

diff --git a/test/fixtures/.empty-repl-history-file b/test/fixtures/.empty-repl-history-file
diff --git a/test/parallel/test-repl-persistent-history.js b/test/parallel/test-repl-persistent-history.js
@@ -65,16 +65,20 @@ const homedirErr = '\nError: Could not get the home directory.\n' +
                    'REPL session history will not be persisted.\n';
 const replFailedRead = '\nError: Could not open history file.\n' +
                        'REPL session history will not be persisted.\n';
+const sameHistoryFilePaths = '\nThe old repl history file has the same name ' +
+                             'and location as the new one i.e., ' +
+                             path.join(common.tmpDir, '.node_repl_history') +
+                             ' and is empty.\nUsing it as is.\n';
 // File paths
 const fixtures = path.join(common.testDir, 'fixtures');
 const historyFixturePath = path.join(fixtures, '.node_repl_history');
 const historyPath = path.join(common.tmpDir, '.fixture_copy_repl_history');
 const historyPathFail = path.join(common.tmpDir, '.node_repl\u0000_history');
 const oldHistoryPath = path.join(fixtures, 'old-repl-history-file.json');
 const enoentHistoryPath = path.join(fixtures, 'enoent-repl-history-file.json');
+const emptyHistoryPath = path.join(fixtures, '.empty-repl-history-file');
 const defaultHistoryPath = path.join(common.tmpDir, '.node_repl_history');
 
-
 const tests = [{
   env: { NODE_REPL_HISTORY: '' },
   test: [UP],
@@ -92,6 +96,16 @@ const tests = [{
   test: [UP],
   expected: [prompt, replDisabled, prompt]
 },
+{
+  env: { NODE_REPL_HISTORY_FILE: emptyHistoryPath },
+  test: [UP],
+  expected: [prompt, convertMsg, prompt]
+},
+{
+  env: { NODE_REPL_HISTORY_FILE: defaultHistoryPath },
+  test: [UP],
+  expected: [prompt, sameHistoryFilePaths, prompt]
+},
 {
   env: { NODE_REPL_HISTORY: historyPath },
   test: [UP, CLEAR],