From: Alan T. DeKok Date: Tue, 5 Jul 2022 17:25:38 +0000 (-0400) Subject: update casting rules X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f4665c3ae4afb24678e84c242b1d3e888965905a;p=thirdparty%2Ffreeradius-server.git update casting rules (string)&Foo now means "print to string". This change only affects 'octets' types, as other types were already printed to a string --- diff --git a/doc/antora/modules/reference/pages/type/double.adoc b/doc/antora/modules/reference/pages/type/double.adoc index ee5af7e6c0c..4085a3209b0 100644 --- a/doc/antora/modules/reference/pages/type/double.adoc +++ b/doc/antora/modules/reference/pages/type/double.adoc @@ -47,7 +47,7 @@ In general, creating strings via xref:xlat/index.adoc[dynamic expansions] will result in the _printed_ version of the expansion being used. -.Example +.Example of double-quoted string expansion [source,unlang] ---- "User-Name is %{User-Name}" @@ -65,36 +65,27 @@ When a string is created via an xref:unlang/expression.adoc[expression] using the `+` operator, the resulting string can be quite different, depending on the inputs. -.Example +.Example of casting to 'string' [source,unlang] ---- "User-Name is " + &User-Name "IP Address is " + (string) &reply.Framed-IP-Address ---- -The first expansion here returns the same string as the previous -example `"User-Name is %{User-Name}"`. This behavior is because the -`User-Name` attribute is defined as type `string`. So appending a -`string` to a `string` yields a `string`. +The output strings here (with casting) are the same as for the +previous example. Note that we do not have to cast `&User-Name`, +because it is already a string. -However, the second example returns the string `"IP Address is "` -followed by four (4) bytes of the actual IP address, with embedded -zeros, and other non-ASCII characters - -This behavior is because the xref:type/cast.doc[cast] operator -`(string)` will convert the data type from `ipv4addr` to `string`, but -it will not change or modify the _contents_ of the data. The result -of the `+` concatenation is therefore the original `string`, followed -by the 4 bytes of the `ipv4addr` data. - -If the printed version of the data is needed, the expansion should be -enclosed in double-quotes, as in the first example. - -=== To Remember +.Example of casting to 'octets' +[source,unlang] +---- +"User-Name is " + (octets) &User-Name +"IP Address is " + (octets) &reply.Framed-IP-Address +---- -`"foo" + (string) &Bar` produces `"foo"` plus `&Bar` _cast_ to a string. +The output strings here are completely different than for the previous +examples. The output data type is `octets`, and not `string`. -`"foo%{Bar}"` produces `"foo"` plus `&Bar` _printed_ to a string. // Copyright (C) 2021 Network RADIUS SAS. Licenced under CC-by-NC 4.0. // Development of this documentation was sponsored by Network RADIUS SAS. diff --git a/src/lib/server/tmpl_eval.c b/src/lib/server/tmpl_eval.c index ee9f11457f3..32848e72618 100644 --- a/src/lib/server/tmpl_eval.c +++ b/src/lib/server/tmpl_eval.c @@ -1488,11 +1488,17 @@ int tmpl_eval_cast(TALLOC_CTX *ctx, fr_value_box_list_t *list, tmpl_t const *vpt if (!fr_dlist_next(list, vb)) { return fr_value_box_cast_in_place(vb, vb, cast, NULL); } + FALL_THROUGH; + /* + * Strings aren't *cast* to the output. They're *printed* to the output. + */ + case FR_TYPE_STRING: FR_SBUFF_TALLOC_THREAD_LOCAL(&agg, 256, 256); - slen = fr_value_box_list_concat_as_string(&tainted, agg, list, NULL, 0, NULL, - FR_VALUE_BOX_LIST_FREE_BOX, true); + slen = fr_value_box_list_concat_as_string(&tainted, agg, list, NULL, 0, + (cast == FR_TYPE_STRING) ? &fr_value_escape_double : NULL, + FR_VALUE_BOX_LIST_FREE_BOX, true, true); if (slen < 0) return -1; MEM(vb = fr_value_box_alloc_null(ctx)); @@ -1504,7 +1510,6 @@ int tmpl_eval_cast(TALLOC_CTX *ctx, fr_value_box_list_t *list, tmpl_t const *vpt fr_dlist_insert_tail(list, vb); break; - case FR_TYPE_STRING: case FR_TYPE_OCTETS: return fr_value_box_list_concat_in_place(vb, vb, list, cast, FR_VALUE_BOX_LIST_FREE_BOX, true, SIZE_MAX); diff --git a/src/lib/unlang/xlat_builtin.c b/src/lib/unlang/xlat_builtin.c index c672b10dd74..cc44ca87f7e 100644 --- a/src/lib/unlang/xlat_builtin.c +++ b/src/lib/unlang/xlat_builtin.c @@ -2090,6 +2090,31 @@ static xlat_action_t xlat_func_cast(UNUSED TALLOC_CTX *ctx, fr_dcursor_t *out, } } + /* + * Cast to string means *print* to string. + */ + if (type == FR_TYPE_STRING) { + fr_sbuff_t *agg; + fr_value_box_t *dst; + + (void) fr_dlist_pop_head(in); + talloc_free(name); + + FR_SBUFF_TALLOC_THREAD_LOCAL(&agg, 256, 8192); + + MEM(dst = fr_value_box_alloc_null(ctx)); + if (fr_value_box_list_concat_as_string(NULL, agg, in, NULL, 0, &fr_value_escape_double, + FR_VALUE_BOX_LIST_FREE_BOX, true, true) < 0) { + RPEDEBUG("Failed concatenating string"); + return XLAT_ACTION_FAIL; + } + + fr_value_box_bstrndup_shallow(dst, NULL, fr_sbuff_start(agg), fr_sbuff_used(agg), false); + fr_dcursor_append(out, dst); + + return XLAT_ACTION_DONE; + } + /* * Copy inputs to outputs, casting them along the way. */ @@ -2552,56 +2577,6 @@ static xlat_action_t xlat_func_pairs(TALLOC_CTX *ctx, fr_dcursor_t *out, } -static xlat_arg_parser_t const xlat_func_print_args[] = { - { .required = true, .variadic = true, .type = FR_TYPE_VOID }, - XLAT_ARG_PARSER_TERMINATOR -}; - -/** Create printable versions of things. - * -@verbatim -%(print:&Tmp-Octets-0) results in the printable version of the attribute. -@endverbatim - * - * @ingroup xlat_functions - */ -static xlat_action_t xlat_func_print(TALLOC_CTX *ctx, fr_dcursor_t *out, - UNUSED xlat_ctx_t const *xctx, - request_t *request, fr_value_box_list_t *in) -{ - fr_sbuff_t *agg; - fr_value_box_t *dst; - - FR_SBUFF_TALLOC_THREAD_LOCAL(&agg, 256, 8192); - - fr_dlist_foreach(in, fr_value_box_t, arg) { - fr_assert(arg->type == FR_TYPE_GROUP); - - fr_dlist_foreach(&arg->vb_group, fr_value_box_t, vb) { - ssize_t slen; - - /* - * If the input is tainted, escape it using the normal double-quoting rules. - * - * The resulting output string is not tainted. - */ - slen = fr_value_box_print(agg, vb, vb->tainted ? &fr_value_escape_double : NULL); - if (slen < 0) { - RPEDEBUG("Failed printing %pV to data type 'string'", vb); - return XLAT_ACTION_FAIL; - } - } - } - - MEM(dst = fr_value_box_alloc_null(ctx)); - fr_value_box_bstrndup_shallow(dst, NULL, fr_sbuff_start(agg), fr_sbuff_used(agg), false); - fr_dcursor_append(out, dst); - - fr_dlist_talloc_free(in); - - return XLAT_ACTION_DONE; -} - static xlat_arg_parser_t const xlat_func_rand_arg = { .required = true, .single = true, @@ -3821,7 +3796,6 @@ do { \ XLAT_REGISTER_ARGS("length", xlat_func_length, xlat_func_length_args); XLAT_REGISTER_ARGS("lpad", xlat_func_lpad, xlat_func_pad_args); XLAT_REGISTER_ARGS("rpad", xlat_func_rpad, xlat_func_pad_args); - XLAT_REGISTER_ARGS("print", xlat_func_print, xlat_func_print_args); /* * The inputs to these functions are variable. diff --git a/src/lib/unlang/xlat_expr.c b/src/lib/unlang/xlat_expr.c index db2027fe877..902082035e6 100644 --- a/src/lib/unlang/xlat_expr.c +++ b/src/lib/unlang/xlat_expr.c @@ -590,7 +590,7 @@ static xlat_action_t xlat_regex_resume(TALLOC_CTX *ctx, fr_dcursor_t *out, * concatenate it here. We escape the various untrusted inputs. */ if (fr_value_box_list_concat_as_string(NULL, agg, &rctx->list, NULL, 0, ®ex_escape_rules, - FR_VALUE_BOX_LIST_FREE_BOX, true) < 0) { + FR_VALUE_BOX_LIST_FREE_BOX, true, false) < 0) { RPEDEBUG("Failed concatenating regular expression string"); return XLAT_ACTION_FAIL; } @@ -1365,7 +1365,7 @@ static xlat_action_t xlat_exists_resume(TALLOC_CTX *ctx, fr_dcursor_t *out, * concatenate it here. We escape the various untrusted inputs. */ if (fr_value_box_list_concat_as_string(NULL, agg, &rctx->list, NULL, 0, NULL, - FR_VALUE_BOX_LIST_FREE_BOX, true) < 0) { + FR_VALUE_BOX_LIST_FREE_BOX, true, true) < 0) { RPEDEBUG("Failed concatenating attribute name string"); return XLAT_ACTION_FAIL; } @@ -2141,6 +2141,7 @@ static ssize_t tokenize_field(xlat_exp_head_t *head, xlat_exp_t **out, fr_sbuff_ if (tmpl_contains_xlat(vpt) && !tmpl_is_exec(vpt) && !tmpl_contains_regex(vpt)) { xlat_exp_head_t *xlat = tmpl_xlat(vpt); xlat_exp_t *cast; + fr_type_t type; talloc_steal(node, xlat); node->fmt = talloc_typed_strdup(node, node->fmt); @@ -2154,23 +2155,24 @@ static ssize_t tokenize_field(xlat_exp_head_t *head, xlat_exp_t **out, fr_sbuff_ if (quote != T_BARE_WORD) { if (tmpl_rules_cast(vpt) != FR_TYPE_NULL) { - MEM(cast = expr_cast_alloc(head, tmpl_rules_cast(vpt))); + type = tmpl_rules_cast(vpt); } else { + type = FR_TYPE_STRING; + /* * The string is T_DOUBLE_QUOTED_STRING, or T_BACK_QUOTED_STRING, * both of which have double-quoting rules. * * 'foo' can't contain an xlat. * /foo/ is forbidden, as we don't expect a regex here. + * + * We rely on xlat_func_cast() to see the `string` cast, and then to + * _print_ the result to a string. */ - MEM(cast = xlat_exp_alloc(head, XLAT_FUNC, "print", 5)); - MEM(cast->call.args = xlat_exp_head_alloc(cast)); - MEM(cast->call.func = xlat_func_find("print", 5)); - fr_assert(cast->call.func != NULL); - cast->flags = cast->call.func->flags; } + MEM(cast = expr_cast_alloc(head, type)); xlat_func_append_arg(cast, node, false); node = cast; } diff --git a/src/lib/util/value.c b/src/lib/util/value.c index bfdb213472c..3d59d49eff4 100644 --- a/src/lib/util/value.c +++ b/src/lib/util/value.c @@ -5235,7 +5235,7 @@ ssize_t fr_value_box_print(fr_sbuff_t *out, fr_value_box_t const *data, fr_sbuff FR_SBUFF_RETURN(fr_value_box_list_concat_as_string, NULL, &our_out, UNCONST(fr_value_box_list_t *, &data->vb_group), ", ", (sizeof(", ") - 1), e_rules, - 0, false); + 0, false, true); FR_SBUFF_IN_CHAR_RETURN(&our_out, '}'); break; @@ -5305,6 +5305,7 @@ ssize_t fr_value_box_print_quoted(fr_sbuff_t *out, fr_value_box_t const *data, f * @param[in] flatten If true and we encounter a #FR_TYPE_GROUP, * we concat the contents of its children together. * If false, the contents will be cast to #type. + * @param[in] printable Convert 'octets' to printable strings. * @return * - >=0 the number of bytes written to the sbuff. * - <0 how many additional bytes we would have needed to @@ -5312,7 +5313,7 @@ ssize_t fr_value_box_print_quoted(fr_sbuff_t *out, fr_value_box_t const *data, f */ ssize_t fr_value_box_list_concat_as_string(bool *tainted, fr_sbuff_t *sbuff, fr_value_box_list_t *list, char const *sep, size_t sep_len, fr_sbuff_escape_rules_t const *e_rules, - fr_value_box_list_action_t proc_action, bool flatten) + fr_value_box_list_action_t proc_action, bool flatten, bool printable) { fr_sbuff_t our_sbuff = FR_SBUFF(sbuff); ssize_t slen; @@ -5322,13 +5323,15 @@ ssize_t fr_value_box_list_concat_as_string(bool *tainted, fr_sbuff_t *sbuff, fr_ fr_dlist_foreach(list, fr_value_box_t, vb) { switch (vb->type) { case FR_TYPE_GROUP: - if (!flatten) goto cast; + if (!flatten) goto print; slen = fr_value_box_list_concat_as_string(tainted, &our_sbuff, &vb->vb_group, sep, sep_len, e_rules, - proc_action, flatten); + proc_action, flatten, printable); break; case FR_TYPE_OCTETS: + if (printable) goto print; /* even if !tainted */ + /* * Copy the raw string over, if necessary with escaping. */ @@ -5340,13 +5343,13 @@ ssize_t fr_value_box_list_concat_as_string(bool *tainted, fr_sbuff_t *sbuff, fr_ break; case FR_TYPE_STRING: - if (vb->tainted && e_rules) goto cast; + if (vb->tainted && e_rules) goto print; slen = fr_sbuff_in_bstrncpy(&our_sbuff, vb->vb_strvalue, vb->vb_length); break; default: - cast: + print: slen = fr_value_box_print(&our_sbuff, vb, e_rules); break; } @@ -5545,10 +5548,13 @@ int fr_value_box_list_concat_in_place(TALLOC_CTX *ctx, * Head gets dealt with specially as we don't * want to free it, and we don't want to free * the buffer associated with it (just yet). + * + * Note that we don't convert 'octets' to a printable string + * here. Doing so breaks the keyword tests. */ if (fr_value_box_list_concat_as_string(&tainted, &sbuff, list, NULL, 0, NULL, - FR_VALUE_BOX_LIST_REMOVE, flatten) < 0) { + FR_VALUE_BOX_LIST_REMOVE, flatten, false) < 0) { fr_strerror_printf("Concatenation exceeded max_size (%zu)", max_size); error: switch (type) { @@ -5571,7 +5577,7 @@ int fr_value_box_list_concat_in_place(TALLOC_CTX *ctx, */ if (fr_value_box_list_concat_as_string(&tainted, &sbuff, list, NULL, 0, NULL, - proc_action, flatten) < 0) { + proc_action, flatten, true) < 0) { fr_dlist_insert_head(list, head_vb); goto error; } @@ -5613,7 +5619,7 @@ int fr_value_box_list_concat_in_place(TALLOC_CTX *ctx, case FR_TYPE_STRING: if (fr_value_box_list_concat_as_string(&tainted, &sbuff, list, NULL, 0, NULL, - proc_action, flatten) < 0) goto error; + proc_action, flatten, true) < 0) goto error; (void)fr_sbuff_trim_talloc(&sbuff, SIZE_MAX); entry = out->entry; diff --git a/src/lib/util/value.h b/src/lib/util/value.h index b9cc4cc59ec..6ca56f7a363 100644 --- a/src/lib/util/value.h +++ b/src/lib/util/value.h @@ -983,7 +983,7 @@ ssize_t fr_value_box_from_str(TALLOC_CTX *ctx, fr_value_box_t *dst, */ ssize_t fr_value_box_list_concat_as_string(bool *tainted, fr_sbuff_t *sbuff, fr_value_box_list_t *list, char const *sep, size_t sep_len, fr_sbuff_escape_rules_t const *e_rules, - fr_value_box_list_action_t proc_action, bool flatten) + fr_value_box_list_action_t proc_action, bool flatten, bool printable) CC_HINT(nonnull(2,3)); ssize_t fr_value_box_list_concat_as_octets(bool *tainted, fr_dbuff_t *dbuff, fr_value_box_list_t *list, diff --git a/src/tests/unit/xlat/cond_base.txt b/src/tests/unit/xlat/cond_base.txt index c58e5b56855..97b20c00157 100644 --- a/src/tests/unit/xlat/cond_base.txt +++ b/src/tests/unit/xlat/cond_base.txt @@ -239,7 +239,7 @@ match (&Session-Timeout == '10') # @todo - yuck. Suppress full path? # xlat_purify &Event-Timestamp == "January 1, 2012 %{User-Name}" -match (&Event-Timestamp == %(print:"January 1, 2012 %{Request[0].User-Name}")) +match (&Event-Timestamp == %(cast:string "January 1, 2012 %{Request[0].User-Name}")) # This one is NOT an expansion, so it's parsed into normal form xlat_purify &Event-Timestamp == 'January 1 2012' @@ -493,7 +493,7 @@ match ((ipaddr)&PMIP6-Home-IPv4-HoA == &Framed-IP-Address) # but these are allowed xlat_purify &Tmp-uint64-0 == "%{module: foo}" -match ((ether)&Tmp-uint64-0 == %(print:"%{module: foo}")) +match ((ether)&Tmp-uint64-0 == %(cast:string "%{module: foo}")) xlat_purify &Filter-Id == &Framed-IP-Address match ((ipaddr)&Filter-Id == &Framed-IP-Address) @@ -522,7 +522,7 @@ match (&User-Name[n] == "bob") #match &Foo-Bar xlat_purify &Acct-Input-Octets > "%{Session-Timeout}" -match (&Acct-Input-Octets > %(print:"%{Request[0].Session-Timeout}")) +match (&Acct-Input-Octets > %(cast:string "%{Request[0].Session-Timeout}")) xlat_purify &Acct-Input-Octets > &Session-Timeout match (&Acct-Input-Octets > &Session-Timeout) diff --git a/src/tests/xlat/expr.txt b/src/tests/xlat/expr.txt index cb4cbde2e5a..4adf3e7b08f 100644 --- a/src/tests/xlat/expr.txt +++ b/src/tests/xlat/expr.txt @@ -98,18 +98,12 @@ match 0x666f6f7f000001 # any escaping. # xlat_expr "foo" + (string)&Packet-Authentication-Vector -match foo +match foo0x00000000000000000000000000000000 +# string + octets gets promoted to octets xlat_expr "foo" + &Packet-Authentication-Vector match 0x666f6f00000000000000000000000000000000 -# -# Really "foo" + 16 octets of 0, but it isn't -# printed correctly. See xlat_expr in unit_test_module. -# -xlat_expr "foo" + (string) &Packet-Authentication-Vector -match foo - # this is "foo" + PRINTABLE version of &Packet-Authentication-Vector xlat_expr "foo%{Packet-Authentication-Vector}" match foo0x00000000000000000000000000000000