From: Arran Cudbard-Bell Date: Sun, 19 May 2024 01:10:52 +0000 (-0600) Subject: Only update request->rcode for a few unlang_ops X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=845b478fa1d6f96b9dc418aa94a608fe4c7f66cc;p=thirdparty%2Ffreeradius-server.git Only update request->rcode for a few unlang_ops --- diff --git a/src/lib/unlang/call.c b/src/lib/unlang/call.c index 1e4c03e19ef..68d0e4bdda1 100644 --- a/src/lib/unlang/call.c +++ b/src/lib/unlang/call.c @@ -29,7 +29,7 @@ RCSID("$Id$") #include "call_priv.h" -static unlang_action_t unlang_call_resume(rlm_rcode_t *p_result, request_t *request, +static unlang_action_t unlang_call_resume(UNUSED rlm_rcode_t *p_result, request_t *request, unlang_stack_frame_t *frame) { unlang_group_t *g = unlang_generic_to_group(frame->instruction); @@ -252,7 +252,7 @@ void unlang_call_init(void) &(unlang_op_t){ .name = "call", .interpret = unlang_call_frame_init, + .rcode_set = true, .debug_braces = true, }); } - diff --git a/src/lib/unlang/condition.c b/src/lib/unlang/condition.c index 27d00e0e43a..26325f36617 100644 --- a/src/lib/unlang/condition.c +++ b/src/lib/unlang/condition.c @@ -94,13 +94,6 @@ static unlang_action_t unlang_if(rlm_rcode_t *p_result, request_t *request, unla fr_value_box_list_init(&state->out); - /* - * Make the rcode available to the caller. Note that the caller can't call - * unlang_interpret_stack_result(), as that returns the result from the xlat frame, and not from - * the calling frame. - */ - request->rcode = *p_result; - if (unlang_xlat_push(state, &state->success, &state->out, request, gext->head, UNLANG_SUB_FRAME) < 0) return UNLANG_ACTION_FAIL; diff --git a/src/lib/unlang/interpret.c b/src/lib/unlang/interpret.c index a8ba0eaa51a..3817ea0179c 100644 --- a/src/lib/unlang/interpret.c +++ b/src/lib/unlang/interpret.c @@ -317,6 +317,16 @@ unlang_frame_action_t result_calculate(request_t *request, unlang_stack_frame_t fr_table_str_by_value(mod_rcode_table, *result, ""), *priority); + /* + * Update request->rcode if the instruction says we should + * We don't care about priorities for this. + */ + if (unlang_ops[instruction->type].rcode_set) { + RDEBUG3("Setting rcode to '%s'", + fr_table_str_by_value(rcode_table, *result, "")); + request->rcode = *result; + } + /* * Don't set action or priority if we don't have one. */ diff --git a/src/lib/unlang/load_balance.c b/src/lib/unlang/load_balance.c index edef0458775..37742b3bcc9 100644 --- a/src/lib/unlang/load_balance.c +++ b/src/lib/unlang/load_balance.c @@ -251,6 +251,7 @@ void unlang_load_balance_init(void) &(unlang_op_t){ .name = "load-balance group", .interpret = unlang_load_balance, + .rcode_set = true, .debug_braces = true, .frame_state_size = sizeof(unlang_frame_state_redundant_t), .frame_state_type = "unlang_frame_state_redundant_t", @@ -260,6 +261,7 @@ void unlang_load_balance_init(void) &(unlang_op_t){ .name = "redundant-load-balance group", .interpret = unlang_redundant_load_balance, + .rcode_set = true, .debug_braces = true, .frame_state_size = sizeof(unlang_frame_state_redundant_t), .frame_state_type = "unlang_frame_state_redundant_t", diff --git a/src/lib/unlang/map.c b/src/lib/unlang/map.c index 5c65cef060a..87a3f254794 100644 --- a/src/lib/unlang/map.c +++ b/src/lib/unlang/map.c @@ -383,6 +383,7 @@ void unlang_map_init(void) unlang_register(UNLANG_TYPE_MAP, &(unlang_op_t){ .name = "map", + .rcode_set = true, .interpret = unlang_map_state_init, .frame_state_size = sizeof(unlang_frame_state_map_proc_t), .frame_state_type = "unlang_frame_state_map_proc_t", diff --git a/src/lib/unlang/module.c b/src/lib/unlang/module.c index 0a18ac9b9dd..ef256b622c2 100644 --- a/src/lib/unlang/module.c +++ b/src/lib/unlang/module.c @@ -1069,6 +1069,7 @@ void unlang_module_init(void) &(unlang_op_t){ .name = "module", .interpret = unlang_module, + .rcode_set = true, .signal = unlang_module_signal, .frame_state_size = sizeof(unlang_frame_state_module_t), .frame_state_type = "unlang_frame_state_module_t", diff --git a/src/lib/unlang/parallel.c b/src/lib/unlang/parallel.c index db419c166c5..f3ec4650a1b 100644 --- a/src/lib/unlang/parallel.c +++ b/src/lib/unlang/parallel.c @@ -497,6 +497,7 @@ void unlang_parallel_init(void) .name = "parallel", .interpret = unlang_parallel, .signal = unlang_parallel_signal, + .rcode_set = true, .debug_braces = true }); } diff --git a/src/lib/unlang/subrequest.c b/src/lib/unlang/subrequest.c index 105a806e07d..35c8e18c71d 100644 --- a/src/lib/unlang/subrequest.c +++ b/src/lib/unlang/subrequest.c @@ -299,6 +299,7 @@ int unlang_subrequest_op_init(void) .name = "subrequest", .interpret = unlang_subrequest_parent_init, .signal = unlang_subrequest_signal_child, + .rcode_set = true, .debug_braces = true, .frame_state_size = sizeof(unlang_frame_state_subrequest_t), .frame_state_type = "unlang_frame_state_subrequest_t", diff --git a/src/lib/unlang/unlang_priv.h b/src/lib/unlang/unlang_priv.h index ae9087c7078..7ce023a2028 100644 --- a/src/lib/unlang/unlang_priv.h +++ b/src/lib/unlang/unlang_priv.h @@ -218,6 +218,8 @@ typedef struct { bool debug_braces; //!< Whether the operation needs to print braces ///< in debug mode. + bool rcode_set; //!< Set request->rcode to the result of this operation. + size_t frame_state_size; //!< size of instance data in the stack frame char const *frame_state_type; //!< talloc name of the frame instance data diff --git a/src/tests/keywords/if-elsif-rcode b/src/tests/keywords/if-elsif-rcode new file mode 100644 index 00000000000..54fb45dda60 --- /dev/null +++ b/src/tests/keywords/if-elsif-rcode @@ -0,0 +1,12 @@ +# PRE: if if-else + +# This is a regression test. We saw the request->rcode being lost after the first condition + +notfound +if (ok || updated) { + test_fail +} elsif (!notfound) { + test_fail +} + +success diff --git a/src/tests/keywords/subrequest-rcode b/src/tests/keywords/subrequest-rcode new file mode 100644 index 00000000000..ff85da29400 --- /dev/null +++ b/src/tests/keywords/subrequest-rcode @@ -0,0 +1,16 @@ + +# request->rcode is updated to the result of the subrequest section +subrequest Access-Request { + ok { + ok = 10 + } + noop { + fail = 1 + } +} + +if (ok) { + success +} elsif (noop) { + test_fail +}