From 9419191aa653da91474b8a65b56d44036d59484e Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sun, 24 Aug 2014 14:28:31 -0700 Subject: [PATCH] Clean up variable expansion, and properly handle embedded NULs --- expand.cpp | 123 ++++++++++++++++++-------------------- fish_tests.cpp | 156 ++++++++++++++++++++++++++++++++++++------------- wildcard.cpp | 33 +++++++---- wildcard.h | 4 +- 4 files changed, 199 insertions(+), 117 deletions(-) diff --git a/expand.cpp b/expand.cpp index 5f96fb2f9..c3cb95031 100644 --- a/expand.cpp +++ b/expand.cpp @@ -1049,21 +1049,24 @@ static size_t parse_slice(const wchar_t *in, wchar_t **end_ptr, std::vector &out, long last_idx, parse_error_list_t *errors); - -static int expand_variables2(parser_t &parser, const wcstring &instr, std::vector &out, long last_idx, parse_error_list_t *errors) +static int expand_variables(parser_t &parser, const wcstring &instr, std::vector &out, long last_idx, parse_error_list_t *errors) { - wchar_t *in = wcsdup(instr.c_str()); - int result = expand_variables_internal(parser, in, out, last_idx, errors); - free(in); - return result; -} - -static int expand_variables_internal(parser_t &parser, wchar_t * const in, std::vector &out, long last_idx, parse_error_list_t *errors) -{ - int is_ok= 1; - int empty=0; + // We permit last_idx to be beyond the end of the string if and only if the string is empty + assert(instr.empty() || (last_idx >= 0 && (size_t)last_idx < instr.size())); + + // Make this explicit + if (instr.empty()) + { + append_completion(out, instr); + return true; + } + + bool is_ok = true; + bool empty = false; + const size_t insize = instr.size(); wcstring var_tmp; @@ -1077,7 +1080,7 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std:: for (long i=last_idx; (i>=0) && is_ok && !empty; i--) { - const wchar_t c = in[i]; + const wchar_t c = instr.at(i); if ((c == VARIABLE_EXPAND) || (c == VARIABLE_EXPAND_SINGLE)) { long start_pos = i+1; @@ -1087,17 +1090,15 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std:: stop_pos = start_pos; - while (1) + while (stop_pos < insize) { - const wchar_t nc = in[stop_pos]; - if (!(nc)) - break; + const wchar_t nc = instr.at(stop_pos); if (nc == VARIABLE_EXPAND_EMPTY) { stop_pos++; break; } - if (!(wcsvarchr(nc))) + if (!wcsvarchr(nc)) break; stop_pos++; @@ -1109,13 +1110,13 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std:: if (var_len == 0) { - expand_variable_error(parser, in, stop_pos-1, -1, errors); + expand_variable_error(parser, instr, stop_pos-1, -1, errors); - is_ok = 0; + is_ok = false; break; } - var_tmp.append(in + start_pos, var_len); + var_tmp.append(instr, start_pos, var_len); env_var_t var_val; if (var_len == 1 && var_tmp[0] == VARIABLE_EXPAND_EMPTY) { @@ -1133,22 +1134,22 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std:: if (is_ok) { - tokenize_variable_array(var_val.c_str(), var_item_list); + tokenize_variable_array(var_val, var_item_list); const size_t slice_start = stop_pos; - if (in[slice_start] == L'[') + if (slice_start < insize && instr.at(slice_start) == L'[') { wchar_t *slice_end; size_t bad_pos; all_vars=0; - + const wchar_t *in = instr.c_str(); bad_pos = parse_slice(in + slice_start, &slice_end, var_idx_list, var_pos_list, var_item_list.size()); if (bad_pos != 0) { append_syntax_error(errors, stop_pos + bad_pos, L"Invalid index value"); - is_ok = 0; + is_ok = false; break; } stop_pos = (slice_end-in); @@ -1168,7 +1169,7 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std:: append_syntax_error(errors, slice_start + var_src_pos, ARRAY_BOUNDS_ERR); - is_ok=0; + is_ok = false; var_idx_list.resize(j); break; } @@ -1187,14 +1188,12 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std:: if (is_ok) { - if (is_single) { - in[i]=0; - wcstring res = in; + wcstring res(instr, 0, i); if (i > 0) { - if (in[i-1] != VARIABLE_EXPAND_SINGLE) + if (instr.at(i-1) != VARIABLE_EXPAND_SINGLE) { res.push_back(INTERNAL_SEPARATOR); } @@ -1215,15 +1214,16 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std:: res.append(next); } } - res.append(in + stop_pos); - is_ok &= expand_variables2(parser, res, out, i, errors); + assert(stop_pos <= insize); + res.append(instr, stop_pos, insize - stop_pos); + is_ok &= expand_variables(parser, res, out, i, errors); } else { for (size_t j=0; j 0) { - if (in[i-1] != VARIABLE_EXPAND) + if (instr.at(i-1) != VARIABLE_EXPAND) { new_in.push_back(INTERNAL_SEPARATOR); } @@ -1246,9 +1246,10 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std:: new_in.push_back(VARIABLE_EXPAND_EMPTY); } } + assert(stop_pos <= insize); new_in.append(next); - new_in.append(in + stop_pos); - is_ok &= expand_variables2(parser, new_in, out, i, errors); + new_in.append(instr, stop_pos, insize - stop_pos); + is_ok &= expand_variables(parser, new_in, out, i, errors); } } @@ -1263,8 +1264,9 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std:: // even with no value, we still need to parse out slice syntax // Behave as though we had 1 value, so $foo[1] always works. const size_t slice_start = stop_pos; - if (in[slice_start] == L'[') + if (slice_start < insize && instr.at(slice_start) == L'[') { + const wchar_t *in = instr.c_str(); wchar_t *slice_end; size_t bad_pos; @@ -1294,42 +1296,35 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std:: } } } - - /* - Expand a non-existing variable - */ + + /* Expand a non-existing variable */ if (c == VARIABLE_EXPAND) { - /* - Regular expansion, i.e. expand this argument to nothing - */ - empty = 1; + /* Regular expansion, i.e. expand this argument to nothing */ + empty = true; } else { - /* - Expansion to single argument. - */ + /* Expansion to single argument. */ wcstring res; - res.append(in, i); - if (i > 0 && in[i-1] == VARIABLE_EXPAND_SINGLE) + res.append(instr, 0, i); + if (i > 0 && instr.at(i-1) == VARIABLE_EXPAND_SINGLE) { res.push_back(VARIABLE_EXPAND_EMPTY); } - res.append(in + stop_pos); + assert(stop_pos <= insize); + res.append(instr, stop_pos, insize - stop_pos); - is_ok &= expand_variables2(parser, res, out, i, errors); + is_ok &= expand_variables(parser, res, out, i, errors); return is_ok; } } - - } } if (!empty) { - append_completion(out, in); + append_completion(out, instr); } return is_ok; @@ -1814,7 +1809,7 @@ int expand_string(const wcstring &input, std::vector &output, expa } else { - if (!expand_variables2(parser, next, *out, next.size() - 1, errors)) + if (!expand_variables(parser, next, *out, next.size() - 1, errors)) { return EXPAND_ERROR; } @@ -1878,12 +1873,10 @@ int expand_string(const wcstring &input, std::vector &output, expa for (i=0; i < in->size(); i++) { - wcstring next_str = in->at(i).completion; + wcstring next = in->at(i).completion; int wc_res; - remove_internal_separator(next_str, (EXPAND_SKIP_WILDCARDS & flags) ? true : false); - const wchar_t *next = next_str.c_str(); - + remove_internal_separator(next, (EXPAND_SKIP_WILDCARDS & flags) ? true : false); const bool has_wildcard = wildcard_has(next, 1); if (has_wildcard && (flags & EXECUTABLES_ONLY)) @@ -1893,12 +1886,12 @@ int expand_string(const wcstring &input, std::vector &output, expa else if (((flags & ACCEPT_INCOMPLETE) && (!(flags & EXPAND_SKIP_WILDCARDS))) || has_wildcard) { - const wchar_t *start, *rest; + wcstring start, rest; if (next[0] == '/') { start = L"/"; - rest = &next[1]; + rest = next.substr(1); } else { @@ -1943,7 +1936,7 @@ int expand_string(const wcstring &input, std::vector &output, expa { if (!(flags & ACCEPT_INCOMPLETE)) { - append_completion(*out, next_str); + append_completion(*out, next); } } } diff --git a/fish_tests.cpp b/fish_tests.cpp index 70337af12..744da79d9 100644 --- a/fish_tests.cpp +++ b/fish_tests.cpp @@ -1341,6 +1341,11 @@ static int expand_test(const wchar_t *in, int flags, ...) i++; } va_end(va); + + if (output.size() != i) + { + res = false; + } return res; @@ -1367,6 +1372,11 @@ static void test_expand() { err(L"Cannot skip wildcard expansion"); } + + if (!expand_test(L"/bin/l\\0", ACCEPT_INCOMPLETE, 0)) + { + err(L"Failed to handle null escape in expansion"); + } if (system("mkdir -p /tmp/fish_expand_test/")) err(L"mkdir failed"); if (system("touch /tmp/fish_expand_test/.foo")) err(L"touch failed"); @@ -1858,6 +1868,7 @@ static void test_colors() static void test_complete(void) { say(L"Testing complete"); + const wchar_t *name_strs[] = {L"Foo1", L"Foo2", L"Foo3", L"Bar1", L"Bar2", L"Bar3"}; size_t count = sizeof name_strs / sizeof *name_strs; const wcstring_list_t names(name_strs, name_strs + count); @@ -1937,7 +1948,47 @@ static void test_complete(void) completions.clear(); complete(L"echo \\$Foo", completions, COMPLETION_REQUEST_DEFAULT); do_test(completions.empty()); + + /* File completions */ + char saved_wd[PATH_MAX + 1] = {}; + getcwd(saved_wd, sizeof saved_wd); + if (system("mkdir -p '/tmp/complete_test/'")) err(L"mkdir failed"); + if (system("touch '/tmp/complete_test/testfile'")) err(L"touch failed"); + if (chdir("/tmp/complete_test/")) err(L"chdir failed"); + complete(L"cat te", completions, COMPLETION_REQUEST_DEFAULT); + do_test(completions.size() == 1); + do_test(completions.at(0).completion == L"stfile"); + completions.clear(); + complete(L"cat /tmp/complete_test/te", completions, COMPLETION_REQUEST_DEFAULT); + do_test(completions.size() == 1); + do_test(completions.at(0).completion == L"stfile"); + completions.clear(); + complete(L"echo sup > /tmp/complete_test/te", completions, COMPLETION_REQUEST_DEFAULT); + do_test(completions.size() == 1); + do_test(completions.at(0).completion == L"stfile"); + completions.clear(); + complete(L"echo sup > /tmp/complete_test/te", completions, COMPLETION_REQUEST_DEFAULT); + do_test(completions.size() == 1); + do_test(completions.at(0).completion == L"stfile"); + completions.clear(); + + // Zero escapes can cause problems. See #1631 + complete(L"cat foo\\0", completions, COMPLETION_REQUEST_DEFAULT); + do_test(completions.empty()); + completions.clear(); + complete(L"cat foo\\0bar", completions, COMPLETION_REQUEST_DEFAULT); + do_test(completions.empty()); + completions.clear(); + complete(L"cat \\0", completions, COMPLETION_REQUEST_DEFAULT); + do_test(completions.empty()); + completions.clear(); + complete(L"cat te\\0", completions, COMPLETION_REQUEST_DEFAULT); + do_test(completions.empty()); + completions.clear(); + if (chdir(saved_wd)) err(L"chdir failed"); + if (system("rm -Rf '/tmp/complete_test/'")) err(L"rm failed"); + complete_set_variable_names(NULL); /* Test wraps */ @@ -2004,7 +2055,7 @@ static void test_completion_insertions() TEST_1_COMPLETION(L"'foo^", L"bar", COMPLETE_REPLACES_TOKEN, false, L"bar ^"); } -static void perform_one_autosuggestion_test(const wcstring &command, const wcstring &wd, const wcstring &expected, long line) +static void perform_one_autosuggestion_special_test(const wcstring &command, const wcstring &wd, const wcstring &expected, long line) { wcstring suggestion; bool success = autosuggest_suggest_special(command, wd, suggestion); @@ -2034,57 +2085,81 @@ static void test_autosuggest_suggest_special() if (system("mkdir -p ~/test_autosuggest_suggest_special/")) err(L"mkdir failed"); //make sure tilde is handled const wcstring wd = L"/tmp/autosuggest_test/"; - perform_one_autosuggestion_test(L"cd /tmp/autosuggest_test/0", wd, L"cd /tmp/autosuggest_test/0foobar/", __LINE__); - perform_one_autosuggestion_test(L"cd \"/tmp/autosuggest_test/0", wd, L"cd \"/tmp/autosuggest_test/0foobar/\"", __LINE__); - perform_one_autosuggestion_test(L"cd '/tmp/autosuggest_test/0", wd, L"cd '/tmp/autosuggest_test/0foobar/'", __LINE__); - perform_one_autosuggestion_test(L"cd 0", wd, L"cd 0foobar/", __LINE__); - perform_one_autosuggestion_test(L"cd \"0", wd, L"cd \"0foobar/\"", __LINE__); - perform_one_autosuggestion_test(L"cd '0", wd, L"cd '0foobar/'", __LINE__); + perform_one_autosuggestion_special_test(L"cd /tmp/autosuggest_test/0", wd, L"cd /tmp/autosuggest_test/0foobar/", __LINE__); + perform_one_autosuggestion_special_test(L"cd \"/tmp/autosuggest_test/0", wd, L"cd \"/tmp/autosuggest_test/0foobar/\"", __LINE__); + perform_one_autosuggestion_special_test(L"cd '/tmp/autosuggest_test/0", wd, L"cd '/tmp/autosuggest_test/0foobar/'", __LINE__); + perform_one_autosuggestion_special_test(L"cd 0", wd, L"cd 0foobar/", __LINE__); + perform_one_autosuggestion_special_test(L"cd \"0", wd, L"cd \"0foobar/\"", __LINE__); + perform_one_autosuggestion_special_test(L"cd '0", wd, L"cd '0foobar/'", __LINE__); - perform_one_autosuggestion_test(L"cd /tmp/autosuggest_test/1", wd, L"cd /tmp/autosuggest_test/1foo\\ bar/", __LINE__); - perform_one_autosuggestion_test(L"cd \"/tmp/autosuggest_test/1", wd, L"cd \"/tmp/autosuggest_test/1foo bar/\"", __LINE__); - perform_one_autosuggestion_test(L"cd '/tmp/autosuggest_test/1", wd, L"cd '/tmp/autosuggest_test/1foo bar/'", __LINE__); - perform_one_autosuggestion_test(L"cd 1", wd, L"cd 1foo\\ bar/", __LINE__); - perform_one_autosuggestion_test(L"cd \"1", wd, L"cd \"1foo bar/\"", __LINE__); - perform_one_autosuggestion_test(L"cd '1", wd, L"cd '1foo bar/'", __LINE__); + perform_one_autosuggestion_special_test(L"cd /tmp/autosuggest_test/1", wd, L"cd /tmp/autosuggest_test/1foo\\ bar/", __LINE__); + perform_one_autosuggestion_special_test(L"cd \"/tmp/autosuggest_test/1", wd, L"cd \"/tmp/autosuggest_test/1foo bar/\"", __LINE__); + perform_one_autosuggestion_special_test(L"cd '/tmp/autosuggest_test/1", wd, L"cd '/tmp/autosuggest_test/1foo bar/'", __LINE__); + perform_one_autosuggestion_special_test(L"cd 1", wd, L"cd 1foo\\ bar/", __LINE__); + perform_one_autosuggestion_special_test(L"cd \"1", wd, L"cd \"1foo bar/\"", __LINE__); + perform_one_autosuggestion_special_test(L"cd '1", wd, L"cd '1foo bar/'", __LINE__); - perform_one_autosuggestion_test(L"cd /tmp/autosuggest_test/2", wd, L"cd /tmp/autosuggest_test/2foo\\ \\ bar/", __LINE__); - perform_one_autosuggestion_test(L"cd \"/tmp/autosuggest_test/2", wd, L"cd \"/tmp/autosuggest_test/2foo bar/\"", __LINE__); - perform_one_autosuggestion_test(L"cd '/tmp/autosuggest_test/2", wd, L"cd '/tmp/autosuggest_test/2foo bar/'", __LINE__); - perform_one_autosuggestion_test(L"cd 2", wd, L"cd 2foo\\ \\ bar/", __LINE__); - perform_one_autosuggestion_test(L"cd \"2", wd, L"cd \"2foo bar/\"", __LINE__); - perform_one_autosuggestion_test(L"cd '2", wd, L"cd '2foo bar/'", __LINE__); + perform_one_autosuggestion_special_test(L"cd /tmp/autosuggest_test/2", wd, L"cd /tmp/autosuggest_test/2foo\\ \\ bar/", __LINE__); + perform_one_autosuggestion_special_test(L"cd \"/tmp/autosuggest_test/2", wd, L"cd \"/tmp/autosuggest_test/2foo bar/\"", __LINE__); + perform_one_autosuggestion_special_test(L"cd '/tmp/autosuggest_test/2", wd, L"cd '/tmp/autosuggest_test/2foo bar/'", __LINE__); + perform_one_autosuggestion_special_test(L"cd 2", wd, L"cd 2foo\\ \\ bar/", __LINE__); + perform_one_autosuggestion_special_test(L"cd \"2", wd, L"cd \"2foo bar/\"", __LINE__); + perform_one_autosuggestion_special_test(L"cd '2", wd, L"cd '2foo bar/'", __LINE__); - perform_one_autosuggestion_test(L"cd /tmp/autosuggest_test/3", wd, L"cd /tmp/autosuggest_test/3foo\\\\bar/", __LINE__); - perform_one_autosuggestion_test(L"cd \"/tmp/autosuggest_test/3", wd, L"cd \"/tmp/autosuggest_test/3foo\\bar/\"", __LINE__); - perform_one_autosuggestion_test(L"cd '/tmp/autosuggest_test/3", wd, L"cd '/tmp/autosuggest_test/3foo\\bar/'", __LINE__); - perform_one_autosuggestion_test(L"cd 3", wd, L"cd 3foo\\\\bar/", __LINE__); - perform_one_autosuggestion_test(L"cd \"3", wd, L"cd \"3foo\\bar/\"", __LINE__); - perform_one_autosuggestion_test(L"cd '3", wd, L"cd '3foo\\bar/'", __LINE__); + perform_one_autosuggestion_special_test(L"cd /tmp/autosuggest_test/3", wd, L"cd /tmp/autosuggest_test/3foo\\\\bar/", __LINE__); + perform_one_autosuggestion_special_test(L"cd \"/tmp/autosuggest_test/3", wd, L"cd \"/tmp/autosuggest_test/3foo\\bar/\"", __LINE__); + perform_one_autosuggestion_special_test(L"cd '/tmp/autosuggest_test/3", wd, L"cd '/tmp/autosuggest_test/3foo\\bar/'", __LINE__); + perform_one_autosuggestion_special_test(L"cd 3", wd, L"cd 3foo\\\\bar/", __LINE__); + perform_one_autosuggestion_special_test(L"cd \"3", wd, L"cd \"3foo\\bar/\"", __LINE__); + perform_one_autosuggestion_special_test(L"cd '3", wd, L"cd '3foo\\bar/'", __LINE__); - perform_one_autosuggestion_test(L"cd /tmp/autosuggest_test/4", wd, L"cd /tmp/autosuggest_test/4foo\\'bar/", __LINE__); - perform_one_autosuggestion_test(L"cd \"/tmp/autosuggest_test/4", wd, L"cd \"/tmp/autosuggest_test/4foo'bar/\"", __LINE__); - perform_one_autosuggestion_test(L"cd '/tmp/autosuggest_test/4", wd, L"cd '/tmp/autosuggest_test/4foo\\'bar/'", __LINE__); - perform_one_autosuggestion_test(L"cd 4", wd, L"cd 4foo\\'bar/", __LINE__); - perform_one_autosuggestion_test(L"cd \"4", wd, L"cd \"4foo'bar/\"", __LINE__); - perform_one_autosuggestion_test(L"cd '4", wd, L"cd '4foo\\'bar/'", __LINE__); + perform_one_autosuggestion_special_test(L"cd /tmp/autosuggest_test/4", wd, L"cd /tmp/autosuggest_test/4foo\\'bar/", __LINE__); + perform_one_autosuggestion_special_test(L"cd \"/tmp/autosuggest_test/4", wd, L"cd \"/tmp/autosuggest_test/4foo'bar/\"", __LINE__); + perform_one_autosuggestion_special_test(L"cd '/tmp/autosuggest_test/4", wd, L"cd '/tmp/autosuggest_test/4foo\\'bar/'", __LINE__); + perform_one_autosuggestion_special_test(L"cd 4", wd, L"cd 4foo\\'bar/", __LINE__); + perform_one_autosuggestion_special_test(L"cd \"4", wd, L"cd \"4foo'bar/\"", __LINE__); + perform_one_autosuggestion_special_test(L"cd '4", wd, L"cd '4foo\\'bar/'", __LINE__); - perform_one_autosuggestion_test(L"cd /tmp/autosuggest_test/5", wd, L"cd /tmp/autosuggest_test/5foo\\\"bar/", __LINE__); - perform_one_autosuggestion_test(L"cd \"/tmp/autosuggest_test/5", wd, L"cd \"/tmp/autosuggest_test/5foo\\\"bar/\"", __LINE__); - perform_one_autosuggestion_test(L"cd '/tmp/autosuggest_test/5", wd, L"cd '/tmp/autosuggest_test/5foo\"bar/'", __LINE__); - perform_one_autosuggestion_test(L"cd 5", wd, L"cd 5foo\\\"bar/", __LINE__); - perform_one_autosuggestion_test(L"cd \"5", wd, L"cd \"5foo\\\"bar/\"", __LINE__); - perform_one_autosuggestion_test(L"cd '5", wd, L"cd '5foo\"bar/'", __LINE__); - - perform_one_autosuggestion_test(L"cd ~/test_autosuggest_suggest_specia", wd, L"cd ~/test_autosuggest_suggest_special/", __LINE__); + perform_one_autosuggestion_special_test(L"cd /tmp/autosuggest_test/5", wd, L"cd /tmp/autosuggest_test/5foo\\\"bar/", __LINE__); + perform_one_autosuggestion_special_test(L"cd \"/tmp/autosuggest_test/5", wd, L"cd \"/tmp/autosuggest_test/5foo\\\"bar/\"", __LINE__); + perform_one_autosuggestion_special_test(L"cd '/tmp/autosuggest_test/5", wd, L"cd '/tmp/autosuggest_test/5foo\"bar/'", __LINE__); + perform_one_autosuggestion_special_test(L"cd 5", wd, L"cd 5foo\\\"bar/", __LINE__); + perform_one_autosuggestion_special_test(L"cd \"5", wd, L"cd \"5foo\\\"bar/\"", __LINE__); + perform_one_autosuggestion_special_test(L"cd '5", wd, L"cd '5foo\"bar/'", __LINE__); + perform_one_autosuggestion_special_test(L"cd ~/test_autosuggest_suggest_specia", wd, L"cd ~/test_autosuggest_suggest_special/", __LINE__); + // A single quote should defeat tilde expansion - perform_one_autosuggestion_test(L"cd '~/test_autosuggest_suggest_specia'", wd, L"", __LINE__); + perform_one_autosuggestion_special_test(L"cd '~/test_autosuggest_suggest_specia'", wd, L"", __LINE__); if (system("rm -Rf '/tmp/autosuggest_test/'")) err(L"rm failed"); if (system("rm -Rf ~/test_autosuggest_suggest_special/")) err(L"rm failed"); } +static void perform_one_autosuggestion_should_ignore_test(const wcstring &command, const wcstring &wd, long line) +{ + completion_list_t comps; + complete(command, comps, COMPLETION_REQUEST_AUTOSUGGESTION); + do_test(comps.empty()); + if (! comps.empty()) + { + const wcstring &suggestion = comps.front().completion; + printf("line %ld: complete() expected to return nothing for %ls\n", line, command.c_str()); + printf(" instead got: %ls\n", suggestion.c_str()); + } +} + +static void test_autosuggestion_ignores() +{ + say(L"Testing scenarios that should produce no autosuggestions"); + const wcstring wd = L"/tmp/autosuggest_test/"; + // Do not do file autosuggestions immediately after certain statement terminators - see #1631 + perform_one_autosuggestion_should_ignore_test(L"echo PIPE_TEST|", wd, __LINE__); + perform_one_autosuggestion_should_ignore_test(L"echo PIPE_TEST&", wd, __LINE__); + perform_one_autosuggestion_should_ignore_test(L"echo PIPE_TEST#comment", wd, __LINE__); + perform_one_autosuggestion_should_ignore_test(L"echo PIPE_TEST;", wd, __LINE__); +} + static void test_autosuggestion_combining() { say(L"Testing autosuggestion combining"); @@ -3631,6 +3706,7 @@ int main(int argc, char **argv) if (should_test_function("universal")) test_universal_callbacks(); if (should_test_function("notifiers")) test_universal_notifiers(); if (should_test_function("completion_insertions")) test_completion_insertions(); + if (should_test_function("autosuggestion_ignores")) test_autosuggestion_ignores(); if (should_test_function("autosuggestion_combining")) test_autosuggestion_combining(); if (should_test_function("autosuggest_suggest_special")) test_autosuggest_suggest_special(); if (should_test_function("history")) history_tests_t::test_history(); diff --git a/wildcard.cpp b/wildcard.cpp index 3ec1a8e53..fde6ddc60 100644 --- a/wildcard.cpp +++ b/wildcard.cpp @@ -105,18 +105,14 @@ wildcards using **. /** Hashtable containing all descriptions that describe an executable */ static std::map suffix_map; - -bool wildcard_has(const wchar_t *str, bool internal) +// Implementation of wildcard_has. Needs to take the length to handle embedded nulls (#1631) +static bool wildcard_has_impl(const wchar_t *str, size_t len, bool internal) { - if (!str) - { - debug(2, L"Got null string on line %d of file %s", __LINE__, __FILE__); - return false; - } - + assert(str != NULL); + const wchar_t *end = str + len; if (internal) { - for (; *str; str++) + for (; str < end; str++) { if ((*str == ANY_CHAR) || (*str == ANY_STRING) || (*str == ANY_STRING_RECURSIVE)) return true; @@ -125,7 +121,7 @@ bool wildcard_has(const wchar_t *str, bool internal) else { wchar_t prev=0; - for (; *str; str++) + for (; str < end; str++) { if (((*str == L'*') || (*str == L'?')) && (prev != L'\\')) return true; @@ -136,6 +132,18 @@ bool wildcard_has(const wchar_t *str, bool internal) return false; } +bool wildcard_has(const wchar_t *str, bool internal) +{ + assert(str != NULL); + return wildcard_has_impl(str, wcslen(str), internal); +} + +bool wildcard_has(const wcstring &str, bool internal) +{ + return wildcard_has_impl(str.data(), str.size(), internal); +} + + /** Check whether the string str matches the wildcard string wc. @@ -1084,6 +1092,11 @@ int wildcard_expand(const wchar_t *wc, int wildcard_expand_string(const wcstring &wc, const wcstring &base_dir, expand_flags_t flags, std::vector &outputs) { + /* Hackish fix for 1631. We are about to call c_str(), which will produce a string truncated at any embedded nulls. We could fix this by passing around the size, etc. However embedded nulls are never allowed in a filename, so we just check for them and return 0 (no matches) if there is an embedded null. This isn't quite right, e.g. it will fail for \0?, but that is an edge case. */ + if (wc.find(L'\0') != wcstring::npos) + { + return 0; + } // PCA: not convinced this temporary variable is really necessary std::vector lst; int res = wildcard_expand(wc.c_str(), base_dir.c_str(), flags, lst); diff --git a/wildcard.h b/wildcard.h index 5a3e291a0..8c542221a 100644 --- a/wildcard.h +++ b/wildcard.h @@ -79,9 +79,9 @@ int wildcard_expand_string(const wcstring &wc, const wcstring &base_dir, expand_ */ bool wildcard_match(const wcstring &str, const wcstring &wc, bool leading_dots_fail_to_match = false); - /** Check if the specified string contains wildcards */ -bool wildcard_has(const wchar_t *str, bool internal); +bool wildcard_has(const wcstring &, bool internal); +bool wildcard_has(const wchar_t *, bool internal); /** Test wildcard completion