diff --git a/doc/ChangeLog.md b/doc/ChangeLog.md index ed2dabde..6fd02932 100644 --- a/doc/ChangeLog.md +++ b/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, diff --git a/src/conf.c b/src/conf.c index fb3f954e..b5af5b4e 100644 --- a/src/conf.c +++ b/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); } /* diff --git a/src/service.c b/src/service.c index ecd5f6b1..d426f8cb 100644 --- a/src/service.c +++ b/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. diff --git a/src/service.h b/src/service.h index efea5a85..6277cf95 100644 --- a/src/service.h +++ b/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); diff --git a/test/conf-command.sh b/test/conf-command.sh index 4bca34d6..1ebaf797 100755 --- a/test/conf-command.sh +++ b/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"