From 03d173cbd8e72c356512a0e19e356b07d518627a Mon Sep 17 00:00:00 2001 From: nicm Date: Wed, 25 Aug 2021 08:51:55 +0000 Subject: [PATCH] Validate command argument types (string or command list) and give more useful error messages. --- arguments.c | 87 +++++++++++++++++++++++++++++++++++++++----- cmd-bind-key.c | 16 +++++++- cmd-command-prompt.c | 19 +++++++--- cmd-confirm-before.c | 15 ++++++-- cmd-display-menu.c | 40 ++++++++++++++++---- cmd-display-panes.c | 19 +++++++--- cmd-if-shell.c | 16 +++++++- cmd-run-shell.c | 16 +++++++- cmd-set-option.c | 20 ++++++++-- cmd.c | 10 ++++- key-bindings.c | 4 +- tmux.h | 12 +++++- 12 files changed, 230 insertions(+), 44 deletions(-) diff --git a/arguments.c b/arguments.c index bddb6365..fca57f08 100644 --- a/arguments.c +++ b/arguments.c @@ -124,10 +124,11 @@ args_create(void) /* Parse arguments into a new argument set. */ struct args * args_parse(const struct args_parse *parse, struct args_value *values, - u_int count) + u_int count, char **cause) { struct args *args; u_int i; + enum args_parse_type type; struct args_value *value, *new; u_char flag, argument; const char *found, *string, *s; @@ -153,11 +154,13 @@ args_parse(const struct args_parse *parse, struct args_value *values, if (flag == '\0') break; if (!isalnum(flag)) { + xasprintf(cause, "invalid flag -%c", flag); args_free(args); return (NULL); } found = strchr(parse->template, flag); if (found == NULL) { + xasprintf(cause, "unknown flag -%c", flag); args_free(args); return (NULL); } @@ -167,12 +170,22 @@ args_parse(const struct args_parse *parse, struct args_value *values, args_set(args, flag, NULL); continue; } - new = xcalloc(1, sizeof *value); + new = xcalloc(1, sizeof *new); if (*string != '\0') { new->type = ARGS_STRING; new->string = xstrdup(string); } else { if (i == count) { + xasprintf(cause, + "-%c expects an argument", + flag); + args_free(args); + return (NULL); + } + if (values[i].type != ARGS_STRING) { + xasprintf(cause, + "-%c argument must be a string", + flag); args_free(args); return (NULL); } @@ -192,14 +205,60 @@ args_parse(const struct args_parse *parse, struct args_value *values, s = args_value_as_string(value); log_debug("%s: %u = %s", __func__, i, s); + if (parse->cb != NULL) { + type = parse->cb(args, args->count, cause); + if (type == ARGS_PARSE_INVALID) { + args_free(args); + return (NULL); + } + } else + type = ARGS_PARSE_STRING; + args->values = xrecallocarray(args->values, args->count, args->count + 1, sizeof *args->values); - args_copy_value(&args->values[args->count++], value); + new = &args->values[args->count++]; + + switch (type) { + case ARGS_PARSE_INVALID: + fatalx("unexpected argument type"); + case ARGS_PARSE_STRING: + if (value->type != ARGS_STRING) { + xasprintf(cause, + "argument %u must be \"string\"", + args->count); + args_free(args); + return (NULL); + } + args_copy_value(new, value); + break; + case ARGS_PARSE_COMMANDS_OR_STRING: + args_copy_value(new, value); + break; + case ARGS_PARSE_COMMANDS: + if (value->type != ARGS_COMMANDS) { + xasprintf(cause, + "argument %u must be { commands }", + args->count); + args_free(args); + return (NULL); + } + args_copy_value(new, value); + break; + } } } - if ((parse->lower != -1 && args->count < (u_int)parse->lower) || - (parse->upper != -1 && args->count > (u_int)parse->upper)) { + if (parse->lower != -1 && args->count < (u_int)parse->lower) { + xasprintf(cause, + "too few arguments (need at least %u)", + parse->lower); + args_free(args); + return (NULL); + } + if (parse->upper != -1 && args->count > (u_int)parse->upper) { + xasprintf(cause, + "too many arguments (need at most %u)", + parse->upper); args_free(args); return (NULL); } @@ -254,15 +313,25 @@ args_free(struct args *args) void args_vector(struct args *args, int *argc, char ***argv) { - struct args_value *value; - u_int i; + char *s; + u_int i; *argc = 0; *argv = NULL; for (i = 0; i < args->count; i++) { - value = &args->values[i]; - cmd_append_argv(argc, argv, args_value_as_string(value)); + switch (args->values[i].type) { + case ARGS_NONE: + break; + case ARGS_STRING: + cmd_append_argv(argc, argv, args->values[i].string); + break; + case ARGS_COMMANDS: + s = cmd_list_print(args->values[i].cmdlist, 0); + cmd_append_argv(argc, argv, s); + free(s); + break; + } } } diff --git a/cmd-bind-key.c b/cmd-bind-key.c index bb905bce..778d655b 100644 --- a/cmd-bind-key.c +++ b/cmd-bind-key.c @@ -27,13 +27,16 @@ * Bind a key to a command. */ -static enum cmd_retval cmd_bind_key_exec(struct cmd *, struct cmdq_item *); +static enum args_parse_type cmd_bind_key_args_parse(struct args *, u_int, + char **); +static enum cmd_retval cmd_bind_key_exec(struct cmd *, + struct cmdq_item *); const struct cmd_entry cmd_bind_key_entry = { .name = "bind-key", .alias = "bind", - .args = { "nrN:T:", 1, -1, NULL }, + .args = { "nrN:T:", 1, -1, cmd_bind_key_args_parse }, .usage = "[-nr] [-T key-table] [-N note] key " "[command [arguments]]", @@ -41,6 +44,15 @@ const struct cmd_entry cmd_bind_key_entry = { .exec = cmd_bind_key_exec }; +static enum args_parse_type +cmd_bind_key_args_parse(__unused struct args *args, u_int idx, + __unused char **cause) +{ + if (idx == 1) + return (ARGS_PARSE_COMMANDS_OR_STRING); + return (ARGS_PARSE_STRING); +} + static enum cmd_retval cmd_bind_key_exec(struct cmd *self, struct cmdq_item *item) { diff --git a/cmd-command-prompt.c b/cmd-command-prompt.c index bca1a7fc..737c44c7 100644 --- a/cmd-command-prompt.c +++ b/cmd-command-prompt.c @@ -29,8 +29,10 @@ * Prompt for command in client. */ -static enum cmd_retval cmd_command_prompt_exec(struct cmd *, - struct cmdq_item *); +static enum args_parse_type cmd_command_prompt_args_parse(struct args *, + u_int, char **); +static enum cmd_retval cmd_command_prompt_exec(struct cmd *, + struct cmdq_item *); static int cmd_command_prompt_callback(struct client *, void *, const char *, int); @@ -40,7 +42,7 @@ const struct cmd_entry cmd_command_prompt_entry = { .name = "command-prompt", .alias = NULL, - .args = { "1bFkiI:Np:t:T:", 0, 1, NULL }, + .args = { "1bFkiI:Np:t:T:", 0, 1, cmd_command_prompt_args_parse }, .usage = "[-1bFkiN] [-I inputs] [-p prompts] " CMD_TARGET_CLIENT_USAGE " [-T type] [template]", @@ -68,6 +70,13 @@ struct cmd_command_prompt_cdata { char **argv; }; +static enum args_parse_type +cmd_command_prompt_args_parse(__unused struct args *args, __unused u_int idx, + __unused char **cause) +{ + return (ARGS_PARSE_COMMANDS_OR_STRING); +} + static enum cmd_retval cmd_command_prompt_exec(struct cmd *self, struct cmdq_item *item) { @@ -197,8 +206,8 @@ cmd_command_prompt_callback(struct client *c, void *data, const char *s, return (1); out: - if (item != NULL) - cmdq_continue(item); + if (item != NULL) + cmdq_continue(item); return (0); } diff --git a/cmd-confirm-before.c b/cmd-confirm-before.c index 4fe43302..8f50ba2d 100644 --- a/cmd-confirm-before.c +++ b/cmd-confirm-before.c @@ -28,8 +28,10 @@ * Asks for confirmation before executing a command. */ -static enum cmd_retval cmd_confirm_before_exec(struct cmd *, - struct cmdq_item *); +static enum args_parse_type cmd_confirm_before_args_parse(struct args *, + u_int, char **); +static enum cmd_retval cmd_confirm_before_exec(struct cmd *, + struct cmdq_item *); static int cmd_confirm_before_callback(struct client *, void *, const char *, int); @@ -39,7 +41,7 @@ const struct cmd_entry cmd_confirm_before_entry = { .name = "confirm-before", .alias = "confirm", - .args = { "bp:t:", 1, 1, NULL }, + .args = { "bp:t:", 1, 1, cmd_confirm_before_args_parse }, .usage = "[-b] [-p prompt] " CMD_TARGET_CLIENT_USAGE " command", .flags = CMD_CLIENT_TFLAG, @@ -51,6 +53,13 @@ struct cmd_confirm_before_data { struct cmd_list *cmdlist; }; +static enum args_parse_type +cmd_confirm_before_args_parse(__unused struct args *args, __unused u_int idx, + __unused char **cause) +{ + return (ARGS_PARSE_COMMANDS_OR_STRING); +} + static enum cmd_retval cmd_confirm_before_exec(struct cmd *self, struct cmdq_item *item) { diff --git a/cmd-display-menu.c b/cmd-display-menu.c index 378c62e1..b36894f4 100644 --- a/cmd-display-menu.c +++ b/cmd-display-menu.c @@ -28,16 +28,18 @@ * Display a menu on a client. */ -static enum cmd_retval cmd_display_menu_exec(struct cmd *, - struct cmdq_item *); -static enum cmd_retval cmd_display_popup_exec(struct cmd *, - struct cmdq_item *); +static enum args_parse_type cmd_display_menu_args_parse(struct args *, + u_int, char **); +static enum cmd_retval cmd_display_menu_exec(struct cmd *, + struct cmdq_item *); +static enum cmd_retval cmd_display_popup_exec(struct cmd *, + struct cmdq_item *); const struct cmd_entry cmd_display_menu_entry = { .name = "display-menu", .alias = "menu", - .args = { "c:t:OT:x:y:", 1, -1, NULL }, + .args = { "c:t:OT:x:y:", 1, -1, cmd_display_menu_args_parse }, .usage = "[-O] [-c target-client] " CMD_TARGET_PANE_USAGE " [-T title] " "[-x position] [-y position] name key command ...", @@ -53,8 +55,8 @@ const struct cmd_entry cmd_display_popup_entry = { .args = { "BCc:d:Eh:t:w:x:y:", 0, -1, NULL }, .usage = "[-BCE] [-c target-client] [-d start-directory] [-h height] " - CMD_TARGET_PANE_USAGE " [-w width] " - "[-x position] [-y position] [command]", + CMD_TARGET_PANE_USAGE " [-w width] " + "[-x position] [-y position] [shell-command]", .target = { 't', CMD_FIND_PANE, 0 }, @@ -62,6 +64,30 @@ const struct cmd_entry cmd_display_popup_entry = { .exec = cmd_display_popup_exec }; +static enum args_parse_type +cmd_display_menu_args_parse(struct args *args, u_int idx, __unused char **cause) +{ + u_int i = 0; + enum args_parse_type type = ARGS_PARSE_STRING; + + for (;;) { + type = ARGS_PARSE_STRING; + if (i == idx) + break; + if (*args_string(args, i++) == '\0') + continue; + + type = ARGS_PARSE_STRING; + if (i++ == idx) + break; + + type = ARGS_PARSE_COMMANDS_OR_STRING; + if (i++ == idx) + break; + } + return (type); +} + static int cmd_display_menu_get_position(struct client *tc, struct cmdq_item *item, struct args *args, u_int *px, u_int *py, u_int w, u_int h) diff --git a/cmd-display-panes.c b/cmd-display-panes.c index 169ab3d5..5773a2d0 100644 --- a/cmd-display-panes.c +++ b/cmd-display-panes.c @@ -27,14 +27,16 @@ * Display panes on a client. */ -static enum cmd_retval cmd_display_panes_exec(struct cmd *, - struct cmdq_item *); +static enum args_parse_type cmd_display_panes_args_parse(struct args *, + u_int, char **); +static enum cmd_retval cmd_display_panes_exec(struct cmd *, + struct cmdq_item *); const struct cmd_entry cmd_display_panes_entry = { .name = "display-panes", .alias = "displayp", - .args = { "bd:Nt:", 0, 1, NULL }, + .args = { "bd:Nt:", 0, 1, cmd_display_panes_args_parse }, .usage = "[-bN] [-d duration] " CMD_TARGET_CLIENT_USAGE " [template]", .flags = CMD_AFTERHOOK|CMD_CLIENT_TFLAG, @@ -46,6 +48,13 @@ struct cmd_display_panes_data { struct args_command_state *state; }; +static enum args_parse_type +cmd_display_panes_args_parse(__unused struct args *args, __unused u_int idx, + __unused char **cause) +{ + return (ARGS_PARSE_COMMANDS_OR_STRING); +} + static void cmd_display_panes_draw_pane(struct screen_redraw_ctx *ctx, struct window_pane *wp) @@ -139,7 +148,7 @@ cmd_display_panes_draw_pane(struct screen_redraw_ctx *ctx, if (sx >= len + llen + 1) { len += llen + 1; tty_cursor(tty, xoff + px - len / 2, yoff + py); - tty_putn(tty, buf, len, len); + tty_putn(tty, buf, len, len); tty_putn(tty, " ", 1, 1); tty_putn(tty, lbuf, llen, llen); } else { @@ -263,7 +272,7 @@ cmd_display_panes_exec(struct cmd *self, struct cmdq_item *item) struct args *args = cmd_get_args(self); struct client *tc = cmdq_get_target_client(item); struct session *s = tc->session; - u_int delay; + u_int delay; char *cause; struct cmd_display_panes_data *cdata; int wait = !args_has(args, 'b'); diff --git a/cmd-if-shell.c b/cmd-if-shell.c index 7804ef27..907a96e1 100644 --- a/cmd-if-shell.c +++ b/cmd-if-shell.c @@ -29,7 +29,10 @@ * Executes a tmux command if a shell command returns true or false. */ -static enum cmd_retval cmd_if_shell_exec(struct cmd *, struct cmdq_item *); +static enum args_parse_type cmd_if_shell_args_parse(struct args *, u_int, + char **); +static enum cmd_retval cmd_if_shell_exec(struct cmd *, + struct cmdq_item *); static void cmd_if_shell_callback(struct job *); static void cmd_if_shell_free(void *); @@ -38,7 +41,7 @@ const struct cmd_entry cmd_if_shell_entry = { .name = "if-shell", .alias = "if", - .args = { "bFt:", 2, 3, NULL }, + .args = { "bFt:", 2, 3, cmd_if_shell_args_parse }, .usage = "[-bF] " CMD_TARGET_PANE_USAGE " shell-command command " "[command]", @@ -56,6 +59,15 @@ struct cmd_if_shell_data { struct cmdq_item *item; }; +static enum args_parse_type +cmd_if_shell_args_parse(__unused struct args *args, u_int idx, + __unused char **cause) +{ + if (idx == 1 || idx == 2) + return (ARGS_PARSE_COMMANDS_OR_STRING); + return (ARGS_PARSE_STRING); +} + static enum cmd_retval cmd_if_shell_exec(struct cmd *self, struct cmdq_item *item) { diff --git a/cmd-run-shell.c b/cmd-run-shell.c index f23a8ec3..662312fb 100644 --- a/cmd-run-shell.c +++ b/cmd-run-shell.c @@ -30,7 +30,10 @@ * Runs a command without a window. */ -static enum cmd_retval cmd_run_shell_exec(struct cmd *, struct cmdq_item *); +static enum args_parse_type cmd_run_shell_args_parse(struct args *, u_int, + char **); +static enum cmd_retval cmd_run_shell_exec(struct cmd *, + struct cmdq_item *); static void cmd_run_shell_timer(int, short, void *); static void cmd_run_shell_callback(struct job *); @@ -41,7 +44,7 @@ const struct cmd_entry cmd_run_shell_entry = { .name = "run-shell", .alias = "run", - .args = { "bd:Ct:", 0, 1, NULL }, + .args = { "bd:Ct:", 0, 1, cmd_run_shell_args_parse }, .usage = "[-bC] [-d delay] " CMD_TARGET_PANE_USAGE " [shell-command]", .target = { 't', CMD_FIND_PANE, CMD_FIND_CANFAIL }, @@ -62,6 +65,15 @@ struct cmd_run_shell_data { int flags; }; +static enum args_parse_type +cmd_run_shell_args_parse(struct args *args, __unused u_int idx, + __unused char **cause) +{ + if (args_has(args, 'C')) + return (ARGS_PARSE_COMMANDS_OR_STRING); + return (ARGS_PARSE_STRING); +} + static void cmd_run_shell_print(struct job *job, const char *msg) { diff --git a/cmd-set-option.c b/cmd-set-option.c index 8839dc0d..fbe32cfa 100644 --- a/cmd-set-option.c +++ b/cmd-set-option.c @@ -27,13 +27,16 @@ * Set an option. */ -static enum cmd_retval cmd_set_option_exec(struct cmd *, struct cmdq_item *); +static enum args_parse_type cmd_set_option_args_parse(struct args *, + u_int, char **); +static enum cmd_retval cmd_set_option_exec(struct cmd *, + struct cmdq_item *); const struct cmd_entry cmd_set_option_entry = { .name = "set-option", .alias = "set", - .args = { "aFgopqst:uUw", 1, 2, NULL }, + .args = { "aFgopqst:uUw", 1, 2, cmd_set_option_args_parse }, .usage = "[-aFgopqsuUw] " CMD_TARGET_PANE_USAGE " option [value]", .target = { 't', CMD_FIND_PANE, CMD_FIND_CANFAIL }, @@ -46,7 +49,7 @@ const struct cmd_entry cmd_set_window_option_entry = { .name = "set-window-option", .alias = "setw", - .args = { "aFgoqt:u", 1, 2, NULL }, + .args = { "aFgoqt:u", 1, 2, cmd_set_option_args_parse }, .usage = "[-aFgoqu] " CMD_TARGET_WINDOW_USAGE " option [value]", .target = { 't', CMD_FIND_WINDOW, CMD_FIND_CANFAIL }, @@ -59,7 +62,7 @@ const struct cmd_entry cmd_set_hook_entry = { .name = "set-hook", .alias = NULL, - .args = { "agpRt:uw", 1, 2, NULL }, + .args = { "agpRt:uw", 1, 2, cmd_set_option_args_parse }, .usage = "[-agpRuw] " CMD_TARGET_PANE_USAGE " hook [command]", .target = { 't', CMD_FIND_PANE, CMD_FIND_CANFAIL }, @@ -68,6 +71,15 @@ const struct cmd_entry cmd_set_hook_entry = { .exec = cmd_set_option_exec }; +static enum args_parse_type +cmd_set_option_args_parse(__unused struct args *args, u_int idx, + __unused char **cause) +{ + if (idx == 1) + return (ARGS_PARSE_COMMANDS_OR_STRING); + return (ARGS_PARSE_STRING); +} + static enum cmd_retval cmd_set_option_exec(struct cmd *self, struct cmdq_item *item) { diff --git a/cmd.c b/cmd.c index 930ee56c..62c8b9ab 100644 --- a/cmd.c +++ b/cmd.c @@ -502,6 +502,7 @@ cmd_parse(struct args_value *values, u_int count, const char *file, u_int line, const struct cmd_entry *entry; struct cmd *cmd; struct args *args; + char *error; if (count == 0 || values[0].type != ARGS_STRING) { xasprintf(cause, "no command"); @@ -511,11 +512,16 @@ cmd_parse(struct args_value *values, u_int count, const char *file, u_int line, if (entry == NULL) return (NULL); - args = args_parse(&entry->args, values, count); - if (args == NULL) { + args = args_parse(&entry->args, values, count, &error); + if (args == NULL && error == NULL) { xasprintf(cause, "usage: %s %s", entry->name, entry->usage); return (NULL); } + if (args == NULL) { + xasprintf(cause, "command %s: %s", entry->name, error); + free(error); + return (NULL); + } cmd = xcalloc(1, sizeof *cmd); cmd->entry = entry; diff --git a/key-bindings.c b/key-bindings.c index e3b21d61..66e1ec9f 100644 --- a/key-bindings.c +++ b/key-bindings.c @@ -634,8 +634,10 @@ key_bindings_init(void) for (i = 0; i < nitems(defaults); i++) { pr = cmd_parse_from_string(defaults[i], NULL); - if (pr->status != CMD_PARSE_SUCCESS) + if (pr->status != CMD_PARSE_SUCCESS) { + log_debug("%s", pr->error); fatalx("bad default key: %s", defaults[i]); + } cmdq_append(NULL, cmdq_get_command(pr->cmdlist, NULL)); cmd_list_free(pr->cmdlist); } diff --git a/tmux.h b/tmux.h index 668ca270..96baaf4d 100644 --- a/tmux.h +++ b/tmux.h @@ -1378,8 +1378,16 @@ struct args_value { struct args_entry; RB_HEAD(args_tree, args_entry); +/* Arguments parsing type. */ +enum args_parse_type { + ARGS_PARSE_INVALID, + ARGS_PARSE_STRING, + ARGS_PARSE_COMMANDS_OR_STRING, + ARGS_PARSE_COMMANDS +}; + /* Arguments parsing state. */ -typedef enum args_type (*args_parse_cb)(struct args *, u_int); +typedef enum args_parse_type (*args_parse_cb)(struct args *, u_int, char **); struct args_parse { const char *template; int lower; @@ -2201,7 +2209,7 @@ int tty_keys_next(struct tty *); void args_set(struct args *, u_char, struct args_value *); struct args *args_create(void); struct args *args_parse(const struct args_parse *, struct args_value *, - u_int); + u_int, char **); void args_vector(struct args *, int *, char ***); void args_free_value(struct args_value *); void args_free(struct args *);