diff --git a/src/libsystemd/sd-varlink/sd-varlink-idl.c b/src/libsystemd/sd-varlink/sd-varlink-idl.c index 76a2499561..58edcf02f7 100644 --- a/src/libsystemd/sd-varlink/sd-varlink-idl.c +++ b/src/libsystemd/sd-varlink/sd-varlink-idl.c @@ -374,6 +374,20 @@ static int varlink_idl_format_symbol( break; case SD_VARLINK_METHOD: + + /* Sooner or later we want to export this in a proper IDL language construct, see + * https://github.com/varlink/varlink.github.io/issues/26 – but for now export this as a + * comment. */ + if ((symbol->symbol_flags & (SD_VARLINK_REQUIRES_MORE|SD_VARLINK_SUPPORTS_MORE)) != 0) { + fputs(colors[COLOR_COMMENT], f); + if (FLAGS_SET(symbol->symbol_flags, SD_VARLINK_REQUIRES_MORE)) + fputs("# [Requires 'more' flag]", f); + else + fputs("# [Supports 'more' flag]", f); + fputs(colors[COLOR_RESET], f); + fputs("\n", f); + } + fputs(colors[COLOR_SYMBOL_TYPE], f); fputs("method ", f); fputs(colors[COLOR_IDENTIFIER], f); @@ -1761,15 +1775,15 @@ static int varlink_idl_validate_field(const sd_varlink_field *field, sd_json_var return 0; } -static int varlink_idl_validate_symbol(const sd_varlink_symbol *symbol, sd_json_variant *v, sd_varlink_field_direction_t direction, const char **bad_field) { +static int varlink_idl_validate_symbol(const sd_varlink_symbol *symbol, sd_json_variant *v, sd_varlink_field_direction_t direction, const char **reterr_bad_field) { int r; assert(symbol); assert(!IN_SET(symbol->symbol_type, _SD_VARLINK_SYMBOL_COMMENT, _SD_VARLINK_INTERFACE_COMMENT)); if (!v) { - if (bad_field) - *bad_field = NULL; + if (reterr_bad_field) + *reterr_bad_field = NULL; return varlink_idl_log(SYNTHETIC_ERRNO(EMEDIUMTYPE), "Null object passed, refusing."); } @@ -1780,8 +1794,8 @@ static int varlink_idl_validate_symbol(const sd_varlink_symbol *symbol, sd_json_ const char *s; if (!sd_json_variant_is_string(v)) { - if (bad_field) - *bad_field = symbol->name; + if (reterr_bad_field) + *reterr_bad_field = symbol->name; return varlink_idl_log(SYNTHETIC_ERRNO(EMEDIUMTYPE), "Passed non-string to enum field '%s', refusing.", strna(symbol->name)); } @@ -1801,8 +1815,8 @@ static int varlink_idl_validate_symbol(const sd_varlink_symbol *symbol, sd_json_ } if (!found) { - if (bad_field) - *bad_field = s; + if (reterr_bad_field) + *reterr_bad_field = s; return varlink_idl_log(SYNTHETIC_ERRNO(EMEDIUMTYPE), "Passed unrecognized string '%s' to enum field '%s', refusing.", s, strna(symbol->name)); } @@ -1813,8 +1827,8 @@ static int varlink_idl_validate_symbol(const sd_varlink_symbol *symbol, sd_json_ case SD_VARLINK_METHOD: case SD_VARLINK_ERROR: { if (!sd_json_variant_is_object(v)) { - if (bad_field) - *bad_field = symbol->name; + if (reterr_bad_field) + *reterr_bad_field = symbol->name; return varlink_idl_log(SYNTHETIC_ERRNO(EMEDIUMTYPE), "Passed non-object to field '%s', refusing.", strna(symbol->name)); } @@ -1828,8 +1842,8 @@ static int varlink_idl_validate_symbol(const sd_varlink_symbol *symbol, sd_json_ r = varlink_idl_validate_field(field, sd_json_variant_by_key(v, field->name)); if (r < 0) { - if (bad_field) - *bad_field = field->name; + if (reterr_bad_field) + *reterr_bad_field = field->name; return r; } } @@ -1838,8 +1852,8 @@ static int varlink_idl_validate_symbol(const sd_varlink_symbol *symbol, sd_json_ const char *name; JSON_VARIANT_OBJECT_FOREACH(name, e, v) { if (!varlink_idl_find_field(symbol, name)) { - if (bad_field) - *bad_field = name; + if (reterr_bad_field) + *reterr_bad_field = name; return varlink_idl_log(SYNTHETIC_ERRNO(EBUSY), "Field '%s' not defined for object, refusing.", name); } } @@ -1858,32 +1872,40 @@ static int varlink_idl_validate_symbol(const sd_varlink_symbol *symbol, sd_json_ return 1; /* validated */ } -static int varlink_idl_validate_method(const sd_varlink_symbol *method, sd_json_variant *v, sd_varlink_field_direction_t direction, const char **bad_field) { - assert(IN_SET(direction, SD_VARLINK_INPUT, SD_VARLINK_OUTPUT)); +int varlink_idl_validate_method_call(const sd_varlink_symbol *method, sd_json_variant *v, sd_varlink_method_flags_t flags, const char **reterr_bad_field) { if (!method) return 0; /* Can't validate */ if (method->symbol_type != SD_VARLINK_METHOD) return -EBADMSG; - return varlink_idl_validate_symbol(method, v, direction, bad_field); + /* If method calls require the "more" flag, but none is given, return a recognizable error */ + if (FLAGS_SET(method->symbol_flags, SD_VARLINK_REQUIRES_MORE) && !FLAGS_SET(flags, SD_VARLINK_METHOD_MORE)) + return -EBADE; + + return varlink_idl_validate_symbol(method, v, SD_VARLINK_INPUT, reterr_bad_field); } -int varlink_idl_validate_method_call(const sd_varlink_symbol *method, sd_json_variant *v, const char **bad_field) { - return varlink_idl_validate_method(method, v, SD_VARLINK_INPUT, bad_field); +int varlink_idl_validate_method_reply(const sd_varlink_symbol *method, sd_json_variant *v, sd_varlink_reply_flags_t flags, const char **reterr_bad_field) { + if (!method) + return 0; /* Can't validate */ + if (method->symbol_type != SD_VARLINK_METHOD) + return -EBADMSG; + + /* If method replies have the "continues" flag set, but the method is not allowed to generate that, return a recognizable error */ + if (FLAGS_SET(flags, SD_VARLINK_REPLY_CONTINUES) && (method->symbol_type & (SD_VARLINK_SUPPORTS_MORE|SD_VARLINK_REQUIRES_MORE)) == 0) + return -EBADE; + + return varlink_idl_validate_symbol(method, v, SD_VARLINK_OUTPUT, reterr_bad_field); } -int varlink_idl_validate_method_reply(const sd_varlink_symbol *method, sd_json_variant *v, const char **bad_field) { - return varlink_idl_validate_method(method, v, SD_VARLINK_OUTPUT, bad_field); -} - -int varlink_idl_validate_error(const sd_varlink_symbol *error, sd_json_variant *v, const char **bad_field) { +int varlink_idl_validate_error(const sd_varlink_symbol *error, sd_json_variant *v, const char **reterr_bad_field) { if (!error) return 0; /* Can't validate */ if (error->symbol_type != SD_VARLINK_ERROR) return -EBADMSG; - return varlink_idl_validate_symbol(error, v, SD_VARLINK_REGULAR, bad_field); + return varlink_idl_validate_symbol(error, v, SD_VARLINK_REGULAR, reterr_bad_field); } const sd_varlink_symbol* varlink_idl_find_symbol( diff --git a/src/libsystemd/sd-varlink/sd-varlink.c b/src/libsystemd/sd-varlink/sd-varlink.c index 007746f64f..70615bb3e3 100644 --- a/src/libsystemd/sd-varlink/sd-varlink.c +++ b/src/libsystemd/sd-varlink/sd-varlink.c @@ -1354,8 +1354,17 @@ static int varlink_dispatch_method(sd_varlink *v) { else { const char *bad_field; - r = varlink_idl_validate_method_call(v->current_method, parameters, &bad_field); - if (r < 0) { + r = varlink_idl_validate_method_call(v->current_method, parameters, flags, &bad_field); + if (r == -EBADE) { + varlink_log_errno(v, r, "Method %s() called without 'more' flag, but flag needs to be set: %m", + method); + + if (v->state == VARLINK_PROCESSING_METHOD) { + r = sd_varlink_error(v, SD_VARLINK_ERROR_EXPECTED_MORE, NULL); + if (r < 0) + return r; + } + } else if (r < 0) { /* Please adjust test/units/end.sh when updating the log message. */ varlink_log_errno(v, r, "Parameters for method %s() didn't pass validation on field '%s': %m", method, strna(bad_field)); @@ -1365,8 +1374,9 @@ static int varlink_dispatch_method(sd_varlink *v) { if (r < 0) return r; } - invalid = true; } + + invalid = r < 0; } if (!invalid) { @@ -2439,7 +2449,7 @@ _public_ int sd_varlink_reply(sd_varlink *v, sd_json_variant *parameters) { if (v->current_method) { const char *bad_field = NULL; - r = varlink_idl_validate_method_reply(v->current_method, parameters, &bad_field); + r = varlink_idl_validate_method_reply(v->current_method, parameters, /* flags= */ 0, &bad_field); if (r < 0) /* Please adjust test/units/end.sh when updating the log message. */ varlink_log_errno(v, r, "Return parameters for method reply %s() didn't pass validation on field '%s', ignoring: %m", @@ -2652,8 +2662,11 @@ _public_ int sd_varlink_notify(sd_varlink *v, sd_json_variant *parameters) { if (v->current_method) { const char *bad_field = NULL; - r = varlink_idl_validate_method_reply(v->current_method, parameters, &bad_field); - if (r < 0) + r = varlink_idl_validate_method_reply(v->current_method, parameters, SD_VARLINK_REPLY_CONTINUES, &bad_field); + if (r == -EBADE) + varlink_log_errno(v, r, "Method reply for %s() has 'continues' flag set, but IDL structure doesn't allow that, ignoring: %m", + v->current_method->name); + else if (r < 0) /* Please adjust test/units/end.sh when updating the log message. */ varlink_log_errno(v, r, "Return parameters for method reply %s() didn't pass validation on field '%s', ignoring: %m", v->current_method->name, strna(bad_field)); diff --git a/src/libsystemd/sd-varlink/varlink-idl-util.h b/src/libsystemd/sd-varlink/varlink-idl-util.h index fab836a723..8c9274eefa 100644 --- a/src/libsystemd/sd-varlink/varlink-idl-util.h +++ b/src/libsystemd/sd-varlink/varlink-idl-util.h @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #pragma once +#include "sd-varlink.h" #include "sd-varlink-idl.h" #include "macro.h" @@ -20,6 +21,6 @@ int varlink_idl_consistent(const sd_varlink_interface *interface, int level); const sd_varlink_symbol* varlink_idl_find_symbol(const sd_varlink_interface *interface, sd_varlink_symbol_type_t type, const char *name); const sd_varlink_field* varlink_idl_find_field(const sd_varlink_symbol *symbol, const char *name); -int varlink_idl_validate_method_call(const sd_varlink_symbol *method, sd_json_variant *v, const char **bad_field); -int varlink_idl_validate_method_reply(const sd_varlink_symbol *method, sd_json_variant *v, const char **bad_field); +int varlink_idl_validate_method_call(const sd_varlink_symbol *method, sd_json_variant *v, sd_varlink_method_flags_t flags, const char **reterr_bad_field); +int varlink_idl_validate_method_reply(const sd_varlink_symbol *method, sd_json_variant *v, sd_varlink_reply_flags_t flags, const char **reterr_bad_field); int varlink_idl_validate_error(const sd_varlink_symbol *error, sd_json_variant *v, const char **bad_field); diff --git a/src/shared/varlink-io.systemd.BootControl.c b/src/shared/varlink-io.systemd.BootControl.c index 449cc98bb4..364fa8470e 100644 --- a/src/shared/varlink-io.systemd.BootControl.c +++ b/src/shared/varlink-io.systemd.BootControl.c @@ -43,8 +43,9 @@ static SD_VARLINK_DEFINE_STRUCT_TYPE( SD_VARLINK_FIELD_COMMENT("Indicates whether this entry has been booted."), SD_VARLINK_DEFINE_FIELD(isSelected, SD_VARLINK_BOOL, SD_VARLINK_NULLABLE)); -static SD_VARLINK_DEFINE_METHOD( +static SD_VARLINK_DEFINE_METHOD_FULL( ListBootEntries, + SD_VARLINK_REQUIRES_MORE, SD_VARLINK_FIELD_COMMENT("A boot menu entry structure"), SD_VARLINK_DEFINE_OUTPUT_BY_TYPE(entry, BootEntry, SD_VARLINK_NULLABLE)); diff --git a/src/shared/varlink-io.systemd.Import.c b/src/shared/varlink-io.systemd.Import.c index 03af5a98c3..02eb70d91d 100644 --- a/src/shared/varlink-io.systemd.Import.c +++ b/src/shared/varlink-io.systemd.Import.c @@ -53,8 +53,9 @@ static SD_VARLINK_DEFINE_STRUCT_TYPE( SD_VARLINK_FIELD_COMMENT("The priority of the log message, using the BSD syslog priority levels"), SD_VARLINK_DEFINE_FIELD(priority, SD_VARLINK_INT, 0)); -static SD_VARLINK_DEFINE_METHOD( +static SD_VARLINK_DEFINE_METHOD_FULL( ListTransfers, + SD_VARLINK_REQUIRES_MORE, SD_VARLINK_FIELD_COMMENT("Image class to filter by"), SD_VARLINK_DEFINE_INPUT_BY_TYPE(class, ImageClass, SD_VARLINK_NULLABLE), SD_VARLINK_FIELD_COMMENT("A unique numeric identifier for the ongoing transfer"), @@ -70,8 +71,9 @@ static SD_VARLINK_DEFINE_METHOD( SD_VARLINK_FIELD_COMMENT("Progress in percent"), SD_VARLINK_DEFINE_OUTPUT(percent, SD_VARLINK_FLOAT, 0)); -static SD_VARLINK_DEFINE_METHOD( +static SD_VARLINK_DEFINE_METHOD_FULL( Pull, + SD_VARLINK_SUPPORTS_MORE, SD_VARLINK_FIELD_COMMENT("The remote URL to download from"), SD_VARLINK_DEFINE_INPUT(remote, SD_VARLINK_STRING, 0), SD_VARLINK_FIELD_COMMENT("The local image name to download to"), diff --git a/src/shared/varlink-io.systemd.Machine.c b/src/shared/varlink-io.systemd.Machine.c index 1c8f39e3c7..5841bf02ce 100644 --- a/src/shared/varlink-io.systemd.Machine.c +++ b/src/shared/varlink-io.systemd.Machine.c @@ -28,8 +28,9 @@ static SD_VARLINK_DEFINE_STRUCT_TYPE( SD_VARLINK_FIELD_COMMENT("Timestamp in µs in the CLOCK_MONOTONIC clock"), SD_VARLINK_DEFINE_FIELD(monotonic, SD_VARLINK_INT, SD_VARLINK_NULLABLE)); -static SD_VARLINK_DEFINE_METHOD( +static SD_VARLINK_DEFINE_METHOD_FULL( List, + SD_VARLINK_SUPPORTS_MORE, SD_VARLINK_FIELD_COMMENT("If non-null the name of a running machine to report details on. If null/unspecified enumerates all running machines."), SD_VARLINK_DEFINE_INPUT(name, SD_VARLINK_STRING, SD_VARLINK_NULLABLE), SD_VARLINK_FIELD_COMMENT("Name of the machine"), diff --git a/src/shared/varlink-io.systemd.ManagedOOM.c b/src/shared/varlink-io.systemd.ManagedOOM.c index 8fc329b707..763b0abfbd 100644 --- a/src/shared/varlink-io.systemd.ManagedOOM.c +++ b/src/shared/varlink-io.systemd.ManagedOOM.c @@ -9,8 +9,9 @@ * * Compare with io.systemd.oom where the client/server roles of oomd and the service manager are swapped! */ -static SD_VARLINK_DEFINE_METHOD( +static SD_VARLINK_DEFINE_METHOD_FULL( SubscribeManagedOOMCGroups, + SD_VARLINK_SUPPORTS_MORE, SD_VARLINK_DEFINE_OUTPUT_BY_TYPE(cgroups, ControlGroup, SD_VARLINK_ARRAY)); static SD_VARLINK_DEFINE_ERROR(SubscriptionTaken); diff --git a/src/shared/varlink-io.systemd.PCRLock.c b/src/shared/varlink-io.systemd.PCRLock.c index f385eb31c3..15e37cd284 100644 --- a/src/shared/varlink-io.systemd.PCRLock.c +++ b/src/shared/varlink-io.systemd.PCRLock.c @@ -2,8 +2,9 @@ #include "varlink-io.systemd.PCRLock.h" -static SD_VARLINK_DEFINE_METHOD( +static SD_VARLINK_DEFINE_METHOD_FULL( ReadEventLog, + SD_VARLINK_REQUIRES_MORE, SD_VARLINK_DEFINE_OUTPUT(record, SD_VARLINK_OBJECT, 0)); static SD_VARLINK_DEFINE_METHOD( diff --git a/src/shared/varlink-io.systemd.Resolve.Monitor.c b/src/shared/varlink-io.systemd.Resolve.Monitor.c index 8c9fb51469..f5650b1011 100644 --- a/src/shared/varlink-io.systemd.Resolve.Monitor.c +++ b/src/shared/varlink-io.systemd.Resolve.Monitor.c @@ -17,8 +17,9 @@ static SD_VARLINK_DEFINE_STRUCT_TYPE( SD_VARLINK_DEFINE_FIELD(raw, SD_VARLINK_STRING, 0), SD_VARLINK_DEFINE_FIELD(ifindex, SD_VARLINK_INT, SD_VARLINK_NULLABLE)); -static SD_VARLINK_DEFINE_METHOD( +static SD_VARLINK_DEFINE_METHOD_FULL( SubscribeQueryResults, + SD_VARLINK_REQUIRES_MORE, SD_VARLINK_DEFINE_INPUT(allowInteractiveAuthentication, SD_VARLINK_BOOL, SD_VARLINK_NULLABLE), /* First reply */ SD_VARLINK_DEFINE_OUTPUT(ready, SD_VARLINK_BOOL, SD_VARLINK_NULLABLE), diff --git a/src/shared/varlink-io.systemd.UserDatabase.c b/src/shared/varlink-io.systemd.UserDatabase.c index b714550f7d..3dc72c0257 100644 --- a/src/shared/varlink-io.systemd.UserDatabase.c +++ b/src/shared/varlink-io.systemd.UserDatabase.c @@ -2,24 +2,27 @@ #include "varlink-io.systemd.UserDatabase.h" -static SD_VARLINK_DEFINE_METHOD( +static SD_VARLINK_DEFINE_METHOD_FULL( GetUserRecord, + SD_VARLINK_SUPPORTS_MORE, SD_VARLINK_DEFINE_INPUT(uid, SD_VARLINK_INT, SD_VARLINK_NULLABLE), SD_VARLINK_DEFINE_INPUT(userName, SD_VARLINK_STRING, SD_VARLINK_NULLABLE), SD_VARLINK_DEFINE_INPUT(service, SD_VARLINK_STRING, 0), SD_VARLINK_DEFINE_OUTPUT(record, SD_VARLINK_OBJECT, 0), SD_VARLINK_DEFINE_OUTPUT(incomplete, SD_VARLINK_BOOL, SD_VARLINK_NULLABLE)); -static SD_VARLINK_DEFINE_METHOD( +static SD_VARLINK_DEFINE_METHOD_FULL( GetGroupRecord, + SD_VARLINK_SUPPORTS_MORE, SD_VARLINK_DEFINE_INPUT(gid, SD_VARLINK_INT, SD_VARLINK_NULLABLE), SD_VARLINK_DEFINE_INPUT(groupName, SD_VARLINK_STRING, SD_VARLINK_NULLABLE), SD_VARLINK_DEFINE_INPUT(service, SD_VARLINK_STRING, 0), SD_VARLINK_DEFINE_OUTPUT(record, SD_VARLINK_OBJECT, 0), SD_VARLINK_DEFINE_OUTPUT(incomplete, SD_VARLINK_BOOL, SD_VARLINK_NULLABLE)); -static SD_VARLINK_DEFINE_METHOD( +static SD_VARLINK_DEFINE_METHOD_FULL( GetMemberships, + SD_VARLINK_SUPPORTS_MORE, SD_VARLINK_DEFINE_INPUT(userName, SD_VARLINK_STRING, SD_VARLINK_NULLABLE), SD_VARLINK_DEFINE_INPUT(groupName, SD_VARLINK_STRING, SD_VARLINK_NULLABLE), SD_VARLINK_DEFINE_INPUT(service, SD_VARLINK_STRING, 0), diff --git a/src/shared/varlink-io.systemd.sysext.c b/src/shared/varlink-io.systemd.sysext.c index b20afb2950..90eb8177d1 100644 --- a/src/shared/varlink-io.systemd.sysext.c +++ b/src/shared/varlink-io.systemd.sysext.c @@ -33,8 +33,9 @@ static SD_VARLINK_DEFINE_METHOD( SD_VARLINK_DEFINE_INPUT(noReload, SD_VARLINK_BOOL, SD_VARLINK_NULLABLE), SD_VARLINK_DEFINE_INPUT(noexec, SD_VARLINK_BOOL, SD_VARLINK_NULLABLE)); -static SD_VARLINK_DEFINE_METHOD( +static SD_VARLINK_DEFINE_METHOD_FULL( List, + SD_VARLINK_REQUIRES_MORE, SD_VARLINK_DEFINE_INPUT_BY_TYPE(class, ImageClass, SD_VARLINK_NULLABLE), SD_VARLINK_DEFINE_OUTPUT_BY_TYPE(Class, ImageClass, 0), SD_VARLINK_DEFINE_OUTPUT_BY_TYPE(Type, ImageType, 0), diff --git a/src/systemd/sd-varlink-idl.h b/src/systemd/sd-varlink-idl.h index d2fd458cac..e5e8a57498 100644 --- a/src/systemd/sd-varlink-idl.h +++ b/src/systemd/sd-varlink-idl.h @@ -53,7 +53,9 @@ __extension__ typedef enum _SD_ENUM_TYPE_S64(sd_varlink_symbol_type_t) { } sd_varlink_symbol_type_t; __extension__ typedef enum _SD_ENUM_TYPE_S64(sd_varlink_symbol_flags_t) { - _SD_VARLINK_SYMBOL_FLAGS_MAX = (1 << 0) - 1, + SD_VARLINK_SUPPORTS_MORE = 1 << 0, /* Call supports "more" flag */ + SD_VARLINK_REQUIRES_MORE = 1 << 1, /* Call requires "more" flag */ + _SD_VARLINK_SYMBOL_FLAGS_MAX = (1 << 2) - 1, _SD_VARLINK_SYMBOL_FLAGS_INVALID = -EINVAL, _SD_ENUM_FORCE_S64(SD_VARLINK_SYMBOL_FLAGS) } sd_varlink_symbol_flags_t; @@ -163,6 +165,14 @@ struct sd_varlink_interface { .fields = { __VA_ARGS__ __VA_OPT__(,) {}}, \ } +#define SD_VARLINK_DEFINE_METHOD_FULL(_name, _flags, ...) \ + const sd_varlink_symbol vl_method_ ## _name = { \ + .name = #_name, \ + .symbol_type = SD_VARLINK_METHOD, \ + .symbol_flags = _flags, \ + .fields = { __VA_ARGS__ __VA_OPT__(,) {}}, \ + } + #define SD_VARLINK_DEFINE_ERROR(_name, ...) \ const sd_varlink_symbol vl_error_ ## _name = { \ .name = #_name, \ diff --git a/src/test/test-varlink-idl.c b/src/test/test-varlink-idl.c index 4283ae7c01..af48216865 100644 --- a/src/test/test-varlink-idl.c +++ b/src/test/test-varlink-idl.c @@ -305,7 +305,7 @@ TEST(validate_json) { const sd_varlink_symbol* symbol = ASSERT_PTR(varlink_idl_find_symbol(parsed, SD_VARLINK_METHOD, "Mymethod")); - assert_se(varlink_idl_validate_method_call(symbol, v, NULL) >= 0); + assert_se(varlink_idl_validate_method_call(symbol, v, /* flags= */ 0, /* reterr_bad_field= */ NULL) >= 0); } static int test_recursive_one(unsigned depth) {