From 6c4a51d56e9f45f3fbedfb4807c5a5b665055095 Mon Sep 17 00:00:00 2001
From: Kurtis Rader <krader@skepticism.us>
Date: Sat, 20 May 2017 21:03:31 -0700
Subject: [PATCH] improve bash history importing

Reject more invalid commands from the bash history file.

Fixes #3636
---
 src/fish_tests.cpp        |  9 +++++++--
 src/history.cpp           | 30 ++++++++++++++++++++----------
 tests/history_sample_bash |  7 ++++++-
 3 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp
index 95eb8b7e2..d72370e77 100644
--- a/src/fish_tests.cpp
+++ b/src/fish_tests.cpp
@@ -3206,8 +3206,13 @@ void history_tests_t::test_history_formats(void) {
     } else {
         // The results are in the reverse order that they appear in the bash history file.
         // We don't expect whitespace to be elided.
-        const wchar_t *expected[] = {L"sleep 123",      L"    final line", L"echo supsup",
-                                     L"history --help", L"echo foo",       NULL};
+        const wchar_t *expected[] = {L"sleep 123",
+                                     L"    final line",
+                                     L"echo supsup",
+                                     L"export XVAR='exported'",
+                                     L"history --help",
+                                     L"echo foo",
+                                     NULL};
         history_t &test_history = history_t::history_with_name(L"bash_import");
         test_history.populate_from_bash(f);
         if (!history_equals(test_history, expected)) {
diff --git a/src/history.cpp b/src/history.cpp
index 19afc52c0..d87b09c03 100644
--- a/src/history.cpp
+++ b/src/history.cpp
@@ -36,6 +36,7 @@
 #include "lru.h"
 #include "parse_constants.h"
 #include "parse_tree.h"
+#include "parse_util.h"
 #include "path.h"
 #include "reader.h"
 #include "signal.h"
@@ -1677,21 +1678,30 @@ void history_t::populate_from_config_path() {
 static bool should_import_bash_history_line(const std::string &line) {
     if (line.empty()) return false;
 
-    // Very naive tests! Skip `export` and comments.
-    // TODO: We should probably should skip other commands.
-    const char *const ignore_prefixes[] = {"export ", "#"};
+    parse_node_tree_t parse_tree;
+    wcstring wide_line = str2wcstring(line);
+    if (!parse_tree_from_string(wide_line, parse_flag_none, &parse_tree, NULL)) return false;
 
-    for (size_t i = 0; i < sizeof ignore_prefixes / sizeof *ignore_prefixes; i++) {
-        const char *prefix = ignore_prefixes[i];
-        if (!line.compare(0, strlen(prefix), prefix)) {
-            return false;
-        }
-    }
+    // In doing this test do not allow incomplete strings. Hence the "false" argument.
+    parse_error_list_t errors;
+    parse_util_detect_errors(wide_line, &errors, false);
+    if (!errors.empty()) return false;
+
+    // The following are Very naive tests!
+
+    // Skip comments.
+    if (line[0] == '#') return false;
 
     // Skip lines with backticks.
     if (line.find('`') != std::string::npos) return false;
 
-    // Skip lines that end with a backslash since we do not handle multiline commands from bash.
+    // Skip lines with [[...]] and ((...)) since we don't handle those constructs.
+    if (line.find("[[") != std::string::npos) return false;
+    if (line.find("]]") != std::string::npos) return false;
+    if (line.find("((") != std::string::npos) return false;
+    if (line.find("))") != std::string::npos) return false;
+
+    // Skip lines that end with a backslash. We do not handle multiline commands from bash history.
     if (line.back() == '\\') return false;
 
     return true;
diff --git a/tests/history_sample_bash b/tests/history_sample_bash
index bd47a18b9..d800f0397 100644
--- a/tests/history_sample_bash
+++ b/tests/history_sample_bash
@@ -1,7 +1,7 @@
 echo foo
 history --help
 #1339718290
-export HISTTIMEFORMAT='%F %T '
+export XVAR='exported'
 #1339718298
 echo supsup
 #abcde
@@ -11,4 +11,9 @@ echo hello \
     final line
 another `command
     and arg` to skip
+backticks `are not allowed`
+a && echo invalid construct
+[[ x = y ]] && echo double brackets not allowed
+(( 1 = 2 )) && echo double parens not allowed
+posix_cmd_sub $(is not supported)
 sleep 123