shared/bus-unit-util: tweak bus_append_exec_command to use Ex prop only if necessary

This changes little in behaviour, the conceptual part is more important. The
non-Ex variant is the actual name on the command line, and we should use the
non-Ex D-Bus property too, if it works. This increases compatibility with old
versions. But the code was mostly doing the right thing. Even the tests tested
the right thing.

Follow-up for b3d593673c and
898fc00e79.

The test is simplified by taking advantage of the fact that both names
on the commandline are supposed to behave identically.

Partially resolves https://github.com/systemd/systemd/issues/37174.
This commit is contained in:
Zbigniew Jędrzejewski-Szmek
2025-07-04 19:32:51 +02:00
parent 6f06afde59
commit fb98c75e0e
2 changed files with 27 additions and 39 deletions

View File

@@ -561,9 +561,8 @@ static int bus_append_socket_filter(sd_bus_message *m, const char *field, const
static int bus_append_exec_command(sd_bus_message *m, const char *field, const char *eq) {
bool explicit_path = false, done = false, ambient_hack = false;
_cleanup_strv_free_ char **cmdline = NULL, **ex_opts = NULL;
_cleanup_free_ char *_path = NULL, *upgraded_name = NULL;
_cleanup_free_ char *_path = NULL;
ExecCommandFlags flags = 0;
bool is_ex_prop = endswith(field, "Ex");
int r;
do {
@@ -638,20 +637,18 @@ static int bus_append_exec_command(sd_bus_message *m, const char *field, const c
}
} while (!done);
if (!is_ex_prop && (flags & (EXEC_COMMAND_NO_ENV_EXPAND|EXEC_COMMAND_FULLY_PRIVILEGED|EXEC_COMMAND_NO_SETUID|EXEC_COMMAND_VIA_SHELL))) {
/* Upgrade the ExecXYZ= property to ExecXYZEx= for convenience */
is_ex_prop = true;
bool ex_prop = flags & (EXEC_COMMAND_NO_ENV_EXPAND|EXEC_COMMAND_FULLY_PRIVILEGED|EXEC_COMMAND_NO_SETUID|EXEC_COMMAND_VIA_SHELL);
if (ex_prop) {
/* We need to use ExecXYZEx=. */
if (!endswith(field, "Ex"))
field = strjoina(field, "Ex");
upgraded_name = strjoin(field, "Ex");
if (!upgraded_name)
return log_oom();
field = upgraded_name;
}
if (is_ex_prop) {
r = exec_command_flags_to_strv(flags, &ex_opts);
if (r < 0)
return log_error_errno(r, "Failed to serialize ExecCommand flags: %m");
} else {
if (endswith(field, "Ex"))
field = strndupa_safe(field, strlen(field) - 2);
}
const char *path = NULL;
@@ -686,16 +683,16 @@ static int bus_append_exec_command(sd_bus_message *m, const char *field, const c
if (r < 0)
return bus_log_create_error(r);
r = sd_bus_message_open_container(m, 'v', is_ex_prop ? "a(sasas)" : "a(sasb)");
r = sd_bus_message_open_container(m, 'v', ex_prop ? "a(sasas)" : "a(sasb)");
if (r < 0)
return bus_log_create_error(r);
r = sd_bus_message_open_container(m, 'a', is_ex_prop ? "(sasas)" : "(sasb)");
r = sd_bus_message_open_container(m, 'a', ex_prop ? "(sasas)" : "(sasb)");
if (r < 0)
return bus_log_create_error(r);
if (!strv_isempty(cmdline)) {
r = sd_bus_message_open_container(m, 'r', is_ex_prop ? "sasas" : "sasb");
r = sd_bus_message_open_container(m, 'r', ex_prop ? "sasas" : "sasb");
if (r < 0)
return bus_log_create_error(r);
@@ -707,8 +704,8 @@ static int bus_append_exec_command(sd_bus_message *m, const char *field, const c
if (r < 0)
return bus_log_create_error(r);
r = is_ex_prop ? sd_bus_message_append_strv(m, ex_opts) :
sd_bus_message_append(m, "b", FLAGS_SET(flags, EXEC_COMMAND_IGNORE_FAILURE));
r = ex_prop ? sd_bus_message_append_strv(m, ex_opts) :
sd_bus_message_append(m, "b", FLAGS_SET(flags, EXEC_COMMAND_IGNORE_FAILURE));
if (r < 0)
return bus_log_create_error(r);
@@ -2584,19 +2581,19 @@ static const BusProperty service_properties[] = {
{ "FileDescriptorStoreMax", bus_append_safe_atou },
{ "RestartSteps", bus_append_safe_atou },
{ "ExecCondition", bus_append_exec_command },
{ "ExecConditionEx", bus_append_exec_command }, /* compat */
{ "ExecStartPre", bus_append_exec_command },
{ "ExecStartPreEx", bus_append_exec_command }, /* compat */
{ "ExecStart", bus_append_exec_command },
{ "ExecStartEx", bus_append_exec_command }, /* compat */
{ "ExecStartPost", bus_append_exec_command },
{ "ExecConditionEx", bus_append_exec_command },
{ "ExecStartPreEx", bus_append_exec_command },
{ "ExecStartEx", bus_append_exec_command },
{ "ExecStartPostEx", bus_append_exec_command },
{ "ExecStartPostEx", bus_append_exec_command }, /* compat */
{ "ExecReload", bus_append_exec_command },
{ "ExecReloadEx", bus_append_exec_command }, /* compat */
{ "ExecStop", bus_append_exec_command },
{ "ExecStopEx", bus_append_exec_command }, /* compat */
{ "ExecStopPost", bus_append_exec_command },
{ "ExecReloadEx", bus_append_exec_command },
{ "ExecStopEx", bus_append_exec_command },
{ "ExecStopPostEx", bus_append_exec_command },
{ "ExecStopPostEx", bus_append_exec_command }, /* compat */
{ "RestartPreventExitStatus", bus_append_exit_status },
{ "RestartForceExitStatus", bus_append_exit_status },
{ "SuccessExitStatus", bus_append_exit_status },

View File

@@ -20,25 +20,16 @@ property[7_seven]=ExecStopPost
# These should all get upgraded to the corresponding Ex property as the non-Ex variant
# does not support the ":" prefix (no-env-expand).
for c in "${!property[@]}"; do
systemd-run --unit="$c" -r -p "Type=oneshot" -p "${property[$c]}=:/bin/echo \${$c}" /bin/true
systemd-run --unit="$c" -r -p "Type=oneshot" -p "${property[$c]}=:/bin/echo \${$c}" true
systemctl show -p "${property[$c]}" "$c" | grep -F "path=/bin/echo ; argv[]=/bin/echo \${$c} ; ignore_errors=no"
systemctl show -p "${property[$c]}Ex" "$c" | grep -F "path=/bin/echo ; argv[]=/bin/echo \${$c} ; flags=no-env-expand"
done
declare -A property_ex
property_ex[1_one_ex]=ExecConditionEx
property_ex[2_two_ex]=ExecStartPreEx
property_ex[3_three_ex]=ExecStartEx
property_ex[4_four_ex]=ExecStartPostEx
property_ex[5_five_ex]=ExecReloadEx
property_ex[6_six_ex]=ExecStopEx
property_ex[7_seven_ex]=ExecStopPostEx
for c in "${!property_ex[@]}"; do
systemd-run --unit="$c" -r -p "Type=oneshot" -p "${property_ex[$c]}=:/bin/echo \${$c}" /bin/true
systemctl show -p "${property_ex[$c]%??}" "$c" | grep -F "path=/bin/echo ; argv[]=/bin/echo \${$c} ; ignore_errors=no"
systemctl show -p "${property_ex[$c]}" "$c" | grep -F "path=/bin/echo ; argv[]=/bin/echo \${$c} ; flags=no-env-expand"
# Ex names on the commandline are supported for backward compat.
for c in "${!property[@]}"; do
systemd-run --unit="${c}_ex" -r -p "Type=oneshot" -p "${property[$c]}Ex=:/bin/echo \${$c}" true
systemctl show -p "${property[$c]}" "$c" | grep -F "path=/bin/echo ; argv[]=/bin/echo \${$c} ; ignore_errors=no"
systemctl show -p "${property[$c]}Ex" "$c" | grep -F "path=/bin/echo ; argv[]=/bin/echo \${$c} ; flags=no-env-expand"
done
systemd-analyze log-level info