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

@@ -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;

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);

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;
     }

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;