conf: pass -- in a block command to the program

A command that takes -- as an argument lost it, and everything after
it, to the description:

    service name {
        command     = "cmd -- arg"
        description = "a cmd"
    }

ran `cmd` with description `arg`.

The block translator builds a legacy one-liner, appending the
description as "-- text", and service_register() splits that line at
the first "-- ", here the one in the command.

Split the description off only for legacy one-liners.  Blocks now hand
it to the new service_register_desc() as an argument.

Fixes #512

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

doc/ChangeLog.md

@@ -153,6 +153,9 @@ All relevant changes are documented in this file.
 
 ### Fixes
 
+- 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ș
 - 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/conf.c

@@ -1126,7 +1126,7 @@ static int sec_getbool(cfg_t *sec, const char *key, const char *alias)
  * The line-based format could only spell this as one stanza per
  * candidate, relying on nowarn to skip the ones that were not
  * installed.  A candidate not being there is not an error here, so
- * only the winner is looked up again by service_register().
+ * only the winner is looked up again by service_register_desc().
  *
  * A leading '-' still says a missing binary is expected, cf. systemd
  * ExecStart=-.
@@ -1443,7 +1443,7 @@ static void provides_translate(cfg_t *sec, svc_t *svc, char *file)
 /*
  * These have no legacy token, they are validated by service_set_dir()
  * and stored directly on the registered svc.  Empty means unset,
- * service_register() has already cleared the fields.
+ * service_register_desc() has already cleared the fields.
  */
 static void dirs_translate(cfg_t *sec, svc_t *svc, char *file)
 {
@@ -1728,11 +1728,9 @@ static void svc_translate(cfg_t *sec, int type, struct rlimit rlimit[], char *fi
 	/* the command and its arguments, verbatim */
 	addtok(line, sizeof(line), "%s", cmd);
 
-	if ((str = sec_getstr(sec, "description", "desc")))
-		addtok(line, sizeof(line), "-- %s", str);
-
+	str = sec_getstr(sec, "description", "desc");
 	dbg("translated: %s", line);
-	svc = service_register(type, line, rlimit, file);
+	svc = service_register_desc(type, line, str, rlimit, file);
 	if (!svc)
 		return;
 
@@ -1807,7 +1805,7 @@ static void tty_translate(cfg_t *sec, struct rlimit rlimit[], char *file)
 		addtok(line, sizeof(line), "%s", str);
 
 	dbg("translated: tty %s", line);
-	service_register(SVC_TYPE_TTY, line, rlimit, file);
+	service_register_desc(SVC_TYPE_TTY, line, NULL, rlimit, file);
 }
 
 /*

src/service.c

@@ -2156,9 +2156,10 @@ static void parse_cmdline_args(svc_t *svc, char *cmd, char **args)
 }
 
 /**
- * service_register - Register service, task or run commands
+ * service_register_desc - Register service, task or run commands
  * @type:   %SVC_TYPE_SERVICE(0), %SVC_TYPE_TASK(1), %SVC_TYPE_RUN(2)
- * @cfg:    Configuration, complete command, with -- for description text
+ * @cfg:    Configuration, complete command, without description text
+ * @desc:   Description text, or %NULL
  * @rlimit: Limits for this service/task/run, may be global limits
  * @file:   The file name service was loaded from
  *
@@ -2216,9 +2217,10 @@ static void parse_cmdline_args(svc_t *svc, char *cmd, char **args)
  * skipped on purpose -- conditional loading, bootstrap over, nowarn --
  * also returns %NULL, with @errno zero.
  */
-svc_t *service_register(int type, char *cfg, struct rlimit rlimit[], char *file)
+svc_t *service_register_desc(int type, char *cfg, const char *desc,
+			     struct rlimit rlimit[], char *file)
 {
-	char *cmd, *desc, *runlevels = NULL, *cond = NULL;
+	char *cmd, *runlevels = NULL, *cond = NULL;
 	char *username = NULL, *log = NULL, *pid = NULL;
 	char *name = NULL, *halt = NULL, *delay = NULL;
 	char *id = NULL, *env = NULL, *cgroup = NULL;
@@ -2254,24 +2256,6 @@ svc_t *service_register(int type, char *cfg, struct rlimit rlimit[], char *file)
 		return NULL;
 	}
 
-	desc = strstr(line, "-- ");
-	if (desc) {
-		*desc = 0;
-		desc += 3;
-
-		while (*desc && isblank(*desc))
-			desc++;
-	} else {
-		int pos;
-
-		/* Find "--\n" to denote empty/no description */
-		pos = (int)strlen(line) - 2;
-		if (pos > 0 && !strcmp(&line[pos], "--")) {
-			line[pos] = 0;
-			desc = &line[pos];
-		}
-	}
-
 	args = line;
 	cmd = parse_args(&args);
 	if (!cmd) {
@@ -2739,6 +2723,45 @@ svc_t *service_register(int type, char *cfg, struct rlimit rlimit[], char *file)
 	return svc;
 }
 
+/**
+ * service_register - Register from a legacy one-liner
+ * @type:   %SVC_TYPE_SERVICE(0), %SVC_TYPE_TASK(1), %SVC_TYPE_RUN(2)
+ * @cfg:    Configuration, complete command, with -- for description text
+ * @rlimit: Limits for this service/task/run, may be global limits
+ * @file:   The file name service was loaded from
+ *
+ * The first "-- " in @cfg starts the description, the rest is handed
+ * to service_register_desc().
+ */
+svc_t *service_register(int type, char *cfg, struct rlimit rlimit[], char *file)
+{
+	char *line, *desc;
+
+	if (!cfg)
+		return service_register_desc(type, cfg, NULL, rlimit, file);
+
+	line = strdupa(cfg);
+	desc = strstr(line, "-- ");
+	if (desc) {
+		*desc = 0;
+		desc += 3;
+
+		while (*desc && isblank(*desc))
+			desc++;
+	} else {
+		int pos;
+
+		/* Find "--\n" to denote empty/no description */
+		pos = (int)strlen(line) - 2;
+		if (pos > 0 && !strcmp(&line[pos], "--")) {
+			line[pos] = 0;
+			desc = &line[pos];
+		}
+	}
+
+	return service_register_desc(type, line, desc, rlimit, file);
+}
+
 /*
  * This function is called at the end of a runlevel change or reload
  * command to delete all remnants of removed services.

src/service.h

@@ -44,6 +44,8 @@ extern const struct svcdir svcdirs[NUM_SVCDIRS];
 
 int	  service_set_dir	 (svc_t *svc, const struct svcdir *sd, const char *name);
 svc_t	 *service_register	 (int type, char *line, struct rlimit rlimit[], char *file);
+svc_t	 *service_register_desc  (int type, char *line, const char *desc,
+				  struct rlimit rlimit[], char *file);
 void      service_unregister     (svc_t *svc);
 
 void      service_runtask_clean  (void);

test/conf-command.sh

@@ -76,6 +76,19 @@ retry 'assert_cmd pick "serv -np -i pick"'
 assert_desc "Single command" pick
 assert_num_children 1 serv
 
+# The description has its own setting, so a -- in the command is an
+# argument to it, see issue #512.
+say 'A -- in the command is passed on to it'
+run "echo 'service pick {'                                >  $FINIT_CONF"
+run "echo '    description = \"Dashes\"'                   >> $FINIT_CONF"
+run "echo '    command     = \"serv -np -i pick -- extra\"' >> $FINIT_CONF"
+run "echo '}'                                             >> $FINIT_CONF"
+run "initctl reload"
+
+retry 'assert_cmd pick "serv -np -i pick -- extra"'
+assert_desc "Dashes" pick
+assert_num_children 1 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.
@@ -90,7 +103,7 @@ retry 'assert_cmd tty "serv -np -i ttypick"'
 
 # Nothing resolves, so the block is skipped the same way a single
 # missing command is.  The leading - only decides whether that is
-# quiet, service_register() bails before svc_new() either way.
+# quiet, service_register_desc() bails before svc_new() either way.
 say 'A block whose candidates are all missing is skipped'
 run "echo 'service ghost {'                                >  $FINIT_CONF"
 run "echo '    command = { \"-/no/such/one\", \"-/no/such/two\" }' >> $FINIT_CONF"