From: Alan T. DeKok <aland@freeradius.org>
Date: Tue, 22 Nov 2022 21:11:18 +0000 (-0500)
Subject: allow for failed xlats to return empty strings, as a special case
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e8b7de494ea072e58b22b3cefbdcc2bfcc1bdde0;p=thirdparty%2Ffreeradius-server.git

allow for failed xlats to return empty strings, as a special case

	&foo := "%{failed xlat}"

will return

	&foo == ""

But

	&foo := %{failed xlat}

without quotes, will not create &foo.
---

diff --git a/src/lib/unlang/edit.c b/src/lib/unlang/edit.c
index 0dee31bd1e2..b56f0303228 100644
--- a/src/lib/unlang/edit.c
+++ b/src/lib/unlang/edit.c
@@ -484,15 +484,18 @@ static int apply_edits_to_leaf(request_t *request, unlang_frame_state_edit_t *st
 			/*
 			 *	The caller asked for a string, so instead of returning a list, return a string.
 			 *
-			 *	@todo - this should arguably be the responsibility of xlat_push(), or
-			 *	tmpl_push().  If the input xlat/tmpl is quoted, then the output should be a
-			 *	single value-box which is the final string.
+			 *	If there's no output, then it's an empty string.
+			 *
+			 *	We have to check this here, because the quote is part of the tmpl, and we call
+			 *	xlat_push(), which doesn't know about the quote.
 			 */
 			box = fr_value_box_list_head(&current->rhs.result);
 
-			if (!box) goto no_rhs;
+			if (!box) {
+				MEM(box = fr_value_box_alloc(state, FR_TYPE_STRING, NULL, false));
+				fr_value_box_list_insert_tail(&current->rhs.result, box);
 
-			if (fr_value_box_list_concat_in_place(box, box, &current->rhs.result, FR_TYPE_STRING,
+			} else if (fr_value_box_list_concat_in_place(box, box, &current->rhs.result, FR_TYPE_STRING,
 							      FR_VALUE_BOX_LIST_FREE_BOX, true, 8192) < 0) {
 				RWDEBUG("Failed converting result to string");
 				return -1;
@@ -533,7 +536,6 @@ static int apply_edits_to_leaf(request_t *request, unlang_frame_state_edit_t *st
 	}
 
 	if (!box) {
-	no_rhs:
 		RWDEBUG("%s %s ... - Assignment failed to having no value on right-hand side", map->lhs->name, fr_tokens[map->op]);
 		return -1;
 	}
diff --git a/src/tests/keywords/date b/src/tests/keywords/date
index 9a54108c416..a5e4f76c831 100644
--- a/src/tests/keywords/date
+++ b/src/tests/keywords/date
@@ -47,7 +47,7 @@ if (&Tmp-String-2 != '2022-08-08 19:04:19') {
 
 # Invalid format
 &Tmp-String-3 := '201-32-22 17:25:00'
-&Tmp-String-4 := "%(sqldate:%{Tmp-String-3})"
+&Tmp-String-4 := %(sqldate:%{Tmp-String-3})
 
 # This shouldn't exist, as the RHS above is NULL, and therefore the assignment will fail
 if (&Tmp-String-4) {
diff --git a/src/tests/keywords/mschap b/src/tests/keywords/mschap
index b3fceb6f689..e7879066a1d 100644
--- a/src/tests/keywords/mschap
+++ b/src/tests/keywords/mschap
@@ -2,33 +2,37 @@
 #  PRE: if
 #
 
+#
+#  over-write the existing User-Name
+#
+&User-Name := 'EXAMPLE\bob'
+
 # MS CHAPv1
 &request += {
-	&User-Name = 'EXAMPLE\bob'
 	&Vendor-Specific.Microsoft.CHAP-Challenge = 0xe96e4fff2955c4f1
 	&Vendor-Specific.Microsoft.CHAP-Response = 0x00010000000000000000000000000000000000000000000000008860bbaac0cd3960b4ce5dc6f0f7a462b897ef530484e80f
 }
 
 &request += {
-	&Tmp-Octets-0 = "%(mschap:Challenge)"
-	&Tmp-Octets-1 = "%(mschap:NT-Response)"
+	&Tmp-Octets-0 = %(mschap:Challenge)
+	&Tmp-Octets-1 = %(mschap:NT-Response)
 	&Tmp-String-0 = "%(mschap:NT-Domain)"
 	&Tmp-String-1 = "%(mschap:User-Name)"
 }
 
-if (&Tmp-Octets-0 != 0xe96e4fff2955c4f1) {
+if !(&Tmp-Octets-0 == 0xe96e4fff2955c4f1) {
 	test_fail
 }
 
-if (&Tmp-Octets-1 != 0x8860bbaac0cd3960b4ce5dc6f0f7a462b897ef530484e80f) {
+if !(&Tmp-Octets-1 == 0x8860bbaac0cd3960b4ce5dc6f0f7a462b897ef530484e80f) {
 	test_fail
 }
 
-if (&Tmp-String-0 != 'EXAMPLE') {
+if !(&Tmp-String-0 == 'EXAMPLE') {
 	test_fail
 }
 
-if (&Tmp-String-1 != 'bob') {
+if !(&Tmp-String-1 == 'bob') {
 	test_fail
 }
 
@@ -36,19 +40,19 @@ if (&Tmp-String-1 != 'bob') {
 
 &Tmp-Octets-0 := "%(mschap:LM-Response)"
 
-if (&Tmp-Octets-0 != 0x6937d7935bb28a4c1dafe6a193bdea7a853a74d8bcf6db15) {
+if !(&Tmp-Octets-0 == 0x6937d7935bb28a4c1dafe6a193bdea7a853a74d8bcf6db15) {
 	test_fail
 }
 
 # Hashing
-&Tmp-Octets-0 := "%(mschap:NT-Hash testing_123)"
-&Tmp-Octets-1 := "%(mschap:LM-Hash testing_123)"
+&Tmp-Octets-0 := %(mschap:NT-Hash testing_123)
+&Tmp-Octets-1 := %(mschap:LM-Hash testing_123)
 
-if (&Tmp-Octets-0 != 0xfa782604f85eb3564f555648341b53e4) {
+if !(&Tmp-Octets-0 == 0xfa782604f85eb3564f555648341b53e4) {
 	test_fail
 }
 
-if (&Tmp-Octets-1 != 0x2d5545077d7b7d2ae4343f96ab15c596) {
+if !(&Tmp-Octets-1 == 0x2d5545077d7b7d2ae4343f96ab15c596) {
 	test_fail
 }
 
@@ -58,14 +62,14 @@ if (&Tmp-Octets-1 != 0x2d5545077d7b7d2ae4343f96ab15c596) {
 &Vendor-Specific.Microsoft.CHAP-Challenge := 0x04408dc2a98dae1ce351dfc53f57d08e
 &Vendor-Specific.Microsoft.CHAP2-Response := 0x00010e93cfbfcef8d5b6af42d2b2ca5b43180000000000000000bc068d1e8c54de5e9db78e6736d686eb88a999dd7fa239b200
 
-&Tmp-Octets-0 := "%(mschap:Challenge)"
-&Tmp-Octets-1 := "%(mschap:NT-Response)"
+&Tmp-Octets-0 := %(mschap:Challenge)
+&Tmp-Octets-1 := %(mschap:NT-Response)
 
-if (&Tmp-Octets-0 != 0xad18b6b8e1478b4c) {
+if !(&Tmp-Octets-0 == 0xad18b6b8e1478b4c) {
 	test_fail
 }
 
-if (&Tmp-Octets-1 != 0xbc068d1e8c54de5e9db78e6736d686eb88a999dd7fa239b2) {
+if !(&Tmp-Octets-1 == 0xbc068d1e8c54de5e9db78e6736d686eb88a999dd7fa239b2) {
 	test_fail
 }
 
@@ -73,7 +77,7 @@ if (&Tmp-Octets-1 != 0xbc068d1e8c54de5e9db78e6736d686eb88a999dd7fa239b2) {
 &Vendor-Specific.Microsoft.CHAP-Challenge := 0xf0eac4151d5346662ba8c5e428
 &Tmp-String-0 := "%(mschap:Challenge)"
 
-if (&Tmp-String-0 != "") {
+if !(&Tmp-String-0 == "") {
 	test_fail
 }
 
diff --git a/src/tests/keywords/rand b/src/tests/keywords/rand
index ef3070c2faf..760d3b385ba 100644
--- a/src/tests/keywords/rand
+++ b/src/tests/keywords/rand
@@ -3,7 +3,7 @@
 #
 #  Negative limit should have failed assignment
 #
-if (&Tmp-String-0) {
+if !(&Tmp-String-0 == "") {
 	test_fail
 }
 
@@ -12,11 +12,15 @@ if (&Tmp-String-0) {
 #
 #  Invalid limit should have failed assignment
 #
-if (&Tmp-String-0) {
+if !(&Tmp-String-0 == "") {
 	test_fail
 }
 
-&Tmp-Integer-0 := "%{rand:123}"
+&Tmp-Integer-0 := %{rand:123}
+
+if (!&Tmp-Integer-0) {
+	test_fail
+}
 
 #
 #  Make sure random number is within limit
diff --git a/src/tests/keywords/randstr b/src/tests/keywords/randstr
index fe75538c886..24b50ac0a61 100644
--- a/src/tests/keywords/randstr
+++ b/src/tests/keywords/randstr
@@ -6,48 +6,48 @@
 }
 
 #
-#  These next two assignments fail, so they can't go
-#  into the above list.  If they were there, then the
-#  entire list assignment would fail.
+#  This next assignment fails, so it can't go into the above list.  If
+#  it was there, then the entire list assignment would fail.
 #
 &Tmp-String-0 := "%{randstr:%{Tmp-String-0}}"
-&Tmp-String-4 := "%{randstr:G}"
 
 #
 #  Empty output on empty input
 #
-if (&Tmp-String-0 != "") {
+if !(&Tmp-String-0 == "") {
 	test_fail
 }
 
 #
 #  Check length of output
 #
-if ("%{strlen:%{Tmp-String-1}}" != 3) {
+if (%{strlen:%{Tmp-String-1}} != 3) {
 	test_fail
 }
 
-if ("%{strlen:%{Tmp-String-2}}" != 24) {
+if (%{strlen:%{Tmp-String-2}} != 24) {
 	test_fail
 }
 #
 #  Check maximum repetition
 #
-if ("%{strlen:%{Tmp-String-3}}" != 1025) {
+if (%{strlen:%{Tmp-String-3}} != 1025) {
 	test_fail
 }
 
 #
 #  Check invalid character class
 #
-if (&Tmp-String-4) {
+&Tmp-String-4 := "%{randstr:G}"
+
+if !(&Tmp-String-4 == "") {
 	test_fail
 }
 
 #
 #  Check repetition of binary output
 #
-if ("%(length:%{Tmp-String-5})" != 10) {
+if (%(length:%{Tmp-String-5}) != 10) {
 	test_fail
 }
 
diff --git a/src/tests/keywords/subrequest-request-qualifiers b/src/tests/keywords/subrequest-request-qualifiers
index f34d18c02d4..6c38de1e4b5 100644
--- a/src/tests/keywords/subrequest-request-qualifiers
+++ b/src/tests/keywords/subrequest-request-qualifiers
@@ -20,54 +20,50 @@ subrequest Access-Request {
 				&Tmp-String-4 = &outer.User-Name
 				&Tmp-String-5 = &current.outer.User-Name
 				&Tmp-String-6 = &parent.current.outer.User-Name
+				&Tmp-String-7 = "%{parent.parent.parent.parent.User-Name}"
+				&Tmp-String-8 = "%{outer.parent.User-Name}"
 			}
-
-			#
-			#  These fail, so they belong on their own.
-			#
-			&outer.request.Tmp-String-7 := "%{parent.parent.parent.parent.User-Name}"
-			&outer.request.Tmp-String-8 := "%{outer.parent.User-Name}"
 		}
 	}
 }
 
-if (&Outer-Realm-Name != 'testing123') {
+if !(&Outer-Realm-Name == 'testing123') {
 	test_fail
 }
 
-if (&Tmp-String-0 != 'joe') {
+if !(&Tmp-String-0 == 'joe') {
 	test_fail
 }
 
-if (&Tmp-String-1 != 'jim') {
+if !(&Tmp-String-1 == 'jim') {
 	test_fail
 }
 
-if (&tmp-String-2 != 'obo') {
+if !(&Tmp-String-2 == 'obo') {
 	test_fail
 }
 
-if (&Tmp-String-3 != 'bob') {
+if !(&Tmp-String-3 == 'bob') {
 	test_fail
 }
 
-if (&Tmp-String-4 != 'bob') {
+if !(&Tmp-String-4 == 'bob') {
 	test_fail
 }
 
-if (&Tmp-String-5 != 'bob') {
+if !(&Tmp-String-5 == 'bob') {
 	test_fail
 }
 
-if (&Tmp-String-6 != 'bob') {
+if !(&Tmp-String-6 == 'bob') {
 	test_fail
 }
 
-if (&Tmp-String-7) {
+if !(&Tmp-String-7 == "") {
 	test_fail
 }
 
-if (&Tmp-String-8) {
+if !(&Tmp-String-8 == "") {
 	test_fail
 }
 
diff --git a/src/tests/keywords/tolower b/src/tests/keywords/tolower
index b1a2b6b629b..5283ea49041 100644
--- a/src/tests/keywords/tolower
+++ b/src/tests/keywords/tolower
@@ -2,9 +2,7 @@
 # PRE: if
 #
 &Tmp-String-0	:= "AbCdE"
-
 &Tmp-String-0	:= "%{tolower:%{Tmp-String-0}}"
-
 &Tmp-String-1	:= "%{tolower:AAbbCCdd}"
 
 #
@@ -21,7 +19,7 @@ if (&Tmp-String-1 != "aabbccdd") {
 	test_fail
 }
 
-if (&Tmp-String-2) {
+if !(&Tmp-String-2 == "") {
 	test_fail
 }
 
diff --git a/src/tests/keywords/toupper b/src/tests/keywords/toupper
index efa94393571..e08425dbed3 100644
--- a/src/tests/keywords/toupper
+++ b/src/tests/keywords/toupper
@@ -15,7 +15,7 @@ if (&Tmp-String-1 != "AABBCCDD") {
 	test_fail
 }
 
-if (&Tmp-String-2) {
+if !(&Tmp-String-2 == "") {
 	test_fail
 }
 
diff --git a/src/tests/keywords/unpack b/src/tests/keywords/unpack
index ce04e6b6bf8..a8dc488f563 100644
--- a/src/tests/keywords/unpack
+++ b/src/tests/keywords/unpack
@@ -7,11 +7,11 @@
 &Tmp-String-0 := "%(unpack:%{Tmp-Octets-0} 0 ipaddr)"
 &Tmp-IP-Address-0 := "%(unpack:%{Tmp-Octets-0} 0 ipaddr)"
 
-if (&Tmp-String-0 != "127.0.0.1") {
+if !(&Tmp-String-0 == "127.0.0.1") {
 	test_fail
 }
 
-if (&Tmp-IP-Address-0 != 127.0.0.1) {
+if !(&Tmp-IP-Address-0 == 127.0.0.1) {
 	test_fail
 }
 
@@ -19,20 +19,20 @@ if (&Tmp-IP-Address-0 != 127.0.0.1) {
 &Tmp-Integer-0 := "%(unpack:%{Tmp-Octets-0} 4 uint16)"
 
 # Octets 4 and 5 == 0x0304 == 772
-if (&Tmp-Integer-0 != 772) {
+if ~(&Tmp-Integer-0 == 772) {
 	test_fail
 }
 
 # truncation
 &Tmp-String-0 := "0x0011223344556677"
 &Tmp-String-1 := "%(unpack:%{Tmp-String-0} 0 ether)"
-if (&Tmp-String-1 != "00:11:22:33:44:55") {
+if !(&Tmp-String-1 == "00:11:22:33:44:55") {
 	test_fail
 }
 
 &Tmp-String-0 := "0x48656C6C6F"
 &Tmp-String-1 := "%(unpack:%{Tmp-String-0} 0 string)"
-if (&Tmp-String-1 != "Hello") {
+if !(&Tmp-String-1 == "Hello") {
 	test_fail
 }
 
@@ -40,7 +40,7 @@ if (&Tmp-String-1 != "Hello") {
 &request -= &Tmp-String-1[*]
 &Tmp-String-1 := "%(unpack:%{Tmp-String-0} 10 string)"
 
-if (&Tmp-String-1) {
+if !(&Tmp-String-1 == "") {
 	test_fail
 }
 
@@ -51,7 +51,7 @@ if (&Module-Failure-Message != "unpack offset 10 is larger than input data lengt
 
 # Invalid destination data type
 &Tmp-String-1 := "%(unpack:%{Tmp-String-0} 0 thing)"
-if (&Tmp-String-1) {
+if !(&Tmp-String-1 == "") {
 	test_fail
 }
 
@@ -61,7 +61,7 @@ if (&Module-Failure-Message != "Invalid data type 'thing'") {
 
 # Invalid source data type
 &Tmp-String-1 := "%(unpack:%{Tmp-Integer-0} 0 string)"
-if (&Tmp-String-1) {
+if !(&Tmp-String-1 == "") {
 	test_fail
 }
 
@@ -74,7 +74,7 @@ if (&Module-Failure-Message != "unpack requires the input attribute to be 'strin
 &Tmp-String-0 := '0x014sdgw'
 &Tmp-String-1 := "%(unpack:%{Tmp-String-0} 0 string)"
 
-if (&Tmp-String-1) {
+if !(&Tmp-String-1 == "") {
 	test_fail
 }
 
@@ -87,7 +87,7 @@ if (&Module-Failure-Message != "Invalid hex string in '0x014sdgw'") {
 &Tmp-String-0 := '0x'
 &Tmp-String-1 := "%(unpack:%{Tmp-String-0} 0 string)"
 
-if (&Tmp-String-1) {
+if !(&Tmp-String-1 == "") {
 	test_fail
 }
 
diff --git a/src/tests/keywords/urlquote b/src/tests/keywords/urlquote
index f44aeca93bb..3b71d90ca88 100644
--- a/src/tests/keywords/urlquote
+++ b/src/tests/keywords/urlquote
@@ -49,7 +49,7 @@ if (&Tmp-String-1 != &Tmp-String-0) {
 }
 
 # Test decoding invalid encoded string
-if (&Tmp-String-2) {
+if !(&Tmp-String-2 == "") {
 	test_fail
 }
 
diff --git a/src/tests/keywords/xlat-alternation b/src/tests/keywords/xlat-alternation
index 851cd4c3f1c..fb5e7a2acf3 100644
--- a/src/tests/keywords/xlat-alternation
+++ b/src/tests/keywords/xlat-alternation
@@ -8,8 +8,8 @@
 #
 #  First choice
 #
-&Tmp-String-2 := "%{%{Tmp-String-0[0]}:-%{Tmp-String-1[0]}}"
-if (&Tmp-String-2[0] != 'foo') {
+&Tmp-String-2 := "%{%{Tmp-String-0}:-%{Tmp-String-1}}"
+if (&Tmp-String-2 != 'foo') {
 	test_fail
 }
 
@@ -17,16 +17,16 @@ if (&Tmp-String-2[0] != 'foo') {
 #  Second choice
 #
 &request -= &Tmp-String-0[*]
-&Tmp-String-2 := "%{%{Tmp-String-0[0]}:-%{Tmp-String-1[0]}}"
-if (&Tmp-String-2[0] != 'bar') {
+&Tmp-String-2 := "%{%{Tmp-String-0}:-%{Tmp-String-1}}"
+if (&Tmp-String-2 != 'bar') {
 	test_fail
 }
 
 #
 #  Multiple things in an alternation
 #
-&Tmp-String-2 := "%{%{Tmp-String-0[0]}:-%{Tmp-String-1[0]} foo}"
-if !(&Tmp-String-2[0] == 'bar foo') {
+&Tmp-String-2 := "%{%{Tmp-String-0}:-%{Tmp-String-1} foo}"
+if !(&Tmp-String-2 == 'bar foo') {
 	test_fail
 }
 
@@ -37,11 +37,22 @@ if !(&Tmp-String-2[0] == 'bar foo') {
 &request -= &Tmp-String-2[*]
 
 #
-#  Both sides are failing, so the assignment here fails, too.
+#  Both sides are failing, so the assignment returns a NULL string
 #
-&Tmp-String-2 := "%{%{Tmp-String-0[0]}:-%{Tmp-String-1[0]}}"
-if (&Tmp-String-2[0]) {
+&Tmp-String-2 := "%{%{Tmp-String-0}:-%{Tmp-String-1}}"
+if !(&Tmp-String-2 == "") {
 	test_fail
 }
 
+#
+#  And unquoted results return nothing.
+#
+&request -= &Tmp-String-2[*]
+
+&Tmp-String-2 := %{%{Tmp-String-0}:-%{Tmp-String-1}}
+if (&Tmp-String-2) {
+	test_fail
+}
+
+
 success
diff --git a/src/tests/modules/client/xlat.unlang b/src/tests/modules/client/xlat.unlang
index 1cfaaadf695..508147987ae 100644
--- a/src/tests/modules/client/xlat.unlang
+++ b/src/tests/modules/client/xlat.unlang
@@ -26,13 +26,13 @@ if (&Tmp-String-0 != 'b_type') {
 #  Test non-existent client properties
 #
 &Tmp-String-3 := "%(client:non-existent-attr)"
-&Tmp-String-4 += "%(client:non-existing-attr2)"
+&Tmp-String-4 := "%(client:non-existing-attr2)"
 
-if (&Tmp-String-3) {
+if !(&Tmp-String-3 == "") {
 	test_fail
 }
 
-if (&Tmp-String-4) {
+if !(&Tmp-String-4 == "") {
 	test_fail
 }
 
diff --git a/src/tests/modules/exec/async.unlang b/src/tests/modules/exec/async.unlang
index 80ebf21c13e..2bba7443623 100644
--- a/src/tests/modules/exec/async.unlang
+++ b/src/tests/modules/exec/async.unlang
@@ -3,7 +3,7 @@
 #  because we don't wait for the response.
 #
 &Tmp-String-0 := "%(exec_async:/bin/sh -c 'echo -n hello')"
-if (&Tmp-String-0) {
+if !(&Tmp-String-0 == "") {
 	test_fail
 }
 
@@ -26,6 +26,6 @@ if (&reply.Reply-Message == 'hello') {
 #  after the test exits.
 #
 &Tmp-String-0 := "%(exec_async:/bin/sh -c 'sleep 1')"
-if (&Tmp-String-0) {
+if !(&Tmp-String-0 == "") {
 	test_fail
 }
diff --git a/src/tests/modules/exec/sync.unlang b/src/tests/modules/exec/sync.unlang
index 51f8b3a01cc..b78d5430539 100644
--- a/src/tests/modules/exec/sync.unlang
+++ b/src/tests/modules/exec/sync.unlang
@@ -16,14 +16,14 @@ if (&Module-Failure-Message[*] != "Timeout running program - killing it and fail
 	test_fail
 }
 
-if (&Tmp-String-0) {
+if !(&Tmp-String-0 == "") {
 	test_fail
 }
 
 &request -= &Tmp-String-0
 &Tmp-String-0 := "%(exec_sync:/bin/sh $ENV{MODULE_TEST_DIR}/fail.sh)"
 
-if (&Tmp-String-0) {
+if !(&Tmp-String-0 == "") {
 	test_fail
 }