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 111faad75dfea8a5c7232ed08feab2d7aeea6fa4..ca8f8cb3149d2aed2b3beebb81e1481cc0ea23a9 100644 (file)
--- 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 58aaf2ca2b719c8cda1ff52d1db713823129bc5a..046c22983351733738f7d12c4318e70d7a620834 100644 (file)
--- 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 3d58ee1418b46376801aa2258f7ddc7115ef89b5..8836d9eab6b76dff4a76aa1bce88412124397f5b 100644 (file)
--- 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 (file)
index 0000000..c3652af
--- /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