add unions to struct decoder

and add test case for it

diff --git a/src/lib/util/struct.c b/src/lib/util/struct.c
index 51fa2025229dd66cb16587e40aeb872b7ce9c470..f4c5205c4c6c5e1cdd3e99315cd5937856be1f14 100644 (file)
--- a/src/lib/util/struct.c
+++ b/src/lib/util/struct.c
@@ -36,7 +36,7 @@ ssize_t fr_struct_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out,
 {
        unsigned int            child_num;
        uint8_t const           *p = data, *end = data + data_len;
-       fr_dict_attr_t const    *child;
+       fr_dict_attr_t const    *child, *substruct_da;
        fr_pair_list_t          child_list_head;
        fr_pair_list_t          *child_list;
        fr_pair_t               *vp, *key_vp, *struct_vp = NULL;
@@ -58,8 +58,7 @@ ssize_t fr_struct_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out,
 
        struct_vp = fr_pair_afrom_da(ctx, parent);
        if (!struct_vp) {
-               fr_strerror_const("out of memory");
-               return -1;
+               return PAIR_DECODE_OOM;
        }
 
        fr_pair_list_init(&child_list_head); /* still used elsewhere */
@@ -161,6 +160,8 @@ ssize_t fr_struct_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out,
                        vp = fr_pair_afrom_da(child_ctx, child);
                        if (!vp) {
                                FR_PROTO_TRACE("fr_struct_from_network - failed allocating child VP");
+                       oom:
+                               talloc_free(struct_vp);
                                return PAIR_DECODE_OOM;
                        }
 
@@ -199,19 +200,67 @@ ssize_t fr_struct_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out,
                }
                offset = 0;     /* reset for non-bit-field attributes */
 
+               child_length = child->flags.length;
+
+               /*
+                *      If this field overflows the input, then *all*
+                *      of the input is suspect.
+                */
+               if (child_length > (size_t) (end - p)) {        
+                       fprintf(stderr, "SHIT %s have %zd want %zd\n",
+                               child->name, (end - p), child_length);
+                       FR_PROTO_TRACE("fr_struct_from_network - child length %zu overflows buffer", child_length);
+                       goto unknown;
+               }
+
+               /*
+                *      The child is variable sized, OR it's an array.
+                *      Eat up the rest of the data.
+                */
+               if (!child_length || (child->flags.array)) {
+                       child_length = end - p;
+
+               } else if ((size_t) (end - p) < child_length) {
+                       fprintf(stderr, "SHIT %s have %zd want %zd\n",
+                               child->name, (end - p), child_length);
+                       FR_PROTO_TRACE("fr_struct_from_network - child length %zu underflows buffer", child_length);
+                       goto unknown;
+               }
+
+               /*
+                *      We only allow a limited number of data types
+                *      inside of a struct.
+                */
+               switch (child->type) {
+               default:
+                       /*
+                        *      Other structural types can only be decoded by a callback.
+                        */
+                       if (!decode_value) {
+                               FR_PROTO_TRACE("fr_struct_from_network - unknown child type");
+                               goto unknown;
+                       }
+                       break;
+
+               case FR_TYPE_LEAF:
+                       break;
+
                /*
                 *      Decode child TLVs, according to the parent attribute.
                 */
-               if (child->type == FR_TYPE_TLV) {
+               case FR_TYPE_TLV:
                        fr_assert(!key_vp);
 
                        if (!decode_tlv) {
                                fr_strerror_const("Decoding TLVs requires a decode_tlv() function to be passed");
+                               talloc_free(struct_vp);
                                return -(p - data);
                        }
 
                        /*
-                        *      Decode EVERYTHING as a TLV.
+                        *      Decode all of the remaining data as
+                        *      TLVs.  Any malformed TLVs are appended
+                        *      as raw VP.
                         */
                        while (p < end) {
                                slen = decode_tlv(child_ctx, child_list, child, p, end - p, decode_ctx);
@@ -223,29 +272,27 @@ ssize_t fr_struct_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out,
                        }
 
                        goto done;
-               }
-
-               child_length = child->flags.length;
 
                /*
-                *      If this field overflows the input, then *all*
-                *      of the input is suspect.
+                *      The child is a union, it MUST be at the end of
+                *      the struct, and we must have seen a key before
+                *      we reach the union.  See dict_tokenize.
                 */
-               if (child_length > (size_t) (end - p)) {
-                       FR_PROTO_TRACE("fr_struct_from_network - child length %zu overflows buffer", child_length);
-                       goto unknown;
-               }
+               case FR_TYPE_UNION:
+                       fr_assert(!fr_dict_attr_child_by_num(parent, child_num + 1));
+                       if (!fr_cond_assert(key_vp)) goto unknown;
 
-               /*
-                *      The child is variable sized, OR it's an array.
-                *      Eat up the rest of the data.
-                */
-               if (!child_length || (child->flags.array)) {
-                       child_length = end - p;
+                       /*
+                        *      Create the union wrapper, and reset the child_ctx and child_list to it.
+                        */
+                       vp = fr_pair_afrom_da(child_ctx, child);
+                       if (!vp) goto oom;
 
-               } else if ((size_t) (end - p) < child_length) {
-                       FR_PROTO_TRACE("fr_struct_from_network - child length %zu underflows buffer", child_length);
-                       goto unknown;
+                       fr_pair_append(child_list, vp);
+                       substruct_da = child;
+                       child_ctx = vp;
+                       child_list = &vp->vp_group;
+                       goto substruct;
                }
 
                /*
@@ -273,19 +320,6 @@ ssize_t fr_struct_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out,
                        continue;
                }
 
-               /*
-                *      We only allow a limited number of data types
-                *      inside of a struct.
-                */
-               switch (child->type) {
-               default:
-                       FR_PROTO_TRACE("fr_struct_from_network - unknown child type");
-                       goto unknown;
-
-               case FR_TYPE_LEAF:
-                       break;
-               }
-
                /*
                 *      We don't handle this yet here.
                 */
@@ -335,6 +369,10 @@ ssize_t fr_struct_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out,
         */
        if (key_vp) {
                fr_dict_enum_value_t const *enumv;
+
+               substruct_da = key_vp->da;
+
+       substruct:
                child = NULL;
 
                FR_PROTO_HEX_DUMP(p, (end - p), "fr_struct_from_network - substruct");
@@ -360,7 +398,7 @@ ssize_t fr_struct_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out,
                         *      incomparable.  And thus can all be
                         *      given the same number.
                         */
-                       child = fr_dict_attr_unknown_raw_afrom_num(child_ctx, key_vp->da, 0);
+                       child = fr_dict_attr_unknown_raw_afrom_num(child_ctx, substruct_da, 0);
                        if (!child) {
                                FR_PROTO_TRACE("failed allocating unknown child for key VP %s - %s",
                                               key_vp->da->name, fr_strerror());

diff --git a/src/tests/unit/protocols/radius/dictionary.test b/src/tests/unit/protocols/radius/dictionary.test
index 512aa2fd7d46bc5d906439a3f2e390169c4a94de..1839b97b04c4eab5a3f9cc167c784fa5fd29efd6 100644 (file)
--- a/src/tests/unit/protocols/radius/dictionary.test
+++ b/src/tests/unit/protocols/radius/dictionary.test
@@ -46,8 +46,8 @@ MEMBER                Data                                    union   key=Key-Field
 
 BEGIN Test-Struct2.Data
 ATTRIBUTE      Sub-Struct                              1       struct
-MEMBER         Nested-Sub1                             uint8
-MEMBER         Nested-Sub2                             uint8
+MEMBER         Nested-Uint1                            uint8
+MEMBER         Nested-Uint2                            uint8
 END Test-Struct2.Data
 
 ATTRIBUTE      Unit-TLV                                254     tlv

diff --git a/src/tests/unit/protocols/radius/union.txt b/src/tests/unit/protocols/radius/union.txt
new file mode 100644 (file)
index 0000000..0cd371f
--- /dev/null
+++ b/src/tests/unit/protocols/radius/union.txt
@@ -0,0 +1,32 @@
+#
+#  UNION tests
+#
+#  src/tests/dict checks if the dictionaries can be parsed.
+#  This file tests if the contents are OK.
+#
+proto radius
+proto-dictionary radius
+load-dictionary dictionary.test
+fuzzer-out radius
+
+#
+#  Too small.
+#
+decode-pair fd 04 01 21
+match Test-Struct2 = { Key-Field = ::Sub-Struct, Data = { Sub-Struct = { Nested-Uint1 = 33 } } }
+#match raw.Test-Struct2 = 0x0121
+
+#
+#  Too large.  Extra data is silently ignored.
+#
+decode-pair fd 06 01 21 12 33
+match Test-Struct2 = { Key-Field = ::Sub-Struct, Data = { Sub-Struct = { Nested-Uint1 = 33, Nested-Uint2 = 18 } } }
+
+#
+#  Just right.
+#
+decode-pair fd 05 01 21 12
+match Test-Struct2 = { Key-Field = ::Sub-Struct, Data = { Sub-Struct = { Nested-Uint1 = 33, Nested-Uint2 = 18 } } }
+
+count
+match 10