Skip to content

Commit

Permalink
move test to new edit framework
Browse files Browse the repository at this point in the history
and fix issue where

	&foo := &non-existent

was different from

	&foo := {}

or

	%foo := %function_that_returns_nothing()
  • Loading branch information
alandekok committed Oct 22, 2023
1 parent 0fca014 commit 0bea2b1
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 77 deletions.
142 changes: 80 additions & 62 deletions src/lib/unlang/edit.c
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,77 @@ static int apply_edits_to_list(request_t *request, unlang_frame_state_edit_t *st
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:
*
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}

/*
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/tests/keywords/all.mk
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
28 changes: 14 additions & 14 deletions src/tests/keywords/edit-attr-ref-null
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}

Expand All @@ -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) {
Expand Down

0 comments on commit 0bea2b1

Please sign in to comment.