limit bind mode names to the rules for var names

The bind mode names can be, and are, used in the construction of fish
variable names. So don't allow users to use names that are not legal as
a variable name. This should not break anything since, AFAICT, no
existing fish scripts, including those provided by Oh-My-Fish and
Fisherman define bind modes that would not be legal with this change.

Fixes #3965
This commit is contained in:
Kurtis Rader 2017-04-22 20:33:56 -07:00
parent 275d658616
commit f1d40a3c7c
5 changed files with 59 additions and 44 deletions

View file

@ -450,86 +450,84 @@ static void builtin_bind_list_modes(io_streams_t &streams) {
/// The bind builtin, used for setting character sequences. /// The bind builtin, used for setting character sequences.
static int builtin_bind(parser_t &parser, io_streams_t &streams, wchar_t **argv) { static int builtin_bind(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
wgetopter_t w;
enum { BIND_INSERT, BIND_ERASE, BIND_KEY_NAMES, BIND_FUNCTION_NAMES }; enum { BIND_INSERT, BIND_ERASE, BIND_KEY_NAMES, BIND_FUNCTION_NAMES };
wchar_t *cmd = argv[0];
int argc = builtin_count_args(argv); int argc = builtin_count_args(argv);
int mode = BIND_INSERT; int mode = BIND_INSERT;
int res = STATUS_BUILTIN_OK; int res = STATUS_BUILTIN_OK;
int all = 0; bool all = false;
bool use_terminfo = false;
const wchar_t *bind_mode = DEFAULT_BIND_MODE; const wchar_t *bind_mode = DEFAULT_BIND_MODE;
bool bind_mode_given = false; bool bind_mode_given = false;
const wchar_t *sets_bind_mode = L""; const wchar_t *sets_bind_mode = L"";
int use_terminfo = 0;
w.woptind = 0; static const wchar_t *short_options = L"aehkKfM:Lm:";
static const struct woption long_options[] = {{L"all", no_argument, NULL, 'a'},
static const struct woption long_options[] = {{L"all", no_argument, 0, 'a'}, {L"erase", no_argument, NULL, 'e'},
{L"erase", no_argument, 0, 'e'}, {L"function-names", no_argument, NULL, 'f'},
{L"function-names", no_argument, 0, 'f'}, {L"help", no_argument, NULL, 'h'},
{L"help", no_argument, 0, 'h'}, {L"key", no_argument, NULL, 'k'},
{L"key", no_argument, 0, 'k'}, {L"key-names", no_argument, NULL, 'K'},
{L"key-names", no_argument, 0, 'K'}, {L"mode", required_argument, NULL, 'M'},
{L"mode", required_argument, 0, 'M'}, {L"list-modes", no_argument, NULL, 'L'},
{L"list-modes", no_argument, 0, 'L'}, {L"sets-mode", required_argument, NULL, 'm'},
{L"sets-mode", required_argument, 0, 'm'}, {NULL, 0, NULL, 0}};
{0, 0, 0, 0}};
while (1) {
int opt_index = 0;
int opt = w.wgetopt_long_only(argc, argv, L"aehkKfM:Lm:", long_options, &opt_index);
if (opt == -1) break;
int opt;
wgetopter_t w;
while ((opt = w.wgetopt_long_only(argc, argv, short_options, long_options, NULL)) != -1) {
switch (opt) { switch (opt) {
case 0: { case L'a': {
if (long_options[opt_index].flag != 0) break; all = true;
streams.err.append_format(BUILTIN_ERR_UNKNOWN, argv[0],
long_options[opt_index].name);
builtin_print_help(parser, streams, argv[0], streams.err);
return STATUS_BUILTIN_ERROR;
}
case 'a': {
all = 1;
break; break;
} }
case 'e': { case L'e': {
mode = BIND_ERASE; mode = BIND_ERASE;
break; break;
} }
case 'h': { case L'h': {
builtin_print_help(parser, streams, argv[0], streams.out); builtin_print_help(parser, streams, argv[0], streams.out);
return STATUS_BUILTIN_OK; return STATUS_BUILTIN_OK;
} }
case 'k': { case L'k': {
use_terminfo = 1; use_terminfo = true;
break; break;
} }
case 'K': { case L'K': {
mode = BIND_KEY_NAMES; mode = BIND_KEY_NAMES;
break; break;
} }
case 'f': { case L'f': {
mode = BIND_FUNCTION_NAMES; mode = BIND_FUNCTION_NAMES;
break; break;
} }
case 'M': { case L'M': {
if (!valid_var_name(w.woptarg)) {
streams.err.append_format(BUILTIN_ERR_BIND_MODE, cmd, w.woptarg);
return STATUS_BUILTIN_ERROR;
}
bind_mode = w.woptarg; bind_mode = w.woptarg;
bind_mode_given = true; bind_mode_given = true;
break; break;
} }
case 'm': { case L'm': {
if (!valid_var_name(w.woptarg)) {
streams.err.append_format(BUILTIN_ERR_BIND_MODE, cmd, w.woptarg);
return STATUS_BUILTIN_ERROR;
}
sets_bind_mode = w.woptarg; sets_bind_mode = w.woptarg;
break; break;
} }
case 'L': { case L'L': {
builtin_bind_list_modes(streams); builtin_bind_list_modes(streams);
return STATUS_BUILTIN_OK; return STATUS_BUILTIN_OK;
} }
case '?': { case L'?': {
builtin_unknown_option(parser, streams, argv[0], argv[w.woptind - 1]); builtin_unknown_option(parser, streams, argv[0], argv[w.woptind - 1]);
return STATUS_BUILTIN_ERROR; return STATUS_BUILTIN_ERROR;
} }
default: { default: {
DIE("unexpected opt"); DIE("unexpected retval from wgetopt_long_only");
break; break;
} }
} }
@ -1668,7 +1666,7 @@ int builtin_function(parser_t &parser, io_streams_t &streams, const wcstring_lis
} }
case 'v': { case 'v': {
if (!valid_var_name(w.woptarg)) { if (!valid_var_name(w.woptarg)) {
append_format(*out_err, _(L"%ls: Invalid variable name '%ls'"), cmd, w.woptarg); append_format(*out_err, BUILTIN_ERR_VARNAME, cmd, w.woptarg);
return STATUS_BUILTIN_ERROR; return STATUS_BUILTIN_ERROR;
} }
@ -1738,10 +1736,9 @@ int builtin_function(parser_t &parser, io_streams_t &streams, const wcstring_lis
} }
case 'V': { case 'V': {
if (!valid_var_name(w.woptarg)) { if (!valid_var_name(w.woptarg)) {
append_format(*out_err, _(L"%ls: Invalid variable name '%ls'"), cmd, w.woptarg); append_format(*out_err, BUILTIN_ERR_VARNAME, cmd, w.woptarg);
return STATUS_BUILTIN_ERROR; return STATUS_BUILTIN_ERROR;
} }
inherit_vars.push_back(w.woptarg); inherit_vars.push_back(w.woptarg);
break; break;
} }
@ -2110,10 +2107,10 @@ static int read_one_char_at_a_time(int fd, wcstring &buff, int nchars, bool spli
/// The read builtin. Reads from stdin and stores the values in environment variables. /// The read builtin. Reads from stdin and stores the values in environment variables.
static int builtin_read(parser_t &parser, io_streams_t &streams, wchar_t **argv) { static int builtin_read(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
wcstring buff;
wchar_t *cmd = argv[0]; wchar_t *cmd = argv[0];
int argc = builtin_count_args(argv); int argc = builtin_count_args(argv);
int place = ENV_USER; int place = ENV_USER;
wcstring buff;
wcstring prompt_cmd; wcstring prompt_cmd;
const wchar_t *prompt = NULL; const wchar_t *prompt = NULL;
const wchar_t *prompt_str = NULL; const wchar_t *prompt_str = NULL;

View file

@ -60,6 +60,9 @@ enum { COMMAND_NOT_BUILTIN, BUILTIN_REGULAR, BUILTIN_FUNCTION };
/// Error message for invalid variable name. /// Error message for invalid variable name.
#define BUILTIN_ERR_VARNAME _(L"%ls: Variable name '%ls' is not valid. See `help identifiers`.\n") #define BUILTIN_ERR_VARNAME _(L"%ls: Variable name '%ls' is not valid. See `help identifiers`.\n")
/// Error message for invalid bind mode name.
#define BUILTIN_ERR_BIND_MODE _(L"%ls: mode name '%ls' is not valid. See `help identifiers`.\n")
/// Error message when too many arguments are supplied to a builtin. /// Error message when too many arguments are supplied to a builtin.
#define BUILTIN_ERR_TOO_MANY_ARGUMENTS _(L"%ls: Too many arguments\n") #define BUILTIN_ERR_TOO_MANY_ARGUMENTS _(L"%ls: Too many arguments\n")

4
tests/bind.err Normal file
View file

@ -0,0 +1,4 @@
# Verify that an invalid bind mode is rejected.
bind: mode name 'bad bind mode' is not valid. See `help identifiers`.
# Verify that an invalid bind mode target is rejected.
bind: mode name 'bind-mode' is not valid. See `help identifiers`.

11
tests/bind.in Normal file
View file

@ -0,0 +1,11 @@
# Test various `bind` command invocations. This is meant to verify that
# invalid flags, mode names, etc. are caught as well as to verify that valid
# ones are allowed.
echo \# Verify that an invalid bind mode is rejected. >&2
bind -m 'bad bind mode' \cX true
echo \# Verify that an invalid bind mode target is rejected. >&2
bind -M bind-mode \cX true
# This should succeed and result in a success, zero, status.
bind -M bind_mode \cX true

0
tests/bind.out Normal file
View file