]> git.ipfire.org Git - thirdparty/freeradius-server.git/commitdiff
Fix breaking out of foreach sections
authorArran Cudbard-Bell <a.cudbardb@freeradius.org>
Fri, 3 Dec 2021 14:23:51 +0000 (08:23 -0600)
committerArran Cudbard-Bell <a.cudbardb@freeradius.org>
Sat, 4 Dec 2021 19:21:55 +0000 (14:21 -0500)
Not entirely sure how this ever worked, but the code is now a lot more explicit, and clears unwind flags properly after reaching a frame that consumes them.

src/lib/unlang/foreach.c
src/lib/unlang/interpret.c
src/lib/unlang/unlang_priv.h

index c1ed3d90df5b41abdea55b73eb23ffa543aad09b..bcd9775ebc6cb8d1d5a81825c6d227d8ee8eac48 100644 (file)
@@ -80,6 +80,9 @@ static unlang_action_t unlang_foreach_next(rlm_rcode_t *p_result, request_t *req
        fr_pair_t                       *vp;
        unlang_frame_state_foreach_t    *foreach = talloc_get_type_abort(frame->state, unlang_frame_state_foreach_t);
        unlang_group_t                  *g = unlang_generic_to_group(frame->instruction);
+
+       if (is_stack_unwinding_to_break(request->stack)) return UNLANG_ACTION_CALCULATE_RESULT;
+
        vp = fr_dcursor_current(&foreach->cursor);
        if (!vp) {
                *p_result = frame->result;
index f0485e55d0eb519d47a343736a5fb7f5d6cc4c11..c748cc6fea4f8bb9bd9d85fd04df8d354731e8fb 100644 (file)
@@ -99,20 +99,42 @@ static void frame_dump(request_t *request, unlang_stack_frame_t *frame)
        REXDENT();
 }
 
+static char *stack_unwind_flag_dump(uint8_t unwind)
+{
+       static __thread char buf[256];
+       size_t len;
+
+#define UNWIND_FLAG_DUMP(attrib) \
+       if (unwind & attrib) strcat(buf, #attrib" ");
+
+       snprintf(buf, sizeof(buf), "unwind=0x%x (", unwind);
+
+       UNWIND_FLAG_DUMP(UNWIND_FLAG_TOP_FRAME);
+       UNWIND_FLAG_DUMP(UNWIND_FLAG_BREAK_POINT);
+       UNWIND_FLAG_DUMP(UNWIND_FLAG_RETURN_POINT);
+
+       len = strlen(buf);
+       if (buf[len - 1] == ' ') buf[len - 1] = '\0';    /* Trim trailing space */
+       strcat(buf, ")");
+
+#undef UNWIND_FLAG_DUMP
+
+       return buf;
+}
+
 static void stack_dump(request_t *request)
 {
        int i;
        unlang_stack_t *stack = request->stack;
 
-       RDEBUG2("----- Begin stack debug [depth %i, unwind %i] -----", stack->depth, stack->unwind);
+       RDEBUG2("----- Begin stack debug [depth %i, %s] -----", stack->depth, stack_unwind_flag_dump(stack->unwind));
        for (i = stack->depth; i >= 0; i--) {
                unlang_stack_frame_t *frame = &stack->frame[i];
 
                RDEBUG2("[%d] Frame contents", i);
                frame_dump(request, frame);
        }
-
-       RDEBUG2("----- End stack debug [depth %i, unwind %i] -------", stack->depth, stack->unwind);
+       RDEBUG2("----- End stack debug [depth %i, %s] -------", stack->depth, stack_unwind_flag_dump(stack->unwind));
 }
 #define DUMP_STACK if (DEBUG_ENABLED5) stack_dump(request)
 #else
@@ -477,6 +499,24 @@ unlang_frame_action_t frame_eval(request_t *request, unlang_stack_frame_t *frame
                repeatable_clear(frame);
                ua = frame->process(result, request, frame);
 
+               /*
+                *      If this frame is breaking or returning
+                *      frame then clear that unwind flag,
+                *      it's been consumed by this call.
+                *
+                *      We leave the unwind flags for the eval
+                *      call so that the process function knows
+                *      that the stack is being unwound.
+                */
+               if (is_break_point(frame)) {
+                       stack_unwind_break_clear(stack);
+                       stack_unwind_top_frame_clear(stack);
+               }
+               if (is_return_point(frame)) {
+                       stack_unwind_return_clear(stack);
+                       stack_unwind_top_frame_clear(stack);
+               }
+
                RDEBUG4("** [%i] %s << %s (%d)", stack->depth, __FUNCTION__,
                        fr_table_str_by_value(unlang_action_table, ua, "<INVALID>"), *priority);
 
@@ -672,7 +712,7 @@ CC_HINT(hot) rlm_rcode_t unlang_interpret(request_t *request)
                        /*
                         *      Head on back up the stack
                         */
-                       frame_pop(stack);
+                       frame_pop(request, stack);
                        frame = &stack->frame[stack->depth];
                        DUMP_STACK;
 
@@ -1386,7 +1426,7 @@ unlang_interpret_t *unlang_interpret_init(TALLOC_CTX *ctx,
  */
 void unlang_interpet_frame_discard(request_t *request)
 {
-       frame_pop(request->stack);
+       frame_pop(request, request->stack);
 }
 
 /** Set a specific interpreter for a request
index 67770b45ef9aff8d2e8625004013352a4d1f3d4d..1ee51e50284227c875d9eac346107e3e4cd0eb67 100644 (file)
@@ -365,6 +365,13 @@ static inline unlang_action_t unwind_all(unlang_stack_t *stack)
        return UNLANG_ACTION_UNWIND;
 }
 
+static inline bool is_stack_unwinding_to_top_frame(unlang_stack_t *stack)      { return stack->unwind & UNWIND_FLAG_TOP_FRAME; }
+static inline bool is_stack_unwinding_to_break(unlang_stack_t *stack)          { return stack->unwind & UNWIND_FLAG_BREAK_POINT; }
+static inline bool is_stack_unwinding_to_return(unlang_stack_t *stack)         { return stack->unwind & UNWIND_FLAG_RETURN_POINT; }
+static inline void stack_unwind_top_frame_clear(unlang_stack_t *stack)         { stack->unwind &= ~UNWIND_FLAG_TOP_FRAME; }
+static inline void stack_unwind_break_clear(unlang_stack_t *stack)             { stack->unwind &= ~UNWIND_FLAG_BREAK_POINT; }
+static inline void stack_unwind_return_clear(unlang_stack_t *stack)            { stack->unwind &= ~UNWIND_FLAG_RETURN_POINT; }
+
 static inline unlang_stack_frame_t *frame_current(request_t *request)
 {
        unlang_stack_t *stack = request->stack;
@@ -458,9 +465,10 @@ static inline void frame_next(unlang_stack_t *stack, unlang_stack_frame_t *frame
 
 /** Pop a stack frame, removing any associated dynamically allocated state
  *
+ * @param[in] request  The current request.
  * @param[in] stack    frame to pop.
  */
-static inline void frame_pop(unlang_stack_t *stack)
+static inline void frame_pop(request_t *request, unlang_stack_t *stack)
 {
        unlang_stack_frame_t *frame;
 
@@ -481,7 +489,14 @@ static inline void frame_pop(unlang_stack_t *stack)
 
        frame = &stack->frame[--stack->depth];
 
-       if (stack->unwind && is_repeatable(frame) && !is_break_point(frame) && !is_return_point(frame)) {
+       /*
+        *      Signal the frame to get it back into a consistent state
+        *      as we won't be calling the resume function.
+        */
+       if (stack->unwind && is_repeatable(frame) &&
+           ((is_stack_unwinding_to_break(stack) && !is_break_point(frame)) ||
+            (is_stack_unwinding_to_return(stack) && !is_return_point(frame)))) {
+               if (frame->signal) frame->signal(request, frame, FR_SIGNAL_CANCEL);
                repeatable_clear(frame);
        }
 }