From: Arran Cudbard-Bell <a.cudbardb@freeradius.org>
Date: Wed, 29 Mar 2023 02:50:18 +0000 (-0600)
Subject: subrequest: Detached subrequest sections should not return calculate the result of... 
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=cd3c7b5770fe6f2a39fec0dea1aa11ba681ad7b8;p=thirdparty%2Ffreeradius-server.git

subrequest: Detached subrequest sections should not return calculate the result of the section

What was happening previously was the current section rcode was being evaluated with the priorities of the subrequest section, and causing spurious failures.
---

diff --git a/src/lib/unlang/subrequest.c b/src/lib/unlang/subrequest.c
index 074b3f63788..0607097b465 100644
--- a/src/lib/unlang/subrequest.c
+++ b/src/lib/unlang/subrequest.c
@@ -26,6 +26,7 @@ RCSID("$Id$")
 
 #include <freeradius-devel/server/state.h>
 #include <freeradius-devel/server/tmpl_dcursor.h>
+#include <freeradius-devel/unlang/action.h>
 #include "unlang_priv.h"
 #include "interpret_priv.h"
 #include "subrequest_priv.h"
@@ -74,7 +75,13 @@ static unlang_action_t unlang_subrequest_parent_resume(rlm_rcode_t *p_result, re
 	 */
 	if (!state->child) {
 		RDEBUG3("Child has detached");
-		return UNLANG_ACTION_CALCULATE_RESULT;
+
+		/*
+		 *	If the child detached the subrequest section
+		 *	should become entirely transparent, and
+		 *	should not update the section rcode.
+		 */
+		return UNLANG_ACTION_EXECUTE_NEXT;
 	}
 
 	/*
diff --git a/src/tests/keywords/subrequest-detach b/src/tests/keywords/subrequest-detach
new file mode 100644
index 00000000000..58456cb351c
--- /dev/null
+++ b/src/tests/keywords/subrequest-detach
@@ -0,0 +1,44 @@
+#
+#  PRE: subrequest
+#
+&Tmp-String-1 := 'testing'
+
+# Cancel in subrequest
+subrequest Access-Request {
+	detach
+
+	# Smoke test, check for crashes
+	if (&parent.Tmp-String-1 == 'testing') {
+		test_fail
+	}
+
+	&parent.Tmp-String-2 = 'testing'
+
+	# As we're detached this failure doesn't do anything
+	test_fail
+}
+
+# Should not have been set from detached child
+if (&Tmp-String-2 && (&Tmp-String-2 == 'testing')) {
+	test_fail
+}
+
+# Regression test - Previous behaviour of detached subrequest
+# sections was to leave the previous rcode set, but evaluate
+# it with the priorities of the subrequest section, which was
+# wrong.
+#
+# New behaviour should be to avoid calculating a new rcode.
+fail {
+	fail = 1
+}
+
+# Check that detached requests can be cancelled
+subrequest Access-Request {
+	detach
+
+	# Smoke test for cancelling detached requests
+	%(cancel:)
+}
+
+success