sd-varlink: skip IDL validation on synthetic empty terminator
When a streaming method's callback returns 0 with no replies enqueued and sd_varlink_set_sentinel() was passed NULL (POINTER_MAX), the framework emits an empty parameters reply via sd_varlink_reply(v, NULL) as a clean stream terminator. That call validated the empty parameters against the method's output IDL — which always failed when the IDL declared any mandatory output field, generating a spurious 'didn't pass validation' warning even though the empty terminator is correct by construction. Move the body of sd_varlink_reply() into a static varlink_reply_internal() that takes a skip_validation flag. The public sd_varlink_reply() keeps its existing behavior (validation enabled). Add a static varlink_reply_terminator() helper that wraps varlink_reply_internal(v, NULL, /* skip_validation= */ true), and route varlink_dispatch_sentinel() through it for the synthetic empty terminator, so both the synchronous dispatch path (varlink_dispatch_method()) and the fiber path (varlink_fiber_entry()) skip validation in one place. Without this, callers who want 'streaming method, possibly empty result' semantics had to either fire a logically wrong 'NotFound'-style error sentinel on empty (sysext.List, BootControl.ListBootEntries pattern) or mark every output field NULLABLE just to placate the validator (the io.systemd.Manager.EnqueueMarkedJobs approach), misrepresenting the contract — exactly the tradeoff the FIXME in pcrlock.c flagged. With this change, the empty terminator is invisible to the IDL validator and the FIXME is no longer needed. Co-developed-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -73,6 +73,7 @@ static const char* const varlink_state_table[_VARLINK_STATE_MAX] = {
|
||||
DEFINE_PRIVATE_STRING_TABLE_LOOKUP_TO_STRING(varlink_state, VarlinkState);
|
||||
|
||||
static void varlink_server_test_exit_on_idle(sd_varlink_server *s);
|
||||
static int varlink_reply_terminator(sd_varlink *v);
|
||||
|
||||
static void varlink_set_state(sd_varlink *v, VarlinkState state) {
|
||||
assert(v);
|
||||
@@ -989,7 +990,9 @@ static int varlink_dispatch_sentinel(sd_varlink *v) {
|
||||
/* Propagate the sentinel to the client if one was configured and no replies were enqueued by
|
||||
* the callback. */
|
||||
if (sentinel == POINTER_MAX)
|
||||
r = sd_varlink_reply(v, NULL);
|
||||
/* Synthetic empty terminator. Skip IDL validation since the empty parameters wouldn't
|
||||
* satisfy any mandatory output fields the method declares. */
|
||||
r = varlink_reply_terminator(v);
|
||||
else {
|
||||
r = sd_varlink_error(v, sentinel, NULL);
|
||||
/* sd_varlink_error() deliberately returns a negative
|
||||
@@ -2298,10 +2301,10 @@ _public_ int sd_varlink_collectb(
|
||||
return sd_varlink_collect_full(v, method, parameters, ret_parameters, ret_error_id, NULL);
|
||||
}
|
||||
|
||||
_public_ int sd_varlink_reply(sd_varlink *v, sd_json_variant *parameters) {
|
||||
static int varlink_reply_internal(sd_varlink *v, sd_json_variant *parameters, bool skip_validation) {
|
||||
int r;
|
||||
|
||||
assert_return(v, -EINVAL);
|
||||
assert(v);
|
||||
|
||||
if (v->state == VARLINK_DISCONNECTED)
|
||||
return varlink_log_errno(v, SYNTHETIC_ERRNO(ENOTCONN), "Not connected.");
|
||||
@@ -2313,8 +2316,9 @@ _public_ int sd_varlink_reply(sd_varlink *v, sd_json_variant *parameters) {
|
||||
|
||||
bool more = IN_SET(v->state, VARLINK_PROCESSING_METHOD_MORE, VARLINK_PENDING_METHOD_MORE);
|
||||
|
||||
/* Validate parameters BEFORE sanitization */
|
||||
if (v->current_method) {
|
||||
/* Validate parameters BEFORE sanitization. Validation should be skipped for the synthetic
|
||||
* empty terminator. */
|
||||
if (!skip_validation && v->current_method) {
|
||||
const char *bad_field = NULL;
|
||||
|
||||
r = varlink_idl_validate_method_reply(v->current_method, parameters, more && v->sentinel ? SD_VARLINK_REPLY_CONTINUES : 0, &bad_field);
|
||||
@@ -2373,6 +2377,16 @@ _public_ int sd_varlink_reply(sd_varlink *v, sd_json_variant *parameters) {
|
||||
return 1;
|
||||
}
|
||||
|
||||
static int varlink_reply_terminator(sd_varlink *v) {
|
||||
return varlink_reply_internal(v, /* parameters= */ NULL, /* skip_validation= */ true);
|
||||
}
|
||||
|
||||
_public_ int sd_varlink_reply(sd_varlink *v, sd_json_variant *parameters) {
|
||||
assert_return(v, -EINVAL);
|
||||
|
||||
return varlink_reply_internal(v, parameters, /* skip_validation= */ false);
|
||||
}
|
||||
|
||||
_public_ int sd_varlink_replyb(sd_varlink *v, ...) {
|
||||
_cleanup_(sd_json_variant_unrefp) sd_json_variant *parameters = NULL;
|
||||
va_list ap;
|
||||
|
||||
@@ -5406,7 +5406,6 @@ static int vl_method_read_event_log(sd_varlink *link, sd_json_variant *parameter
|
||||
if (r < 0)
|
||||
return r;
|
||||
|
||||
// FIXME: We can't use a NULL sentinel here because the output fields in the IDL are non-nullable.
|
||||
r = sd_varlink_set_sentinel(link, NULL);
|
||||
if (r < 0)
|
||||
return r;
|
||||
|
||||
Reference in New Issue
Block a user