From 8d36f78b8e85f2a292c24411a481dc0a02c7498d Mon Sep 17 00:00:00 2001 From: =?utf8?q?Ingo=20B=C3=BCrk?= Date: Wed, 9 Dec 2015 13:39:08 +0100 Subject: [PATCH] Reject invalid match criteria with an error. Previously, using a command like [con_id=foo] kill would kill the currently focused window because while an error for not being able to parse the con_id was logged, no further action was taken, which caused the criterion to be ignored. In this case, the fallback behavior of using the focused window took over. For con_id, id and window_type we now reject incorrect values with an error and abort the command. fixes #2091 --- include/data.h | 3 +++ src/commands.c | 14 ++++++++++++++ src/match.c | 25 +++++++++++++++---------- testcases/t/260-invalid-criteria.t | 27 +++++++++++++++++++++++++++ 4 files changed, 59 insertions(+), 10 deletions(-) create mode 100644 testcases/t/260-invalid-criteria.t diff --git a/include/data.h b/include/data.h index 636092d2..b122dbfd 100644 --- a/include/data.h +++ b/include/data.h @@ -436,6 +436,9 @@ struct Window { * */ struct Match { + /* Set if a criterion was specified incorrectly. */ + char *error; + struct regex *title; struct regex *application; struct regex *class; diff --git a/src/commands.c b/src/commands.c index 5b28d03e..78b1e993 100644 --- a/src/commands.c +++ b/src/commands.c @@ -42,6 +42,16 @@ } \ } while (0) +/** If an error occured during parsing of the criteria, we want to exit instead + * of relying on fallback behavior. See #2091. */ +#define HANDLE_INVALID_MATCH \ + do { \ + if (current_match->error != NULL) { \ + yerror("Invalid match: %s", current_match->error); \ + return; \ + } \ + } while (0) + /** When the command did not include match criteria (!), we use the currently * focused container. Do not confuse this case with a command which included * criteria but which did not match any windows. This macro has to be called in @@ -49,6 +59,8 @@ */ #define HANDLE_EMPTY_MATCH \ do { \ + HANDLE_INVALID_MATCH; \ + \ if (match_is_empty(current_match)) { \ owindow *ow = smalloc(sizeof(owindow)); \ ow->con = focused; \ @@ -1233,6 +1245,8 @@ void cmd_kill(I3_CMD, const char *kill_mode_str) { return; } + HANDLE_INVALID_MATCH; + /* check if the match is empty, not if the result is empty */ if (match_is_empty(current_match)) tree_close_con(kill_mode); diff --git a/src/match.c b/src/match.c index 950e0fe3..caeb909f 100644 --- a/src/match.c +++ b/src/match.c @@ -238,6 +238,7 @@ bool match_matches_window(Match *match, i3Window *window) { * */ void match_free(Match *match) { + FREE(match->error); regex_free(match->title); regex_free(match->application); regex_free(match->class); @@ -286,6 +287,7 @@ void match_parse_property(Match *match, const char *ctype, const char *cvalue) { parsed < 0 || (end && *end != '\0')) { ELOG("Could not parse con id \"%s\"\n", cvalue); + match->error = sstrdup("invalid con_id"); } else { match->con_id = (Con *)parsed; DLOG("id as int = %p\n", match->con_id); @@ -301,6 +303,7 @@ void match_parse_property(Match *match, const char *ctype, const char *cvalue) { parsed < 0 || (end && *end != '\0')) { ELOG("Could not parse window id \"%s\"\n", cvalue); + match->error = sstrdup("invalid id"); } else { match->id = parsed; DLOG("window id as int = %d\n", match->id); @@ -309,26 +312,28 @@ void match_parse_property(Match *match, const char *ctype, const char *cvalue) { } if (strcmp(ctype, "window_type") == 0) { - if (strcasecmp(cvalue, "normal") == 0) + if (strcasecmp(cvalue, "normal") == 0) { match->window_type = A__NET_WM_WINDOW_TYPE_NORMAL; - else if (strcasecmp(cvalue, "dialog") == 0) + } else if (strcasecmp(cvalue, "dialog") == 0) { match->window_type = A__NET_WM_WINDOW_TYPE_DIALOG; - else if (strcasecmp(cvalue, "utility") == 0) + } else if (strcasecmp(cvalue, "utility") == 0) { match->window_type = A__NET_WM_WINDOW_TYPE_UTILITY; - else if (strcasecmp(cvalue, "toolbar") == 0) + } else if (strcasecmp(cvalue, "toolbar") == 0) { match->window_type = A__NET_WM_WINDOW_TYPE_TOOLBAR; - else if (strcasecmp(cvalue, "splash") == 0) + } else if (strcasecmp(cvalue, "splash") == 0) { match->window_type = A__NET_WM_WINDOW_TYPE_SPLASH; - else if (strcasecmp(cvalue, "menu") == 0) + } else if (strcasecmp(cvalue, "menu") == 0) { match->window_type = A__NET_WM_WINDOW_TYPE_MENU; - else if (strcasecmp(cvalue, "dropdown_menu") == 0) + } else if (strcasecmp(cvalue, "dropdown_menu") == 0) { match->window_type = A__NET_WM_WINDOW_TYPE_DROPDOWN_MENU; - else if (strcasecmp(cvalue, "popup_menu") == 0) + } else if (strcasecmp(cvalue, "popup_menu") == 0) { match->window_type = A__NET_WM_WINDOW_TYPE_POPUP_MENU; - else if (strcasecmp(cvalue, "tooltip") == 0) + } else if (strcasecmp(cvalue, "tooltip") == 0) { match->window_type = A__NET_WM_WINDOW_TYPE_TOOLTIP; - else + } else { ELOG("unknown window_type value \"%s\"\n", cvalue); + match->error = sstrdup("unknown window_type value"); + } return; } diff --git a/testcases/t/260-invalid-criteria.t b/testcases/t/260-invalid-criteria.t new file mode 100644 index 00000000..4c1830f9 --- /dev/null +++ b/testcases/t/260-invalid-criteria.t @@ -0,0 +1,27 @@ +#!perl +# vim:ts=4:sw=4:expandtab +# +# Please read the following documents before working on tests: +# • http://build.i3wm.org/docs/testsuite.html +# (or docs/testsuite) +# +# • http://build.i3wm.org/docs/lib-i3test.html +# (alternatively: perldoc ./testcases/lib/i3test.pm) +# +# • http://build.i3wm.org/docs/ipc.html +# (or docs/ipc) +# +# • http://onyxneon.com/books/modern_perl/modern_perl_a4.pdf +# (unless you are already familiar with Perl) +# +# Ticket: #2091 +use i3test; + +my $ws = fresh_workspace; +open_window; + +my $result = cmd '[con_id=foobar] kill'; +is($result->[0]->{success}, 0, 'command was unsuccessful'); +is($result->[0]->{error}, 'Invalid match: invalid con_id', 'correct error is returned'); + +done_testing; -- 2.39.2