service: don't take a command named like an option for one

A run/task/service whose command starts like an option without a ':'
lost its command, e.g. the example in finit.conf(5):

    run [2345] @joe:users logger "Hello world"

was registered as the log option, running "Hello world".  Same for
pidof, pidproxy, cgroupfs-mount, and in block format too.

The option parser matches with MATCH_CMD(), a prefix match, also for
the options with no trailing ':' to end the prefix: log, pid, cgroup,
respawn, nowarn, and the rest.

Match these against the whole token, allowing only the separator that
starts a value: "log:", "pid:", "cgroup." and "cgroup:".

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>

doc/ChangeLog.md

@@ -156,6 +156,9 @@ All relevant changes are documented in this file.
 - Fix #512: `--` in the `command` of a block-format .conf, it was taken as the
   start of the description, so `command = "cmd -- arg"` ran `cmd` with
   description `arg`.  Found by Alexandru Paniș
+- Fix a command whose name starts like a service option, e.g. `logger` or
+  `pidof`.  It was taken as the `log` or `pid` option and the next argument
+  ran as the command, which also broke the `logger` example in finit.conf(5)
 - Reject a .conf file that declares the same block title twice.  The title is
   the service identity and libConfuse merges two sections sharing one, so the
   file loaded as a single service holding a mix of both declarations,

src/service.c

@@ -1812,6 +1812,24 @@ static char *parse_args(char **line)
 	return NULL;
 }
 
+/*
+ * Match an option that takes a value, @opt on its own or followed by
+ * one of the characters in @sep, e.g. "log" and "log:/path".  Returns
+ * the value, or NULL, so a command like logger or pidof is not taken
+ * for an option.
+ */
+static char *is_opt(char *tok, const char *opt, const char *sep)
+{
+	size_t len = strlen(opt);
+
+	if (strncasecmp(tok, opt, len))
+		return NULL;
+	if (tok[len] && !strchr(sep, tok[len]))
+		return NULL;
+
+	return &tok[len];
+}
+
 /*
  * log:/path/to/logfile,priority:facility.level,tag:ident
  */
@@ -2265,6 +2283,10 @@ svc_t *service_register_desc(int type, char *cfg, const char *desc,
 		return NULL;
 	}
 
+	/*
+	 * Options ending in ':' can be prefix matched, the rest must match
+	 * the whole token or a command like logger is taken for one.
+	 */
 	while (cmd) {
 		char *arg;
 
@@ -2279,19 +2301,19 @@ svc_t *service_register_desc(int type, char *cfg, const char *desc,
 			cond = &cmd[1];
 		else if (cmd[0] == ':')	/* :ID */
 			id = &cmd[1];
-		else if (MATCH_CMD(cmd, "log", arg))
+		else if (is_opt(cmd, "log", ":"))
 			log = cmd;
-		else if (MATCH_CMD(cmd, "pid", arg))
+		else if (is_opt(cmd, "pid", ":"))
 			pid = cmd;
 		else if (MATCH_CMD(cmd, "name:", arg))
 			name = cmd;
 		else if (MATCH_CMD(cmd, "notify:", arg))
 			notify = arg;
-		else if (MATCH_CMD(cmd, "type:forking", arg))
+		else if (!strcasecmp(cmd, "type:forking"))
 			forking = 1;
-		else if (MATCH_CMD(cmd, "manual:yes", arg))
+		else if (!strcasecmp(cmd, "manual:yes"))
 			manual = 1;
-		else if (MATCH_CMD(cmd, "remain:yes", arg))
+		else if (!strcasecmp(cmd, "remain:yes"))
 			remain = 1;
 		else if (MATCH_CMD(cmd, "restart:", arg)) {
 			if (MATCH_CMD(arg, "always", arg))
@@ -2303,9 +2325,9 @@ svc_t *service_register_desc(int type, char *cfg, const char *desc,
 			restart_tmo = atoi(arg) * 1000;
 		else if (MATCH_CMD(cmd, "restart_sec:", arg))
 			restart_tmo = atoi(arg) * 1000;
-		else if (MATCH_CMD(cmd, "norestart", arg))
+		else if (!strcasecmp(cmd, "norestart"))
 			restart_max = 0;
-		else if (MATCH_CMD(cmd, "nowarn", arg))
+		else if (!strcasecmp(cmd, "nowarn"))
 			nowarn = 1;
 		else if (MATCH_CMD(cmd, "oncrash:", arg)) {
 			if (MATCH_CMD(arg, "reboot", arg))
@@ -2313,7 +2335,7 @@ svc_t *service_register_desc(int type, char *cfg, const char *desc,
 			if (MATCH_CMD(arg, "script", arg))
 				oncrash_action = SVC_ONCRASH_SCRIPT;
 		}
-		else if (MATCH_CMD(cmd, "respawn", arg))
+		else if (!strcasecmp(cmd, "respawn"))
 			respawn = 1;
 		else if (MATCH_CMD(cmd, "halt:", arg))
 			halt = arg;
@@ -2338,7 +2360,7 @@ svc_t *service_register_desc(int type, char *cfg, const char *desc,
 		else if (MATCH_CMD(cmd, "tty:", arg))
 			ctty = arg;
 		/* catch both cgroup: and cgroup. handled in parse_cgroup() */
-		else if (MATCH_CMD(cmd, "cgroup", arg))
+		else if ((arg = is_opt(cmd, "cgroup", ".:")))
 			cgroup = arg;
 		else if (MATCH_CMD(cmd, "conflict:", arg))
 			conflict = arg;

test/conf-command.sh

@@ -89,6 +89,16 @@ retry 'assert_cmd pick "serv -np -i pick -- extra"'
 assert_desc "Dashes" pick
 assert_num_children 1 serv
 
+# Options are matched as whole tokens, so a command that merely starts
+# like one, pidof vs. pid, is still the command.
+say 'A command named like an option is the command'
+run "echo 'task lookalike {'             >  $FINIT_CONF"
+run "echo '    command = \"pidof serv\"' >> $FINIT_CONF"
+run "echo '}'                            >> $FINIT_CONF"
+run "initctl reload"
+
+retry 'assert_cmd lookalike "pidof serv"'
+
 # A tty block takes the same list, its command is the getty to run.
 # Asserting it is loaded is enough here, a getty in the test namespace
 # has no terminal to attach to.