From: Alan T. DeKok <aland@freeradius.org>
Date: Fri, 14 Jul 2023 20:37:13 +0000 (-0400)
Subject: don't automatically create the key field for nested structs.
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d328d57012d807a20a142dc98f5e04810a91b808;p=thirdparty%2Ffreeradius-server.git

don't automatically create the key field for nested structs.

The old code didn't do it, so we shouldn't do it either.
---

diff --git a/src/lib/util/pair.c b/src/lib/util/pair.c
index 2707f95dbbd..4e1cb793c2d 100644
--- a/src/lib/util/pair.c
+++ b/src/lib/util/pair.c
@@ -461,7 +461,7 @@ fr_pair_t *fr_pair_afrom_child_num(TALLOC_CTX *ctx, fr_dict_attr_t const *parent
  */
 fr_pair_t *fr_pair_afrom_da_depth_nested(TALLOC_CTX *ctx, fr_pair_list_t *list, fr_dict_attr_t const *da, int start)
 {
-	fr_pair_t		*vp, *key_vp = NULL;
+	fr_pair_t		*vp;
 	unsigned int		i;
 	TALLOC_CTX		*cur_ctx;
 	fr_dict_attr_t const	*find;		/* DA currently being looked for */
@@ -475,10 +475,15 @@ fr_pair_t *fr_pair_afrom_da_depth_nested(TALLOC_CTX *ctx, fr_pair_list_t *list,
 	for (i = start; i <= da->depth; i++) {
 		find = da_stack.da[i];
 
-		if (key_vp) {
-			fr_assert(find->type == FR_TYPE_STRUCT);
-			key_vp->vp_uint32 = find->attr;
-			key_vp = NULL;
+		/*
+		 *	If we're asked to create a key field, then do it.
+		 *
+		 *	Otherwise if we're creating a child struct (which is magically parented by the key
+		 *	field), then don't bother creating the key field.  It will be automatically filled in
+		 *	by the encoder.
+		 */
+		if ((find != da) && fr_dict_attr_is_key_field(find)) {
+			continue;
 		}
 
 		vp = fr_pair_find_by_da(cur_list, NULL, find);
@@ -488,16 +493,6 @@ fr_pair_t *fr_pair_afrom_da_depth_nested(TALLOC_CTX *ctx, fr_pair_list_t *list,
 
 		if (find == da) return vp;
 
-		/*
-		 *	Create the key field, but do _not_ put the child struct into it.  Instead, the child
-		 *	struct goes appended into this struct.  However, we _do_ automatically set the value
-		 *	of the key field, depending on the number of the child structure.
-		 */
-		if (fr_dict_attr_is_key_field(find)) {
-			key_vp = vp;
-			continue;
-		}
-
 		fr_assert(fr_type_is_structural(vp->da->type));
 
 		cur_ctx = vp;
diff --git a/src/tests/keywords/all.mk b/src/tests/keywords/all.mk
index 8845f95e5fe..85596157423 100644
--- a/src/tests/keywords/all.mk
+++ b/src/tests/keywords/all.mk
@@ -73,9 +73,6 @@ KEYWORD_UPDATE_REWRITE_TESTS := update-all update-array update-delete update-rem
 # Tests which can use new conditions, but which can't use tmpl_tokenize_all_nested=yes
 KEYWORD_UPDATE_TMPL_TESTS	:= foreach-regex xlat-dhcpv4
 
-# Tests which can't use pair_legacy_nested=yes
-KEYWORD_PAIR_LEGACY_TESTS	:= map-xlat-struct
-
 #
 #  Migration support.  Some of the tests don't run under the new
 #  conditions, so we don't run them under the new conditions.
@@ -91,8 +88,7 @@ $(OUTPUT)/${1}: NEW_COND=-S use_new_conditions=yes -S rewrite_update=yes
 
 else ifneq "$(findstring ${1}, $(KEYWORD_UPDATE_TMPL_TESTS))" ""
 $(OUTPUT)/${1}: NEW_COND=-S use_new_conditions=yes -S forbid_update=yes
-else ifneq "$(findstring ${1}, $(KEYWORD_PAIR_LEGACY_TESTS))" ""
-$(OUTPUT)/${1}: NEW_COND=-S use_new_conditions=yes -S forbid_update=yes -S tmpl_tokenize_all_nested=yes
+
 else
 $(OUTPUT)/${1}: NEW_COND=-S use_new_conditions=yes -S forbid_update=yes -S tmpl_tokenize_all_nested=yes -S pair_legacy_nested=true