From: Alan T. DeKok <aland@freeradius.org>
Date: Tue, 31 Oct 2023 14:47:24 +0000 (-0400)
Subject: note that we can't do &list1 := &list2 + &list3
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ec82df1cb1d587251c555bc8e8f9c847dda7104a;p=thirdparty%2Ffreeradius-server.git

note that we can't do &list1 := &list2 + &list3

it's better to give a descriptive error than crash
---

diff --git a/src/lib/server/map.c b/src/lib/server/map.c
index 893dd242daf..2b81362844e 100644
--- a/src/lib/server/map.c
+++ b/src/lib/server/map.c
@@ -196,7 +196,7 @@ int map_afrom_cp(TALLOC_CTX *ctx, map_t **out, map_t *parent, CONF_PAIR *cp,
 		rhs_rules = &my_rhs_rules;
 
 		da = tmpl_attr_tail_da(map->lhs);
-		if (edit && fr_type_is_leaf(da->type)) my_rhs_rules.enumv = tmpl_attr_tail_da(map->lhs);
+		if (edit) my_rhs_rules.enumv = tmpl_attr_tail_da(map->lhs);
 		break;
 	}
 
@@ -246,6 +246,18 @@ int map_afrom_cp(TALLOC_CTX *ctx, map_t **out, map_t *parent, CONF_PAIR *cp,
 		slen = talloc_array_length(value) - 1;
 	}
 
+	/*
+	 *	If we're assigning to a structural attribute, AND the
+	 *	RHS side is a string, THEN don't parse the RHS as a
+	 *	"group".  The edit code will take the string, create
+	 *	pairs, and work on that.
+	 */
+	if (edit && my_rhs_rules.enumv && fr_type_is_structural(my_rhs_rules.enumv->type) &&
+	    ((type == T_DOUBLE_QUOTED_STRING) || (type == T_BACK_QUOTED_STRING) || (type == T_SINGLE_QUOTED_STRING))) {
+		my_rhs_rules.enumv = NULL;
+	}
+
+
 	slen = tmpl_afrom_substr(map, &map->rhs,
 				 &FR_SBUFF_IN(value, slen),
 				 type,
diff --git a/src/lib/unlang/xlat_expr.c b/src/lib/unlang/xlat_expr.c
index 548c3959cfe..79eac596ebb 100644
--- a/src/lib/unlang/xlat_expr.c
+++ b/src/lib/unlang/xlat_expr.c
@@ -2720,14 +2720,25 @@ redo:
 	XLAT_DEBUG("    operator <-- %pV", fr_box_strvalue_len(fr_sbuff_current(&our_in), fr_sbuff_remaining(&our_in)));
 	fr_sbuff_out_by_longest_prefix(&slen, &op, expr_assignment_op_table, &our_in, T_INVALID);
 	if (op == T_INVALID) {
+		fr_strerror_const("Invalid operator");
 		talloc_free(lhs);
-		fr_strerror_printf("Invalid operator");
 		FR_SBUFF_ERROR_RETURN(&our_in);
 	}
 
 	if (!binary_ops[op].str) {
-		fr_strerror_printf("Invalid operator");
+		fr_strerror_const("Invalid operator");
 		fr_sbuff_set(&our_in, &m_op);
+		talloc_free(lhs);
+		FR_SBUFF_ERROR_RETURN(&our_in);
+	}
+
+	/*
+	 *	We can't (yet) do &list1 = &list2 + &list3
+	 */
+	if (fr_binary_op[op] && t_rules->enumv && fr_type_is_structural(t_rules->enumv->type)) {
+		fr_strerror_const("Invalid operator for structural attribute");
+		fr_sbuff_set(&our_in, &m_op);
+		talloc_free(lhs);
 		FR_SBUFF_ERROR_RETURN(&our_in);
 	}
 
diff --git a/src/tests/keywords/list-add-error b/src/tests/keywords/list-add-error
new file mode 100644
index 00000000000..dc4c346e29a
--- /dev/null
+++ b/src/tests/keywords/list-add-error
@@ -0,0 +1,3 @@
+group foo
+
+&foo := &request + &control # ERROR