Fix segfaults caused by faulty command parsing

This patch fixes faulty command parsing introduced by
f0f5de9a9e. When that commit allowed
criteria reset on ';' delimeters in commands lists, it failed to account
for its inner ','-parsing loop eating threw the entire rest of the
string.

This patch refactors argsep to use a list of multiple separators, and
(optionally) return the separator that it matched against in this
iteration via a pointer. This allows it to hint at the command parser
which separator was used at the end of the last command, allowing it to
trigger a potential secondary read of the criteria.

Fixes #4239

common/stringop.c

@@ -251,37 +251,61 @@ char *join_args(char **argv, int argc) {
 	return res;
 }
 
-char *argsep(char **stringp, const char *delim) {
+static inline char *argsep_next_interesting(const char *src, const char *delim) {
+	char *special = strpbrk(src, "\"'\\");
+	char *next_delim = strpbrk(src, delim);
+	if (!special) {
+		return next_delim;
+	}
+	if (!next_delim) {
+		return special;
+	}
+	return (next_delim < special) ? next_delim : special;
+}
+
+char *argsep(char **stringp, const char *delim, char *matched) {
 	char *start = *stringp;
 	char *end = start;
 	bool in_string = false;
 	bool in_char = false;
 	bool escaped = false;
-	while (1) {
+	char *interesting = NULL;
-		if (*end == '"' && !in_char && !escaped) {
+
-			in_string = !in_string;
+	while ((interesting = argsep_next_interesting(end, delim))) {
-		} else if (*end == '\'' && !in_string && !escaped) {
+		if (escaped && interesting != end) {
-			in_char = !in_char;
-		} else if (*end == '\\') {
-			escaped = !escaped;
-		} else if (*end == '\0') {
-			*stringp = NULL;
-			break;
-		} else if (!in_string && !in_char && !escaped && strchr(delim, *end)) {
-			if (end - start) {
-				*(end++) = 0;
-				*stringp = end + strspn(end, delim);;
-				if (!**stringp) *stringp = NULL;
-				break;
-			} else {
-				++start;
-				end = start;
-			}
-		}
-		if (*end != '\\') {
 			escaped = false;
 		}
-		++end;
+		if (*interesting == '"' && !in_char && !escaped) {
+			in_string = !in_string;
+			end = interesting + 1;
+		} else if (*interesting == '\'' && !in_string && !escaped) {
+			in_char = !in_char;
+			end = interesting + 1;
+		} else if (*interesting == '\\') {
+			escaped = !escaped;
+			end = interesting + 1;
+		} else if (!in_string && !in_char && !escaped) {
+			// We must have matched a separator
+			end = interesting;
+			if (matched) {
+				*matched = *end;
+			}
+			if (end - start) {
+				*(end++) = 0;
+				*stringp = end;
+				break;
+			} else {
+				end = ++start;
+			}
+		} else {
+			end++;
+		}
+	}
+	if (!interesting) {
+		*stringp = NULL;
+		if (matched) {
+			*matched = '\0';
+		}
 	}
 	return start;
 }

include/stringop.h

@@ -24,6 +24,6 @@ int unescape_string(char *string);
 char *join_args(char **argv, int argc);
 
 // Split string into 2 by delim, handle quotes
-char *argsep(char **stringp, const char *delim);
+char *argsep(char **stringp, const char *delim, char *matched_delim);
 
 #endif

sway/commands.c

@@ -211,8 +211,8 @@ list_t *execute_command(char *_exec, struct sway_seat *seat,
 	list_t *res_list = create_list();
 	char *exec = strdup(_exec);
 	char *head = exec;
-	char *cmdlist;
 	char *cmd;
+	char matched_delim = ';';
 	list_t *views = NULL;
 
 	if (seat == NULL) {
@@ -227,16 +227,13 @@ list_t *execute_command(char *_exec, struct sway_seat *seat,
 
 	head = exec;
 	do {
-		// Split command list
+		for (; isspace(*head); ++head) {}
-		cmdlist = argsep(&head, ";");
+		// Extract criteria (valid for this command list only).
-		do {
+		if (matched_delim == ';') {
-			// Skip leading whitespace
-			for (; isspace(*cmdlist); ++cmdlist) {}
-			// Extract criteria (valid for this command chain only).
 			config->handler_context.using_criteria = false;
-			if (*cmdlist == '[') {
+			if (*head == '[') {
 				char *error = NULL;
-				struct criteria *criteria = criteria_parse(cmdlist, &error);
+				struct criteria *criteria = criteria_parse(head, &error);
 				if (!criteria) {
 					list_add(res_list,
 							cmd_results_new(CMD_INVALID, "%s", error));
@@ -245,15 +242,17 @@ list_t *execute_command(char *_exec, struct sway_seat *seat,
 				}
 				list_free(views);
 				views = criteria_get_views(criteria);
-				cmdlist += strlen(criteria->raw);
+				head += strlen(criteria->raw);
 				criteria_destroy(criteria);
 				config->handler_context.using_criteria = true;
 				// Skip leading whitespace
-				for (; isspace(*cmdlist); ++cmdlist) {}
+				for (; isspace(*head); ++head) {}
 			}
-			// Split command chain into commands
+		}
-			cmd = argsep(&cmdlist, ",");
+		// Split command list
+		cmd = argsep(&head, ";,", &matched_delim);
 		for (; isspace(*cmd); ++cmd) {}
+
 		if (strcmp(cmd, "") == 0) {
 			sway_log(SWAY_INFO, "Ignoring empty command.");
 			continue;
@@ -265,8 +264,7 @@ list_t *execute_command(char *_exec, struct sway_seat *seat,
 		if (strcmp(argv[0], "exec") != 0 &&
 				strcmp(argv[0], "exec_always") != 0 &&
 				strcmp(argv[0], "mode") != 0) {
-				int i;
+			for (int i = 1; i < argc; ++i) {
-				for (i = 1; i < argc; ++i) {
 				if (*argv[i] == '\"' || *argv[i] == '\'') {
 					strip_quotes(argv[i]);
 				}
@@ -309,7 +307,6 @@ list_t *execute_command(char *_exec, struct sway_seat *seat,
 			}
 		}
 		free_argv(argc, argv);
-		} while(cmdlist);
 	} while(head);
 cleanup:
 	free(exec);

sway/tree/workspace.c

@@ -212,9 +212,9 @@ static void workspace_name_from_binding(const struct sway_binding * binding,
 	char *name = NULL;
 
 	// workspace n
-	char *cmd = argsep(&cmdlist, " ");
+	char *cmd = argsep(&cmdlist, " ", NULL);
 	if (cmdlist) {
-		name = argsep(&cmdlist, ",;");
+		name = argsep(&cmdlist, ",;", NULL);
 	}
 
 	// TODO: support "move container to workspace" bindings as well