From: Alan T. DeKok Date: Wed, 4 Sep 2024 15:11:28 +0000 (-0400) Subject: add looping over TLVs X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=69c4da742b3c0bb6e1d0c1dd2fde0f46b98dcc9c;p=thirdparty%2Ffreeradius-server.git add looping over TLVs and ensure that we can access child members. via a small amount of code which is a terrible hack. --- diff --git a/doc/antora/modules/reference/pages/unlang/foreach.adoc b/doc/antora/modules/reference/pages/unlang/foreach.adoc index 47181a7fff..8b2caaeccc 100644 --- a/doc/antora/modules/reference/pages/unlang/foreach.adoc +++ b/doc/antora/modules/reference/pages/unlang/foreach.adoc @@ -3,7 +3,7 @@ .Syntax [source,unlang] ---- -foreach () { +foreach [] () { [ statements ] } ---- @@ -14,14 +14,16 @@ xref:unlang/break.adoc[break] keyword. There is no limit on how many `foreach` statements can be nested. +:: + +An optional data tye for the `` local variable. When looping over attributes, the data type can be omitted. The data type of the local variable is then taken from the attribute reference. + :: The name of the local variable which is used as the name of key when iterating over the attributes. The local variable is created automatically when the `foreach` loop is entered, and is deleted automatically when the `foreach` loop exits. -The data type of the local variable is taken from the attribute reference. - The `` can be modified during the course of the `foreach` loop. Modifications to the variable are copied back to the referenced attribute when the loop is done. See below for an example. The only limitation on the `` is that it must be unique. @@ -68,6 +70,36 @@ loop over each i in attribute[0..n] copy the key variable back to attribute[i] ---- +=== Structural Data Types + +It is possible to loop over the children of a structural data type, as given in the example below. Since the loop is over the child (i.e. leaf) attributes, the values are copied back. + +In this example, we have to explicitly give a data type `string`. The data type is needed because there may be multiple children of the `TLV-Thing` attribute, and the children may not all have the same data type. + +.Example of Looping over children of a structural type. +[source,unlang] +---- +foreach string child (&TLV-Thing.[*]) { + &out += &child + &out += " " +} +---- + + +When using `foreach` to loop over multiple structural data types, the values can be +examined, but cannot be changed. This is a limitation of the current interpreter, and may be changed in the future. + +.Example of Looping over children of a structural type. +[source,unlang] +---- +foreach thing (&Tmp-TLV-0[*]) { + &out += &thing.c + &out += " " +} +---- + +This example can read the child attribute `c`, but cannot modify it. + // Copyright (C) 2024 Network RADIUS SAS. Licenced under CC-by-NC 4.0. // This documentation was developed by Network RADIUS SAS. diff --git a/src/lib/unlang/foreach.c b/src/lib/unlang/foreach.c index e49dec138d..b80573f8d6 100644 --- a/src/lib/unlang/foreach.c +++ b/src/lib/unlang/foreach.c @@ -65,6 +65,73 @@ typedef struct { #endif } unlang_frame_state_foreach_t; +/* + * Brute-force things instead of doing it the "right" way. + * + * We would ideally like to have the local variable be a ref to the current vp from the cursor. However, + * that isn't (yet) supported. In order to support that, we would likely have to add a new data type + * FR_TYPE_DCURSOR, and put the cursor into in vp->vp_ptr. We would then have to update a lot of things: + * + * - the foreach code has to put the dcursor into state->key->vp_ptr. + * - the pair code (all of it, perhaps) has to check for "is this thing a cursor), and if so + * return the next pair from the cursor instead of the given pair. This is a huge change. + * - update all of the pair / value-box APIs to handle the new data type + * - check performance, etc, and that nothing else breaks. + * - also need to ensure that the pair with the cursor _cannot_ be copied, as that would add two + * refs to the cursor. + * - if we're lucky, we could perhaps _instead_ update only the tmpl code, but the cursor + * still has to be in the pair. + * - we can update tmpl_eval_pair(), because that's what's used in the xlat code. That gets us all + * references to the _source_ VP. + * - we also have to update the edit.c code, which calls tmpl_dcursor_init() to get pairs from + * a tmpl_t of type ATTR. + * - for LHS assignment, the edit code has to be updated: apply_edits_to_leaf() and apply_edits_to_list() + * which calls fr_edit_list_apply_pair_assignment() to do the actual work. But we could likely just + * check current->lhs.vp, and dereference that to get the underlying thing. + * + * What we ACTUALLY do instead is in the compiler when we call define_local_variable(), we clone the "da" + * hierarchy via fr_dict_attr_acopy_local(). That function which should go away when we add refs. + * + * Then this horrific function copies the pairs by number, which re-parents them to the correct + * destination da. It's brute-force and expensive, but it's easy. And for now, it's less work than + * re-doing substantial parts of the server core and utility libraries. + */ +static int unlang_foreach_pair_copy(fr_pair_t *to, fr_pair_t *from, fr_dict_attr_t const *from_parent) +{ + fr_assert(fr_type_is_structural(to->vp_type)); + fr_assert(fr_type_is_structural(from->vp_type)); + + fr_pair_list_foreach(&from->vp_group, vp) { + fr_pair_t *child; + + /* + * We only copy children of the parent TLV, but we can copy internal attributes, as they + * can exist anywhere. + */ + if (vp->da->parent != from_parent) { + if (vp->da->flags.internal) { + child = fr_pair_copy(to, vp); + if (child) fr_pair_append(&to->vp_group, child); + } + continue; + } + + child = fr_pair_afrom_child_num(to, to->da, vp->da->attr); + if (!child) continue; + + fr_pair_append(&to->vp_group, child); + + if (fr_type_is_leaf(child->vp_type)) { + if (fr_value_box_copy(child, &child->data, &vp->data) < 0) return -1; + continue; + } + + if (unlang_foreach_pair_copy(child, vp, vp->da) < 0) return -1; + } + + return 0; +} + static xlat_action_t unlang_foreach_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out, xlat_ctx_t const *xctx, request_t *request, UNUSED fr_value_box_list_t *in); @@ -183,14 +250,28 @@ next: /* * Copy the data. */ - if (fr_type_is_structural(vp->vp_type)) { + if (vp->vp_type == FR_TYPE_GROUP) { + fr_assert(state->key->vp_type == FR_TYPE_GROUP); + fr_pair_list_free(&state->key->vp_group); if (fr_pair_list_copy(state->key, &state->key->vp_group, &vp->vp_group) < 0) { + REDEBUG("Failed copying members of %s", state->key->da->name); + *p_result = RLM_MODULE_FAIL; + return UNLANG_ACTION_CALCULATE_RESULT; + } + + } else if (fr_type_is_structural(vp->vp_type)) { + fr_assert(state->key->vp_type == vp->vp_type); + + fr_pair_list_free(&state->key->vp_group); + + if (unlang_foreach_pair_copy(state->key, vp, vp->da) < 0) { REDEBUG("Failed copying children of %s", state->key->da->name); *p_result = RLM_MODULE_FAIL; return UNLANG_ACTION_CALCULATE_RESULT; } + } else { fr_value_box_clear_value(&state->key->data); if (fr_value_box_cast(state->key, &state->key->data, state->key->vp_type, state->key->da, &vp->data) < 0) { @@ -276,12 +357,24 @@ static unlang_action_t unlang_foreach(rlm_rcode_t *p_result, request_t *request, } fr_assert(state->key != NULL); - if (fr_type_is_structural(vp->vp_type)) { + if (vp->vp_type == FR_TYPE_GROUP) { + fr_assert(state->key->vp_type == FR_TYPE_GROUP); + if (fr_pair_list_copy(state->key, &state->key->vp_group, &vp->vp_group) < 0) { - REDEBUG("Failed copying children of %s", gext->key->name); + REDEBUG("Failed copying members of %s", gext->key->name); + *p_result = RLM_MODULE_FAIL; + return UNLANG_ACTION_CALCULATE_RESULT; + } + + } else if (fr_type_is_structural(vp->vp_type)) { + fr_assert(state->key->vp_type == vp->vp_type); + + if (unlang_foreach_pair_copy(state->key, vp, vp->da) < 0) { + REDEBUG("Failed copying children of %s", state->key->da->name); *p_result = RLM_MODULE_FAIL; return UNLANG_ACTION_CALCULATE_RESULT; } + } else { fr_value_box_clear_value(&state->key->data); while (vp && (fr_value_box_cast(state->key, &state->key->data, state->key->vp_type, state->key->da, &vp->data) < 0)) { diff --git a/src/tests/keywords/foreach-tlv.ignore b/src/tests/keywords/foreach-tlv similarity index 56% rename from src/tests/keywords/foreach-tlv.ignore rename to src/tests/keywords/foreach-tlv index f6c8aab895..9fb4307c8e 100644 --- a/src/tests/keywords/foreach-tlv.ignore +++ b/src/tests/keywords/foreach-tlv @@ -23,17 +23,24 @@ string out } } - &out = "" # -# Home-brew concat! +# Don't use "tlv" as the attribute name, because "tlv" is a data type. # -foreach child (&Tmp-TLV-0[*]) { - &out += &child.c +foreach thing (&Tmp-TLV-0[*]) { + &out += &thing.c &out += " " } &out -= " " -"%{out}" +# +# And we have updated the string from multiple TLVs, and one particular +# child from each TLV. +# +if (&out != 'foo bar') { + test_fail +} + +success