various tools: be more explicit when a glob is passed when not supported

See https://bugzilla.redhat.com/show_bug.cgi?id=1763488: when we say that
'foo@*.service' is not a valid unit name, this is not clear enough. Let's
include the name of the operation that does not support globbing in the
error message:

$ build/systemctl enable 'foo@*.service'
Glob pattern passed to enable, but globs are not supported for this.
Invalid unit name "foo@*.service" escaped as "foo@\x2a.service".
...
This commit is contained in:
Zbigniew Jędrzejewski-Szmek 2019-10-24 14:09:11 +02:00 committed by Yu Watanabe
parent 1c089741d3
commit df7c4eb62a
7 changed files with 50 additions and 32 deletions

View File

@ -2093,7 +2093,7 @@ int analyze_security(sd_bus *bus, char **units, AnalyzeSecurityFlags flags) {
fflush(stdout);
}
r = unit_name_mangle_with_suffix(*i, 0, ".service", &mangled);
r = unit_name_mangle(*i, 0, &mangled);
if (r < 0)
return log_error_errno(r, "Failed to mangle unit name '%s': %m", *i);

View File

@ -597,10 +597,10 @@ static bool do_escape_mangle(const char *f, bool allow_globs, char *t) {
*
* If @allow_globs, globs characters are preserved. Otherwise, they are escaped.
*/
int unit_name_mangle_with_suffix(const char *name, UnitNameMangle flags, const char *suffix, char **ret) {
int unit_name_mangle_with_suffix(const char *name, const char *operation, UnitNameMangle flags, const char *suffix, char **ret) {
char *s;
int r;
bool mangled;
bool mangled, suggest_escape = true;
assert(name);
assert(suffix);
@ -617,10 +617,14 @@ int unit_name_mangle_with_suffix(const char *name, UnitNameMangle flags, const c
goto good;
/* Already a fully valid globbing expression? If so, no mangling is necessary either... */
if ((flags & UNIT_NAME_MANGLE_GLOB) &&
string_is_glob(name) &&
in_charset(name, VALID_CHARS_GLOB))
goto good;
if (string_is_glob(name) && in_charset(name, VALID_CHARS_GLOB)) {
if (flags & UNIT_NAME_MANGLE_GLOB)
goto good;
log_full(flags & UNIT_NAME_MANGLE_WARN ? LOG_NOTICE : LOG_DEBUG,
"Glob pattern passed%s%s, but globs are not supported for this.",
operation ? " " : "", operation ?: "");
suggest_escape = false;
}
if (is_device_path(name)) {
r = unit_name_from_path(name, ".device", ret);
@ -645,11 +649,12 @@ int unit_name_mangle_with_suffix(const char *name, UnitNameMangle flags, const c
mangled = do_escape_mangle(name, flags & UNIT_NAME_MANGLE_GLOB, s);
if (mangled)
log_full(flags & UNIT_NAME_MANGLE_WARN ? LOG_NOTICE : LOG_DEBUG,
"Invalid unit name \"%s\" was escaped as \"%s\" (maybe you should use systemd-escape?)",
name, s);
"Invalid unit name \"%s\" escaped as \"%s\"%s.",
name, s,
suggest_escape ? " (maybe you should use systemd-escape?)" : "");
/* Append a suffix if it doesn't have any, but only if this is not a glob, so that we can allow "foo.*" as a
* valid glob. */
/* Append a suffix if it doesn't have any, but only if this is not a glob, so that we can allow
* "foo.*" as a valid glob. */
if ((!(flags & UNIT_NAME_MANGLE_GLOB) || !string_is_glob(s)) && unit_name_to_type(s) < 0)
strcat(s, suffix);

View File

@ -52,10 +52,10 @@ typedef enum UnitNameMangle {
UNIT_NAME_MANGLE_WARN = 1 << 1,
} UnitNameMangle;
int unit_name_mangle_with_suffix(const char *name, UnitNameMangle flags, const char *suffix, char **ret);
int unit_name_mangle_with_suffix(const char *name, const char *operation, UnitNameMangle flags, const char *suffix, char **ret);
static inline int unit_name_mangle(const char *name, UnitNameMangle flags, char **ret) {
return unit_name_mangle_with_suffix(name, flags, ".service", ret);
return unit_name_mangle_with_suffix(name, NULL, flags, ".service", ret);
}
bool service_unit_name_is_valid(const char *name);

View File

@ -227,7 +227,7 @@ static int write_dependency(FILE *f, const char *opts,
STRV_FOREACH(s, names) {
char *x;
r = unit_name_mangle_with_suffix(*s, 0, ".mount", &x);
r = unit_name_mangle_with_suffix(*s, "as dependency", 0, ".mount", &x);
if (r < 0)
return log_error_errno(r, "Failed to generate unit name: %m");
r = strv_consume(&units, x);

View File

@ -258,7 +258,7 @@ int allocate_scope(
if (r < 0)
return log_error_errno(r, "Could not watch job: %m");
r = unit_name_mangle_with_suffix(machine_name, 0, ".scope", &scope);
r = unit_name_mangle_with_suffix(machine_name, "as machine name", 0, ".scope", &scope);
if (r < 0)
return log_error_errno(r, "Failed to mangle scope name: %m");
@ -350,7 +350,7 @@ int terminate_scope(
_cleanup_free_ char *scope = NULL;
int r;
r = unit_name_mangle_with_suffix(machine_name, 0, ".scope", &scope);
r = unit_name_mangle_with_suffix(machine_name, "to terminate", 0, ".scope", &scope);
if (r < 0)
return log_error_errno(r, "Failed to mangle scope name: %m");

View File

@ -641,7 +641,9 @@ static int transient_cgroup_set_properties(sd_bus_message *m) {
if (!isempty(arg_slice)) {
_cleanup_free_ char *slice = NULL;
r = unit_name_mangle_with_suffix(arg_slice, arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, ".slice", &slice);
r = unit_name_mangle_with_suffix(arg_slice, "as slice",
arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN,
".slice", &slice);
if (r < 0)
return log_error_errno(r, "Failed to mangle name '%s': %m", arg_slice);
@ -1112,7 +1114,9 @@ static int start_transient_service(
}
if (arg_unit) {
r = unit_name_mangle_with_suffix(arg_unit, arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, ".service", &service);
r = unit_name_mangle_with_suffix(arg_unit, "as unit",
arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN,
".service", &service);
if (r < 0)
return log_error_errno(r, "Failed to mangle unit name: %m");
} else {
@ -1355,7 +1359,9 @@ static int start_transient_scope(sd_bus *bus) {
return log_oom();
if (arg_unit) {
r = unit_name_mangle_with_suffix(arg_unit, arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, ".scope", &scope);
r = unit_name_mangle_with_suffix(arg_unit, "as unit",
arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN,
".scope", &scope);
if (r < 0)
return log_error_errno(r, "Failed to mangle scope name: %m");
} else {
@ -1530,11 +1536,15 @@ static int start_transient_trigger(
break;
default:
r = unit_name_mangle_with_suffix(arg_unit, arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, ".service", &service);
r = unit_name_mangle_with_suffix(arg_unit, "as unit",
arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN,
".service", &service);
if (r < 0)
return log_error_errno(r, "Failed to mangle unit name: %m");
r = unit_name_mangle_with_suffix(arg_unit, arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, suffix, &trigger);
r = unit_name_mangle_with_suffix(arg_unit, "as trigger",
arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN,
suffix, &trigger);
if (r < 0)
return log_error_errno(r, "Failed to mangle unit name: %m");

View File

@ -761,10 +761,7 @@ static int expand_names(sd_bus *bus, char **names, const char* suffix, char ***r
char *t;
UnitNameMangle options = UNIT_NAME_MANGLE_GLOB | (arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN);
if (suffix)
r = unit_name_mangle_with_suffix(*name, options, suffix, &t);
else
r = unit_name_mangle(*name, options, &t);
r = unit_name_mangle_with_suffix(*name, NULL, options, suffix ?: ".service", &t);
if (r < 0)
return log_error_errno(r, "Failed to mangle name: %m");
@ -2182,7 +2179,9 @@ static int set_default(int argc, char *argv[], void *userdata) {
assert(argc >= 2);
assert(argv);
r = unit_name_mangle_with_suffix(argv[1], arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, ".target", &unit);
r = unit_name_mangle_with_suffix(argv[1], "set-default",
arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN,
".target", &unit);
if (r < 0)
return log_error_errno(r, "Failed to mangle unit name: %m");
@ -6565,7 +6564,7 @@ static int enable_sysv_units(const char *verb, char **args) {
return r;
}
static int mangle_names(char **original_names, char ***mangled_names) {
static int mangle_names(const char *operation, char **original_names, char ***mangled_names) {
char **i, **l, **name;
int r;
@ -6585,7 +6584,9 @@ static int mangle_names(char **original_names, char ***mangled_names) {
return log_oom();
}
} else {
r = unit_name_mangle(*name, arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, i);
r = unit_name_mangle_with_suffix(*name, operation,
arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN,
".service", i);
if (r < 0) {
*i = NULL;
strv_free(l);
@ -6696,7 +6697,7 @@ static int enable_unit(int argc, char *argv[], void *userdata) {
if (!argv[1])
return 0;
r = mangle_names(strv_skip(argv, 1), &names);
r = mangle_names("to enable", strv_skip(argv, 1), &names);
if (r < 0)
return r;
@ -6923,11 +6924,13 @@ static int add_dependency(int argc, char *argv[], void *userdata) {
if (!argv[1])
return 0;
r = unit_name_mangle_with_suffix(argv[1], arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, ".target", &target);
r = unit_name_mangle_with_suffix(argv[1], "as target",
arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN,
".target", &target);
if (r < 0)
return log_error_errno(r, "Failed to mangle unit name: %m");
r = mangle_names(strv_skip(argv, 2), &names);
r = mangle_names("as dependency", strv_skip(argv, 2), &names);
if (r < 0)
return r;
@ -7113,7 +7116,7 @@ static int unit_is_enabled(int argc, char *argv[], void *userdata) {
char **name;
int r;
r = mangle_names(strv_skip(argv, 1), &names);
r = mangle_names("to check", strv_skip(argv, 1), &names);
if (r < 0)
return r;