update fr_pair_verify() with argument to verify the values

so that we can check more often for invalid values.  The default
is "true" for the PAIR_VERIFY() and REQUEST_VERIFY() macros.

Update the various parsers to pass "false" if they add a VP to a
list before setting the value.  This lets the tests continue to
pass, but also ensures that at normal run-time, we do the full
checks.

diff --git a/src/lib/server/request.c b/src/lib/server/request.c
index 3c45dc3a118de537badf4b1b7f968c144a63bdd1..192a07ba912101b1e67127b93fbedb5a7d66555f 100644 (file)
--- a/src/lib/server/request.c
+++ b/src/lib/server/request.c
@@ -664,11 +664,11 @@ void request_verify(char const *file, int line, request_t const *request)
        fr_assert(request->magic == REQUEST_MAGIC);
 
        (void)talloc_get_type_abort(request->request_ctx, fr_pair_t);
-       fr_pair_list_verify(file, line, request->request_ctx, &request->request_pairs);
+       fr_pair_list_verify(file, line, request->request_ctx, &request->request_pairs, true);
        (void)talloc_get_type_abort(request->reply_ctx, fr_pair_t);
-       fr_pair_list_verify(file, line, request->reply_ctx, &request->reply_pairs);
+       fr_pair_list_verify(file, line, request->reply_ctx, &request->reply_pairs, true);
        (void)talloc_get_type_abort(request->control_ctx, fr_pair_t);
-       fr_pair_list_verify(file, line, request->control_ctx, &request->control_pairs);
+       fr_pair_list_verify(file, line, request->control_ctx, &request->control_pairs, true);
        (void)talloc_get_type_abort(request->session_state_ctx, fr_pair_t);
 
 #ifndef NDEBUG
@@ -681,8 +681,8 @@ void request_verify(char const *file, int line, request_t const *request)
        }
 #endif
 
-       fr_pair_list_verify(file, line, request->session_state_ctx, &request->session_state_pairs);
-       fr_pair_list_verify(file, line, request->local_ctx, &request->local_pairs);
+       fr_pair_list_verify(file, line, request->session_state_ctx, &request->session_state_pairs, true);
+       fr_pair_list_verify(file, line, request->local_ctx, &request->local_pairs, true);
 
        fr_assert(request->proto_dict != NULL);
        fr_assert(request->local_dict != NULL);

diff --git a/src/lib/server/state.c b/src/lib/server/state.c
index 011ad1d65b96835e85061cc0fef30d68ee2995b8..e7857366cf41e10b8c1ba3baf093a85004440c3a 100644 (file)
--- a/src/lib/server/state.c
+++ b/src/lib/server/state.c
@@ -755,7 +755,7 @@ int fr_request_to_state(fr_state_tree_t *state, request_t *request)
                 *      are parented correctly, else we'll get
                 *      memory errors when we restore.
                 */
-               fr_pair_list_verify(__FILE__, __LINE__, request->session_state_ctx, &request->session_state_pairs);
+               fr_pair_list_verify(__FILE__, __LINE__, request->session_state_ctx, &request->session_state_pairs, true);
 #endif
        }
 

diff --git a/src/lib/util/pair.c b/src/lib/util/pair.c
index e4c375cc40d398aaede093d716b786385e5b68d7..bd7cb76274f97f670e88b5998fefc4b027387c61 100644 (file)
--- a/src/lib/util/pair.c
+++ b/src/lib/util/pair.c
@@ -3090,7 +3090,8 @@ int fr_pair_value_enum_box(fr_value_box_t const **out, fr_pair_t *vp)
 /*
  *     Verify a fr_pair_t
  */
-void fr_pair_verify(char const *file, int line, fr_dict_attr_t const *parent_da, fr_pair_list_t const *list, fr_pair_t const *vp)
+void fr_pair_verify(char const *file, int line, fr_dict_attr_t const *parent_da,
+                   fr_pair_list_t const *list, fr_pair_t const *vp, bool verify_values)
 {
        (void) talloc_get_type_abort_const(vp, fr_pair_t);
 
@@ -3139,14 +3140,12 @@ void fr_pair_verify(char const *file, int line, fr_dict_attr_t const *parent_da,
                                             file, line, vp->da->name, vp->da->parent->name, parent->da->name);
                }
 
-#if 0
                /*
                 *      We would like to enable this, but there's a
                 *      lot of code like fr_pair_append_by_da() which
                 *      creates the #fr_pair_t with no value.
                 */
-               fr_value_box_verify(file, line, &vp->data);
-#endif
+               if (verify_values) fr_value_box_verify(file, line, &vp->data);
 
        } else {
                fr_pair_t *parent = fr_pair_parent(vp);
@@ -3156,7 +3155,7 @@ void fr_pair_verify(char const *file, int line, fr_dict_attr_t const *parent_da,
                                             file, line, vp->da->name);
                }
 
-               fr_pair_list_verify(file, line, vp, &vp->vp_group);
+               fr_pair_list_verify(file, line, vp, &vp->vp_group, verify_values);
        }
 
        switch (vp->vp_type) {
@@ -3283,7 +3282,7 @@ void fr_pair_verify(char const *file, int line, fr_dict_attr_t const *parent_da,
                                            file, line,
                                            child->da->name, child->da->parent->name, vp->da->name);
 
-                       fr_pair_verify(file, line, vp->da, &vp->vp_group, child);
+                       fr_pair_verify(file, line, vp->da, &vp->vp_group, child, verify_values);
                }
 
               UNCONST(fr_pair_t *, vp)->vp_group.verified = true;
@@ -3339,8 +3338,9 @@ void fr_pair_verify(char const *file, int line, fr_dict_attr_t const *parent_da,
  * @param[in] line     number in file
  * @param[in] expected talloc ctx pairs should have been allocated in
  * @param[in] list     of fr_pair_ts to verify
+ * @param[in] verify_values whether we verify the values, too.
  */
-void fr_pair_list_verify(char const *file, int line, TALLOC_CTX const *expected, fr_pair_list_t const *list)
+void fr_pair_list_verify(char const *file, int line, TALLOC_CTX const *expected, fr_pair_list_t const *list, bool verify_values)
 {
        fr_pair_t               *slow, *fast;
        TALLOC_CTX              *parent;
@@ -3355,7 +3355,7 @@ void fr_pair_list_verify(char const *file, int line, TALLOC_CTX const *expected,
        for (slow = fr_pair_list_head(list), fast = fr_pair_list_head(list);
             slow && fast;
             slow = fr_pair_list_next(list, slow), fast = fr_pair_list_next(list, fast)) {
-               PAIR_VERIFY_WITH_LIST(list, slow);
+               fr_pair_verify(__FILE__, __LINE__, NULL, list, slow, verify_values);
 
                /*
                 *      Advances twice as fast as slow...
@@ -3384,7 +3384,7 @@ void fr_pair_list_verify(char const *file, int line, TALLOC_CTX const *expected,
         *      Check the remaining pairs
         */
        for (; slow; slow = fr_pair_list_next(list, slow)) {
-               PAIR_VERIFY_WITH_LIST(list, slow);
+               fr_pair_verify(__FILE__, __LINE__, NULL, list, slow, verify_values);
 
                parent = talloc_parent(slow);
                if (expected && (parent != expected)) goto bad_parent;

diff --git a/src/lib/util/pair.h b/src/lib/util/pair.h
index c42853ca715302f173b1151393fa83b325521187..d4b5b49c2751ad3cb50482267608755f7a0ad416 100644 (file)
--- a/src/lib/util/pair.h
+++ b/src/lib/util/pair.h
@@ -158,16 +158,17 @@ struct value_pair_s {
  *
  */
 #ifdef WITH_VERIFY_PTR
-void           fr_pair_verify(char const *file, int line, fr_dict_attr_t const *parent, fr_pair_list_t const *list, fr_pair_t const *vp) CC_HINT(nonnull(5));
+void           fr_pair_verify(char const *file, int line, fr_dict_attr_t const *parent,
+                              fr_pair_list_t const *list, fr_pair_t const *vp, bool verify_values) CC_HINT(nonnull(5));
 
 void           fr_pair_list_verify(char const *file, int line,
-                                   TALLOC_CTX const *expected, fr_pair_list_t const *list) CC_HINT(nonnull(4));
+                                   TALLOC_CTX const *expected, fr_pair_list_t const *list, bool verify_values) CC_HINT(nonnull(4));
 
-#  define PAIR_VERIFY(_x)              fr_pair_verify(__FILE__, __LINE__, NULL, NULL, _x)
-#  define PAIR_VERIFY_WITH_LIST(_l, _x)                fr_pair_verify(__FILE__, __LINE__, NULL, _l, _x)
-#  define PAIR_LIST_VERIFY(_x) fr_pair_list_verify(__FILE__, __LINE__, NULL, _x)
-#  define PAIR_VERIFY_WITH_PARENT_VP(_p, _x)  fr_pair_verify(__FILE__, __LINE__, (_p)->da, &(_p)->vp_group, _x)
-#  define PAIR_VERIFY_WITH_PARENT_DA(_p, _x)  fr_pair_verify(__FILE__, __LINE__, _p, NULL, _x)
+#  define PAIR_VERIFY(_x)              fr_pair_verify(__FILE__, __LINE__, NULL, NULL, _x, true)
+#  define PAIR_VERIFY_WITH_LIST(_l, _x)                fr_pair_verify(__FILE__, __LINE__, NULL, _l, _x, true)
+#  define PAIR_LIST_VERIFY(_x) fr_pair_list_verify(__FILE__, __LINE__, NULL, _x, true)
+#  define PAIR_VERIFY_WITH_PARENT_VP(_p, _x)  fr_pair_verify(__FILE__, __LINE__, (_p)->da, &(_p)->vp_group, _x, true)
+#  define PAIR_VERIFY_WITH_PARENT_DA(_p, _x)  fr_pair_verify(__FILE__, __LINE__, _p, NULL, _x, true)
 
 #define PAIR_ALLOCED(_x) do { (_x)->filename = __FILE__; (_x)->line = __LINE__; } while (0)
 #else

diff --git a/src/lib/util/pair_legacy.c b/src/lib/util/pair_legacy.c
index eafe41d913acd423451e28f82a5bb031134458da..d3e2f4d2f9cc65a38931be0d6bef5cfd0db49075 100644 (file)
--- a/src/lib/util/pair_legacy.c
+++ b/src/lib/util/pair_legacy.c
@@ -326,6 +326,8 @@ redo:
                slen = fr_dict_oid_component(&err, &da, relative->da, &our_in, &bareword_terminals);
                if (err == FR_DICT_ATTR_NOTFOUND) {
                        if (raw) {
+                               fr_pair_t *parent_vp;
+
                                /*
                                 *      We looked up raw.FOO, and FOO wasn't found.  The component must be a number.
                                 */
@@ -357,8 +359,15 @@ redo:
 
                                if (!vp) return fr_sbuff_error(&our_in);
 
+                               parent_vp = fr_pair_parent(vp);
+                               fr_assert(parent_vp);
+
+                               /*
+                                *      @todo - check parent_vp->da, too?  But we have to handle the case of
+                                *      groups, changing dictionaries, etc.
+                                */
                                PAIR_ALLOCED(vp);
-                               PAIR_VERIFY(vp);
+                               fr_pair_verify(__FILE__, __LINE__, NULL, &parent_vp->vp_group, vp, false);
 
                                /*
                                 *      The above function MAY have jumped ahead a few levels.  Ensure
@@ -366,10 +375,6 @@ redo:
                                 *      but only if the parent changed.
                                 */
                                if (relative->da != vp->da->parent) {
-                                       fr_pair_t *parent_vp;
-
-                                       parent_vp = fr_pair_parent(vp);
-                                       fr_assert(parent_vp);
 
                                        relative->ctx = parent_vp;
                                        relative->da = parent_vp->da;
@@ -638,6 +643,9 @@ redo:
        if (slen <= 0) return fr_sbuff_error(&our_in) + slen;
 
 done:
+       /*
+        *      Now that we have a value, verify the full VP.
+        */
        PAIR_VERIFY(vp);
 
        keep_going = false;