From: Alan T. DeKok <aland@freeradius.org>
Date: Wed, 7 May 2025 11:12:10 +0000 (-0400)
Subject: rewrite cf_reference_item()
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=41a08c09a0ae2b5c3b8cf435c0d248801022353b;p=thirdparty%2Ffreeradius-server.git

rewrite cf_reference_item()

while it's now more code, the code is at least clearer, and also
returns descriptive errors sayng exactly what went wrong.

Update the callers to print out these errors
---

diff --git a/src/lib/server/cf_file.c b/src/lib/server/cf_file.c
index 12a3fc8d3f4..dd53ff46354 100644
--- a/src/lib/server/cf_file.c
+++ b/src/lib/server/cf_file.c
@@ -295,7 +295,7 @@ char const *cf_expand_variables(char const *cf, int lineno,
 			ci = cf_reference_item(parent_cs, outer_cs, name);
 			if (!ci) {
 				if (soft_fail) *soft_fail = true;
-				ERROR("%s[%d]: Reference \"${%s}\" not found", cf, lineno, name);
+				PERROR("%s[%d]: Failed finding reference \"${%s}\"", cf, lineno, name);
 				return NULL;
 			}
 
@@ -1215,8 +1215,8 @@ static int process_template(cf_stack_t *stack)
 
 	ci = cf_reference_item(parent_cs, templatecs, stack->buff[2]);
 	if (!ci || (ci->type != CONF_ITEM_SECTION)) {
-		ERROR("%s[%d]: No such template \"%s\" in the 'templates' section.",
-		      frame->filename, frame->lineno, stack->buff[2]);
+		PERROR("%s[%d]: Failed finding item \"%s\" in the 'templates' section.",
+		       frame->filename, frame->lineno, stack->buff[2]);
 		return -1;
 	}
 
@@ -3734,10 +3734,18 @@ CONF_ITEM *cf_reference_item(CONF_SECTION const *parent_cs,
 	CONF_PAIR		*cp;
 	CONF_SECTION		*next;
 	CONF_SECTION const	*cs = outer_cs;
-	char			name[8192];
-	char			*p;
+	char			name[8192], *p;
+	char const		*name2;
 
-	if (!ptr || (!parent_cs && !outer_cs)) return NULL;
+	if (!ptr || (!parent_cs && !outer_cs)) {
+		fr_strerror_const("Invalid argument");
+		return NULL;
+	}
+
+	if (!*ptr) {
+		fr_strerror_const("Empty string is invalid");
+		return NULL;
+	}
 
 	strlcpy(name, ptr, sizeof(name));
 
@@ -3752,7 +3760,7 @@ CONF_ITEM *cf_reference_item(CONF_SECTION const *parent_cs,
 		/*
 		 *	Just '.' means the current section
 		 */
-		if (*p == '\0') return cf_section_to_item(cs);
+		if (*p == '\0') return cf_section_to_item(cs); /* const issues */
 
 		/*
 		 *	..foo means "foo from the section
@@ -3765,95 +3773,190 @@ CONF_ITEM *cf_reference_item(CONF_SECTION const *parent_cs,
 			 *	.. means the section
 			 *	enclosing this section
 			 */
-			if (!*++p) return cf_section_to_item(cs);
+			if (!*++p) return cf_section_to_item(cs); /* const issues */
 		}
 
 		/*
-		 *	"foo.bar.baz" means "from the root"
+		 *	"foo.bar.baz" means "from the given root"
 		 */
 	} else if (strchr(p, '.') != NULL) {
-		if (!parent_cs) return NULL;
+		if (!parent_cs) {
+		missing_parent:
+			fr_strerror_const("Missing parent configuration section");
+			return NULL;
+		}
+		cs = parent_cs;
+
+		/*
+		 *	"foo" could be from the current section, either as a
+		 *	section or as a pair.
+		 *
+		 *	If that isn't found, search from the given root.
+		 */
+	} else {
+		next = cf_section_find(cs, p, NULL);
+		if (!next && cs->template) next = cf_section_find(cs->template, p, NULL);
+		if (next) return &(next->item);
+
+		cp = cf_pair_find(cs, p);
+		if (!cp && cs->template) cp = cf_pair_find(cs->template, p);
+		if (cp) return &(cp->item);
+
+		if (!parent_cs) goto missing_parent;
 		cs = parent_cs;
 	}
 
+	/*
+	 *	Chop the string into pieces, and look up the pieces.
+	 */
 	while (*p) {
-		char *q, *r;
+		char *n1, *n2, *q;
+
+		ERROR("Parent %s %s at %d - %s",
+		      cf_section_name1(cs), cf_section_name2(cs), __LINE__, p);
 
-		r = strchr(p, '[');
-		q = strchr(p, '.');
-		if (!r && !q) break;
+		n1 = p;
+		n2 = NULL;
+		q = p;
 
-		if (r && q > r) q = NULL;
-		if (q && q < r) r = NULL;
+		fr_assert(*q);
 
 		/*
-		 *	Split off name2.
+		 *	Look for a terminating '.' or '[', to get name1 and possibly name2.
 		 */
-		if (r) {
-			q = strchr(r + 1, ']');
-			if (!q) return NULL; /* parse error */
-
+		while (*q != '\0') {
 			/*
-			 *	Points to foo[bar]xx: parse error,
-			 *	it should be foo[bar] or foo[bar].baz
+			 *	foo.bar -> return "foo"
 			 */
-			if (q[1] && q[1] != '.') return NULL;
+			if (*q == '.') {
+				*q++ = '\0'; /* leave 'q' after the '.' */
+				break;
+			}
 
-			*r = '\0';
-			*q = '\0';
-			next = cf_section_find(cs, p, r + 1);
-			if (!next && cs->template) next = cf_section_find(cs->template, p, r + 1);
-			*r = '[';
-			*q = ']';
+			/*
+			 *	name1 is anything up to '[' or EOS.
+			 */
+			if (*q != '[') {
+				q++;
+				continue;
+			}
 
 			/*
-			 *	Points to a named instance of a section.
+			 *	Found "name1[", look for "name2]" or "name2]."
 			 */
-			if (!q[1]) {
-				if (!next) return NULL;
-				return &(next->item);
+			*q++ = '\0';
+			n2 = q;
+
+			while (*q != '\0') {
+				if (*q == '[') {
+					fr_strerror_const("Invalid reference, '[' cannot be used inside of a '[...]' block");
+					return NULL;
+				}
+
+				if (*q != ']') {
+					q++;
+					continue;
+				}
+
+				/*
+				 *	We've found the trailing ']'
+				 */
+				*q++ = '\0';
+
+				/*
+				 *	"name2]"
+				 */
+				if (!*q) break;
+
+				/*
+				 *	Must be "name2]."
+				 */
+				if (*q++ == '.') break;
+
+				fr_strerror_const("Invalid reference, ']' is not followed by '.'");
+				return NULL;
 			}
 
-			q++;	/* ensure we skip the ']' and '.' */
+			if (n2) break;
 
-		} else {
-			*q = '\0';
-			next = cf_section_find(cs, p, NULL);
-			if (!next && cs->template) next = cf_section_find(cs->template, p, NULL);
-			*q = '.';
+			/*
+			 *	"name1[name2", but not "name1[name2]"
+			 */
+			fr_strerror_printf("Invalid reference after '%s', missing close ']'", n2);
+			return NULL;
 		}
+		p = q;		/* get it ready for the next round */
 
-		if (!next) break; /* it MAY be a pair in this section! */
+		/*
+		 *	End of the string.  The thing could be a section with
+		 *	two names, a section with one name, or a pair.
+		 *
+		 *	And if we don't find the thing we're looking for here,
+		 *	check the template section.
+		 */
+		if (!*p) {
+			/*
+			 *	Two names, must be a section.
+			 */
+			if (n2) {
+				next = cf_section_find(cs, n1, n2);
+				if (!next && cs->template) next = cf_section_find(cs->template, n1, n2);
+				if (next) return &(next->item);
+
+			fail:
+				name2 = cf_section_name2(cs);
+				fr_strerror_printf("Parent section %s%s%s { ... } does not contain a %s %s { ... } configuration section",
+						   cf_section_name1(cs),
+						   name2 ? " " : "", name2 ? name2 : "",
+						   n1, n2);
+				return NULL;
+			}
 
-		cs = next;
-		p = q + 1;
-	}
+			/*
+			 *	One name, the final thing can be a section or a pair.
+			 */
+			next = cf_section_find(cs, n1, NULL);
+			if (!next && cs->template) next = cf_section_find(cs->template, n1, NULL);
 
-	if (!*p) return NULL;
+			if (next) return &(next->item);
 
-retry:
-	/*
-	 *	Find it in the current referenced
-	 *	section.
-	 */
-	cp = cf_pair_find(cs, p);
-	if (!cp && cs->template) cp = cf_pair_find(cs->template, p);
-	if (cp) {
-		cp->referenced = true;	/* conf pairs which are referenced count as used */
-		return &(cp->item);
-	}
+			cp = cf_pair_find(cs, n1);
+			if (!cp && cs->template) cp = cf_pair_find(cs->template, n1);
+			if (cp) return &(cp->item);
 
-	next = cf_section_find(cs, p, NULL);
-	if (next) return &(next->item);
+			name2 = cf_section_name2(cs);
+			fr_strerror_printf("Parent section %s%s%s  { ... } does not contain a %s configuration item",
+					   cf_section_name1(cs),
+					   name2 ? " " : "", name2 ? name2 : "",
+					   n1);
+			return NULL;
+		}
 
-	/*
-	 *	"foo" is "in the current section, OR in main".
-	 */
-	if ((p == name) && (parent_cs != NULL) && (cs != parent_cs)) {
-		cs = parent_cs;
-		goto retry;
+		/*
+		 *	There's more to the string.  The thing we're looking
+		 *	for MUST be a configuration section.
+		 */
+		next = cf_section_find(cs, n1, n2);
+		if (!next && cs->template) next = cf_section_find(cs->template, n1, n2);
+		if (next) {
+			cs = next;
+			continue;
+		}
+
+		if (n2) goto fail;
+
+		name2 = cf_section_name2(cs);
+		fr_strerror_printf("Parent section %s%s%s { ... } does not contain a %s { ... } configuration section",
+				   cf_section_name1(cs),
+				   name2 ? " " : "", name2 ? name2 : "",
+				   n1);
+		return NULL;
 	}
 
+	/*
+	 *	We've fallen off of the end of the string.  This should not have happened!
+	 */
+	fr_strerror_const("Cannot parse reference");
 	return NULL;
 }
 
diff --git a/src/lib/server/main_config.c b/src/lib/server/main_config.c
index afe06ee91ad..62becaa5f0e 100644
--- a/src/lib/server/main_config.c
+++ b/src/lib/server/main_config.c
@@ -576,7 +576,7 @@ static xlat_action_t xlat_config(TALLOC_CTX *ctx, fr_dcursor_t *out,
 
 	ci = cf_reference_item(main_config->root_cs, main_config->root_cs, in_head->vb_strvalue);
 	if (!ci || !cf_item_is_pair(ci)) {
-		REDEBUG("Config item \"%pV\" does not exist", in_head);
+		RPEDEBUG("Failed finding configuration item '%s'", in_head->vb_strvalue);
 		return XLAT_ACTION_FAIL;
 	}
 
diff --git a/src/lib/server/trigger.c b/src/lib/server/trigger.c
index e16037bd13b..141438ec49e 100644
--- a/src/lib/server/trigger.c
+++ b/src/lib/server/trigger.c
@@ -277,7 +277,7 @@ int trigger_exec(unlang_interpret_t *intp,
 
 	ci = cf_reference_item(subcs, trigger_exec_main, attr);
 	if (!ci) {
-		DEBUG3("No trigger configured for: %s", attr);
+		DEBUG3("Failed finding trigger '%s'", attr);
 		return -1;
 	}
 
diff --git a/src/lib/unlang/compile.c b/src/lib/unlang/compile.c
index 5027c28807a..9e52ac0f7c4 100644
--- a/src/lib/unlang/compile.c
+++ b/src/lib/unlang/compile.c
@@ -1833,9 +1833,14 @@ bool unlang_compile_actions(unlang_mod_actions_t *actions, CONF_SECTION *action_
 			CONF_ITEM *subci;
 			char const *value = cf_pair_value(cp);
 
+			if (!value) {
+				cf_log_err(csi, "Missing reference string");
+				return false;
+			}
+
 			subci = cf_reference_item(cs, cf_root(cf_section_to_item(action_cs)), value);
 			if (!subci) {
-				cf_log_err(csi, "Unknown reference '%s'", value ? value : "<INVALID>");
+				cf_log_perr(csi, "Failed finding reference '%s'", value);
 				return false;
 			}
 
diff --git a/src/modules/rlm_linelog/rlm_linelog.c b/src/modules/rlm_linelog/rlm_linelog.c
index 6638df15e08..37075e901c6 100644
--- a/src/modules/rlm_linelog/rlm_linelog.c
+++ b/src/modules/rlm_linelog/rlm_linelog.c
@@ -734,7 +734,7 @@ static unlang_action_t CC_HINT(nonnull) mod_do_linelog(rlm_rcode_t *p_result, mo
 
 		ci = cf_reference_item(NULL, inst->cs, p);
 		if (!ci) {
-			RDEBUG2("Path \"%s\" doesn't exist", p);
+			RPDEBUG2("Failed finding reference '%s'", p);
 			goto default_msg;
 		}