From: Nick Porter Date: Thu, 11 Aug 2022 12:54:01 +0000 (+0100) Subject: Add callback to tmpl_dcursors to build out missing pairs (#4657) X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d6dddec6d86c95aa44228b26d99cacef18fd1056;p=thirdparty%2Ffreeradius-server.git 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 --- diff --git a/src/lib/server/tmpl_dcursor.c b/src/lib/server/tmpl_dcursor.c index d2d35cc240e..fd54fd8740e 100644 --- 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 0b2f42139d0..957d77248c5 100644 --- 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 bfe1b73275c..34f49816873 100644 --- 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 e7acc94e865..42865614847 100644 --- 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; }