cleanups and comments for future work

diff --git a/src/lib/util/dict.h b/src/lib/util/dict.h
index 4b4f0dd42c0e7fdc77728c2a5a15adf9c8bbefc2..dcbe278857830c06e8ba377bece22b5fc4368eb2 100644 (file)
--- a/src/lib/util/dict.h
+++ b/src/lib/util/dict.h
@@ -87,23 +87,39 @@ typedef struct {
 
        unsigned int            virtual : 1;                    //!< for dynamic expansion
 
+       /*
+        *      @todo - if we want to clean these fields up, make
+        *      "subtype" and "type_size" both 4-bit bitfields.  That
+        *      gives us an extra 8 bits for adding new flags, and we
+        *      can likely get rid of "extra", in order to save one
+        *      more bit.
+        */
        unsigned int            extra : 1;                      //!< really "subtype is used by dict, not by protocol"
 
+       /*
+        *      main: extra is set, then this field is is key, bit, or a uint16 length field.
+        *      radius: is one of 9 options for flags
+        *      dhcp v4/v6: DNS label, or partial DNS label
+        */
        uint8_t                 subtype;                        //!< protocol-specific values, OR key fields
 
+       /*
+        *      Length in bytes for most attributes.
+        *      Length in bits for da_is_bit_field(da)
+        */
        uint8_t                 length;                         //!< length of the attribute
 
        /*
         *      TLVs: 1, 2, or 4.
         *      date / time types: fr_time_res_t, which has 4 possible values.
-        *      bit fields: offset in the byte where this bit field ends.
+        *      bit fields: offset in the byte where this bit field ends, which is only
+        *      used as a caching mechanism during parsing of the dictionaries.
         */
        uint8_t                 type_size;                      //!< For TLV2 and root attributes.
 } fr_dict_attr_flags_t;
 
 #define flag_time_res type_size
 #define flag_byte_offset type_size
-#define flag_struct_offset type_size
 
 /** subtype values for the dictionary when extra=1
  *

diff --git a/src/lib/util/dict_tokenize.c b/src/lib/util/dict_tokenize.c
index dda9b950933c9e1320fe03314542dce5c879e432..713d44d11c821adff0336f3252d4620073426432 100644 (file)
--- a/src/lib/util/dict_tokenize.c
+++ b/src/lib/util/dict_tokenize.c
@@ -203,7 +203,10 @@ static int dict_process_type_field(dict_tokenize_ctx_t *ctx, char const *name, f
                        return -1;
                }
 
-               if ((length == 0) || (length > 253)) {
+               /*
+                *      "length" has to fit into a uint8_t field.
+                */
+               if ((length == 0) || (length > 255)) {
                        fr_strerror_printf("Invalid length for '%s[...]'", name);
                        return -1;
                }
@@ -245,7 +248,11 @@ static int dict_process_type_field(dict_tokenize_ctx_t *ctx, char const *name, f
                        }
 
                        /*
-                        *      We track where on a byte boundary this bit field ends.
+                        *      Cache where on a byte boundary this
+                        *      bit field ends.  We could have the
+                        *      validation function loop through all
+                        *      previous siblings, but that's
+                        *      annoying.
                         */
                        flags->flag_byte_offset = length;
 
@@ -948,8 +955,7 @@ static int dict_read_process_member(dict_tokenize_ctx_t *ctx, char **argv, int a
                         *      located.
                         */
                        if (flags.extra && (flags.subtype == FLAG_BIT_FIELD)) {
-                               flags.flag_byte_offset += previous->flags.flag_byte_offset;
-                               flags.flag_byte_offset &= 0x07;
+                               flags.flag_byte_offset = (flags.length + previous->flags.flag_byte_offset) & 0x07;
 
                        } else {
                                if (previous->flags.flag_byte_offset != 0) {

diff --git a/src/protocols/radius/base.c b/src/protocols/radius/base.c
index 9765f1774f2897aaeda4b30ceb4b689128b8d72c..9006d679b02ba868efee5d8ff9439eef66049e85 100644 (file)
--- a/src/protocols/radius/base.c
+++ b/src/protocols/radius/base.c
@@ -1138,6 +1138,11 @@ static bool attr_valid(UNUSED fr_dict_t *dict, fr_dict_attr_t const *parent,
                return false;
        }
 
+       if (flags->length > 253) {
+               fr_strerror_printf("Attributes cannot be more than 253 octets in length");
+               return false;
+       }
+
        if (flags->subtype > FLAG_ENCRYPT_ASCEND_SECRET) {
                fr_strerror_printf("Invalid flag value %u", flags->subtype);
                return false;