add more tests and clean up foreach assert

diff --git a/src/lib/unlang/foreach.c b/src/lib/unlang/foreach.c
index d6107549225139575a002a6b4ed5ef5cee6053ae..5da74d0a372984be6fe4a7af40a651870d2dbda5 100644 (file)
--- a/src/lib/unlang/foreach.c
+++ b/src/lib/unlang/foreach.c
@@ -256,7 +256,7 @@ next:
                *p_result = frame->result;
                return UNLANG_ACTION_CALCULATE_RESULT;
        }
-       
+
        if (unlang_foreach_xlat_key_update(request, state) < 0) goto next;
 
        fr_value_box_clear_value(&state->value->data);
@@ -418,7 +418,7 @@ next:
                }
 
        } else if (fr_type_is_structural(vp->vp_type)) {
-               fr_assert(state->value->vp_type == vp->vp_type);
+               if (state->value->vp_type != vp->vp_type) goto next;
 
                fr_pair_list_free(&state->value->vp_group);
 
@@ -486,6 +486,7 @@ static unlang_action_t unlang_foreach_attr_init(rlm_rcode_t *p_result, request_t
        vp = tmpl_dcursor_init(NULL, NULL, &state->cc, &state->cursor, request, state->vpt);
        fr_assert(vp != NULL);
 
+next:
        /*
         *      Update the key with the current path.  Attribute indexes start at zero.
         */
@@ -500,13 +501,17 @@ static unlang_action_t unlang_foreach_attr_init(rlm_rcode_t *p_result, request_t
                }
 
        } else if (fr_type_is_structural(vp->vp_type)) {
-               if (state->value->vp_type == vp->vp_type) {
-                       if (unlang_foreach_pair_copy(state->value, vp, vp->da) < 0) {
-                               REDEBUG("Failed copying children of %s", state->value->da->name);
-                               goto fail;
-                       }
-               } else {
-                       REDEBUG("Failed initializing loop variable %s - expected %s type, but got input (%pP)", state->value->da->name, fr_type_to_str(state->value->vp_type), vp);
+               if (state->value->vp_type != vp->vp_type) {
+                       vp = fr_dcursor_next(&state->cursor);
+                       if (vp) goto next;
+
+                       *p_result = frame->result;
+                       fr_assert(state->indent == request->log.indent.unlang);
+                       return UNLANG_ACTION_CALCULATE_RESULT;
+               }
+
+               if (unlang_foreach_pair_copy(state->value, vp, vp->da) < 0) {
+                       REDEBUG("Failed copying children of %s", state->value->da->name);
                        goto fail;
                }
 

diff --git a/src/tests/keywords/foreach-list b/src/tests/keywords/foreach-list
new file mode 100644 (file)
index 0000000..17c8176
--- /dev/null
+++ b/src/tests/keywords/foreach-list
@@ -0,0 +1,19 @@
+#
+#  PRE: foreach
+#
+string total
+
+#
+#  Try to delete one of the variables we're looping over.  If the user could delete one,
+#  then the underlying tmpl_dcursor would crash, as it didn't know about the deletion.
+#
+foreach string value (request.[*]) {
+       total += value
+       total += ', '
+}
+
+if (total != "bob, hello, Access-Request, ") {
+       test_fail
+}
+
+success