From 296027122b0a2946bd6936a8128bb3cec5d1eda2 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 27 May 2024 22:31:09 +0200 Subject: [PATCH 1/3] varlink: add helper that validates a qualified Varlink symbol name Qualified Varlink symbol names are the combination of an interface name, followed by a dot, followed by a symbol name. It's a primary concept, after all it's what we send over the wire for method calls and get back for error returns. hence, let's add an explicit validator for it. --- src/shared/varlink-idl.c | 22 ++++++++++++++++++++++ src/shared/varlink-idl.h | 2 ++ src/test/test-varlink-idl.c | 11 +++++++++++ 3 files changed, 35 insertions(+) diff --git a/src/shared/varlink-idl.c b/src/shared/varlink-idl.c index 57765b18c29..f81c38c53dd 100644 --- a/src/shared/varlink-idl.c +++ b/src/shared/varlink-idl.c @@ -1452,6 +1452,28 @@ static bool varlink_idl_comment_is_valid(const char *comment) { return utf8_is_valid(comment); } +int varlink_idl_qualified_symbol_name_is_valid(const char *name) { + const char *dot; + + /* Validates a qualified symbol name (i.e. interface name, followed by a dot, followed by a symbol name) */ + + if (!name) + return false; + + dot = strrchr(name, '.'); + if (!dot) + return false; + + if (!varlink_idl_symbol_name_is_valid(dot + 1)) + return false; + + _cleanup_free_ char *iface = strndup(name, dot - name); + if (!iface) + return -ENOMEM; + + return varlink_idl_interface_name_is_valid(iface); +} + static int varlink_idl_symbol_consistent(const VarlinkInterface *interface, const VarlinkSymbol *symbol, int level); static int varlink_idl_field_consistent( diff --git a/src/shared/varlink-idl.h b/src/shared/varlink-idl.h index 03a1a2b2e48..ffb55f30663 100644 --- a/src/shared/varlink-idl.h +++ b/src/shared/varlink-idl.h @@ -170,6 +170,8 @@ bool varlink_idl_field_name_is_valid(const char *name); bool varlink_idl_symbol_name_is_valid(const char *name); bool varlink_idl_interface_name_is_valid(const char *name); +int varlink_idl_qualified_symbol_name_is_valid(const char *name); + int varlink_idl_consistent(const VarlinkInterface *interface, int level); const VarlinkSymbol* varlink_idl_find_symbol(const VarlinkInterface *interface, VarlinkSymbolType type, const char *name); diff --git a/src/test/test-varlink-idl.c b/src/test/test-varlink-idl.c index f14622bd581..c2be1dfec04 100644 --- a/src/test/test-varlink-idl.c +++ b/src/test/test-varlink-idl.c @@ -261,6 +261,17 @@ TEST(field_name_is_valid) { assert_se(varlink_idl_field_name_is_valid("foo0foo")); } +TEST(qualified_symbol_name_is_valid) { + assert_se(varlink_idl_qualified_symbol_name_is_valid(NULL) == 0); + assert_se(varlink_idl_qualified_symbol_name_is_valid("") == 0); + assert_se(varlink_idl_qualified_symbol_name_is_valid("x") == 0); + assert_se(varlink_idl_qualified_symbol_name_is_valid("xxx") == 0); + assert_se(varlink_idl_qualified_symbol_name_is_valid("xxx.xxx") == 0); + assert_se(varlink_idl_qualified_symbol_name_is_valid("xxx.Xxx") > 0); + assert_se(varlink_idl_qualified_symbol_name_is_valid("xxx.xxx.XXX") > 0); + assert_se(varlink_idl_qualified_symbol_name_is_valid("xxx.xxx.0foo") == 0); +} + TEST(validate_json) { _cleanup_(varlink_interface_freep) VarlinkInterface *parsed = NULL; From da213bb5c0d766ef612fe74537792b92c257c01c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 27 May 2024 22:32:51 +0200 Subject: [PATCH 2/3] varlinkctl: add --graceful= option for optionally marking some errors as successes This is generally useful, but in some cases particularly: when implementing enumeration calls that use the "more" flag to return multiple replies then for the first reply we need to return an error in case the list of objects to enumerate is empty, usually so form of "NoSuchXYZ" error. In many cases this shouldn't really be treated as error, as an empty list probably more than not is as valid as a list with one, two or more entries. --- man/varlinkctl.xml | 14 ++++++++++++ src/varlinkctl/varlinkctl.c | 44 ++++++++++++++++++++++++++++++++++--- 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/man/varlinkctl.xml b/man/varlinkctl.xml index f21e513cb0f..2e8207cbcb3 100644 --- a/man/varlinkctl.xml +++ b/man/varlinkctl.xml @@ -250,6 +250,20 @@ + + + + + Takes a qualified Varlink error name (i.e. an interface name, suffixed by an error name, + separated by a dot; e.g. org.varlink.service.InvalidParameter). Ensures that if + a method call fails with the specified error this will be treated as success, i.e. will cause the + varlinkctl invocation to exit with a zero exit status. This option may be used more + than once in order to treat multiple different errors as successes. + + + + + diff --git a/src/varlinkctl/varlinkctl.c b/src/varlinkctl/varlinkctl.c index 02caf218e79..f4e5cc2cbff 100644 --- a/src/varlinkctl/varlinkctl.c +++ b/src/varlinkctl/varlinkctl.c @@ -22,6 +22,9 @@ static PagerFlags arg_pager_flags = 0; static VarlinkMethodFlags arg_method_flags = 0; static bool arg_collect = false; static bool arg_quiet = false; +static char **arg_graceful = NULL; + +STATIC_DESTRUCTOR_REGISTER(arg_graceful, strv_freep); static int help(void) { _cleanup_free_ char *link = NULL; @@ -58,6 +61,7 @@ static int help(void) { " --json=MODE Output as JSON\n" " -j Same as --json=pretty on tty, --json=short otherwise\n" " -q --quiet Do not output method reply\n" + " --graceful=ERROR Treat specified Varlink error as success\n" "\nSee the %2$s for details.\n", program_invocation_short_name, link, @@ -82,6 +86,7 @@ static int parse_argv(int argc, char *argv[]) { ARG_ONEWAY, ARG_JSON, ARG_COLLECT, + ARG_GRACEFUL, }; static const struct option options[] = { @@ -93,6 +98,7 @@ static int parse_argv(int argc, char *argv[]) { { "json", required_argument, NULL, ARG_JSON }, { "collect", no_argument, NULL, ARG_COLLECT }, { "quiet", no_argument, NULL, 'q' }, + { "graceful", required_argument, NULL, ARG_GRACEFUL }, {}, }; @@ -142,6 +148,18 @@ static int parse_argv(int argc, char *argv[]) { arg_quiet = true; break; + case ARG_GRACEFUL: + r = varlink_idl_qualified_symbol_name_is_valid(optarg); + if (r < 0) + return log_error_errno(r, "Failed to validate Varlink error name '%s': %m", optarg); + if (r == 0) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Not a valid Varlink error name: %s", optarg); + + if (strv_extend(&arg_graceful, optarg) < 0) + return log_oom(); + + break; + case '?': return -EINVAL; @@ -153,6 +171,8 @@ static int parse_argv(int argc, char *argv[]) { if (FLAGS_SET(arg_method_flags, VARLINK_METHOD_MORE)) arg_json_format_flags |= SD_JSON_FORMAT_SEQ; + strv_sort_uniq(arg_graceful); + return 1; } @@ -438,7 +458,13 @@ static int reply_callback( /* Propagate the error we received via sd_notify() */ (void) sd_notifyf(/* unset_environment= */ false, "VARLINKERROR=%s", error); - r = *ret = log_error_errno(SYNTHETIC_ERRNO(EBADE), "Method call failed: %s", error); + if (strv_contains(arg_graceful, error)) { + log_full(arg_quiet ? LOG_DEBUG : LOG_INFO, + "Method call returned expected error: %s", error); + + r = 0; + } else + r = *ret = log_error_errno(SYNTHETIC_ERRNO(EBADE), "Method call failed: %s", error); } else r = 0; @@ -505,7 +531,13 @@ static int verb_call(int argc, char *argv[], void *userdata) { /* Propagate the error we received via sd_notify() */ (void) sd_notifyf(/* unset_environment= */ false, "VARLINKERROR=%s", error); - r = log_error_errno(SYNTHETIC_ERRNO(EBADE), "Method call %s() failed: %s", method, error); + if (strv_contains(arg_graceful, error)) { + log_full(arg_quiet ? LOG_DEBUG : LOG_INFO, + "Method call %s() returned expected error: %s", method, error); + + r = 0; + } else + r = log_error_errno(SYNTHETIC_ERRNO(EBADE), "Method call %s() failed: %s", method, error); } else r = 0; @@ -570,7 +602,13 @@ static int verb_call(int argc, char *argv[], void *userdata) { /* Propagate the error we received via sd_notify() */ (void) sd_notifyf(/* unset_environment= */ false, "VARLINKERROR=%s", error); - r = log_error_errno(SYNTHETIC_ERRNO(EBADE), "Method call %s() failed: %s", method, error); + if (strv_contains(arg_graceful, error)) { + log_full(arg_quiet ? LOG_DEBUG : LOG_INFO, + "Method call %s() returned expected error: %s", method, error); + + r = 0; + } else + r = log_error_errno(SYNTHETIC_ERRNO(EBADE), "Method call %s() failed: %s", method, error); } else r = 0; From ea441dbd9d90299550c6f8899555b3cc6ce0bf67 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 20 Jun 2024 12:23:53 +0200 Subject: [PATCH 3/3] ci: test new --graceful= switch of varlinkctl --- test/units/TEST-74-AUX-UTILS.bootctl.sh | 6 +++--- test/units/TEST-74-AUX-UTILS.varlinkctl.sh | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/test/units/TEST-74-AUX-UTILS.bootctl.sh b/test/units/TEST-74-AUX-UTILS.bootctl.sh index 5331f5f8b40..1767f1fc897 100755 --- a/test/units/TEST-74-AUX-UTILS.bootctl.sh +++ b/test/units/TEST-74-AUX-UTILS.bootctl.sh @@ -264,7 +264,7 @@ EOF } testcase_bootctl_varlink() { - (varlinkctl call --collect /run/systemd/io.systemd.BootControl io.systemd.BootControl.ListBootEntries '{}' ||:) + varlinkctl call --collect /run/systemd/io.systemd.BootControl io.systemd.BootControl.ListBootEntries '{}' --graceful=io.systemd.BootControl.NoSuchBootEntry # We may have UEFI in the test environment. # If we don't have UEFI then we can test whether bootctl's varlink API fails cleanly. @@ -272,8 +272,8 @@ testcase_bootctl_varlink() { if ! (SYSTEMD_LOG_TARGET=console varlinkctl call --json=short /run/systemd/io.systemd.BootControl io.systemd.BootControl.GetRebootToFirmware '{}' || true) |& grep -q io.systemd.BootControl.RebootToFirmwareNotSupported; then return 0 fi - (SYSTEMD_LOG_TARGET=console varlinkctl call --json=short /run/systemd/io.systemd.BootControl io.systemd.BootControl.SetRebootToFirmware '{"state":true}' || true) |& grep -q io.systemd.BootControl.RebootToFirmwareNotSupported - (SYSTEMD_LOG_TARGET=console varlinkctl call --json=short /run/systemd/io.systemd.BootControl io.systemd.BootControl.SetRebootToFirmware '{"state":false}' || true) |& grep -q io.systemd.BootControl.RebootToFirmwareNotSupported + SYSTEMD_LOG_TARGET=console varlinkctl call --json=short /run/systemd/io.systemd.BootControl io.systemd.BootControl.SetRebootToFirmware '{"state":true}' --graceful=io.systemd.BootControl.RebootToFirmwareNotSupported + SYSTEMD_LOG_TARGET=console varlinkctl call --json=short /run/systemd/io.systemd.BootControl io.systemd.BootControl.SetRebootToFirmware '{"state":false}' --graceful=io.systemd.BootControl.RebootToFirmwareNotSupported } run_testcases diff --git a/test/units/TEST-74-AUX-UTILS.varlinkctl.sh b/test/units/TEST-74-AUX-UTILS.varlinkctl.sh index 030f077c859..5324b9501a1 100755 --- a/test/units/TEST-74-AUX-UTILS.varlinkctl.sh +++ b/test/units/TEST-74-AUX-UTILS.varlinkctl.sh @@ -43,9 +43,9 @@ if command -v userdbctl >/dev/null; then varlinkctl call -q /run/systemd/userdb/io.systemd.Multiplexer io.systemd.UserDatabase.GetUserRecord '{ "userName" : "testuser", "service" : "io.systemd.Multiplexer" }' varlinkctl call -j /run/systemd/userdb/io.systemd.Multiplexer io.systemd.UserDatabase.GetUserRecord '{ "userName" : "testuser", "service" : "io.systemd.Multiplexer" }' | jq . # We ignore the return value of the following two calls, since if no memberships are defined at all this will return a NotFound error, which is OK - (varlinkctl call --more /run/systemd/userdb/io.systemd.Multiplexer io.systemd.UserDatabase.GetMemberships '{ "service" : "io.systemd.Multiplexer" }' ||:) - (varlinkctl call --quiet --more /run/systemd/userdb/io.systemd.Multiplexer io.systemd.UserDatabase.GetMemberships '{ "service" : "io.systemd.Multiplexer" }' ||:) - (varlinkctl call --more -j /run/systemd/userdb/io.systemd.Multiplexer io.systemd.UserDatabase.GetMemberships '{ "service" : "io.systemd.Multiplexer" }' ||:) | jq --seq . + varlinkctl call --more /run/systemd/userdb/io.systemd.Multiplexer io.systemd.UserDatabase.GetMemberships '{ "service" : "io.systemd.Multiplexer" }' --graceful=io.systemd.UserDatabase.NoRecordFound + varlinkctl call --quiet --more /run/systemd/userdb/io.systemd.Multiplexer io.systemd.UserDatabase.GetMemberships '{ "service" : "io.systemd.Multiplexer" }' --graceful=io.systemd.UserDatabase.NoRecordFound + varlinkctl call --more -j /run/systemd/userdb/io.systemd.Multiplexer io.systemd.UserDatabase.GetMemberships '{ "service" : "io.systemd.Multiplexer" }' --graceful=io.systemd.UserDatabase.NoRecordFound | jq --seq . varlinkctl call --oneway /run/systemd/userdb/io.systemd.Multiplexer io.systemd.UserDatabase.GetMemberships '{ "service" : "io.systemd.Multiplexer" }' (! varlinkctl call --oneway /run/systemd/userdb/io.systemd.Multiplexer io.systemd.UserDatabase.GetMemberships '{ "service" : "io.systemd.Multiplexer" }' | grep .) fi