From: Alan T. DeKok <aland@freeradius.org>
Date: Fri, 15 Jan 2021 16:19:15 +0000 (-0500)
Subject: call fr_value_box_from_network()
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ea6e4fe7eecb32f62c0dce23269ac2b96aa7a021;p=thirdparty%2Ffreeradius-server.git

call fr_value_box_from_network()

instead of re-implementing a subset of that functionality

and add a test case for it
---

diff --git a/src/modules/rlm_unpack/rlm_unpack.c b/src/modules/rlm_unpack/rlm_unpack.c
index 111faad75df..ca8f8cb3149 100644
--- a/src/modules/rlm_unpack/rlm_unpack.c
+++ b/src/modules/rlm_unpack/rlm_unpack.c
@@ -63,6 +63,7 @@ static ssize_t unpack_xlat(UNUSED TALLOC_CTX *ctx, char **out, size_t outlen,
 			   UNUSED void const *mod_inst, UNUSED void const *xlat_inst,
 			   request_t *request, char const *fmt)
 {
+	bool tainted = false;
 	char *data_name, *data_size, *data_type;
 	char *p;
 	size_t len, input_len;
@@ -70,7 +71,7 @@ static ssize_t unpack_xlat(UNUSED TALLOC_CTX *ctx, char **out, size_t outlen,
 	int offset;
 	fr_type_t type;
 	fr_dict_attr_t const *da;
-	fr_pair_t *vp, *cast;
+	fr_pair_t *vp;;
 	uint8_t const *input;
 	char buffer[256];
 	uint8_t blob[256];
@@ -114,15 +115,18 @@ static ssize_t unpack_xlat(UNUSED TALLOC_CTX *ctx, char **out, size_t outlen,
 	 *	Attribute reference
 	 */
 	if (*data_name == '&') {
-		if (xlat_fmt_get_vp(&vp, request, data_name) < 0) goto nothing;
+		fr_pair_t *from_vp;
 
-		if ((vp->vp_type != FR_TYPE_OCTETS) &&
-		    (vp->vp_type != FR_TYPE_STRING)) {
+		if (xlat_fmt_get_vp(&from_vp, request, data_name) < 0) goto nothing;
+
+		if ((from_vp->vp_type != FR_TYPE_OCTETS) &&
+		    (from_vp->vp_type != FR_TYPE_STRING)) {
 			REDEBUG("unpack requires the input attribute to be 'string' or 'octets'");
 			goto nothing;
 		}
-		input = vp->vp_octets;
-		input_len = vp->vp_length;
+		input = from_vp->vp_octets;
+		input_len = from_vp->vp_length;
+		tainted = from_vp->vp_tainted;
 
 	} else if ((data_name[0] == '0') && (data_name[1] == 'x')) {
 		/*
@@ -140,8 +144,7 @@ static ssize_t unpack_xlat(UNUSED TALLOC_CTX *ctx, char **out, size_t outlen,
 				goto nothing;
 			}
 		} else {
-			input = blob;
-			input_len = 0;
+			GOTO_ERROR;
 		}
 	} else {
 		GOTO_ERROR;
@@ -149,26 +152,18 @@ static ssize_t unpack_xlat(UNUSED TALLOC_CTX *ctx, char **out, size_t outlen,
 
 	offset = (int) strtoul(data_size, &p, 10);
 	if (*p) {
-		REDEBUG("unpack requires a float64 number, not '%s'", data_size);
+		REDEBUG("unpack requires a decimal number, not '%s'", data_size);
 		goto nothing;
 	}
 
-	type = fr_table_value_by_str(fr_value_box_type_table, data_type, FR_TYPE_INVALID);
-	if (type == FR_TYPE_INVALID) {
-		REDEBUG("Invalid data type '%s'", data_type);
+	if (offset >= input_len) {
+		REDEBUG("unpack offset %d is larger than input data length %zd", offset, input_len);
 		goto nothing;
 	}
 
-	/*
-	 *	Output must be a non-zero limited size.
-	 */
-	if ((min_size(type) ==  0) || !is_fixed_size(type)) {
-		REDEBUG("unpack requires fixed-size output type, not '%s'", data_type);
-		goto nothing;
-	}
-
-	if (input_len < (offset + min_size(type))) {
-		REDEBUG("Insufficient data to unpack '%s' from '%s'", data_type, data_name);
+	type = fr_table_value_by_str(fr_value_box_type_table, data_type, FR_TYPE_INVALID);
+	if (type == FR_TYPE_INVALID) {
+		REDEBUG("Invalid data type '%s'", data_type);
 		goto nothing;
 	}
 
@@ -178,37 +173,25 @@ static ssize_t unpack_xlat(UNUSED TALLOC_CTX *ctx, char **out, size_t outlen,
 		goto nothing;
 	}
 
-	MEM(cast = fr_pair_afrom_da(request, da));
-
-	memcpy(&(cast->data), input + offset, min_size(type));
+	MEM(vp = fr_pair_afrom_da(request, da));
 
 	/*
-	 *	Hacks
+	 *	Call the generic routines to get data from the
+	 *	"network" buffer.
+	 *
+	 *	@todo - just parse / print the value-box directly,
+	 *	instead of putting it into a VP.
+	 *
+	 *	@todo - make this function 'async', and just return a
+	 *	copy of the value-box, instead of printing it to a string.
 	 */
-	switch (type) {
-	case FR_TYPE_INT32:
-	case FR_TYPE_UINT32:
-		cast->vp_uint32 = ntohl(cast->vp_uint32);
-		break;
-
-	case FR_TYPE_UINT16:
-		cast->vp_uint16 = ((input[offset] << 8) | input[offset + 1]);
-		break;
-
-	case FR_TYPE_UINT64:
-		cast->vp_uint64 = ntohll(cast->vp_uint64);
-		break;
-
-	case FR_TYPE_DATE:
-		cast->vp_date = fr_time_from_timeval(&(struct timeval) {.tv_sec = ntohl(cast->vp_uint32)});
-		break;
-
-	default:
-		break;
+	if (fr_value_box_from_network(vp, &vp->data, da->type, NULL, input + offset, input_len - offset, tainted) < 0) {
+		RPEDEBUG("Failed decoding %s", vp->da->name);
+		goto nothing;
 	}
 
-	slen = fr_pair_print_value_quoted(&FR_SBUFF_OUT(*out, outlen), cast, T_BARE_WORD);
-	talloc_free(cast);
+	slen = fr_pair_print_value_quoted(&FR_SBUFF_OUT(*out, outlen), vp, T_BARE_WORD);
+	talloc_free(vp);
 	if (slen < 0) {
 		REDEBUG("Insufficient buffer space to unpack data");
 		goto nothing;
diff --git a/src/tests/keywords/all.mk b/src/tests/keywords/all.mk
index 58aaf2ca2b7..046c2298335 100644
--- a/src/tests/keywords/all.mk
+++ b/src/tests/keywords/all.mk
@@ -80,7 +80,7 @@ KEYWORD_LIBS	:= $(addsuffix .la,$(addprefix rlm_,$(KEYWORD_MODULES))) rlm_cache.
 #  Otherwise, check the log file for a parse error which matches the
 #  ERROR line in the input.
 #
-$(OUTPUT)/%: $(DIR)/% $(TEST_BIN_DIR)/unit_test_module | $(KEYWORD_RADDB) $(KEYWORD_LIBS) build.raddb rlm_cache_rbtree.la rlm_test.la rlm_csv.la
+$(OUTPUT)/%: $(DIR)/% $(TEST_BIN_DIR)/unit_test_module | $(KEYWORD_RADDB) $(KEYWORD_LIBS) build.raddb rlm_cache_rbtree.la rlm_test.la rlm_csv.la rlm_unpack.la
 	@echo "KEYWORD-TEST $(notdir $@)"
 	${Q}cp $(if $(wildcard $<.attrs),$<.attrs,$(dir $<)/default-input.attrs) $@.attrs
 	${Q}if ! KEYWORD=$(notdir $@) $(TEST_BIN)/unit_test_module -D share/dictionary -d src/tests/keywords/ -i "$@.attrs" -f "$@.attrs" -r "$@" -xx > "$@.log" 2>&1 || ! test -f "$@"; then \
diff --git a/src/tests/keywords/unit_test_module.conf b/src/tests/keywords/unit_test_module.conf
index 3d58ee1418b..8836d9eab6b 100644
--- a/src/tests/keywords/unit_test_module.conf
+++ b/src/tests/keywords/unit_test_module.conf
@@ -40,6 +40,11 @@ modules {
 		fields = "field1,,field3"
 		index_field = 'field1'
 	}
+
+	unpack {
+
+	}
+
 }
 
 policy {
diff --git a/src/tests/keywords/unpack b/src/tests/keywords/unpack
new file mode 100644
index 00000000000..c3652afad96
--- /dev/null
+++ b/src/tests/keywords/unpack
@@ -0,0 +1,25 @@
+#
+#  PRE: if update
+#
+update request {
+	&Framed-IP-Address := 127.0.0.1
+}
+
+update request {
+	&Tmp-Octets-0 := &Framed-IP-Address
+}
+
+update request {
+	&Tmp-String-0 := "%{unpack: &Tmp-Octets-0 0 ipaddr}"
+	&Tmp-IP-Address-0 := "%{unpack: &Tmp-Octets-0 0 ipaddr}"
+}
+
+if (&Tmp-String-0 != "127.0.0.1") {
+	test_fail
+}
+
+if (&Tmp-IP-Address-0 != 127.0.0.1) {
+	test_fail
+}
+
+success