From: Arran Cudbard-Bell Date: Mon, 17 Apr 2023 05:51:57 +0000 (+1000) Subject: Add multiple types of variadic argument X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d490c46201db02e4a39014d8e4337528f1f0fd16;p=thirdparty%2Ffreeradius-server.git Add multiple types of variadic argument Fix redis xlats to keep argument order and not crash when empty values are provided --- diff --git a/src/lib/unlang/xlat.h b/src/lib/unlang/xlat.h index 28a564e6eff..0077ae5b39b 100644 --- a/src/lib/unlang/xlat.h +++ b/src/lib/unlang/xlat.h @@ -128,20 +128,28 @@ extern size_t xlat_action_table_len; */ typedef int (*xlat_escape_func_t)(request_t *request, fr_value_box_t *vb, void *uctx); +typedef enum { + XLAT_ARG_VARIADIC_DISABLED = 0, + XLAT_ARG_VARIADIC_EMPTY_SQUASH = 1, //!< Empty argument groups are removed. + XLAT_ARG_VARIADIC_EMPTY_KEEP = 2, //!< Empty argument groups are left alone, + ///< and either passed through as empty groups + ///< or null boxes. +} xlat_arg_parser_variadic_t; + /** Definition for a single argument consumend by an xlat function * */ typedef struct { - bool required; //!< Argument must be present. - bool concat; //!< Concat boxes together. - bool single; //!< Argument must only contain a single box - bool variadic; //!< All additional boxes should be processed - ///< using this definition. - bool always_escape; //!< Pass all arguments to escape function not just - ///< tainted ones. - fr_type_t type; //!< Type to cast argument to. - xlat_escape_func_t func; //!< Function to handle tainted values. - void *uctx; //!< Argument to pass to escape callback. + bool required; //!< Argument must be present, and non-empty. + bool concat; //!< Concat boxes together. + bool single; //!< Argument must only contain a single box + xlat_arg_parser_variadic_t variadic; //!< All additional boxes should be processed + ///< using this definition. + bool always_escape; //!< Pass all arguments to escape function not just + ///< tainted ones. + fr_type_t type; //!< Type to cast argument to. + xlat_escape_func_t func; //!< Function to handle tainted values. + void *uctx; //!< Argument to pass to escape callback. } xlat_arg_parser_t; typedef struct { diff --git a/src/lib/unlang/xlat_builtin.c b/src/lib/unlang/xlat_builtin.c index 213e175aa62..d9e909e234b 100644 --- a/src/lib/unlang/xlat_builtin.c +++ b/src/lib/unlang/xlat_builtin.c @@ -1374,7 +1374,8 @@ finish: static xlat_arg_parser_t const xlat_func_cast_args[] = { { .required = true, .single = true, .type = FR_TYPE_VOID }, - { .required = true, .variadic = true, .type = FR_TYPE_VOID }, + { .required = true, .type = FR_TYPE_VOID }, + { .variadic = XLAT_ARG_VARIADIC_EMPTY_KEEP, .type = FR_TYPE_VOID }, XLAT_ARG_PARSER_TERMINATOR }; @@ -1645,27 +1646,26 @@ static xlat_action_t xlat_func_hmac_sha1(TALLOC_CTX *ctx, fr_dcursor_t *out, } static xlat_arg_parser_t const xlat_func_join_args[] = { - { .required = true, .variadic = true, .type = FR_TYPE_VOID }, + { .required = true, .type = FR_TYPE_VOID }, + { .variadic = XLAT_ARG_VARIADIC_EMPTY_SQUASH, .type = FR_TYPE_VOID }, XLAT_ARG_PARSER_TERMINATOR }; /** Join a series of arguments to form a single list * + * null boxes are not preserved. */ static xlat_action_t xlat_func_join(UNUSED TALLOC_CTX *ctx, fr_dcursor_t *out, UNUSED xlat_ctx_t const *xctx, UNUSED request_t *request, fr_value_box_list_t *in) { - fr_value_box_t *arg = NULL, *vb, *p; - - while ((arg = fr_value_box_list_next(in, arg))) { + fr_value_box_list_foreach(in, arg) { fr_assert(arg->type == FR_TYPE_GROUP); - vb = fr_value_box_list_head(&arg->vb_group); - while (vb) { - p = fr_value_box_list_remove(&arg->vb_group, vb); + + fr_value_box_list_foreach_safe(&arg->vb_group, vb) { + fr_value_box_list_remove(&arg->vb_group, vb); fr_dcursor_append(out, vb); - vb = fr_value_box_list_next(&arg->vb_group, p); - } + }} } return XLAT_ACTION_DONE; } @@ -1701,11 +1701,13 @@ static xlat_action_t xlat_func_ungroup(UNUSED TALLOC_CTX *ctx, fr_dcursor_t *out } static xlat_arg_parser_t const xlat_func_length_args[] = { - { .single = true, .variadic = true, .type = FR_TYPE_VOID }, + { .single = true, .variadic = XLAT_ARG_VARIADIC_EMPTY_KEEP, .type = FR_TYPE_VOID }, XLAT_ARG_PARSER_TERMINATOR }; /** Return the on-the-wire size of the boxes in bytes + * + * skips null values * * Example: @verbatim @@ -1722,13 +1724,11 @@ static xlat_action_t xlat_func_length(TALLOC_CTX *ctx, fr_dcursor_t *out, UNUSED request_t *request, fr_value_box_list_t *in) { - fr_value_box_t *vb = NULL; - - while ((vb = fr_value_box_list_next(in, vb))) { + fr_value_box_list_foreach(in, vb) { fr_value_box_t *my; MEM(my = fr_value_box_alloc(ctx, FR_TYPE_SIZE, NULL, false)); - my->vb_size = fr_value_box_network_length(vb); + if (!fr_type_is_null(vb->type)) my->vb_size = fr_value_box_network_length(vb); fr_dcursor_append(out, my); } @@ -3001,7 +3001,7 @@ static xlat_action_t xlat_func_urlunquote(TALLOC_CTX *ctx, fr_dcursor_t *out, } static xlat_arg_parser_t const protocol_decode_xlat_args[] = { - { .single = true, .variadic = true, .type = FR_TYPE_VOID }, + { .single = true, .variadic = XLAT_ARG_VARIADIC_EMPTY_SQUASH, .type = FR_TYPE_VOID }, XLAT_ARG_PARSER_TERMINATOR }; diff --git a/src/lib/unlang/xlat_eval.c b/src/lib/unlang/xlat_eval.c index cd04eaf2dc5..77d3b211c0b 100644 --- a/src/lib/unlang/xlat_eval.c +++ b/src/lib/unlang/xlat_eval.c @@ -32,6 +32,7 @@ RCSID("$Id$") #include #include #include +#include #include /* Remove when everything uses new xlat API */ @@ -375,7 +376,6 @@ xlat_action_t xlat_process_args(TALLOC_CTX *ctx, fr_value_box_list_t *list, */ if (!func->args) return XLAT_ACTION_DONE; - /* * xlat needs no input processing just return. */ @@ -430,12 +430,25 @@ xlat_action_t xlat_process_args(TALLOC_CTX *ctx, fr_value_box_list_t *list, * This argument doesn't exist. That might be OK, or it may be a fatal error. */ if (fr_value_box_list_empty(&vb->vb_group)) { + /* + * Variadic rules deal with empty boxes differently... + */ + switch (arg_p->variadic) { + case XLAT_ARG_VARIADIC_EMPTY_SQUASH: + fr_value_box_list_talloc_free_head(list); + continue; + + case XLAT_ARG_VARIADIC_EMPTY_KEEP: + goto empty_ok; + + case XLAT_ARG_VARIADIC_DISABLED: + break; + } + /* * Empty groups for optional arguments are OK, we can just stop processing the list. */ if (!arg_p->required) { - fr_assert(!next); - /* * If the caller doesn't care about the type, then we leave the * empty group there. @@ -455,8 +468,7 @@ xlat_action_t xlat_process_args(TALLOC_CTX *ctx, fr_value_box_list_t *list, * accessing the box as vb_uint8, hoping that it's being passed * the right thing. */ - fr_value_box_list_remove(list, vb); - talloc_free(vb); + fr_value_box_list_talloc_free_head(list); break; } @@ -470,13 +482,23 @@ xlat_action_t xlat_process_args(TALLOC_CTX *ctx, fr_value_box_list_t *list, if (arg_p->type != FR_TYPE_VOID) goto missing; } + empty_ok: /* * In some cases we replace the current argument with the head of the group. * * xlat_process_arg_list() has already done concatenations for us. */ if (arg_p->single || arg_p->concat) { - fr_value_box_list_replace(list, vb, fr_value_box_list_pop_head(&vb->vb_group)); + fr_value_box_t *head = fr_value_box_list_pop_head(&vb->vb_group); + + /* + * If we're meant to be smashing the argument + * to a single box, but the group was empty, + * add a null box instead so ordering is maintained + * for subsequent boxes. + */ + if (!head) head = fr_value_box_alloc_null(ctx); + fr_value_box_list_replace(list, vb, head); talloc_free(vb); } diff --git a/src/lib/unlang/xlat_pair.c b/src/lib/unlang/xlat_pair.c index 023c8f2226c..da3a4894813 100644 --- a/src/lib/unlang/xlat_pair.c +++ b/src/lib/unlang/xlat_pair.c @@ -92,14 +92,13 @@ int xlat_decode_value_box_list(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_value_box_list_t *in) { int decoded = 0; - fr_value_box_t *vb = NULL; fr_pair_t *vp = NULL; fr_dict_attr_t const *parent = fr_dict_root(request->dict); fr_pair_list_t head; fr_pair_list_init(&head); - while ((vb = fr_value_box_list_next(in, vb))) { + fr_value_box_list_foreach(in, vb) { ssize_t len; if (vb->type != FR_TYPE_OCTETS) { diff --git a/src/modules/rlm_cipher/rlm_cipher.c b/src/modules/rlm_cipher/rlm_cipher.c index 799fddd2bd1..8729bf1e412 100644 --- a/src/modules/rlm_cipher/rlm_cipher.c +++ b/src/modules/rlm_cipher/rlm_cipher.c @@ -35,6 +35,7 @@ RCSID("$Id$") #include #include #include +#include #include #include @@ -763,9 +764,9 @@ static xlat_action_t cipher_rsa_decrypt_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out, } static xlat_arg_parser_t const cipher_rsa_verify_xlat_arg[] = { - { .required = true, .concat = true, .single = false, .variadic = true, .type = FR_TYPE_STRING, - .func = NULL, .uctx = NULL }, { .required = true, .concat = false, .single = true, .type = FR_TYPE_VOID }, + { .required = true, .concat = true, .type = FR_TYPE_STRING }, + { .variadic = XLAT_ARG_VARIADIC_EMPTY_SQUASH, .concat = true, .type = FR_TYPE_STRING }, XLAT_ARG_PARSER_TERMINATOR }; diff --git a/src/modules/rlm_exec/rlm_exec.c b/src/modules/rlm_exec/rlm_exec.c index b888e76cbd8..4684f26f466 100644 --- a/src/modules/rlm_exec/rlm_exec.c +++ b/src/modules/rlm_exec/rlm_exec.c @@ -101,7 +101,7 @@ static xlat_action_t exec_xlat_oneshot_wait_resume(TALLOC_CTX *ctx, fr_dcursor_t static xlat_arg_parser_t const exec_xlat_args[] = { { .required = true, .type = FR_TYPE_STRING }, - { .variadic = true, .type = FR_TYPE_VOID}, + { .variadic = XLAT_ARG_VARIADIC_EMPTY_KEEP, .type = FR_TYPE_VOID}, XLAT_ARG_PARSER_TERMINATOR }; diff --git a/src/modules/rlm_perl/rlm_perl.c b/src/modules/rlm_perl/rlm_perl.c index ddd999c0094..a1f1f8b33b6 100644 --- a/src/modules/rlm_perl/rlm_perl.c +++ b/src/modules/rlm_perl/rlm_perl.c @@ -30,6 +30,7 @@ RCSID("$Id$") #include #include #include +#include #include DIAG_OFF(DIAG_UNKNOWN_PRAGMAS) @@ -412,7 +413,7 @@ static int perl_sv_to_vblist(TALLOC_CTX *ctx, fr_value_box_list_t *list, request static xlat_arg_parser_t const perl_xlat_args[] = { { .required = true, .single = true, .type = FR_TYPE_STRING }, - { .variadic = true, .type = FR_TYPE_VOID }, + { .variadic = XLAT_ARG_VARIADIC_EMPTY_KEEP, .type = FR_TYPE_VOID }, XLAT_ARG_PARSER_TERMINATOR }; @@ -429,7 +430,7 @@ static xlat_action_t perl_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out, xlat_action_t ret = XLAT_ACTION_FAIL; STRLEN n_a; fr_value_box_t *func = fr_value_box_list_pop_head(in); - fr_value_box_t *arg = NULL, *child; + fr_value_box_t *child; SV *sv; AV *av; fr_value_box_list_t list, sub_list; @@ -449,7 +450,7 @@ static xlat_action_t perl_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out, PUSHMARK(SP); - while ((arg = fr_value_box_list_next(in, arg))) { + fr_value_box_list_foreach(in, arg) { fr_assert(arg->type == FR_TYPE_GROUP); if (fr_value_box_list_empty(&arg->vb_group)) continue; diff --git a/src/modules/rlm_radius/rlm_radius_udp.c b/src/modules/rlm_radius/rlm_radius_udp.c index 41630f3de1f..573c2430cb7 100644 --- a/src/modules/rlm_radius/rlm_radius_udp.c +++ b/src/modules/rlm_radius/rlm_radius_udp.c @@ -1578,7 +1578,6 @@ static bool check_for_zombie(fr_event_list_t *el, fr_trunk_connection_t *tconn, /* * If we're status checking OR already zombie, don't go to zombie - * */ if (h->status_checking || h->zombie_ev) return true; @@ -1595,13 +1594,7 @@ static bool check_for_zombie(fr_event_list_t *el, fr_trunk_connection_t *tconn, if (h->inst->parent->synchronous && fr_time_gt(last_sent, fr_time_wrap(0)) && (fr_time_lt(fr_time_add(last_sent, h->inst->parent->response_window), now))) return false; - /* - * Mark the connection as inactive, but keep sending - * packets on it. - */ WARN("%s - Entering Zombie state - connection %s", h->module_name, h->name); - fr_trunk_connection_signal_inactive(tconn); - if (h->inst->parent->status_check) { h->status_checking = true; diff --git a/src/modules/rlm_redis/rlm_redis.c b/src/modules/rlm_redis/rlm_redis.c index 222e7d5be95..e443bb65af2 100644 --- a/src/modules/rlm_redis/rlm_redis.c +++ b/src/modules/rlm_redis/rlm_redis.c @@ -351,7 +351,7 @@ static xlat_action_t redis_node_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out, static xlat_arg_parser_t const redis_lua_func_args[] = { { .required = true, .single = true, .type = FR_TYPE_UINT64 }, /* key count */ - { .variadic = true, .concat = true, .type = FR_TYPE_STRING }, /* keys and args */ + { .variadic = XLAT_ARG_VARIADIC_EMPTY_KEEP, .concat = true, .type = FR_TYPE_STRING }, /* keys and args */ XLAT_ARG_PARSER_TERMINATOR }; @@ -411,9 +411,20 @@ static xlat_action_t redis_lua_func_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out, REXDENT(); return XLAT_ACTION_FAIL; } + + /* + * Fixup null or empty arguments to be + * zero length strings so that the position + * of subsequent arguments are maintained. + */ + if (!fr_type_is_string(vb->type)) { + argv[argc] = ""; + arg_len[argc++] = 0; + continue; + } + argv[argc] = vb->vb_strvalue; - arg_len[argc] = vb->vb_length; - argc++; + arg_len[argc++] = vb->vb_length; } /* @@ -551,14 +562,15 @@ static int redis_lua_func_instantiate(xlat_inst_ctx_t const *xctx) } static xlat_arg_parser_t const redis_args[] = { - { .required = true, .variadic = true, .concat = true, .type = FR_TYPE_STRING }, + { .required = true, .concat = true, .type = FR_TYPE_STRING }, + { .variadic = XLAT_ARG_VARIADIC_EMPTY_KEEP, .concat = true, .type = FR_TYPE_STRING }, XLAT_ARG_PARSER_TERMINATOR }; /** Xlat to make calls to redis * @verbatim -%{redis:} +%(redis:) @endverbatim * * @ingroup xlat_functions @@ -628,9 +640,19 @@ static xlat_action_t redis_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out, goto fail; } + /* + * Fixup null or empty arguments to be + * zero length strings so that the position + * of subsequent arguments are maintained. + */ + if (!fr_type_is_string(vb->type)) { + argv[argc] = ""; + arg_len[argc++] = 0; + continue; + } + argv[argc] = vb->vb_strvalue; - arg_len[argc] = vb->vb_length; - argc++; + arg_len[argc++] = vb->vb_length; } RDEBUG2("Executing command: %pV", fr_value_box_list_head(in)); diff --git a/src/modules/rlm_rest/rlm_rest.c b/src/modules/rlm_rest/rlm_rest.c index 360fec27355..13cbdd63b96 100644 --- a/src/modules/rlm_rest/rlm_rest.c +++ b/src/modules/rlm_rest/rlm_rest.c @@ -386,7 +386,7 @@ static fr_uri_part_t const rest_uri_parts[] = { }; static xlat_arg_parser_t const rest_xlat_args[] = { - { .required = true, .variadic = true, .type = FR_TYPE_STRING }, + { .required = true, .variadic = XLAT_ARG_VARIADIC_EMPTY_KEEP, .type = FR_TYPE_STRING }, XLAT_ARG_PARSER_TERMINATOR }; diff --git a/src/modules/rlm_test/rlm_test.c b/src/modules/rlm_test/rlm_test.c index d26cb8e0318..210f31cbefb 100644 --- a/src/modules/rlm_test/rlm_test.c +++ b/src/modules/rlm_test/rlm_test.c @@ -380,7 +380,8 @@ static xlat_action_t trigger_test_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out, static xlat_arg_parser_t const test_xlat_args[] = { - { .required = true, .concat = true, .variadic = true, .type = FR_TYPE_STRING }, + { .required = true, .concat = true, .type = FR_TYPE_STRING }, + { .variadic = XLAT_ARG_VARIADIC_EMPTY_KEEP, .concat = true, .type = FR_TYPE_STRING }, XLAT_ARG_PARSER_TERMINATOR }; @@ -393,10 +394,9 @@ static xlat_action_t test_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out, UNUSED xlat_ctx_t const *xctx, UNUSED request_t *request, fr_value_box_list_t *in) { - fr_value_box_t *vb_p = NULL; fr_value_box_t *vb; - while ((vb_p = fr_value_box_list_next(in, vb_p))) { + fr_value_box_list_foreach(in, vb_p) { MEM(vb = fr_value_box_alloc(ctx, FR_TYPE_STRING, NULL, false)); if (fr_value_box_copy(ctx, vb, vb_p) < 0) { diff --git a/src/tests/modules/redis/functions.unlang b/src/tests/modules/redis/functions.unlang index a21f37ee347..6b420184313 100644 --- a/src/tests/modules/redis/functions.unlang +++ b/src/tests/modules/redis/functions.unlang @@ -15,7 +15,7 @@ if (!(%(redis.hello_world:0) == 'hello world')) { # ...and again using an argument that would produce a null result # this is a regression test where the arg parser would require all # arguments to be non-null if the first argument was -if (!(%(redis.hello_world:0 %{Framed-IP-Address}) == 'hello world')) { +if (!(%(redis.hello_world:0 %{Tmp-String-9}) == 'hello world')) { test_fail } @@ -23,6 +23,11 @@ if (!(%(redis.concat_args_keys:1 foo bar baz) == 'foo,bar,baz')) { test_fail } +# Concat with an empty argument. This is a regression test +if (!(%(redis.concat_args_keys:1 foo %{Tmp-String-9} baz) == 'foo,,baz')) { + test_fail +} + if (!(%(redis.multiline:0 0) == 0)) { test_fail }