From: Alan T. DeKok <aland@freeradius.org>
Date: Thu, 11 Aug 2022 15:31:11 +0000 (-0400)
Subject: only groups can use "ref=".
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=528af86fbbb65738e365aa72238b057653a5803f;p=thirdparty%2Ffreeradius-server.git

only groups can use "ref=".

Perhaps when other things get fixed, we can have TLVs use
"ref=" instead of "clone="
---

diff --git a/src/lib/util/dict_tokenize.c b/src/lib/util/dict_tokenize.c
index c9223402fb5..7fb0fdb6d6d 100644
--- a/src/lib/util/dict_tokenize.c
+++ b/src/lib/util/dict_tokenize.c
@@ -888,6 +888,9 @@ static int dict_read_process_attribute(dict_tokenize_ctx_t *ctx, char **argv, in
 			goto check;
 		}
 
+		/*
+		 *	Else we find the reference.
+		 */
 		da = fr_dict_attr_by_oid(NULL, parent, ref);
 		if (da) {
 			dict = ctx->dict;
@@ -900,7 +903,7 @@ static int dict_read_process_attribute(dict_tokenize_ctx_t *ctx, char **argv, in
 		 *	dictionary.
 		 */
 		p = strchr(ref, '.');
-		if (!p) goto save;
+		if (!p) goto fixup;
 
 		/*
 		 *	Get / skip protocol name.
@@ -915,10 +918,13 @@ static int dict_read_process_attribute(dict_tokenize_ctx_t *ctx, char **argv, in
 		 *	No known dictionary, so we're asked to just
 		 *	use the whole string.  Which we did above.  So
 		 *	either it's a bad ref, OR it's a ref to a
-		 *	dictionary which doesn't exist.
+		 *	dictionary which hasn't yet been loaded.
+		 *
+		 *	Save the fixup for later, when we've hopefully
+		 *	loaded the dictionary.
 		 */
 		if (slen == 0) {
-		save:
+		fixup:
 			if (dict_fixup_group(&ctx->fixup, CURRENT_FRAME(ctx)->filename, CURRENT_FRAME(ctx)->line,
 					     self, ref, talloc_array_length(ref) - 1) < 0) {
 			oom:
diff --git a/src/lib/util/dict_util.c b/src/lib/util/dict_util.c
index 701036b9e62..fd3c5f553a8 100644
--- a/src/lib/util/dict_util.c
+++ b/src/lib/util/dict_util.c
@@ -563,13 +563,23 @@ int dict_attr_init(fr_dict_attr_t **da_p,
 	switch (type) {
 	case FR_TYPE_STRUCTURAL:
 	structural:
-		if (dict_attr_ref_init(da_p, NULL) < 0) return -1;	/* Just allocate space */
-
 		/*
-		 *	Groups don't have children or
-		 *	namespaces.
+		 *	Groups don't have children or namespaces.  But
+		 *	they always have refs.  Either to the root of
+		 *	the current dictionary, or to another dictionary,
+		 *	via its top-level TLV.
+		 *
+		 *	Note that when multiple TLVs have the same
+		 *	children, the dictionary has to use "clone="
+		 *	instead of "ref=".  That's because the
+		 *	children of the TLVs all require the correct
+		 *	parentage.  Perhaps that can be changed when
+		 *	the encoders / decoders are updated.  It would be good to just reference the DAs instead of cloning an entire subtree.
 		 */
-		if (type == FR_TYPE_GROUP) break;
+		if (type == FR_TYPE_GROUP) {
+			if (dict_attr_ref_init(da_p, NULL) < 0) return -1;
+			break;
+		}
 
 		if (dict_attr_children_init(da_p) < 0) return -1;
 		if (dict_attr_namespace_init(da_p) < 0) return -1;	/* Needed for all TLV style attributes */