From: Alan T. DeKok <aland@freeradius.org>
Date: Sat, 19 Nov 2022 19:15:34 +0000 (-0500)
Subject: it's OK for the RHS of an expansion to *not* be an attr ref
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e68e22970a328005be3e216db5b66394f2a1160b;p=thirdparty%2Ffreeradius-server.git

it's OK for the RHS of an expansion to *not* be an attr ref

We should really do

	&foo := &%{...}

for "RHS is an attr ref".

A reasonable compromise is instead

	&foo := %{...}

because that's a "bare word".  If the user wants a string on the
RHS, he can do

	&foo := "%{...}"

and we won't try to parse the RHS as an attribute
---

diff --git a/src/lib/unlang/edit.c b/src/lib/unlang/edit.c
index a5c687e2ca2..f2012c59811 100644
--- a/src/lib/unlang/edit.c
+++ b/src/lib/unlang/edit.c
@@ -87,7 +87,7 @@ static fr_pair_t *edit_list_pair_build(fr_pair_t *parent, fr_dcursor_t *cursor,
 /*
  *  Convert a value-box list to a LHS attribute #tmpl_t
  */
-static int tmpl_attr_from_result(TALLOC_CTX *ctx, edit_result_t *out, request_t *request)
+static int tmpl_attr_from_result(TALLOC_CTX *ctx, map_t const *map, edit_result_t *out, request_t *request)
 {
 	ssize_t slen;
 	fr_value_box_t *box = fr_value_box_list_head(&out->result);
@@ -97,6 +97,36 @@ static int tmpl_attr_from_result(TALLOC_CTX *ctx, edit_result_t *out, request_t
 		return -1;
 	}
 
+	/*
+	 *	String outputs can be interpreted as attribute references.
+	 *
+	 *	LHS of an assignment MUST be an attribute.  The RHS
+	 *	MAY be an attribute, if the stars align.
+	 */
+	if (map && map->rhs) {
+		/*
+		 *	&foo := "bar"
+		 *
+		 *	RHS can't be an attribute.
+		 */
+		if (map->rhs->quote != T_BARE_WORD) return 0;
+
+		/*
+		 *	&foo := %{bin:0102030005060708}
+		 *
+		 *	RHS is octets (or other non-string), and can't be an attribute references.
+		 */
+		if (box->type != FR_TYPE_STRING) return 0;
+
+		/*
+		 *	@todo - arguably the assignment should be something like:
+		 *
+		 *	&foo := &%{expansion}
+		 *
+		 *	To say "yes, we really want the RHS to be an attribute reference".
+		 */
+	}
+
 	/*
 	 *	Mash all of the results together.
 	 */
@@ -897,7 +927,7 @@ static int expanded_rhs(request_t *request, unlang_frame_state_edit_t *state, ed
 		return check_rhs(request, state, current);
 	}
 
-	if (tmpl_attr_from_result(state, &current->rhs, request) < 0) return -1;
+	if (tmpl_attr_from_result(state, map, &current->rhs, request) < 0) return -1;
 
 	return check_rhs(request, state, current);
 }
@@ -1248,7 +1278,7 @@ static int expanded_lhs_attribute(request_t *request, unlang_frame_state_edit_t
 {
 	REXDENT();
 
-	if (tmpl_attr_from_result(state, &current->lhs, request) < 0) return -1;
+	if (tmpl_attr_from_result(state, NULL, &current->lhs, request) < 0) return -1;
 
 	return current->check_lhs(request, state, current);
 }
diff --git a/src/tests/keywords/xlat-dhcpv4 b/src/tests/keywords/xlat-dhcpv4
index e4c218cbeb3..e6efb2cdd9a 100644
--- a/src/tests/keywords/xlat-dhcpv4
+++ b/src/tests/keywords/xlat-dhcpv4
@@ -7,20 +7,18 @@ if ("%(decode.dhcpv4:%{Tmp-Octets-0})" != 2) {
 	test_fail
 }
 
-debug_request
-
-if ( &Relay-Agent-Information.Circuit-Id != 0xabcdef ) {
+if (&Relay-Agent-Information.Circuit-Id != 0xabcdef) {
 	test_fail
 }
 
-if ( &Relay-Agent-Information.Remote-Id != 0x010203040506 ) {
+if (&Relay-Agent-Information.Remote-Id != 0x010203040506) {
 	test_fail
 }
 
-&Tmp-Octets-1 := "%(encode.dhcpv4:&request[*])"
-if ( &Tmp-Octets-1 != 0x520d0103abcdef0206010203040506 ) {
+&Tmp-Octets-1 := %(encode.dhcpv4:&request)
+
+if !(&Tmp-Octets-1 == 0x520d0103abcdef0206010203040506) {
 	test_fail
 }
 
-debug_control
 success