improve bash history importing

Reject more invalid commands from the bash history file.

Fixes #3636

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",
+        const wchar_t *expected[] = {L"sleep 123",
-                                     L"history --help", L"echo foo",       NULL};
+                                     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)) {

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.
+    parse_node_tree_t parse_tree;
-    // TODO: We should probably should skip other commands.
+    wcstring wide_line = str2wcstring(line);
-    const char *const ignore_prefixes[] = {"export ", "#"};
+    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++) {
+    // In doing this test do not allow incomplete strings. Hence the "false" argument.
-        const char *prefix = ignore_prefixes[i];
+    parse_error_list_t errors;
-        if (!line.compare(0, strlen(prefix), prefix)) {
+    parse_util_detect_errors(wide_line, &errors, false);
-            return 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;

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