From 6fa09278b8ae30c672451dbcfd05a7d5f072fe71 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 12 Oct 2023 15:21:50 +0200 Subject: [PATCH 1/3] varlink: refuse empty () structs/enums If we encounter an empty struct in the varlink IDL it could also be an empty enum. Refuse this to avoid the ambiguity. The spec doesn't cover this case clearly, hence let's better be on the safe side and refuse it rather than making a decision what it means. --- src/shared/varlink-idl.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/shared/varlink-idl.c b/src/shared/varlink-idl.c index 73f336c522..98d97892ec 100644 --- a/src/shared/varlink-idl.c +++ b/src/shared/varlink-idl.c @@ -828,6 +828,11 @@ static int varlink_idl_subparse_struct_or_enum( } } + /* If we don't know the type of the symbol by now it was an empty () which doesn't allow us to + * determine if we look at an enum or a struct */ + if ((*symbol)->symbol_type < 0) + return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Ambiguous empty () enum/struct is not permitted.", *line, *column); + return 0; } From efe511e910368e04831bd62ada3975c7db8ded6e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 12 Oct 2023 15:23:42 +0200 Subject: [PATCH 2/3] varlink: don't generate %m error message if we are synthesizing the error We are outputting a more useful log message anyway, the "Bad message" error string is just confusing. --- src/shared/varlink-idl.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/shared/varlink-idl.c b/src/shared/varlink-idl.c index 98d97892ec..7d9d7874c1 100644 --- a/src/shared/varlink-idl.c +++ b/src/shared/varlink-idl.c @@ -658,7 +658,7 @@ static int varlink_idl_subparse_field_type( if (r < 0) return r; if (!token) - return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Premature EOF: %m", *line, *column); + return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Premature EOF.", *line, *column); field->named_type = TAKE_PTR(token); field->field_type = VARLINK_NAMED_TYPE; @@ -715,9 +715,9 @@ static int varlink_idl_subparse_struct_or_enum( case STATE_OPEN: if (!token) - return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Premature EOF: %m", *line, *column); + return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Premature EOF.", *line, *column); if (!streq(token, "(")) - return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Unexpected token '%s': %m", *line, *column, token); + return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Unexpected token '%s'.", *line, *column, token); state = STATE_NAME; allowed_delimiters = ")"; @@ -728,7 +728,7 @@ static int varlink_idl_subparse_struct_or_enum( assert(!field_name); if (!token) - return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Premature EOF: %m", *line, *column); + return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Premature EOF.", *line, *column); if (streq(token, "#")) { r = varlink_idl_subparse_comment(p, line, column); if (r < 0) @@ -748,7 +748,7 @@ static int varlink_idl_subparse_struct_or_enum( assert(field_name); if (!token) - return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Premature EOF: %m", *line, *column); + return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Premature EOF.", *line, *column); if (streq(token, ":")) { VarlinkField *field; @@ -756,7 +756,7 @@ static int varlink_idl_subparse_struct_or_enum( if ((*symbol)->symbol_type < 0) (*symbol)->symbol_type = VARLINK_STRUCT_TYPE; if ((*symbol)->symbol_type == VARLINK_ENUM_TYPE) - return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Enum with struct fields, refusing: %m", *line, *column); + return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Enum with struct fields, refusing.", *line, *column); r = varlink_symbol_realloc(symbol, *n_fields + 1); if (r < 0) @@ -783,7 +783,7 @@ static int varlink_idl_subparse_struct_or_enum( if ((*symbol)->symbol_type < 0) (*symbol)->symbol_type = VARLINK_ENUM_TYPE; if ((*symbol)->symbol_type != VARLINK_ENUM_TYPE) - return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Struct with enum fields, refusing: %m", *line, *column); + return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Struct with enum fields, refusing.", *line, *column); r = varlink_symbol_realloc(symbol, *n_fields + 1); if (r < 0) @@ -804,7 +804,7 @@ static int varlink_idl_subparse_struct_or_enum( state = STATE_DONE; } } else - return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Unexpected token '%s': %m", *line, *column, token); + return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Unexpected token '%s'.", *line, *column, token); break; @@ -812,7 +812,7 @@ static int varlink_idl_subparse_struct_or_enum( assert(!field_name); if (!token) - return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Premature EOF: %m", *line, *column); + return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Premature EOF.", *line, *column); if (streq(token, ",")) { state = STATE_NAME; allowed_delimiters = NULL; @@ -820,7 +820,7 @@ static int varlink_idl_subparse_struct_or_enum( } else if (streq(token, ")")) state = STATE_DONE; else - return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Unexpected token '%s': %m", *line, *column, token); + return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Unexpected token '%s'.", *line, *column, token); break; default: @@ -928,7 +928,7 @@ int varlink_idl_parse( case STATE_PRE_INTERFACE: if (!token) - return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Premature EOF: %m", *line, *column); + return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Premature EOF.", *line, *column); if (streq(token, "#")) { r = varlink_idl_subparse_comment(&text, line, column); if (r < 0) @@ -938,7 +938,7 @@ int varlink_idl_parse( allowed_delimiters = NULL; allowed_chars = VALID_CHARS_INTERFACE_NAME; } else - return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Unexpected token '%s': %m", *line, *column, token); + return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Unexpected token '%s'.", *line, *column, token); break; case STATE_INTERFACE: @@ -946,7 +946,7 @@ int varlink_idl_parse( assert(n_symbols == 0); if (!token) - return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Premature EOF: %m", *line, *column); + return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Premature EOF.", *line, *column); r = varlink_interface_realloc(&interface, n_symbols); if (r < 0) @@ -978,7 +978,7 @@ int varlink_idl_parse( state = STATE_ERROR; allowed_chars = VALID_CHARS_IDENTIFIER; } else - return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Unexpected token '%s': %m", *line, *column, token); + return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Unexpected token '%s'.", *line, *column, token); break; @@ -1005,7 +1005,7 @@ int varlink_idl_parse( assert(symbol); if (!streq(token, "->")) - return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Unexpected token '%s': %m", *line, *column, token); + return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Unexpected token '%s'.", *line, *column, token); r = varlink_idl_subparse_struct_or_enum(&text, line, column, &symbol, &n_fields, VARLINK_OUTPUT); if (r < 0) From 5d2ea9b5cfde5998bec93d84856017053c9e3ca3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 12 Oct 2023 15:28:06 +0200 Subject: [PATCH 3/3] test: add simple test for two common kind of errors --- src/test/test-varlink-idl.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/test/test-varlink-idl.c b/src/test/test-varlink-idl.c index a93d717f3f..028408b16e 100644 --- a/src/test/test-varlink-idl.c +++ b/src/test/test-varlink-idl.c @@ -152,6 +152,12 @@ TEST(parse) { assert_se(varlink_idl_parse(text, NULL, NULL, &parsed) >= 0); test_parse_format_one(parsed); + + assert_se(varlink_idl_parse("interface org.freedesktop.Foo\n" + "type Foo (b: bool, c: foo, c: int)", NULL, NULL, NULL) == -ENETUNREACH); /* unresolved type */ + assert_se(varlink_idl_parse("interface org.freedesktop.Foo\n" + "type Foo ()", NULL, NULL, NULL) == -EBADMSG); /* empty struct/enum */ + } TEST(interface_name_is_valid) {