diff --git a/configure.ac b/configure.ac index 90b35c47..0b213b53 100644 --- a/configure.ac +++ b/configure.ac @@ -50,9 +50,11 @@ PKG_PROG_PKG_CONFIG # Check for required libraries PKG_CHECK_MODULES([uev], [libuev >= 2.4.1]) PKG_CHECK_MODULES([lite], [libite >= 2.6.1]) -# 3.3 is the floor: CFGF_KEYSTRVAL, which set {} and the free-form -# cgroup keys are built on, does not exist before it. 3.3 parses both -# correctly, see the XXX in src/conf.c before raising this to 3.4. +# 3.3 is the floor: CFGF_KEYSTRVAL, which environment {} and the +# free-form cgroup keys are built on, does not exist before it. 3.3 +# parses both correctly. Two workarounds hang off this number, the +# spurious KEYSTRVAL warning and cfg_parse_buf() losing the file name +# of a template; grep the XXX notes in src/conf.c before raising it. PKG_CHECK_MODULES([confuse], [libconfuse >= 3.3]) # Check for configured Finit features diff --git a/src/conf.c b/src/conf.c index 34d87458..70f349d6 100644 --- a/src/conf.c +++ b/src/conf.c @@ -1525,6 +1525,105 @@ static int conf_parse_cfg(cfg_t *cfg, char *file, int is_rcsd) return 0; } +/* + * Substitute every %i in LINE with NAME, for template instantiation. + * + * Very simple and crude implementation, only supports '%i' + */ +static char *conf_instantiate(char *line, char *name) +{ + char *ptr, *end = strchr(line, 0); + char *pos = line; + size_t num = 0; + + if (!name[0] || !end) + return line; + + while ((ptr = strchr(pos, '%'))) { + num++; + pos = ptr + 1; + } + + ptr = realloc(line, strlen(line) + num * strlen(name) + 1); + if (!ptr) + return line; + + pos = line = ptr; + while ((ptr = strchr(pos, '%'))) { + if (!strncmp(ptr, "%i", 2)) { + char *rest = &ptr[2]; + char *next = ptr + strlen(name); + + memmove(next, rest, strlen(rest) + 1); + memcpy(ptr, name, strlen(name)); + + pos += strlen(name); + } else + pos++; + } + + return line; +} + +static int conf_is_template(const char *file, char *name, size_t len) +{ + char *ptr, *nm; + size_t i = 0; + + /* the @ convention names the file, a directory may contain one */ + ptr = strchr(basenm((char *)file), '@'); + if (!ptr) + return 0; /* not a template */ + + nm = ptr + 1; + ptr = strstr(nm, ".conf"); + if (!strcmp(nm, ".conf") || !ptr) + return 1; /* template itself or invalid */ + + if (!name) + return 1; + + while (nm < ptr && i < len - 1) + name[i++] = *nm++; + name[i] = 0; + + return 1; /* instantiated template */ +} + +/* + * Parse from FILE, or from BUF when it is set, i.e. for a template + * whose %i has already been substituted. + * + * XXX: Workaround for libConfuse <3.4, whose cfg_parse_buf() replaces + * cfg->filename with "[buf]", so every diagnostic from a template + * would lose the file name. cfg_parse_fp() keeps a name the + * caller has already set, so go through fmemopen() instead. With + * a 3.4 floor this is just cfg_parse_buf(cfg, buf). + */ +static int conf_parse_any(cfg_t *cfg, char *file, char *buf) +{ + FILE *fp; + int rc; + + if (!buf) + return cfg_parse(cfg, file); + + /* fmemopen() rejects a zero length on older GLIBC */ + if (!buf[0]) + return CFG_SUCCESS; + + fp = fmemopen(buf, strlen(buf), "r"); + if (!fp) + return CFG_FILE_ERROR; + + /* cfg_parse_fp() only names the stream when we have not */ + cfg->filename = strdup(file); + rc = cfg_parse_fp(cfg, fp); + fclose(fp); + + return rc; +} + /* * Is this a new-format file, or a legacy one? * @@ -1539,7 +1638,7 @@ static int conf_parse_cfg(cfg_t *cfg, char *file, int is_rcsd) * creating them, so a lenient tree is missing every set{} variable * and every free-form cgroup key. */ -static int is_new_format(char *file) +static int is_new_format(char *file, char *buf) { cfg_t *cfg; int rc; @@ -1549,12 +1648,44 @@ static int is_new_format(char *file) return 0; cfg_set_error_function(cfg, cfg_error_quiet); - rc = cfg_parse(cfg, file); + rc = conf_parse_any(cfg, file, buf); cfg_free(cfg); return rc == CFG_SUCCESS; } +/* + * Read FILE and substitute %i with NAME, for template instantiation. + * Returns a malloc()'ed buffer the caller frees. + */ +static char *conf_read_template(char *file, char *name) +{ + struct stat st; + char *buf = NULL; + size_t len; + FILE *fp; + + fp = fopen(file, "r"); + if (!fp) + return NULL; + + /* fstat() the open fd, no window for the file to change under us */ + if (fstat(fileno(fp), &st) || st.st_size < 0) + goto out; + + buf = malloc((size_t)st.st_size + 1); + if (!buf) + goto out; + + len = fread(buf, 1, (size_t)st.st_size, fp); + buf[len] = 0; + buf = conf_instantiate(buf, name); +out: + fclose(fp); + + return buf; +} + /* * Parse one Finit .conf file, in either format. The legacy .conf * include directive routes back through this so included files are @@ -1562,43 +1693,63 @@ static int is_new_format(char *file) */ int conf_parse_file(char *file, int is_rcsd) { + char name[MAX_ID_LEN] = { 0 }; + char *buf = NULL; cfg_t *cfg; int rc; /* - * Template files (name@.conf, name@id.conf): %i instantiation - * for the new format is not yet supported, so these bypass - * detection entirely and go to the legacy parser, which handles - * templates per line. See issue #148. + * Template files (name@.conf, name@id.conf). A bare name@.conf + * is the template itself, there is nothing to instantiate from + * it. Otherwise %i is substituted over the whole file up front, + * so both the format detection below and, on fallback, the + * legacy parser see the finished text. */ - if (strchr(basenm(file), '@')) - return legacy_parse_conf(file, is_rcsd); + if (conf_is_template(file, name, sizeof(name))) { + if (!name[0]) { + dbg("*** Skipping template file %s", file); + return 0; + } + + dbg("*** instantiating %s from %s ...", name, file); + buf = conf_read_template(file, name); + if (!buf) + return 1; + } cfg = cfg_init(conf_opts, CFGF_NONE); - if (!cfg) - return legacy_parse_conf(file, is_rcsd); + if (!cfg) { + rc = legacy_parse_conf(file, buf, is_rcsd); + goto done; + } cfg_set_error_function(cfg, cfg_error_cb); - rc = cfg_parse(cfg, file); + rc = conf_parse_any(cfg, file, buf); if (rc == CFG_SUCCESS) { dbg("*** Parsing %s (new format)", file); rc = conf_parse_cfg(cfg, file, is_rcsd); cfg_free(cfg); - return rc; + goto done; } cfg_free(cfg); - if (rc == CFG_FILE_ERROR) - return 1; /* like legacy fopen() failure */ + if (rc == CFG_FILE_ERROR) { + rc = 1; /* like legacy fopen() failure */ + goto done; + } - if (is_new_format(file)) { + if (is_new_format(file, buf)) { logit(LOG_ERR, "parse error: %s", cfg_errmsg); - return 1; + rc = 1; + goto done; } dbg("not in new format (%s), falling back to legacy parser", cfg_errmsg); + rc = legacy_parse_conf(file, buf, is_rcsd); +done: + free(buf); - return legacy_parse_conf(file, is_rcsd); + return rc; } static void glob_append(glob_t *gl, int append, const char *fmt, ...) @@ -1849,7 +2000,7 @@ static int conf_change_act(char *dir, char *name, uint32_t mask) strlcpy(fn, dir, sizeof(fn)); dbg("path: %s mask: %08x", fn, mask); - if (strchr(name, '@')) { + if (conf_is_template(name, NULL, 0)) { /* Skip realpath for templates */ rp = strdup(fn); } else { @@ -1903,7 +2054,7 @@ int conf_changed(char *file) if (!file) return 0; - if (strchr(file, '@')) + if (conf_is_template(file, NULL, 0)) rp = strdup(file); else rp = realpath(file, NULL); diff --git a/src/conf.h b/src/conf.h index b2576ff8..aa565fca 100644 --- a/src/conf.h +++ b/src/conf.h @@ -83,6 +83,7 @@ int conf_parse_runlevels (const char *runlevels); void conf_parse_cond (svc_t *svc, char *cond); int conf_parse_file (char *file, int is_rcsd); + #endif /* FINIT_CONF_H_ */ /** diff --git a/src/legacy.c b/src/legacy.c index c817886e..8de09469 100644 --- a/src/legacy.c +++ b/src/legacy.c @@ -319,79 +319,19 @@ static int parse_dynamic(char *line, struct rlimit rlimit[], char *file) } /* - * Very simple and crude implementation, only supports '%i' + * Parse FILE, or BUF when the frontend has already read and + * instantiated it, i.e. for a template. Either way the text arriving + * here is final, no %i is left to substitute. */ -static char *instantiate(char *line, char *name) -{ - char *ptr, *end = strchr(line, 0); - char *pos = line; - size_t num = 0; - - if (!name[0] || !end) - return line; - - while ((ptr = strchr(pos, '%'))) { - num++; - pos++; - } - - ptr = realloc(line, strlen(line) + num * strlen(name) + 1); - if (!ptr) - return line; - - pos = line = ptr; - while ((ptr = strchr(pos, '%'))) { - if (!strncmp(ptr, "%i", 2)) { - char *rest = &ptr[2]; - char *next = ptr + strlen(name); - - memmove(next, rest, strlen(rest) + 1); - memcpy(ptr, name, strlen(name)); - - pos += strlen(name); - } else - pos++; - } - - return line; -} - -static int is_template(const char *file, char *name, size_t len) -{ - char *ptr, *nm; - size_t i = 0; - - ptr = strchr(file, '@'); - if (!ptr) - return 0; /* not a template */ - - nm = ptr + 1; - ptr = strstr(nm, ".conf"); - if (!strcmp(nm, ".conf") || !ptr) - return 1; /* template itself or invalid */ - - while (nm < ptr && i < len - 1) - name[i++] = *nm++; - name[i] = 0; - - return 1; /* instantiated template */ -} - -int legacy_parse_conf(char *file, int is_rcsd) +int legacy_parse_conf(char *file, char *buf, int is_rcsd) { struct rlimit rlimit[RLIMIT_NLIMITS]; - char name[65] = { 0 }; FILE *fp; - if (is_template(file, name, sizeof(name))) { - if (!name[0]) { - dbg("*** Skipping template file %s", file); - return 0; - } - dbg("*** instantiating %s from %s ...", name, file); - } - - fp = fopen(file, "r"); + if (buf) + fp = fmemopen(buf, strlen(buf), "r"); + else + fp = fopen(file, "r"); if (!fp) return 1; @@ -410,9 +350,6 @@ int legacy_parse_conf(char *file, int is_rcsd) continue; tabstospaces(line); -// dbg("raw: %s", line); - line = instantiate(line, name); -// dbg("ins: %s", line); if (!parse_static(line, is_rcsd)) ; diff --git a/src/legacy.h b/src/legacy.h index da16c18d..f623c743 100644 --- a/src/legacy.h +++ b/src/legacy.h @@ -24,7 +24,7 @@ #ifndef FINIT_LEGACY_H_ #define FINIT_LEGACY_H_ -int legacy_parse_conf (char *file, int is_rcsd); +int legacy_parse_conf (char *file, char *buf, int is_rcsd); void legacy_parse_env (char *line); #endif /* FINIT_LEGACY_H_ */ diff --git a/test/Makefile.am b/test/Makefile.am index 9edcca9f..38551d02 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-template.sh EXTRA_DIST += crashing.sh EXTRA_DIST += dep-chain-reload.sh EXTRA_DIST += depserv.sh @@ -77,6 +78,7 @@ TESTS += add-remove-dynamic-service-sub-config.sh TESTS += bootstrap-crash.sh TESTS += cond-start-task.sh TESTS += conf-format.sh +TESTS += conf-template.sh TESTS += crashing.sh TESTS += dep-chain-reload.sh TESTS += depserv.sh diff --git a/test/conf-template.sh b/test/conf-template.sh new file mode 100755 index 00000000..344be694 --- /dev/null +++ b/test/conf-template.sh @@ -0,0 +1,82 @@ +#!/bin/sh +# Verify %i template instantiation for both .conf formats: the block +# format substitutes over the whole file before parsing, the legacy +# one-liner format per line. A bare name@.conf registers nothing. +set -eu + +TEST_DIR=$(dirname "$0") + +test_teardown() +{ + say "Running test teardown." + run "rm -f $FINIT_RCSD/available/serv@.conf" + run "rm -f $FINIT_RCSD/enabled/serv@eth0.conf" + run "rm -f $FINIT_RCSD/enabled/serv@eth1.conf $FINIT_RCSD/enabled/serv@.conf" + run "rm -f /run/serv-eth0.pid /run/serv-eth1.pid" +} + +# shellcheck source=/dev/null +. "$TEST_DIR/lib/setup.sh" + +# %i has to reach three different places: the section title, which +# becomes the instance identity, an ordinary value, and the command +# line. The per-instance PID file covers the last one, it can only +# appear if the command was instantiated. +say 'Install a block-format template' +run "echo 'service serv:%i {' > $FINIT_RCSD/available/serv@.conf" +run "echo ' description = \"Template for %i\"' >> $FINIT_RCSD/available/serv@.conf" +run "echo ' pid = \"/run/serv-%i.pid\"' >> $FINIT_RCSD/available/serv@.conf" +run "echo ' command = \"serv -n -p -P /run/serv-%i.pid\"' >> $FINIT_RCSD/available/serv@.conf" +run "echo '}' >> $FINIT_RCSD/available/serv@.conf" + +say 'Enable two instances' +run "initctl enable serv@eth0.conf" +run "initctl enable serv@eth1.conf" +run "initctl reload" + +retry 'assert_num_services 2 serv' +assert_desc "Template for eth0" serv:eth0 +assert_desc "Template for eth1" serv:eth1 + +say 'Both instances run, each with its own instantiated PID file' +retry 'assert_num_children 2 serv' +retry 'assert_file_exists /run/serv-eth0.pid' +retry 'assert_file_exists /run/serv-eth1.pid' + +# available/ is never globbed, so enabling the bare template is the +# only way the "skip the template itself" branch is reached at all. +say 'A bare template in enabled/ registers nothing' +run "ln -sf ../available/serv@.conf $FINIT_RCSD/enabled/serv@.conf" +run "initctl reload" + +retry 'assert_num_services 2 serv' +run "rm -f $FINIT_RCSD/enabled/serv@.conf" + +# assert_num_services cannot express "exactly one": initctl status +# prints a detail block for a single match and a table only for +# several, so the line count is 13, not 1. Check the survivor and the +# absence of the other instead. +say 'Disable one instance' +run "initctl disable serv@eth1.conf" +run "initctl reload" + +retry 'assert_num_children 1 serv' +assert_desc "Template for eth0" serv:eth0 +assert_num_services 0 serv:eth1 + +say 'Legacy one-liner templates still instantiate' +run "echo 'service :%i pid:/run/serv-%i.pid serv -n -p -P /run/serv-%i.pid -- Legacy template for %i' > $FINIT_RCSD/available/serv@.conf" +run "initctl reload" + +retry 'assert_num_children 1 serv' +assert_desc "Legacy template for eth0" serv:eth0 + +say 'A typo in a block template is rejected, and named after the instance' +run "echo 'service serv:%i {' > $FINIT_RCSD/available/serv@.conf" +run "echo ' descriptoin = \"Template for %i\"' >> $FINIT_RCSD/available/serv@.conf" +run "echo ' command = \"serv -n\"' >> $FINIT_RCSD/available/serv@.conf" +run "echo '}' >> $FINIT_RCSD/available/serv@.conf" +run "initctl reload" + +retry 'assert_num_children 0 serv' +assert_num_services 0 serv