From: Arran Cudbard-Bell <a.cudbardb@freeradius.org>
Date: Wed, 3 Nov 2021 23:20:31 +0000 (-0400)
Subject: Final temporary fix for replacing nested pairs
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7fb850efd16a2650931e7d197f8bf2ed29542113;p=thirdparty%2Ffreeradius-server.git

Final temporary fix for replacing nested pairs
---

diff --git a/src/lib/server/map.c b/src/lib/server/map.c
index 38cf15f328a..f8e8e3c7508 100644
--- a/src/lib/server/map.c
+++ b/src/lib/server/map.c
@@ -1768,12 +1768,13 @@ int map_to_request(request_t *request, map_t const *map, radius_map_getvalue_t f
 		fr_dlist_head_t		interior;
 		fr_pair_t 		*src_vp;
 
-		if (dst) {
-			fr_pair_t *old;
+		src_vp = fr_pair_list_tail(&src_list);
 
+		if (dst) {
 			DEBUG_OVERWRITE(dst, src_vp);
-			old = fr_dcursor_replace(&dst_list, fr_pair_copy(talloc_parent(dst), src_vp));
-			talloc_free(old);	/* Remove the old pair */
+
+			fr_pair_reinit_from_da(NULL, dst, src_vp->da);
+			fr_pair_value_copy(dst, src_vp);
 
 			goto op_set_done;
 		}
@@ -1784,7 +1785,6 @@ int map_to_request(request_t *request, map_t const *map, radius_map_getvalue_t f
 		/*
 		 *	Find out what we need to build and build it
 		 */
-		src_vp = fr_pair_list_tail(&src_list);
 		if ((tmpl_extents_find(tmp_ctx, &leaf, &interior, request, map->lhs) < 0) ||
 		    (tmpl_extents_build_to_leaf(&leaf, &interior, map->lhs) < 0)) {
 		    op_set_error:
diff --git a/src/lib/server/tmpl.h b/src/lib/server/tmpl.h
index 082937d7ee2..7d77a6f1b9d 100644
--- a/src/lib/server/tmpl.h
+++ b/src/lib/server/tmpl.h
@@ -525,12 +525,6 @@ struct tmpl_cursor_nested_s {
 	TALLOC_CTX		*list_ctx;	//!< Track where we should be allocating attributes.
 
 	union {
-		struct {
-			fr_da_stack_t		da_stack;		//!< fr_dict_attr_t hierarchy
-									///< between a->b.
-			fr_dcursor_stack_t	*cursor_stack;		//!< Track state as we traverse VPs.
-		} tlv;
-
 		struct {
 			fr_dcursor_t		cursor;			//!< Group traversal is much easier
 									///< but we still need to keep track
diff --git a/src/lib/util/pair.c b/src/lib/util/pair.c
index d6f88e26274..9f8f36feebd 100644
--- a/src/lib/util/pair.c
+++ b/src/lib/util/pair.c
@@ -940,53 +940,81 @@ int fr_pair_append(fr_pair_list_t *list, fr_pair_t *to_add)
 	return 0;
 }
 
-/** Replace first matching VP
- *
- * Walks over 'list', and replaces the first VP that matches 'replace'.
- * If no match is found the replacement VP is appended to the list.
+/** Add a VP after another VP.
  *
- * @note Memory used by the VP being replaced will be freed.
- * @note Will not work with unknown attributes.
- *
- * @param[in,out] list VP in linked list. Will search and replace in this list.
- * @param[in] replace VP to replace.
+ * @param[in] list	VP in linked list. Will add new VP to this list.
+ * @param[in] pos	to insert pair after.
+ * @param[in] to_add	VP to add to list.
+ * @return
+ *	- 0 on success.
+ *	- -1 on failure (pair already in list).
  */
-void fr_pair_replace(fr_pair_list_t *list, fr_pair_t *replace)
+int fr_pair_insert_after(fr_pair_list_t *list, fr_pair_t *pos, fr_pair_t *to_add)
 {
-	fr_pair_t *i;
+	PAIR_VERIFY(to_add);
 
-	PAIR_VERIFY(replace);
+	if (fr_dlist_entry_in_list(&to_add->order_entry)) {
+		fr_strerror_printf("Pair %pV already inserted into list", to_add);
+		return -1;
+	}
 
-	if (fr_dlist_empty(&list->order)) {
-		fr_pair_append(list, replace);
-		return;
+	if (!fr_dlist_entry_in_list(&to_add->order_entry)) {
+		fr_strerror_printf("Pair %pV not in list", pos);
+		return -1;
 	}
 
-	/*
-	 *	Not an empty list, so find item if it is there, and
-	 *	replace it. Note, we always replace the head one, and
-	 *	we ignore any others that might exist.
-	 */
-	for (i = fr_pair_list_head(list); i; i = fr_pair_list_next(list, i)) {
-		PAIR_VERIFY(i);
+	fr_dlist_insert_after(fr_pair_list_order(list), pos, to_add);
 
-		/*
-		 *	Found the head attribute, replace it,
-		 *	and return.
-		 */
-		if (i->da == replace->da) {
-			i = fr_dlist_replace(&list->order, i, replace);
-			talloc_free(i);
-			return;
-		}
+	return 0;
+}
 
+/** Add a VP before another VP.
+ *
+ * @param[in] list	VP in linked list. Will add new VP to this list.
+ * @param[in] pos	to insert pair after.
+ * @param[in] to_add	VP to add to list.
+ * @return
+ *	- 0 on success.
+ *	- -1 on failure (pair already in list).
+ */
+int fr_pair_insert_before(fr_pair_list_t *list, fr_pair_t *pos, fr_pair_t *to_add)
+{
+	PAIR_VERIFY(to_add);
+
+	if (fr_dlist_entry_in_list(&to_add->order_entry)) {
+		fr_strerror_printf("Pair %pV already inserted into list", to_add);
+		return -1;
 	}
 
-	/*
-	 *	If we got here, we didn't find anything to replace, so
-	 *	we just append.
-	 */
-	fr_pair_append(list, replace);
+	if (!fr_dlist_entry_in_list(&to_add->order_entry)) {
+		fr_strerror_printf("Pair %pV not in list", pos);
+		return -1;
+	}
+
+	fr_dlist_insert_before(fr_pair_list_order(list), pos, to_add);
+
+	return 0;
+}
+
+/** Replace first matching VP
+ *
+ * Walks over 'list', and replaces the first VP that matches 'replace'.
+ * If no match is found the replacement VP is appended to the list.
+ *
+ * @note Memory used by the VP being replaced will be freed.
+ * @note Will not work with unknown attributes.
+ *
+ * @param[in,out] list		pair list containing #to_replace.
+ * @param[in] to_replace	pair to release.
+ * @param[in] vp		New pair to insert.
+ */
+void fr_pair_replace(fr_pair_list_t *list, fr_pair_t *to_replace, fr_pair_t *vp)
+{
+	PAIR_VERIFY(to_replace);
+	PAIR_VERIFY(vp);
+
+	fr_pair_insert_after(list, to_replace, vp);
+	fr_pair_remove(list, to_replace);
 }
 
 /** Alloc a new fr_pair_t (and append)
diff --git a/src/lib/util/pair.h b/src/lib/util/pair.h
index 4d652aa7612..e477f135729 100644
--- a/src/lib/util/pair.h
+++ b/src/lib/util/pair.h
@@ -275,7 +275,11 @@ int		fr_pair_append(fr_pair_list_t *list, fr_pair_t *vp) CC_HINT(nonnull);
 
 int		fr_pair_prepend(fr_pair_list_t *list, fr_pair_t *vp) CC_HINT(nonnull);
 
-void		fr_pair_replace(fr_pair_list_t *list, fr_pair_t *add) CC_HINT(nonnull);
+int		fr_pair_insert_after(fr_pair_list_t *list, fr_pair_t *pos, fr_pair_t *to_add) CC_HINT(nonnull);
+
+int		fr_pair_insert_before(fr_pair_list_t *list, fr_pair_t *pos, fr_pair_t *to_add) CC_HINT(nonnull);
+
+void		fr_pair_replace(fr_pair_list_t *list, fr_pair_t *to_replace, fr_pair_t *vp) CC_HINT(nonnull);
 
 int		fr_pair_delete_by_child_num(fr_pair_list_t *list,
 					    fr_dict_attr_t const *parent, unsigned int attr) CC_HINT(nonnull);
diff --git a/src/tests/keywords/map-xlat-nested-overwrite b/src/tests/keywords/map-xlat-nested-overwrite
new file mode 100644
index 00000000000..46e602dea7c
--- /dev/null
+++ b/src/tests/keywords/map-xlat-nested-overwrite
@@ -0,0 +1,18 @@
+#
+# PRE: update
+#
+#  Test the map xlat
+#
+update {
+	&Tmp-String-0 := "request.Tmp-Group-0.Tmp-String-1 := 'testing123'"
+}
+
+if ("%{map:%{Tmp-String-0}}" != 1) {
+	test_fail
+}
+
+if ("%{map:%{Tmp-String-0}}" != 1) {
+	test_fail
+}
+
+success