Add callback to tmpl_dcursors to build out missing pairs (#4657)

* Define callback routine to populate missing #fr_pair_t from #tmpl_dcursor_t

* Populate build callback when initialising #tmpl_dcursor_t

* When initialising a dcursor, always call fr_dcursor_next()

if the cursor is initialised with an itertator function - even with an
empty list, the iterator may be setting something up.

tmpl_cursors use this to potentially populate attributes into the list.

* If no vp is found, and a fill callback is defined call it

Only valid for attribute references with an unspecified number, [0] or
[n]

* Define simple pair building callback for tmpl_dcursors

* Add list debug output to tmpl_dcursor_tests for test failures.

* Add smaller copy of test pairs to tmpl_dcursor_tests

* Add tmpl_dcursor_tests which populate missing pairs

* Improve comments

Co-authored-by: Arran Cudbard-Bell <a.cudbardb@freeradius.org>

diff --git a/src/lib/server/tmpl_dcursor.c b/src/lib/server/tmpl_dcursor.c
index d2d35cc240ea7f47e8518b3026f81b680790ad3d..fd54fd8740eff82457d1b487de1bbab664e3ebbc 100644 (file)
--- a/src/lib/server/tmpl_dcursor.c
+++ b/src/lib/server/tmpl_dcursor.c
@@ -166,6 +166,21 @@ fr_pair_t *_tmpl_cursor_eval(fr_pair_t *curr, tmpl_dcursor_ctx_t *cc)
                break;
        } else goto all_inst;   /* Used for TMPL_TYPE_LIST */
 
+       /*
+        *      If no pair was found and there is a fill
+        *      callback, call that, depending on the suffix
+        */
+       if ((!vp) && (cc->build) && (ar)) switch (ar->ar_num) {
+       case NUM_UNSPEC:
+       case NUM_LAST:
+       case 0:
+               vp = cc->build(ns->list_ctx, &ns->cursor, ar->da, cc->uctx);
+               break;
+
+       default:
+               break;
+       }
+
        return vp;
 }
 
@@ -260,7 +275,8 @@ static void *_tmpl_cursor_next(UNUSED fr_dlist_head_t *list, void *curr, void *u
  */
 fr_pair_t *tmpl_dcursor_init_relative(int *err, TALLOC_CTX *ctx, tmpl_dcursor_ctx_t *cc,
                                      fr_dcursor_t *cursor,
-                                     request_t *request, fr_pair_t *list, tmpl_t const *vpt)
+                                     request_t *request, fr_pair_t *list, tmpl_t const *vpt,
+                                     tmpl_dcursor_build_t build, void *uctx)
 {
        fr_pair_t               *vp = NULL;
 
@@ -273,7 +289,9 @@ fr_pair_t *tmpl_dcursor_init_relative(int *err, TALLOC_CTX *ctx, tmpl_dcursor_ct
                .vpt = vpt,
                .ctx = ctx,
                .request = request,
-               .list = &list->vp_group
+               .list = &list->vp_group,
+               .build = build,
+               .uctx = uctx
        };
        fr_dlist_init(&cc->nested, tmpl_dcursor_nested_t, entry);
 
@@ -334,8 +352,9 @@ fr_pair_t *tmpl_dcursor_init_relative(int *err, TALLOC_CTX *ctx, tmpl_dcursor_ct
  *
  * @see tmpl_cursor_next
  */
-fr_pair_t *tmpl_dcursor_init(int *err, TALLOC_CTX *ctx, tmpl_dcursor_ctx_t *cc,
-                            fr_dcursor_t *cursor, request_t *request, tmpl_t const *vpt)
+fr_pair_t *_tmpl_dcursor_init(int *err, TALLOC_CTX *ctx, tmpl_dcursor_ctx_t *cc,
+                             fr_dcursor_t *cursor, request_t *request, tmpl_t const *vpt,
+                             tmpl_dcursor_build_t build, void *uctx)
 {
        fr_pair_t               *list;
 
@@ -368,7 +387,7 @@ fr_pair_t *tmpl_dcursor_init(int *err, TALLOC_CTX *ctx, tmpl_dcursor_ctx_t *cc,
                list = request->pair_root;
        }
 
-       return tmpl_dcursor_init_relative(err, ctx, cc, cursor, request, list, vpt);
+       return tmpl_dcursor_init_relative(err, ctx, cc, cursor, request, list, vpt, build, uctx);
 }
 
 /** Clear any temporary state allocations
@@ -388,6 +407,27 @@ void tmpl_dcursor_clear(tmpl_dcursor_ctx_t *cc)
        TALLOC_FREE(cc->pool);
 }
 
+/** Simple pair building callback for use with tmpl_dcursors
+ *
+ * Which always appends the new pair to the tail of the list
+ * since it is only called when no matching pairs were found when
+ * walking the list.
+ *
+ * @param[in] parent           to allocate new pair within.
+ * @param[in,out] cursor       to append new pair to.
+ * @param[in] da               of new pair.
+ * @param[in] uctx             unused.
+ * @return
+ *     - newly allocated #fr_pair_t.
+ *     - NULL on error.
+ */
+fr_pair_t *tmpl_dcursor_pair_build(fr_pair_t *parent, fr_dcursor_t *cursor, fr_dict_attr_t const *da, UNUSED void *uctx)
+{
+       fr_pair_t *vp;
+       vp = fr_pair_afrom_da(parent, da);
+       if (vp) fr_dcursor_append(cursor, vp);
+       return vp;
+}
 
 #define EXTENT_ADD(_out, _ar, _list_ctx, _list) \
        do { \

diff --git a/src/lib/server/tmpl_dcursor.h b/src/lib/server/tmpl_dcursor.h
index 0b2f42139d0813543d79c8864fbe43eb95897d44..957d77248c54c0a7cec4c978cb433a8afd358fd9 100644 (file)
--- a/src/lib/server/tmpl_dcursor.h
+++ b/src/lib/server/tmpl_dcursor.h
@@ -28,6 +28,16 @@ extern "C" {
 typedef struct tmpl_dcursor_ctx_s tmpl_dcursor_ctx_t;
 typedef struct tmpl_dcursor_nested_s tmpl_dcursor_nested_t;
 
+/** Callback function for populating missing pair
+ *
+ * @param[in] parent   to allocate the new pair in.
+ * @param[in] cursor   to append the pair to.
+ * @param[in] da       of the attribute to create.
+ * @param[in] uctx     context data.
+ * @return             newly allocated pair.
+ */
+typedef fr_pair_t *(*tmpl_dcursor_build_t)(fr_pair_t *parent, fr_dcursor_t *cursor, fr_dict_attr_t const *da, void *uctx);
+
 /** State for traversing an attribute reference
  *
  */
@@ -68,14 +78,26 @@ struct tmpl_dcursor_ctx_s {
        tmpl_dcursor_nested_t   leaf;           //!< Pre-allocated leaf state.  We always need
                                                ///< one of these so it doesn't make sense to
                                                ///< allocate it later.
+
+       tmpl_dcursor_build_t    build;          //!< Callback to build missing pairs.
+       void                    *uctx;          //!< Context for building new pairs.
 };
 
 fr_pair_t              *tmpl_dcursor_init_relative(int *err, TALLOC_CTX *ctx, tmpl_dcursor_ctx_t *cc,
                                                    fr_dcursor_t *cursor,
-                                                   request_t *request, fr_pair_t *list, tmpl_t const *vpt);
+                                                   request_t *request, fr_pair_t *list, tmpl_t const *vpt,
+                                                   tmpl_dcursor_build_t build, void *uctx);
 
-fr_pair_t              *tmpl_dcursor_init(int *err, TALLOC_CTX *ctx, tmpl_dcursor_ctx_t *cc,
-                                          fr_dcursor_t *cursor, request_t *request,
-                                          tmpl_t const *vpt);
+fr_pair_t              *_tmpl_dcursor_init(int *err, TALLOC_CTX *ctx, tmpl_dcursor_ctx_t *cc,
+                                           fr_dcursor_t *cursor, request_t *request,
+                                           tmpl_t const *vpt, tmpl_dcursor_build_t build, void *uctx);
 
 void                   tmpl_dcursor_clear(tmpl_dcursor_ctx_t *cc);
+
+fr_pair_t *tmpl_dcursor_pair_build(fr_pair_t *parent, fr_dcursor_t *cursor, fr_dict_attr_t const *da, UNUSED void *uctx);
+
+#define tmpl_dcursor_init(_err, _ctx, _cc, _cursor, _request, _vpt) \
+       _tmpl_dcursor_init(_err, _ctx, _cc, _cursor, _request, _vpt, NULL, NULL)
+
+#define tmpl_dcursor_build_init(_err, _ctx, _cc, _cursor, _request, _vpt, _build, _uctx) \
+       _tmpl_dcursor_init(_err, _ctx, _cc, _cursor, _request, _vpt, _build, _uctx)

diff --git a/src/lib/server/tmpl_dcursor_tests.c b/src/lib/server/tmpl_dcursor_tests.c
index bfe1b73275c77eb744183d22fe988e4d7950affa..34f498168730235c146a31cc1c72d8b025184893 100644 (file)
--- a/src/lib/server/tmpl_dcursor_tests.c
+++ b/src/lib/server/tmpl_dcursor_tests.c
@@ -67,6 +67,13 @@ static request_t *request_fake_alloc(void)
        fr_pair_t       *leaf_string_vp ## _x; \
        fr_pair_t       *leaf_int32_vp ## _x
 
+#define pair_defs_thin(_x) \
+       fr_pair_t       *int32_vp ## _x; \
+       fr_pair_t       *group_vp ## _x; \
+       fr_pair_t       *top_vp ## _x; \
+       fr_pair_t       *mid_vp ## _x; \
+       fr_pair_t       *leaf_int32_vp ## _x
+
 #define pair_populate(_x) \
        pair_append_request(&int32_vp ## _x, fr_dict_attr_test_int32); \
        pair_append_request(&string_vp ## _x, fr_dict_attr_test_string); \
@@ -77,6 +84,13 @@ static request_t *request_fake_alloc(void)
        fr_pair_append_by_da(mid_vp ## _x, &leaf_string_vp ## _x, &mid_vp ## _x->children, fr_dict_attr_test_nested_leaf_string); \
        fr_pair_append_by_da(mid_vp ## _x, &leaf_int32_vp ## _x, &mid_vp ## _x->children, fr_dict_attr_test_nested_leaf_int32);
 
+#define pair_populate_thin(_x) \
+       pair_append_request(&int32_vp ## _x, fr_dict_attr_test_int32); \
+       pair_append_request(&group_vp ## _x, fr_dict_attr_test_group); \
+       pair_append_request(&top_vp ## _x, fr_dict_attr_test_nested_top_tlv); \
+       fr_pair_append_by_da(top_vp ## _x, &mid_vp ## _x, &top_vp ## _x->children, fr_dict_attr_test_nested_child_tlv); \
+       fr_pair_append_by_da(mid_vp ## _x, &leaf_int32_vp ## _x, &mid_vp ## _x->children, fr_dict_attr_test_nested_leaf_int32);
+
 /*
  *     Top level attribute names in the test dictionary all have -0 on the end
  *     due to the ability to add multiple instances of each attribute for the
@@ -109,15 +123,47 @@ static request_t *request_fake_alloc(void)
        tmpl_afrom_attr_substr(autofree, NULL, &vpt, &FR_SBUFF_IN(ref, strlen(ref)), NULL, &(tmpl_rules_t){.attr = {.dict_def = test_dict}}); \
        vp = tmpl_dcursor_init(&err, NULL, &cc, &cursor, request, vpt);
 
+#define tmpl_setup_and_cursor_build_init(_attr) \
+       ref = _attr; \
+       tmpl_afrom_attr_substr(autofree, NULL, &vpt, &FR_SBUFF_IN(ref, strlen(ref)), NULL, &(tmpl_rules_t){.attr = {.dict_def = test_dict}}); \
+       inserted = tmpl_dcursor_build_init(&err, autofree, &cc, &cursor, request, vpt, &tmpl_dcursor_pair_build, NULL);
+
 /*
  *     How every test ends
  */
 #define test_end \
+       debug_attr_list(&request->request_pairs, 0); \
        vp = fr_dcursor_next(&cursor); \
        TEST_CHECK(vp == NULL); \
        tmpl_dcursor_clear(&cc); \
        TEST_CHECK_RET(talloc_free(request), 0)
 
+/*
+ *     Call after "build" cursors
+ *     Checks that no additional attributes are created
+ */
+#define build_test_end \
+       debug_attr_list(&request->request_pairs, 0); \
+       vp = fr_dcursor_next(&cursor); \
+       TEST_CHECK(vp == NULL); \
+       tmpl_dcursor_clear(&cc)
+
+static void debug_attr_list(fr_pair_list_t *list, int indent)
+{
+       fr_pair_t *vp = NULL;
+       while ((vp = fr_pair_list_next(list, vp))) {
+               switch (vp->da->type) {
+               case FR_TYPE_STRUCTURAL:
+                       TEST_MSG("%*s%s => {", indent, "", vp->da->name);
+                       debug_attr_list(&vp->vp_group, indent + 2);
+                       TEST_MSG("%*s}", indent, "");
+                       break;
+               default:
+                       TEST_MSG("%*s%s", indent, "", vp->da->name);
+               }
+       }
+}
+
 /*
  *     One instance of attribute at the top level
  */
@@ -540,6 +586,162 @@ static void test_level_3_two_last(void)
        test_end;
 }
 
+static void test_level_1_build(void)
+{
+       common_vars;
+       pair_defs(1);
+       fr_pair_t       *inserted;
+
+       pair_populate(1);
+       tmpl_setup_and_cursor_build_init("&Test-Int16-0");
+       TEST_CHECK(inserted != NULL);
+       build_test_end;
+
+       tmpl_setup_and_cursor_init("&Test-Int16-0");
+       TEST_CHECK(vp == inserted);
+
+       test_end;
+}
+
+static void test_level_2_build_leaf(void)
+{
+       common_vars;
+       pair_defs(1);
+       fr_pair_t       *inserted;
+
+       pair_populate(1);
+       tmpl_setup_and_cursor_build_init("&Test-Group-0.Test-Int32-0");
+       TEST_CHECK(inserted != NULL);
+       build_test_end;
+
+       tmpl_setup_and_cursor_init("&Test-Group-0.Test-Int32-0");
+       TEST_CHECK(vp == inserted);
+
+       test_end;
+}
+
+static void test_level_2_build_intermediate(void)
+{
+       common_vars;
+       fr_pair_t       *inserted;
+
+       tmpl_setup_and_cursor_build_init("&Test-Group-0.Test-Int16-0");
+       TEST_CHECK(inserted != NULL);
+       build_test_end;
+
+       tmpl_setup_and_cursor_init("&Test-Group-0.Test-Int16-0");
+       TEST_CHECK(vp == inserted);
+
+       test_end;
+}
+
+static void test_level_2_build_multi(void)
+{
+       common_vars;
+       pair_defs_thin(1);
+       pair_defs_thin(2);
+       fr_pair_t       *inserted, *second;
+
+       pair_populate_thin(1);
+       pair_populate_thin(2);
+       tmpl_setup_and_cursor_build_init("&Test-Group-0[*].Test-Int32-0");
+       TEST_CHECK(inserted != NULL);
+
+       second = fr_dcursor_next(&cursor);
+       TEST_CHECK(second != NULL);
+       TEST_CHECK(second != inserted);
+       build_test_end;
+
+       tmpl_setup_and_cursor_init("&Test-Group-0[*].Test-Int32-0");
+       TEST_CHECK(vp == inserted);
+       vp = fr_dcursor_next(&cursor);
+       TEST_CHECK(vp == second);
+
+       test_end;
+}
+
+static void test_level_3_build_leaf(void)
+{
+       common_vars;
+       pair_defs(1);
+       fr_pair_t       *inserted;
+
+       pair_populate(1);
+       tmpl_setup_and_cursor_build_init("&Test-Group-0.Test-Group-0.Test-String-0");
+       TEST_CHECK(inserted != NULL);
+       build_test_end;
+
+       tmpl_setup_and_cursor_init("&Test-Group-0.Test-Group-0.Test-String-0");
+       TEST_CHECK(vp == inserted);
+
+       test_end;
+}
+
+static void test_level_3_build_entire(void)
+{
+       common_vars;
+       fr_pair_t       *inserted;
+
+       tmpl_setup_and_cursor_build_init("&Test-Nested-Top-TLV-0[0].Child-TLV[0].Leaf-String");
+       TEST_CHECK(inserted != NULL);
+       build_test_end;
+
+       tmpl_setup_and_cursor_init("&Test-Nested-Top-TLV-0[0].Child-TLV[0].Leaf-String");
+       TEST_CHECK(vp == inserted);
+
+       test_end;
+}
+
+static void test_level_3_build_partial(void)
+{
+       common_vars;
+       pair_defs(1);
+       pair_defs_thin(2);
+       fr_pair_t       *inserted;
+
+       pair_populate(1);
+       pair_populate_thin(2);
+       tmpl_setup_and_cursor_build_init("&Test-Nested-Top-TLV-0[1].Child-TLV[0].Leaf-String");
+       TEST_CHECK(inserted != NULL);
+       TEST_CHECK(inserted != leaf_string_vp1);
+       build_test_end;
+
+       tmpl_setup_and_cursor_init("&Test-Nested-Top-TLV-0[1].Child-TLV[0].Leaf-String");
+       TEST_CHECK(vp == inserted);
+
+       test_end;
+}
+
+static void test_level_3_build_invalid1(void)
+{
+       common_vars;
+       fr_pair_t       *inserted;
+
+       tmpl_setup_and_cursor_build_init("&Test-Nested-Top-TLV-0[3].Child-TLV[0].Leaf-String");
+       TEST_CHECK(inserted == NULL);
+       build_test_end;
+
+       tmpl_setup_and_cursor_init("&Test-Nested-Top-TLV-0[3].Child-TLV[0].Leaf-String");
+       TEST_CHECK(vp == NULL);
+
+       test_end;
+}
+
+static void test_level_3_build_invalid2(void)
+{
+       common_vars;
+       fr_pair_t       *inserted;
+
+       tmpl_setup_and_cursor_build_init("&Test-Nested-Top-TLV-0[*].Child-TLV[0].Leaf-String");
+       TEST_CHECK(inserted == NULL);
+       build_test_end;
+
+       tmpl_setup_and_cursor_init("&Test-Nested-Top-TLV-0[*].Child-TLV[0].Leaf-String");
+       TEST_CHECK(vp == NULL);
+
+       test_end;
+}
+
 TEST_LIST = {
        { "test_level_1_one",           test_level_1_one },
        { "test_level_1_one_second",    test_level_1_one_second },
@@ -568,5 +770,15 @@ TEST_LIST = {
        { "test_level_3_two_all",       test_level_3_two_all },
        { "test_level_3_two_last",      test_level_3_two_last },
 
+       { "test_level_1_build",                 test_level_1_build },
+       { "test_level_2_build_leaf",            test_level_2_build_leaf },
+       { "test_level_2_build_intermediate",    test_level_2_build_intermediate },
+       { "test_level_2_build_multi",           test_level_2_build_multi },
+       { "test_level_3_build_leaf",            test_level_3_build_leaf },
+       { "test_level_3_build_entire",          test_level_3_build_entire },
+       { "test_level_3_build_partial",         test_level_3_build_partial },
+       { "test_level_3_build_invalid1",        test_level_3_build_invalid1 },
+       { "test_level_3_build_invalid2",        test_level_3_build_invalid2 },
+
        { NULL }
 };

diff --git a/src/lib/util/dcursor.h b/src/lib/util/dcursor.h
index e7acc94e86506549814e2e0ab5eaeed234026bd3..428656148475a72a21f342c749cad52349f681cf 100644 (file)
--- a/src/lib/util/dcursor.h
+++ b/src/lib/util/dcursor.h
@@ -728,6 +728,8 @@ void *_fr_dcursor_init(fr_dcursor_t *cursor, fr_dlist_head_t const *head,
        };
        if (!fr_dlist_empty(cursor->dlist)) return fr_dcursor_next(cursor);     /* Initialise current */
 
+       if (iter) return fr_dcursor_next(cursor);       /* An iterator may do something, even on an empty list */
+
        return NULL;
 }