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

it's better to give a descriptive error than crash
FreeRADIUS · Oct 31, 2023 · ec82df1 · ec82df1
1 parent 5a50f6a
commit ec82df1
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 3 deletions.
diff --git 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
@@ -2720,14 +2720,25 @@ static fr_slen_t tokenize_expression(xlat_exp_head_t *head, xlat_exp_t **out, fr
 	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
@@ -0,0 +1,3 @@
+group foo
+
+&foo := &request + &control # ERROR