From: Arran Cudbard-Bell <a.cudbardb@freeradius.org>
Date: Sun, 1 Jun 2025 01:42:01 +0000 (-0600)
Subject: Make discarding function rcodes official
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=54aa73da0daaa252c719fb8223c8a1ea1a7b3bf0;p=thirdparty%2Ffreeradius-server.git

Make discarding function rcodes official
---

diff --git a/src/lib/unlang/function.c b/src/lib/unlang/function.c
index 623763810ae..a0da288df1c 100644
--- a/src/lib/unlang/function.c
+++ b/src/lib/unlang/function.c
@@ -50,16 +50,24 @@ static unlang_t function_instruction = {
 	.name = "function",
 	.debug_name = "function",
 	.actions = {
+		/*
+		 *	By default, functions don't change the section rcode.
+		 *	We can't make generalisations about what the intent
+		 *	of the function callbacks are, so isntead of having
+		 *	implicit, confusing behaviour, we always discard the
+		 *	rcode UNLESS the function explicitly sets it.
+		 */
 		.actions = {
-			[RLM_MODULE_REJECT]	= 0,
-			[RLM_MODULE_FAIL]	= 0,
-			[RLM_MODULE_OK]		= 0,
-			[RLM_MODULE_HANDLED]	= 0,
-			[RLM_MODULE_INVALID]	= 0,
-			[RLM_MODULE_DISALLOW]	= 0,
-			[RLM_MODULE_NOTFOUND]	= 0,
-			[RLM_MODULE_NOOP]	= 0,
-			[RLM_MODULE_UPDATED]	= 0
+			[RLM_MODULE_REJECT]	= MOD_ACTION_NOT_SET,
+			[RLM_MODULE_FAIL]	= MOD_ACTION_NOT_SET,
+			[RLM_MODULE_OK]		= MOD_ACTION_NOT_SET,
+			[RLM_MODULE_HANDLED]	= MOD_ACTION_NOT_SET,
+			[RLM_MODULE_INVALID]	= MOD_ACTION_NOT_SET,
+			[RLM_MODULE_DISALLOW]	= MOD_ACTION_NOT_SET,
+			[RLM_MODULE_NOTFOUND]	= MOD_ACTION_NOT_SET,
+			[RLM_MODULE_NOOP]	= MOD_ACTION_NOT_SET,
+			[RLM_MODULE_UPDATED]	= MOD_ACTION_NOT_SET,
+			[RLM_MODULE_TIMEOUT]	= MOD_ACTION_NOT_SET
 		},
 		.retry = RETRY_INIT,
 	}
diff --git a/src/lib/unlang/function.h b/src/lib/unlang/function.h
index 4c327b4dd1e..80126c8c54d 100644
--- a/src/lib/unlang/function.h
+++ b/src/lib/unlang/function.h
@@ -96,7 +96,16 @@ int		_unlang_function_repeat_set(request_t *request, unlang_function_t repeat, c
  * These can be pushed by any other type of unlang op to allow a submodule or function
  * deeper in the C call stack to establish a new resumption point.
  *
- * @param[in] _reuslt_p		Where to write the result.
+ * @note If you're pushing a function onto the stack to resume execution in a module, you're probably
+ *	 doing it wrong.  Use unlang_module_yield() instead, and change the process function for the
+ *	 module.
+ *
+ * @note By default the rcodes from functions will be discarded.  This can be altered on a per-function
+ *       basis by the func or repeat functions setting result_p->priority to a non-zero value.
+ *	 Results can also be collected by passing in a non-NULL _result_p pointer.
+ *	 You may disagree with this behaviour, but you're wrong.
+ *
+ * @param[in] _result_p		Where to write the result.
  * @param[in] _request		The current request.
  * @param[in] _func		to call going up the stack.
  * @param[in] _repeat		function to call going back down the stack (may be NULL).
diff --git a/src/lib/unlang/interpret.c b/src/lib/unlang/interpret.c
index 80378423954..bf1857b125a 100644
--- a/src/lib/unlang/interpret.c
+++ b/src/lib/unlang/interpret.c
@@ -455,23 +455,25 @@ unlang_frame_action_t result_calculate(request_t *request, unlang_stack_frame_t
 	 *	This is the field that's evaluated in unlang conditions
 	 *	like `if (ok)`.
 	 */
-	if (frame->instruction && is_rcode_set(frame) && (request->rcode != result->rcode)) {
-		RDEBUG3("Setting request->rcode to '%s'",
-			fr_table_str_by_value(rcode_table, result->rcode, "<INVALID>"));
-		request->rcode = result->rcode;
-	}
+	if (frame->instruction) {
+		if (is_rcode_set(frame) && (request->rcode != result->rcode)) {
+			RDEBUG3("Setting request->rcode to '%s'",
+				fr_table_str_by_value(rcode_table, result->rcode, "<INVALID>"));
+			request->rcode = result->rcode;
+		}
 
-	/*
-	 *	The array holds a default priority for this return
-	 *	code.  Grab it in preference to any unset priority.
-	 */
-	if (result->priority == MOD_ACTION_NOT_SET) {
-		result->priority = instruction->actions.actions[result->rcode];
+		/*
+		*	The array holds a default priority for this return
+		*	code.  Grab it in preference to any unset priority.
+		*/
+		if (result->priority == MOD_ACTION_NOT_SET) {
+			result->priority = instruction->actions.actions[result->rcode];
 
-		RDEBUG4("** [%i] %s - using default instruction priority for %s, %d",
-			stack->depth, __FUNCTION__,
-			fr_table_str_by_value(mod_rcode_table, result->rcode, "<invalid>"),
-			result->priority);
+			RDEBUG4("** [%i] %s - using default instruction priority for %s, %d",
+				stack->depth, __FUNCTION__,
+				fr_table_str_by_value(mod_rcode_table, result->rcode, "<invalid>"),
+				result->priority);
+		}
 	}
 
 	/*