From 512506f0f986cadfa18cefa6dbc10ccab4fd0541 Mon Sep 17 00:00:00 2001 From: Kurtis Rader Date: Tue, 5 Jul 2016 22:33:32 -0700 Subject: [PATCH] don't print internal token in error message Attempting to execute something like `exec "$test"` results in a fish internal token (a Unicode private use char) being printed in the resulting error message. That's obviously not desirable as well as confusing. Fixes #3187 --- src/parse_execution.cpp | 36 +++++++++++++++++++++++++++++------- tests/vars_as_commands.err | 6 ++++++ tests/vars_as_commands.in | 8 ++++++++ tests/vars_as_commands.out | 0 4 files changed, 43 insertions(+), 7 deletions(-) create mode 100644 tests/vars_as_commands.err create mode 100644 tests/vars_as_commands.in create mode 100644 tests/vars_as_commands.out diff --git a/src/parse_execution.cpp b/src/parse_execution.cpp index 7a64f6279..5ee3a12d0 100644 --- a/src/parse_execution.cpp +++ b/src/parse_execution.cpp @@ -17,6 +17,8 @@ #include #include #include + +#include #include #include #include @@ -711,6 +713,29 @@ parse_execution_result_t parse_execution_context_t::report_unmatched_wildcard_er return parse_execution_errored; } +// Given a command string that might contain fish special tokens return a string without those +// tokens. +static wcstring reconstruct_orig_cmd(wcstring cmd_str) { + // TODO(krader1961): Figure out what VARIABLE_EXPAND means in this context. After looking at the + // code and doing various tests I couldn't figure out why that token would be present when this + // code is run. I was therefore unable to determine how to substitute its presence in the error + // message. + wcstring orig_cmd = cmd_str; + + if (cmd_str.find(VARIABLE_EXPAND_SINGLE) != std::string::npos) { + // Variable was quoted to force expansion of multiple elements into a single element. + // + // The following isn't entirely correct. For example, $abc"$def" will become "$abc$def". + // However, anyone writing the former is asking for trouble so I don't feel bad about not + // accurately reconstructing what they typed. + wcstring new_cmd_str = wcstring(cmd_str); + std::replace(new_cmd_str.begin(), new_cmd_str.end(), (wchar_t)VARIABLE_EXPAND_SINGLE, L'$'); + orig_cmd = L"\"" + new_cmd_str + L"\""; + } + + return orig_cmd; +} + /// Handle the case of command not found. parse_execution_result_t parse_execution_context_t::handle_command_not_found( const wcstring &cmd_str, const parse_node_t &statement_node, int err_code) { @@ -744,15 +769,12 @@ parse_execution_result_t parse_execution_context_t::handle_command_not_found( this->report_error(statement_node, ERROR_BAD_COMMAND_ASSIGN_ERR_MSG, name_str.c_str(), val_str.c_str()); } - } else if ((cmd[0] == L'$' || cmd[0] == VARIABLE_EXPAND || cmd[0] == VARIABLE_EXPAND_SINGLE) && - cmd[1] != L'\0') { + } else if (wcschr(cmd, L'$') || wcschr(cmd, VARIABLE_EXPAND_SINGLE) || + wcschr(cmd, VARIABLE_EXPAND)) { + wcstring eval_cmd = reconstruct_orig_cmd(cmd_str); this->report_error(statement_node, _(L"Variables may not be used as commands. In fish, " L"please define a function or use 'eval %ls'."), - cmd); - } else if (wcschr(cmd, L'$')) { - this->report_error( - statement_node, - _(L"Commands may not contain variables. In fish, please use 'eval %ls'."), cmd); + eval_cmd.c_str()); } else if (err_code != ENOENT) { this->report_error(statement_node, _(L"The file '%ls' is not executable by this user"), cmd ? cmd : L"UNKNOWN"); diff --git a/tests/vars_as_commands.err b/tests/vars_as_commands.err new file mode 100644 index 000000000..ccf504cd3 --- /dev/null +++ b/tests/vars_as_commands.err @@ -0,0 +1,6 @@ +Variables may not be used as commands. In fish, please define a function or use 'eval $test'. +fish: exec $test + ^ +Variables may not be used as commands. In fish, please define a function or use 'eval "$test"'. +fish: exec "$test" + ^ diff --git a/tests/vars_as_commands.in b/tests/vars_as_commands.in new file mode 100644 index 000000000..8e42abee2 --- /dev/null +++ b/tests/vars_as_commands.in @@ -0,0 +1,8 @@ +# Test that using variables as command names work correctly. + +# Both of these should generate errors about using variables as command names. +# Verify that the expected errors are written to stderr. +exec $test +exec "$test" + +exit 0 diff --git a/tests/vars_as_commands.out b/tests/vars_as_commands.out new file mode 100644 index 000000000..e69de29bb