]> git.ipfire.org Git - thirdparty/freeradius-server.git/commitdiff
Only update request->rcode for a few unlang_ops
authorArran Cudbard-Bell <a.cudbardb@freeradius.org>
Sun, 19 May 2024 01:10:52 +0000 (19:10 -0600)
committerArran Cudbard-Bell <a.cudbardb@freeradius.org>
Sun, 19 May 2024 17:12:22 +0000 (11:12 -0600)
src/lib/unlang/call.c
src/lib/unlang/condition.c
src/lib/unlang/interpret.c
src/lib/unlang/load_balance.c
src/lib/unlang/map.c
src/lib/unlang/module.c
src/lib/unlang/parallel.c
src/lib/unlang/subrequest.c
src/lib/unlang/unlang_priv.h
src/tests/keywords/if-elsif-rcode [new file with mode: 0644]
src/tests/keywords/subrequest-rcode [new file with mode: 0644]

index 1e4c03e19ef8e0488fa80090bb60e59d4ab36aef..68d0e4bdda18d332034a04ac6ec03659b5517992 100644 (file)
@@ -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,
                           });
 }
-
index 27d00e0e43afffcd24813269d653ac24c5431465..26325f366177232b53a36fecfc6db5fc4d6a8f93 100644 (file)
@@ -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;
 
index a8ba0eaa51adcc722e68333588e692af2f8dc2d8..3817ea0179ccf28a89d73ff72ead270b8a4bbbec 100644 (file)
@@ -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, "<invalid>"),
                *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, "<INVALID>"));
+               request->rcode = *result;
+       }
+
        /*
         *      Don't set action or priority if we don't have one.
         */
index edef045877585838f2d2a214e9382de77f25a457..37742b3bcc9d7e509030a2241d082a18ee2a2f58 100644 (file)
@@ -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",
index 5c65cef060a3a7e819eb8c35fafc447382323e72..87a3f2547940f2524c5c123ffc6c9cc5ce6d3e10 100644 (file)
@@ -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",
index 0a18ac9b9dd9cabfa5dc2afed5bfaf3e84debd93..ef256b622c23dd466faab02f1eb42ec13e29c0c2 100644 (file)
@@ -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",
index db419c166c597fcbe94de8b3300e3f9eae3b8399..f3ec4650a1b416fee4686f8e66c35f41b26b1727 100644 (file)
@@ -497,6 +497,7 @@ void unlang_parallel_init(void)
                                .name = "parallel",
                                .interpret = unlang_parallel,
                                .signal = unlang_parallel_signal,
+                               .rcode_set = true,
                                .debug_braces = true
                           });
 }
index 105a806e07dce1ed415391142632d0ca6d9a1365..35c8e18c71d17cfa0644fa5f01de83c1c76ed96a 100644 (file)
@@ -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",
index ae9087c7078df7b73cb2646b1eaa86d8b48f0921..7ce023a20285895c77791290cb4f8dceeff6524f 100644 (file)
@@ -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 (file)
index 0000000..54fb45d
--- /dev/null
@@ -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 (file)
index 0000000..ff85da2
--- /dev/null
@@ -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
+}