From: Alan T. DeKok <aland@freeradius.org>
Date: Wed, 19 Feb 2025 19:09:23 +0000 (-0500)
Subject: do a better job of purifying xlats
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d5cf2ba8f94e558e0853acf36d78b2046d31d2ef;p=thirdparty%2Ffreeradius-server.git

do a better job of purifying xlats

set the can_purify flag in more places, add tests, and do
associated cleanups
---

diff --git a/src/lib/unlang/xlat_alloc.c b/src/lib/unlang/xlat_alloc.c
index d2c9ccc7454..a9fdb3b7765 100644
--- a/src/lib/unlang/xlat_alloc.c
+++ b/src/lib/unlang/xlat_alloc.c
@@ -39,7 +39,7 @@ xlat_exp_head_t *_xlat_exp_head_alloc(NDEBUG_LOCATION_ARGS TALLOC_CTX *ctx)
 	MEM(head = talloc_zero(ctx, xlat_exp_head_t));
 
 	fr_dlist_init(&head->dlist, xlat_exp_t, entry);
-	head->flags.pure = true;
+	head->flags.pure = head->flags.can_purify = true;
 #ifndef NDEBUG
 	head->file = file;
 	head->line = line;
@@ -107,7 +107,7 @@ static xlat_exp_t *xlat_exp_alloc_pool(NDEBUG_LOCATION_ARGS TALLOC_CTX *ctx, uns
 	xlat_exp_t *node;
 
 	MEM(node = talloc_zero_pooled_object(ctx, xlat_exp_t, extra_hdrs, extra));
-	node->flags.pure = true;	/* everything starts pure */
+	node->flags.pure = node->flags.can_purify = true;	/* everything starts pure */
 	node->quote = T_BARE_WORD;
 #ifndef NDEBUG
 	node->file = file;
diff --git a/src/lib/unlang/xlat_expr.c b/src/lib/unlang/xlat_expr.c
index 236fe9d4a12..f57979e2ebb 100644
--- a/src/lib/unlang/xlat_expr.c
+++ b/src/lib/unlang/xlat_expr.c
@@ -2764,13 +2764,13 @@ static fr_slen_t tokenize_field(xlat_exp_head_t *head, xlat_exp_t **out, fr_sbuf
 		}
 
 		node->flags.constant = true;
-		node->flags.pure = true;
+		node->flags.pure = node->flags.can_purify = true;
 
 	} else if (tmpl_contains_xlat(node->vpt)) {
 		node->flags = tmpl_xlat(vpt)->flags;
 
 	} else if (tmpl_is_attr(node->vpt)) {
-		node->flags.pure = false;
+		node->flags.pure = node->flags.can_purify = false;
 
 #ifndef NDEBUG
 		if (vpt->name[0] == '%') {
diff --git a/src/lib/unlang/xlat_priv.h b/src/lib/unlang/xlat_priv.h
index 08132bb6412..f5bb04e308c 100644
--- a/src/lib/unlang/xlat_priv.h
+++ b/src/lib/unlang/xlat_priv.h
@@ -228,7 +228,7 @@ static inline CC_HINT(nonnull) void xlat_flags_merge(xlat_flags_t *parent, xlat_
 {
 	parent->needs_resolving |= child->needs_resolving;
 	parent->pure &= child->pure; /* purity can only be removed, never added */
-	parent->can_purify |= child->can_purify;
+	parent->can_purify |= child->can_purify; /* there is SOME node under us which can be purified */
 	parent->constant &= child->constant;
 	parent->impure_func |= child->impure_func;
 }
diff --git a/src/lib/unlang/xlat_purify.c b/src/lib/unlang/xlat_purify.c
index 486f2eb58a0..7c8381e6c77 100644
--- a/src/lib/unlang/xlat_purify.c
+++ b/src/lib/unlang/xlat_purify.c
@@ -88,10 +88,18 @@ int xlat_purify_list(xlat_exp_head_t *head, request_t *request)
 				node->flags = child->flags;
 				break;
 			}
-			FALL_THROUGH;
+			break;
+
+		case XLAT_BOX:
+		case XLAT_ONE_LETTER:
+		case XLAT_REGEX:
+			break;
 
-		default:
-			fr_strerror_printf("Internal error - cannot purify xlat");
+		case XLAT_INVALID:
+		case XLAT_FUNC_UNRESOLVED:
+		case XLAT_VIRTUAL:
+		case XLAT_VIRTUAL_UNRESOLVED:
+			fr_assert(0);
 			return -1;
 
 		case XLAT_GROUP:
@@ -205,8 +213,11 @@ int xlat_purify(xlat_exp_head_t *head, unlang_interpret_t *intp)
 
 	rcode = xlat_purify_list(head, request);
 	talloc_free(request);
+	if (rcode < 0) return rcode;
 
-	return rcode;
+	fr_assert(!head->flags.can_purify);
+
+	return 0;
 }
 
 static fr_value_box_t *xlat_value_box(xlat_exp_t *node)
diff --git a/src/tests/unit/xlat/purify.txt b/src/tests/unit/xlat/purify.txt
index 263c601f4f5..9f773514d9c 100644
--- a/src/tests/unit/xlat/purify.txt
+++ b/src/tests/unit/xlat/purify.txt
@@ -44,6 +44,15 @@ match 45
 xlat_purify (2 + 3) * 4 + 5
 match 25
 
+#
+#  Not the same as a regex %{3}, but we can't tell the difference here.
+#
+xlat_purify %{1 + 2}
+match %{3}
+
+xlat_purify %{'a' + 'b'}
+match %{"ab"}
+
 xlat_purify NAS-Port + 5
 match (NAS-Port + 5)
 
@@ -198,13 +207,13 @@ match !%expr.rcode('fail')
 #  @todo - xlat_tokenize() does not call purify
 #
 xlat_purify (string)(%{1 + 2})
-match %cast(string, %{(1 + 2)})
+match "3"
 
 #
 #  This is a different code path than the above.
 #
 xlat_purify (string)%{1 + 2}
-match (string)%{(1 + 2)}
+match (string)%{3}
 
 xlat_purify "hello"
 match "hello"
@@ -218,12 +227,8 @@ match "hello 3"
 xlat_purify "hello " + (string)%{1 + 2} + " bob"
 match "hello 3 bob"
 
-#
-#  @todo - this seems wrong, but likely a bug in unit_test_attribute.c
-#  The real run-time tests work
-#
 xlat_purify "hello %{1 + 2} bob"
-match "hello %{(1 + 2)} bob"
+match "hello %{3} bob"
 
 #
 #  New syntax!
@@ -262,4 +267,4 @@ xlat_purify %md5("foo")
 match 0xacbd18db4cc2f85cedef654fccc4a4d8
 
 count
-match 116
+match 120