From: Alan T. DeKok <aland@freeradius.org>
Date: Fri, 8 Sep 2023 19:01:47 +0000 (-0400)
Subject: hoist TLV checks to before creating the VP
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=977671b37975e6071aae13b1dea4cc989fc41ba9;p=thirdparty%2Ffreeradius-server.git

hoist TLV checks to before creating the VP

because there's no point in creating the VP and then freeing it.
---

diff --git a/src/bin/radsnmp.c b/src/bin/radsnmp.c
index 13ad1b44694..c9358de3fc7 100644
--- a/src/bin/radsnmp.c
+++ b/src/bin/radsnmp.c
@@ -309,6 +309,16 @@ static ssize_t radsnmp_pair_from_oid(TALLOC_CTX *ctx, radsnmp_conf_t *conf, fr_d
 		da = parent;
 	}
 
+	/*
+	 *	TLVs are special, they cannot hold values.  The caller should know this!
+	 */
+	if (da->type == FR_TYPE_TLV) {
+		if (!value) return slen;
+
+		fr_strerror_const("TLVs cannot hold values");
+		return -(slen);
+	}
+
 	MEM(vp = fr_pair_afrom_da(ctx, da));
 
 	/*
@@ -316,17 +326,6 @@ static ssize_t radsnmp_pair_from_oid(TALLOC_CTX *ctx, radsnmp_conf_t *conf, fr_d
 	 */
 	if (!value) {
 		switch (da->type) {
-		/*
-		 *	We can blame the authors of RFC 6929 for
-		 *	this hack.  Apparently the presence or absence
-		 *	of an attribute isn't considered a useful means
-		 *	of conveying information, so empty TLVs are
-		 *	disallowed.
-		 */
-		case FR_TYPE_TLV:
-			fr_pair_to_unknown(vp);
-			FALL_THROUGH;
-
 		case FR_TYPE_OCTETS:
 			fr_pair_value_memdup(vp, (uint8_t const *)"\0", 1, true);
 			break;
@@ -346,11 +345,6 @@ static ssize_t radsnmp_pair_from_oid(TALLOC_CTX *ctx, radsnmp_conf_t *conf, fr_d
 		return slen;
 	}
 
-	if (da->type == FR_TYPE_TLV) {
-		fr_strerror_const("TLVs cannot hold values");
-		return -(slen);
-	}
-
 	ret = fr_pair_value_from_str(vp, value, strlen(value), NULL, true);
 	if (ret < 0) {
 		slen = -(slen);