From: Alan T. DeKok Date: Tue, 9 Aug 2022 15:24:33 +0000 (-0400) Subject: minor cleanups, and allow &Foo += Bar[*] X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c6ce4b71de74381a0e036b04405031fbc4486704;p=thirdparty%2Ffreeradius-server.git minor cleanups, and allow &Foo += Bar[*] --- diff --git a/src/lib/unlang/edit.c b/src/lib/unlang/edit.c index fb1448dc95e..42c33d95d1e 100644 --- a/src/lib/unlang/edit.c +++ b/src/lib/unlang/edit.c @@ -29,6 +29,7 @@ RCSID("$Id$") #include #include #include +#include #include #include #include "edit_priv.h" @@ -79,15 +80,15 @@ typedef struct { edit_map_t first; } unlang_frame_state_edit_t; -static int templatize_lhs(TALLOC_CTX *ctx, edit_result_t *out, request_t *request) CC_HINT(nonnull); -static int templatize_rhs(TALLOC_CTX *ctx, edit_result_t *out, fr_pair_t const *lhs, request_t *request) CC_HINT(nonnull); +static int templatize_to_attribute(TALLOC_CTX *ctx, edit_result_t *out, request_t *request) CC_HINT(nonnull); +static int templatize_to_value(TALLOC_CTX *ctx, edit_result_t *out, fr_pair_t const *lhs, request_t *request) CC_HINT(nonnull); #define MAP_INFO cf_filename(map->ci), cf_lineno(map->ci) /* - * Convert a value-box list to a LHS #tmpl_t + * Convert a value-box list to a LHS attribute #tmpl_t */ -static int templatize_lhs(TALLOC_CTX *ctx, edit_result_t *out, request_t *request) +static int templatize_to_attribute(TALLOC_CTX *ctx, edit_result_t *out, request_t *request) { ssize_t slen; fr_value_box_t *box = fr_dlist_head(&out->result); @@ -129,7 +130,7 @@ static int templatize_lhs(TALLOC_CTX *ctx, edit_result_t *out, request_t *reques * the calling code should parse the RHS as a set of VPs, and return * that. */ -static int templatize_rhs(TALLOC_CTX *ctx, edit_result_t *out, fr_pair_t const *lhs, request_t *request) +static int templatize_to_value(TALLOC_CTX *ctx, edit_result_t *out, fr_pair_t const *lhs, request_t *request) { fr_type_t type = lhs->vp_type; fr_type_t cast_type = FR_TYPE_STRING; @@ -271,7 +272,7 @@ static int remove_vps(request_t *request, edit_map_t *current) * Loop over VPs on the LHS, doing the operation with the RHS. * * For now, we only support one VP on the LHS, and one value-box on - * the RHS. Fixing this means updating templatize_rhs() to peek at + * the RHS. Fixing this means updating templatize_to_value() to peek at * the RHS list, and if they're all of the same data type, AND the * same data type as the expected output, leave them alone. This * lets us do things like: @@ -373,12 +374,8 @@ static int apply_edits_to_list(request_t *request, edit_map_t *current, map_t co } /* - * Remove an attribute from a list. - * - * @todo - ensure RHS is only an attribute which is - * parented from the LHS, and that it has no list - * reference? This probably needs to be done in - * unlang_fixup_edit() + * Remove an attribute from a list. The tmpl_dcursor and tmpl_parser ensures that the RHS + * references are done in the context of the LHS attribute. */ if (map->op == T_OP_SUB_EQ) { if (!tmpl_is_attr(current->rhs.vpt)) { @@ -476,6 +473,8 @@ static int apply_edits_to_leaf(request_t *request, edit_map_t *current, map_t co { fr_pair_t *vp; fr_value_box_t const *rhs_box = NULL; + tmpl_dcursor_ctx_t cc; + fr_dcursor_t cursor; fr_assert(current->lhs.vp != NULL); @@ -524,8 +523,31 @@ static int apply_edits_to_leaf(request_t *request, edit_map_t *current, map_t co */ if (tmpl_is_data(current->rhs.vpt)) { rhs_box = tmpl_value(current->rhs.vpt); - goto assign; + RDEBUG2("%s %s %pV", current->lhs.vpt->name, fr_tokens[map->op], rhs_box); + + /* + * Don't apply the edit, as the VP is in a temporary list. The parent will actually apply it. + */ + if (current->in_parent_list) { + vp = current->lhs.vp; + + return fr_value_box_cast(vp, &vp->data, vp->da->type, vp->da, rhs_box); + } + + /* + * The apply function also takes care of doing data type upcasting and conversion. So we don't + * have to check for compatibility of the data types on the LHS and RHS. + */ + if (fr_edit_list_apply_pair_assignment(current->el, + current->lhs.vp, + map->op, + rhs_box) < 0) { + RPERROR("Failed performing %s operation", fr_tokens[map->op]); + return -1; + } + + return 0; } /* @@ -547,39 +569,36 @@ static int apply_edits_to_leaf(request_t *request, edit_map_t *current, map_t co /* * Find the RHS attribute. + * + * @todo - allow &Tmp-String-0 := &Filter-Id[*], which is a list. */ - if (tmpl_find_vp(&vp, request, current->rhs.vpt) < 0) { + vp = tmpl_dcursor_init(NULL, request, &cc, &cursor, request, current->rhs.vpt); + if (!vp) { REDEBUG("%s[%d] Failed to find attribute reference %s", MAP_INFO, current->rhs.vpt->name); return -1; } - rhs_box = &vp->data; - -assign: - RDEBUG2("%s %s %pV", current->lhs.vpt->name, fr_tokens[map->op], rhs_box); - /* - * Don't apply the edit, as the VP is in a temporary list. The parent will actually apply it. + * Save the VP we're editing. */ - if (current->in_parent_list) { - vp = current->lhs.vp; - - return fr_value_box_cast(vp, &vp->data, vp->da->type, vp->da, rhs_box); + if (fr_edit_list_save_pair_value(current->el, current->lhs.vp) < 0) { + fail: + tmpl_dcursor_clear(&cc); + return -1; } + RDEBUG2("%s %s %s", current->lhs.vpt->name, fr_tokens[map->op], current->rhs.vpt->name); + /* - * The apply function also takes care of - * doing data type upcasting and - * conversion. So we don't have to check - * for compatibility of the data types on - * the LHS and RHS. + * Loop over all input VPs, doing the operation. */ - if (fr_edit_list_apply_pair_assignment(current->el, - current->lhs.vp, - map->op, - rhs_box) < 0) { - RPERROR("Failed performing %s operation", fr_tokens[map->op]); - return -1; + while (vp != NULL) { + int rcode; + + rcode = fr_value_calc_assignment_op(current->lhs.vp, ¤t->lhs.vp->data, map->op, &vp->data); + if (rcode < 0) goto fail; + + vp = fr_dcursor_next(&cursor); } return 0; @@ -615,6 +634,10 @@ redo: repeatable_set(frame); /* Call us again when done */ switch (current->state) { + /* + * Turn the LHS into a tmpl_t. This can involve just referencing an existing + * tmpl in map->lhs, or expanding an xlat to get an attribute name. + */ case UNLANG_EDIT_INIT: fr_assert(fr_dlist_empty(¤t->lhs.result)); /* Should have been consumed */ fr_assert(fr_dlist_empty(¤t->rhs.result)); /* Should have been consumed */ @@ -646,22 +669,18 @@ redo: case UNLANG_EDIT_EXPANDED_LHS: /* - * @todo - if current->is_leaf_list, AND lhs is an XLAT, AND the result is a - * value-box group, AND the LHS data type isn't octets/string, THEN apply each - * individual member of the group. This lets us do: + * In normal situations, the LHS is an attribute name. * - * &Tmp-String-0 := { %{sql:...} } - * - * which will assign one value to the result for each column returned by the SQL query. + * For leaf lists, the LHS is a value, so we templatize it as a value. */ if (!current->is_leaf_list) { - if (templatize_lhs(state, ¤t->lhs, request) < 0) goto error; + if (templatize_to_attribute(state, ¤t->lhs, request) < 0) goto error; } else { - if (templatize_rhs(state, ¤t->lhs, current->parent->lhs.vp, request) < 0) goto error; + if (templatize_to_value(state, ¤t->lhs, current->parent->lhs.vp, request) < 0) goto error; } current->state = UNLANG_EDIT_CHECK_LHS; - FALL_THROUGH; + goto check_lhs; case UNLANG_EDIT_CHECK_LHS: check_lhs: @@ -669,6 +688,21 @@ redo: if (!current->lhs.vpt) return -1; #endif + /* + * @todo - if current->is_leaf_list, AND lhs is an XLAT, AND the result is a + * value-box group, AND the LHS data type isn't octets/string, THEN apply each + * individual member of the group. This lets us do: + * + * &Tmp-String-0 := { %{sql:...} } + * + * which will assign one value to the result for each column returned by the SQL query. + * + * Also if we have &Tmp-String-0 := &Filter-Id[*], we should handle that, too. + * + * The easiest way is likely to just push the values into a #fr_value_box_list + * for the parent, and then don't do anything else. Once the parent leaf is + * capable of handling value-box groups, it can just do everything. + */ if (current->is_leaf_list) { fr_pair_t *vp; @@ -677,9 +711,6 @@ redo: MEM(vp = fr_pair_afrom_da(current, current->parent->lhs.vp->da)); - /* - * @todo - handle lists from xlats? - */ if (tmpl_is_data(current->lhs.vpt)) { if (fr_value_box_copy(vp, &vp->data, tmpl_value(current->lhs.vpt)) < 0) goto error; @@ -692,10 +723,6 @@ redo: goto error; } - /* - * Can only copy the same types. For now, automatic casting - * isn't supported. - */ if (ref->da->type == vp->da->type) { if (fr_value_box_copy(vp, &vp->data, &ref->data) < 0) goto error; @@ -781,6 +808,7 @@ redo: * NOOP. */ goto next; + } else { fr_pair_t *parent; request_t *other = request; @@ -788,8 +816,8 @@ redo: fr_assert(!tmpl_is_list(current->lhs.vpt)); /* - * @todo - What we really need is to create a dcursor, and then do something - * like: + * @todo - for T_OP_SET, What we really need is to create a dcursor, and + * then do something like: * * vp = tmpl_dcursor_init(&err, request, &cc, &cursor, request, vpt); * if (!vp) { @@ -885,6 +913,10 @@ redo: goto redo; } + /* + * Turn the RHS into a tmpl_t. This can involve just referencing an existing + * tmpl in map->rhs, or expanding an xlat to get an attribute name. + */ rcode = template_realize(state, ¤t->rhs.result, request, map->rhs); if (rcode < 0) goto error; @@ -902,7 +934,10 @@ redo: if (!current->lhs.vp) goto error; #endif - if (map->rhs && (templatize_rhs(state, ¤t->rhs, current->lhs.vp, request) < 0)) goto error; + /* + * Get the value of the RHS tmpl. + */ + if (map->rhs && (templatize_to_value(state, ¤t->rhs, current->lhs.vp, request) < 0)) goto error; current->state = UNLANG_EDIT_CHECK_RHS; FALL_THROUGH; diff --git a/src/tests/keywords/edit-leaf-star b/src/tests/keywords/edit-leaf-star new file mode 100644 index 00000000000..270c29e3a78 --- /dev/null +++ b/src/tests/keywords/edit-leaf-star @@ -0,0 +1,22 @@ +# +# PRE: edit +# +&request += { + &Tmp-Integer-0 = 1 + &Tmp-Integer-0 = 3 + &Tmp-Integer-0 = 5 + &Tmp-Integer-0 = 7 + &Tmp-Integer-0 = 11 +} + +&Tmp-Integer-1 := 0 + +# +# Do operations on sets of inputs. +# +&Tmp-Integer-1 += &Tmp-Integer-0[*] +if (&Tmp-Integer-1 != 27) { + test_fail +} + +success