diff --git a/src/conf.c b/src/conf.c index 25451d93..6b8896fa 100644 --- a/src/conf.c +++ b/src/conf.c @@ -313,12 +313,21 @@ static cfg_opt_t tty_opts[] = { CFG_END() }; +/* + * Flags for the sections whose title is a service identity. + * libconfuse merges two sections sharing a title, silently, which for + * these would fold two declarations into one service, so duplicates + * are rejected instead. + */ +#define SVC_SEC_FLAGS (CFGF_MULTI | CFGF_TITLE | CFGF_NO_TITLE_DUPES) + static cfg_opt_t conf_opts[] = { - CFG_SEC ("service", svc_opts, CFGF_MULTI | CFGF_TITLE), - CFG_SEC ("task", svc_opts, CFGF_MULTI | CFGF_TITLE), - CFG_SEC ("run", svc_opts, CFGF_MULTI | CFGF_TITLE), - CFG_SEC ("sysv", svc_opts, CFGF_MULTI | CFGF_TITLE), - CFG_SEC ("tty", tty_opts, CFGF_MULTI | CFGF_TITLE), + CFG_SEC ("service", svc_opts, SVC_SEC_FLAGS), + CFG_SEC ("task", svc_opts, SVC_SEC_FLAGS), + CFG_SEC ("run", svc_opts, SVC_SEC_FLAGS), + CFG_SEC ("sysv", svc_opts, SVC_SEC_FLAGS), + CFG_SEC ("tty", tty_opts, SVC_SEC_FLAGS), + /* cgroup titles name a group, not a service, so merging is fine */ CFG_SEC ("cgroup", cgroup_opts, CFGF_MULTI | CFGF_TITLE | CFGF_KEYSTRVAL), CFG_SEC ("rlimit", rlimit_opts, CFGF_NONE), CFG_SEC ("environment", env_opts, CFGF_KEYSTRVAL), @@ -2003,6 +2012,7 @@ static int conf_parse_any(cfg_t *cfg, char *file, char *buf) */ static int is_new_format(char *file, char *buf) { + cfg_opt_t *opt; cfg_t *cfg; int rc; @@ -2011,6 +2021,17 @@ static int is_new_format(char *file, char *buf) return 0; cfg_set_error_function(cfg, cfg_error_quiet); + + /* + * The verdict must stay syntactic. A duplicate section title is + * a semantic error in a file that is unmistakably block format, + * and answering 'no' here would hand it to the legacy parser, + * burying the real message under its errors. cfg_init() copies + * the option array, so this only relaxes the probe. + */ + for (opt = cfg->opts; opt && opt->name; opt++) + opt->flags &= ~CFGF_NO_TITLE_DUPES; + rc = conf_parse_any(cfg, file, buf); cfg_free(cfg); diff --git a/test/Makefile.am b/test/Makefile.am index 6670cbc2..10431cc7 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -32,6 +32,7 @@ EXTRA_DIST += add-remove-dynamic-service-sub-config.sh EXTRA_DIST += bootstrap-crash.sh EXTRA_DIST += cond-start-task.sh EXTRA_DIST += conf-format.sh +EXTRA_DIST += conf-dup-title.sh EXTRA_DIST += conf-dirs.sh EXTRA_DIST += conf-if.sh EXTRA_DIST += conf-template.sh @@ -81,6 +82,7 @@ TESTS += add-remove-dynamic-service-sub-config.sh TESTS += bootstrap-crash.sh TESTS += cond-start-task.sh TESTS += conf-format.sh +TESTS += conf-dup-title.sh TESTS += conf-dirs.sh TESTS += conf-if.sh TESTS += conf-template.sh diff --git a/test/conf-dup-title.sh b/test/conf-dup-title.sh new file mode 100755 index 00000000..26a7193a --- /dev/null +++ b/test/conf-dup-title.sh @@ -0,0 +1,87 @@ +#!/bin/sh +# Verify duplicate section titles. The title is the service identity, +# and libconfuse merges two sections that share one, silently folding +# two declarations into a single service. A file doing that is +# rejected. Across files the same title is still how an administrator +# overrides a system .conf, so that has to keep working. +set -eu + +TEST_DIR=$(dirname "$0") + +# shellcheck disable=SC2034 +BOOTSTRAP="service service.sh { + description = \"Base\" + runlevel = \"S12345\" + command = \"service.sh\" +}" + +# initctl status prints a detail block for a single match and a table +# only for several, so count lines in the full listing instead. +assert_loaded() +{ + assert "Service $1 loaded: $2" \ + "$(texec initctl -t status | awk -v n="$1" '$2 == n' | wc -l)" -eq "$2" +} + +test_teardown() +{ + say "Running test teardown." + run "rm -f $FINIT_RCSD/dup.conf $FINIT_RCSD/override.conf" +} + +# shellcheck source=/dev/null +. "$TEST_DIR/lib/setup.sh" + +say 'The service from the bootstrap file is running' +retry 'assert_num_children 1 service.sh' +assert_desc "Base" service.sh + +# Written as two blocks the file used to load as one service, gated by +# whichever if came last. Rejecting the file makes that visible. +say 'Two blocks with one title are rejected, and the file loads nothing' +run "echo 'service dupsvc {' > $FINIT_RCSD/dup.conf" +run "echo ' if = \"service.sh\"' >> $FINIT_RCSD/dup.conf" +run "echo ' command = \"serv -np -i dupsvc\"' >> $FINIT_RCSD/dup.conf" +run "echo '}' >> $FINIT_RCSD/dup.conf" +run "echo 'service dupsvc {' >> $FINIT_RCSD/dup.conf" +run "echo ' if = \"nosuchservice\"' >> $FINIT_RCSD/dup.conf" +run "echo ' command = \"serv -np -i dupsvc\"' >> $FINIT_RCSD/dup.conf" +run "echo '}' >> $FINIT_RCSD/dup.conf" +run "initctl reload" + +assert_loaded dupsvc 0 + +# A rejected file must not reach the legacy parser either, and the +# rest of the configuration has to survive it. +say 'The other .conf files are unaffected' +retry 'assert_num_children 1 service.sh' +assert_desc "Base" service.sh + +# Detection of the format has to stay syntactic for this: a probe that +# answers "not block format" on a duplicate title would hand the file +# to the legacy parser, which registers a bogus service per line. +say 'A distinct title in the same file loads as its own service' +run "rm -f $FINIT_RCSD/dup.conf" +run "echo 'service dupsvc:1 {' > $FINIT_RCSD/dup.conf" +run "echo ' command = \"serv -np -i dupsvc1\"' >> $FINIT_RCSD/dup.conf" +run "echo '}' >> $FINIT_RCSD/dup.conf" +run "echo 'service dupsvc:2 {' >> $FINIT_RCSD/dup.conf" +run "echo ' command = \"serv -np -i dupsvc2\"' >> $FINIT_RCSD/dup.conf" +run "echo '}' >> $FINIT_RCSD/dup.conf" +run "initctl reload" + +retry 'assert_loaded dupsvc:1 1' +assert_loaded dupsvc:2 1 + +# finit.d is read after finit.conf, so the later declaration of the +# same identity replaces the earlier one. This is the override path, +# and it is a different file, so no duplicate title is involved. +say 'The same title in another file overrides, it does not conflict' +run "echo 'service service.sh {' > $FINIT_RCSD/override.conf" +run "echo ' description = \"Override\"' >> $FINIT_RCSD/override.conf" +run "echo ' command = \"service.sh\"' >> $FINIT_RCSD/override.conf" +run "echo '}' >> $FINIT_RCSD/override.conf" +run "initctl reload" + +retry 'assert_desc "Override" service.sh' +assert_num_children 1 service.sh