From: Alan T. DeKok Date: Sun, 22 Oct 2023 19:48:00 +0000 (-0400) Subject: move test to new edit framework X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0bea2b1a86150393ad1241bcfbfb63c465a74298;p=thirdparty%2Ffreeradius-server.git move test to new edit framework and fix issue where &foo := &non-existent was different from &foo := {} or %foo := %function_that_returns_nothing() --- diff --git a/src/lib/unlang/edit.c b/src/lib/unlang/edit.c index 142478102b5..362764a4599 100644 --- a/src/lib/unlang/edit.c +++ b/src/lib/unlang/edit.c @@ -493,6 +493,77 @@ done: return rcode; } +static int edit_set_eq(request_t *request, edit_map_t *current, bool delete) +{ + tmpl_dcursor_ctx_t cc; + fr_dcursor_t cursor; + bool first = fr_type_is_structural(tmpl_attr_tail_da(current->lhs.vpt)->type); + + while (true) { + int err; + fr_pair_t *vp, *parent; + + /* + * Reinitialize the cursor for every VP. This is because + * fr_dcursor_remove() does not work with tmpl_dcursors, as the + * tmpl_dcursor code does not set the "remove" callback. + * + * Once that's implemented, we also need to update the edit list API to + * allow for "please delete children"? + */ + vp = tmpl_dcursor_init(&err, current->ctx, &cc, &cursor, request, current->lhs.vpt); + if (!vp) break; + + /* + * For structural attributes, we leave the first one, and delete the subsequent + * ones. That way we leave the main lists alone ("request", "reply", "control", etc.) + * + * For leaf attributes, we just skip this step, as "first" is always "false". + */ + if (first) { + first = false; + if (fr_edit_list_free_pair_children(current->el, vp) < 0) return -1; + vp = fr_dcursor_next(&cursor); + if (!vp) goto clear; + continue; + + } else if (fr_type_is_structural(tmpl_attr_tail_da(current->lhs.vpt)->type)) { + /* + * We skipped the first structural member, so keep skipping it for all of the next vps. + */ + vp = fr_dcursor_next(&cursor); + if (!vp) { + clear: + tmpl_dcursor_clear(&cc); + break; + } + } + + parent = fr_pair_parent(vp); + fr_assert(parent != NULL); + + /* + * We can't delete these ones. + */ + fr_assert(vp != request->pair_list.request); + fr_assert(vp != request->pair_list.reply); + fr_assert(vp != request->pair_list.control); + fr_assert(vp != request->pair_list.state); + + /* + * Delete if we're not over-riding a particular value. + */ + if (delete) { + if (fr_edit_list_pair_delete(current->el, &parent->vp_group, vp) < 0) return -1; + tmpl_dcursor_clear(&cc); + } else { + goto clear; + } + } + + return 0; +} + /* * Apply the edits to a leaf attribute. First we figure out where the results come from: * @@ -592,7 +663,10 @@ static int apply_edits_to_leaf(request_t *request, unlang_frame_state_edit_t *st vp = tmpl_dcursor_init(&err, request, &cc, &pair_cursor, request, current->rhs.vpt); if (!vp) { tmpl_dcursor_clear(&cc); - return 0; + + if (map->op != T_OP_SET) return 0; + + return edit_set_eq(request, current, true); } box = fr_pair_dcursor_nested_init(&cursor, &pair_cursor); // the list is unused @@ -867,65 +941,11 @@ static int check_rhs(request_t *request, unlang_frame_state_edit_t *state, edit_ * * The we just apply the assignment to the LHS, over-writing it's value. */ - if ((map->op == T_OP_SET) && ((tmpl_attr_tail_num(current->lhs.vpt) == NUM_UNSPEC) || !current->map->rhs)) { - tmpl_dcursor_ctx_t cc; - fr_dcursor_t cursor; - bool first = fr_type_is_structural(tmpl_attr_tail_da(current->lhs.vpt)->type); - - while (true) { - int err; - fr_pair_t *vp, *parent; - - /* - * Reinitialize the cursor for every VP. This is because - * fr_dcursor_remove() does not work with tmpl_dcursors, as the - * tmpl_dcursor code does not set the "remove" callback. - * - * Once that's implemented, we also need to update the edit list API to - * allow for "please delete children"? - */ - vp = tmpl_dcursor_init(&err, current->ctx, &cc, &cursor, request, current->lhs.vpt); - if (!vp) break; - - /* - * For structural attributes, we leave the first one, and delete the subsequent - * ones. That way we leave the main lists alone ("request", "reply", "control", etc.) - * - * For leaf attributes, we just skip this step, as "first" is always "false". - */ - if (first) { - first = false; - if (fr_edit_list_free_pair_children(current->el, vp) < 0) return -1; - vp = fr_dcursor_next(&cursor); - if (!vp) goto clear; - continue; - - } else if (fr_type_is_structural(tmpl_attr_tail_da(current->lhs.vpt)->type)) { - /* - * We skipped the first structural member, so keep skipping it for all of the next vps. - */ - vp = fr_dcursor_next(&cursor); - if (!vp) { - clear: - tmpl_dcursor_clear(&cc); - break; - } - } - - parent = fr_pair_parent(vp); - fr_assert(parent != NULL); - - /* - * We can't delete these ones. - */ - fr_assert(vp != request->pair_list.request); - fr_assert(vp != request->pair_list.reply); - fr_assert(vp != request->pair_list.control); - fr_assert(vp != request->pair_list.state); - - if (fr_edit_list_pair_delete(current->el, &parent->vp_group, vp) < 0) return -1; - tmpl_dcursor_clear(&cc); - } + if ((map->op == T_OP_SET) && + ((tmpl_attr_tail_num(current->lhs.vpt) == NUM_UNSPEC) || (tmpl_attr_tail_num(current->lhs.vpt) > 0) || + !current->map->rhs)) { + if (edit_set_eq(request, current, + (tmpl_attr_tail_num(current->lhs.vpt) == NUM_UNSPEC) || !current->map->rhs) < 0) return -1; } /* @@ -1495,8 +1515,6 @@ static unlang_action_t process_edit(rlm_rcode_t *p_result, request_t *request, u XDEBUG("MAP %s ...", state->current->map->lhs->name); } else { XDEBUG("MAP %s ... %s", state->current->map->lhs->name, state->current->map->rhs->name); - - XDEBUG("\t%08x", state->current->map->rhs->type); } rcode = state->current->func(request, state, state->current); diff --git a/src/tests/keywords/all.mk b/src/tests/keywords/all.mk index d304b784229..080890dc85b 100644 --- a/src/tests/keywords/all.mk +++ b/src/tests/keywords/all.mk @@ -65,7 +65,7 @@ endif # # Tests for the "update" keyword -KEYWORD_UPDATE_TESTS := update-attr-ref-null update-error-3 update-group-error update-null-value-assign update-remove-index update-filter vendor-specific-error +KEYWORD_UPDATE_TESTS := update-error-3 update-group-error update-null-value-assign update-remove-index update-filter vendor-specific-error # Tests for rewriting "udpate" KEYWORD_UPDATE_REWRITE_TESTS := update-all update-array update-delete update-remove-any update-group update-hex update-remove-value update-index update-list-error update-remove-list update-prepend unknown-update update-error update-error-2 update-exec-error update-list-null-rhs update-exec diff --git a/src/tests/keywords/edit-attr-ref-null b/src/tests/keywords/edit-attr-ref-null index 81cce2f3ad3..1484a2ebf52 100644 --- a/src/tests/keywords/edit-attr-ref-null +++ b/src/tests/keywords/edit-attr-ref-null @@ -3,13 +3,14 @@ # # Form attribute references with xlats # -update request { - &Tmp-String-0 += 'foo' - &Tmp-String-0 += 'bar' - &Tmp-String-1 += 'baz' - &control.[*] !* ANY +&request += { + &Tmp-String-0 = 'foo' + &Tmp-String-0 = 'bar' + &Tmp-String-1 = 'baz' } +&control = {} + if (!(%{Tmp-String-0[#]} == 2)) { test_fail } @@ -18,16 +19,17 @@ if (!(&Tmp-String-0[0] == 'foo')) { test_fail } -# Delete an attribute by assigning a non-existent attribute to it # -# @fixme - EDIT - the new method is to simply omit the assignment +# Delete an attribute by assigning a non-existent attribute to it # -update { - &Tmp-String-0[1] := &Reply-Message -} +&Tmp-String-0[1] := &Reply-Message # Should only remove 'bar' -if (!(&Tmp-String-0[0] == 'foo')) { +if !(&Tmp-String-0[0] == 'foo') { + test_fail +} + +if (!(&Tmp-String-0[#] == 1)) { test_fail } @@ -40,9 +42,7 @@ if (&Tmp-String-0[2]) { test_fail } -update { - &Tmp-String-0 := &Reply-Message -} +&Tmp-String-0 := &Reply-Message # All instances should be removed if (&Tmp-String-0) {