Fail on unknown (alphanumerical) specifiers

The code intentionally ignored unknown specifiers, treating them as text. This
needs to change because otherwise we can never add a new specifier in a backwards
compatible way. So just treat an unknown (potential) specifier as an error.

In principle this is a break of backwards compatibility, but the previous
behaviour was pretty much useless, since the expanded value could change every
time we add new specifiers, which we do all the time.

As a compromise for backwards compatibility, only fail on alphanumerical
characters. This should cover the most cases where an unescaped percent
character is used, like size=5% and such, which behave the same as before with
this patch. OTOH, this means that we will not be able to use non-alphanumerical
specifiers without breaking backwards compatibility again. I think that's an
acceptable compromise.

v2:
- add NEWS entry

v3:
- only fail on alphanumerical
This commit is contained in:
Zbigniew Jędrzejewski-Szmek 2017-11-24 12:19:40 +01:00
parent 2df36d096d
commit 751223fecf
6 changed files with 31 additions and 11 deletions

7
NEWS
View File

@ -7,6 +7,13 @@ CHANGES WITH 236 in spe:
numdummies=0, preventing the kernel from automatically creating
dummy0. All dummy interfaces must now be explicitly created.
* Unknown specifiers are now rejected. This applies to units and
tmpfiles.d configuration. Any percent characters that are followed by
a letter or digit that are not supposed to be interpreted as the
beginning of a specifier should be escaped by doubling ("%%").
(So "size=5%" is still accepted, as well as "size=5%,foo=bar", but
not "LABEL=x%y%z" since %y and %z are not valid specifiers today.)
* systemd-resolved now maintains a new dynamic
/run/systemd/resolve/stub-resolv.conf compatibility file. It is
recommended to make /etc/resolv.conf a symlink to it. This file

View File

@ -1297,7 +1297,8 @@
<para>Many settings resolve specifiers which may be used to write
generic unit files referring to runtime or unit parameters that
are replaced when the unit files are loaded. The following
are replaced when the unit files are loaded. Specifiers must be known
and resolvable for the setting to be valid. The following
specifiers are understood:</para>
<table>

View File

@ -594,6 +594,7 @@ r! /tmp/.X[0-9]*-lock</programlisting>
<title>Specifiers</title>
<para>Specifiers can be used in the "path" and "argument" fields.
An unknown or unresolvable specifier is treated as invalid configuration.
The following expansions are understood:</para>
<table>
<title>Specifiers available</title>

View File

@ -41,6 +41,10 @@
*
*/
/* Any ASCII character or digit: our pool of potential specifiers,
* and "%" used for escaping. */
#define POSSIBLE_SPECIFIERS ALPHANUMERICAL "%"
int specifier_printf(const char *text, const Specifier table[], void *userdata, char **_ret) {
char *ret, *t;
const char *f;
@ -97,7 +101,10 @@ int specifier_printf(const char *text, const Specifier table[], void *userdata,
ret = n;
t = n + j + k;
} else {
} else if (strchr(POSSIBLE_SPECIFIERS, *f))
/* Oops, an unknown specifier. */
return -EBADSLT;
else {
*(t++) = '%';
*(t++) = *f;
}

View File

@ -32,10 +32,10 @@ if __name__ == '__main__':
test_invalid_line('f++ /too/many/plusses')
test_invalid_line('f+!+ /too/many/plusses')
test_invalid_line('f!+! /too/many/bangs')
#test_invalid_line('w /unresolved/argument - - - - "%Y"')
#test_invalid_line('w /unresolved/argument/sandwich - - - - "%v%Y%v"')
#test_invalid_line('w /unresolved/filename/%Y - - - - "whatever"')
#test_invalid_line('w /unresolved/filename/sandwich/%v%Y%v - - - - "whatever"')
test_invalid_line('w /unresolved/argument - - - - "%Y"')
test_invalid_line('w /unresolved/argument/sandwich - - - - "%v%Y%v"')
test_invalid_line('w /unresolved/filename/%Y - - - - "whatever"')
test_invalid_line('w /unresolved/filename/sandwich/%v%Y%v - - - - "whatever"')
test_invalid_line('w - - - - - "no file specfied"')
test_invalid_line('C - - - - - "no file specfied"')
test_invalid_line('C non/absolute/path - - - - -')

View File

@ -1867,7 +1867,8 @@ static int parse_line(const char *fname, unsigned line, const char *buffer, bool
&age,
NULL);
if (r < 0) {
if (r == -EINVAL) /* invalid quoting and such */
if (IN_SET(r, -EINVAL, -EBADSLT))
/* invalid quoting and such or an unknown specifier */
*invalid_config = true;
return log_error_errno(r, "[%s:%u] Failed to parse line: %m", fname, line);
}
@ -1916,8 +1917,8 @@ static int parse_line(const char *fname, unsigned line, const char *buffer, bool
if (r == -ENOKEY)
return log_unresolvable_specifier(fname, line);
if (r < 0) {
/* ENOMEM is the only return value left after ENOKEY,
* so *invalid_config should not be set. */
if (IN_SET(r, -EINVAL, -EBADSLT))
*invalid_config = true;
return log_error_errno(r, "[%s:%u] Failed to replace specifiers: %s", fname, line, path);
}
@ -2028,7 +2029,7 @@ static int parse_line(const char *fname, unsigned line, const char *buffer, bool
return -EBADMSG;
}
r = parse_attribute_from_arg(&i);
if (r == -EINVAL)
if (IN_SET(r, -EINVAL, -EBADSLT))
*invalid_config = true;
if (r < 0)
return r;
@ -2054,9 +2055,12 @@ static int parse_line(const char *fname, unsigned line, const char *buffer, bool
r = specifier_expansion_from_arg(&i);
if (r == -ENOKEY)
return log_unresolvable_specifier(fname, line);
if (r < 0)
if (r < 0) {
if (IN_SET(r, -EINVAL, -EBADSLT))
*invalid_config = true;
return log_error_errno(r, "[%s:%u] Failed to substitute specifiers in argument: %m",
fname, line);
}
if (arg_root) {
char *p;