]> git.ipfire.org Git - thirdparty/freeradius-server.git/commitdiff
v4: Convert %(cache: ) to new xlat api (#4043)
authorNick Porter <nick@portercomputing.co.uk>
Thu, 29 Apr 2021 14:13:28 +0000 (15:13 +0100)
committerGitHub <noreply@github.com>
Thu, 29 Apr 2021 14:13:28 +0000 (09:13 -0500)
* Convert %(cache: ) to new xlat api

* Add test for %(cache: ) xlat

* Tidy rlm_cache tests

src/modules/rlm_cache/rlm_cache.c
src/modules/rlm_cache/rlm_cache.h
src/tests/modules/cache_rbtree/cache-bin.unlang
src/tests/modules/cache_rbtree/cache-logic.unlang
src/tests/modules/cache_rbtree/cache-update.unlang
src/tests/modules/cache_rbtree/cache-xlat.attrs [new file with mode: 0644]
src/tests/modules/cache_rbtree/cache-xlat.unlang [new file with mode: 0644]

index 44e2144825f2e82f34c4aa26c27fdaf20d9db97c..65a05c4f06e38b66c6f2f96b0017422744450c68 100644 (file)
@@ -799,38 +799,54 @@ finish:
        RETURN_MODULE_RCODE(rcode);
 }
 
+static int mod_xlat_thread_instantiate(UNUSED void *xlat_inst, void *xlat_thread_inst,
+                                      UNUSED xlat_exp_t const *exp, void *uctx)
+{
+       rlm_cache_t                     *inst = talloc_get_type_abort(uctx, rlm_cache_t);
+       cache_xlat_thread_inst_t        *xt = xlat_thread_inst;
+
+       xt->inst = inst;
+       return 0;
+}
+
+static xlat_arg_parser_t const cache_xlat_args[] = {
+       { .required = true, .single = true, .type = FR_TYPE_STRING },
+       XLAT_ARG_PARSER_TERMINATOR
+};
+
 /** Allow single attribute values to be retrieved from the cache
  *
  * @ingroup xlat_functions
  */
-static ssize_t cache_xlat(TALLOC_CTX *ctx, char **out, UNUSED size_t freespace,
-                         void const *mod_inst, UNUSED void const *xlat_inst,
-                         request_t *request, char const *fmt) CC_HINT(nonnull);
-static ssize_t cache_xlat(TALLOC_CTX *ctx, char **out, UNUSED size_t freespace,
-                         void const *mod_inst, UNUSED void const *xlat_inst,
-                         request_t *request, char const *fmt)
+static xlat_action_t cache_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out, request_t *request,
+                               UNUSED void const *xlat_inst, void *xlat_thread_inst,
+                               fr_value_box_list_t *in) CC_HINT(nonnull);
+static xlat_action_t cache_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out, request_t *request,
+                               UNUSED void const *xlat_inst, void *xlat_thread_inst,
+                               fr_value_box_list_t *in)
 {
-       rlm_cache_entry_t       *c = NULL;
-       rlm_cache_t const       *inst = mod_inst;
-       rlm_cache_handle_t      *handle = NULL;
+       rlm_cache_entry_t               *c = NULL;
+       cache_xlat_thread_inst_t        *xti = talloc_get_type_abort(xlat_thread_inst, cache_xlat_thread_inst_t);
+       rlm_cache_handle_t              *handle = NULL;
 
-       ssize_t                 slen;
-       ssize_t                 ret = 0;
+       ssize_t                         slen;
 
-       uint8_t                 buffer[1024];
-       uint8_t const           *key;
-       ssize_t                 key_len;
+       fr_value_box_t                  *attr = fr_dlist_head(in);
+       uint8_t                         buffer[1024];
+       uint8_t const                   *key;
+       ssize_t                         key_len;
+       fr_value_box_t                  *vb;
 
-       tmpl_t                  *target = NULL;
-       map_t                   *map = NULL;
-       rlm_rcode_t             rcode = RLM_MODULE_NOOP;
+       tmpl_t                          *target = NULL;
+       map_t                           *map = NULL;
+       rlm_rcode_t                     rcode = RLM_MODULE_NOOP;
 
        key_len = tmpl_expand((char const **)&key, (char *)buffer, sizeof(buffer),
-                             request, inst->config.key, NULL, NULL);
-       if (key_len < 0) return -1;
+                             request, xti->inst->config.key, NULL, NULL);
+       if (key_len < 0) return XLAT_ACTION_FAIL;
 
        slen = tmpl_afrom_attr_substr(ctx, NULL, &target,
-                                     &FR_SBUFF_IN(fmt, strlen(fmt)),
+                                     &FR_SBUFF_IN(attr->vb_strvalue, attr->vb_length),
                                      NULL,
                                      &(tmpl_rules_t){
                                                .dict_def = request->dict,
@@ -838,33 +854,34 @@ static ssize_t cache_xlat(TALLOC_CTX *ctx, char **out, UNUSED size_t freespace,
                                      });
        if (slen <= 0) {
                RPEDEBUG("Invalid key");
-               return -1;
+               return XLAT_ACTION_FAIL;
        }
 
-       if (cache_acquire(&handle, mod_inst, request) < 0) {
+       if (cache_acquire(&handle, xti->inst, request) < 0) {
                talloc_free(target);
-               return -1;
+               return XLAT_ACTION_FAIL;
        }
 
-       cache_find(&rcode, &c, mod_inst, request, &handle, key, key_len);
+       cache_find(&rcode, &c, xti->inst, request, &handle, key, key_len);
        switch (rcode) {
        case RLM_MODULE_OK:             /* found */
                break;
 
        case RLM_MODULE_NOTFOUND:       /* not found */
-               return 0;
+               return XLAT_ACTION_FAIL;
 
        default:
                talloc_free(target);
-               return -1;
+               return XLAT_ACTION_FAIL;
        }
 
        while ((map = fr_dlist_next(&c->maps, map))) {
                if ((tmpl_da(map->lhs) != tmpl_da(target)) ||
                    (tmpl_list(map->lhs) != tmpl_list(target))) continue;
 
-               fr_value_box_aprint(request, out, tmpl_value(map->rhs), NULL);
-               ret = talloc_array_length(*out) - 1;
+               MEM(vb = fr_value_box_alloc_null(ctx));
+               fr_value_box_copy(ctx, vb, tmpl_value(map->rhs));
+               fr_dcursor_append(out, vb);
                break;
        }
 
@@ -873,12 +890,12 @@ static ssize_t cache_xlat(TALLOC_CTX *ctx, char **out, UNUSED size_t freespace,
        /*
         *      Check if we found a matching map
         */
-       if (!map) return 0;
+       if (!map) return XLAT_ACTION_FAIL;
 
-       cache_free(mod_inst, &c);
-       cache_release(mod_inst, request, &handle);
+       cache_free(xti->inst, &c);
+       cache_release(xti->inst, request, &handle);
 
-       return ret;
+       return XLAT_ACTION_DONE;
 }
 
 /** Free any memory allocated under the instance
@@ -909,6 +926,7 @@ static int mod_bootstrap(void *instance, CONF_SECTION *conf)
        rlm_cache_t     *inst = instance;
        CONF_SECTION    *driver_cs;
        char const      *name;
+       xlat_t          *xlat;
 
        inst->cs = conf;
 
@@ -957,7 +975,9 @@ static int mod_bootstrap(void *instance, CONF_SECTION *conf)
        /*
         *      Register the cache xlat function
         */
-       xlat_register_legacy(inst, inst->config.name, cache_xlat, NULL, NULL, 0, 0);
+       xlat = xlat_register(inst, inst->config.name, cache_xlat, true);
+       xlat_func_args(xlat, cache_xlat_args);
+       xlat_async_thread_instantiate_set(xlat, mod_xlat_thread_instantiate, cache_xlat_thread_inst_t, NULL, inst);
 
        return 0;
 }
index 4119e4e815a262cafbeec72ac01cbcb6891b8b64..0e84875dc83fb2232b2045908aa22437944dc4f3 100644 (file)
@@ -86,6 +86,10 @@ typedef struct {
        fr_map_list_t           maps;                   //!< Head of the maps list.
 } rlm_cache_entry_t;
 
+typedef struct {
+       rlm_cache_t             *inst;                  //!< Instance of rlm_cache
+} cache_xlat_thread_inst_t;
+
 /** Allocate a new cache entry
  *
  */
index 9136bacf8e14ac8d0b9549168c82169afa385362..d4a407a5fe910b3b12b351fb57b89689da12a6c8 100644 (file)
@@ -12,18 +12,13 @@ update {
 }
 
 # 0. Sanity check
-if (&Tmp-String-1 == "foo\000bar\000baz") {
-       test_pass
-} else {
+if (&Tmp-String-1 != "foo\000bar\000baz") {
        test_fail
 }
 
 # 1. Store the entry
 cache_bin_key_octets
-if (ok) {
-       test_pass
-}
-else {
+if (!ok) {
        test_fail
 }
 
@@ -35,10 +30,7 @@ update {
 
 # 2. Should create a *new* entry and not update the existing one
 cache_bin_key_octets
-if (ok) {
-       test_pass
-}
-else {
+if (!ok) {
        test_fail
 }
 
@@ -53,24 +45,15 @@ update {
 }
 
 cache_bin_key_octets
-if (updated) {
-       test_pass
-}
-else {
+if (!updated) {
        test_fail
 }
 
-if ("%(length:%{Tmp-String-1})" == 11) {
-       test_pass
-}
-else {
+if ("%(length:%{Tmp-String-1})" != 11) {
        test_fail
 }
 
-if (&Tmp-String-1 == "foo\000bar\000baz") {
-       test_pass
-}
-else {
+if (&Tmp-String-1 != "foo\000bar\000baz") {
        test_fail
 }
 
@@ -84,24 +67,15 @@ update {
 }
 
 cache_bin_key_octets
-if (updated) {
-       test_pass
-}
-else {
+if (!updated) {
        test_fail
 }
 
-if ("%(length:%{Tmp-String-1})" == 7) {
-       test_pass
-}
-else {
+if ("%(length:%{Tmp-String-1})" != 7) {
        test_fail
 }
 
-if (&Tmp-String-1 == "bar\000baz") {
-       test_pass
-}
-else {
+if (&Tmp-String-1 != "bar\000baz") {
        test_fail
 }
 
@@ -120,10 +94,7 @@ update {
 }
 
 cache_bin_key_ipaddr
-if (ok) {
-       test_pass
-}
-else {
+if (!ok) {
        test_fail
 }
 
@@ -135,10 +106,7 @@ update {
 }
 
 cache_bin_key_ipaddr
-if (ok) {
-       test_pass
-}
-else {
+if (!ok) {
        test_fail
 }
 
@@ -152,24 +120,15 @@ update {
 }
 
 cache_bin_key_ipaddr
-if (updated) {
-       test_pass
-}
-else {
+if (!updated) {
        test_fail
 }
 
-if ("%(length:%{Tmp-String-1})" == 11) {
-       test_pass
-}
-else {
+if ("%(length:%{Tmp-String-1})" != 11) {
        test_fail
 }
 
-if (&Tmp-String-1 == "foo\000bar\000baz") {
-       test_pass
-}
-else {
+if (&Tmp-String-1 != "foo\000bar\000baz") {
        test_fail
 }
 
@@ -183,27 +142,20 @@ update {
 }
 
 cache_bin_key_ipaddr
-if (updated) {
-       test_pass
-}
-else {
+if (!updated) {
        test_fail
 }
 
-if ("%(length:%{Tmp-String-1})" == 7) {
-       test_pass
-}
-else {
+if ("%(length:%{Tmp-String-1})" != 7) {
        test_fail
 }
 
-if (&Tmp-String-1 == "bar\000baz") {
-       test_pass
-}
-else {
+if (&Tmp-String-1 != "bar\000baz") {
        test_fail
 }
 
 update {
        &Tmp-String-1 !* ANY
 }
+
+test_pass
index 44564b1e5386b911bdc615cc5ae9750eb23f6ba0..49aa41724c07244c84db10b137651e3c80c3c2ba 100644 (file)
@@ -17,17 +17,11 @@ cache
 if (!ok) {
        test_fail
 }
-else {
-       test_pass
-}
 
 # 1. Check the module didn't perform a merge
 if (&request.Tmp-String-1) {
        test_fail
 }
-else {
-       test_pass
-}
 
 # 2. Check status-only works correctly (should return ok and consume attribute)
 update control {
@@ -37,34 +31,22 @@ cache
 if (!ok) {
        test_fail
 }
-else {
-       test_pass
-}
 
 # 3.
 if (&control.Cache-Status-Only) {
        test_fail
 }
-else {
-       test_pass
-}
 
 # 4. Retrieve the entry (should be copied to request list)
 cache
 if (!updated) {
        test_fail
 }
-else {
-       test_pass
-}
 
 # 5.
 if (&request.Tmp-String-1 != &control.Tmp-String-1) {
        test_fail
 }
-else {
-       test_pass
-}
 
 # 6. Retrieving the entry should not expire it
 update request {
@@ -75,9 +57,6 @@ cache
 if (!updated) {
        test_fail
 }
-else {
-       test_pass
-}
 
 # 7.
 if (&request.Tmp-String-1 != &control.Tmp-String-1) {
@@ -97,9 +76,6 @@ cache
 if (!ok) {
        test_fail
 }
-else {
-       test_pass
-}
 
 # 9. Check status-only works correctly (should return notfound and consume attribute)
 update control {
@@ -109,17 +85,11 @@ cache
 if (!notfound) {
        test_fail
 }
-else {
-       test_pass
-}
 
 # 10.
 if (&control.Cache-Status-Only) {
        test_fail
 }
-else {
-       test_pass
-}
 
 # 11. Check merge-only works correctly (should return notfound and consume attribute)
 update control {
@@ -130,17 +100,11 @@ cache
 if (!notfound) {
        test_fail
 }
-else {
-       test_pass
-}
 
 # 12.
 if (&control.Cache-Allow-Merge) {
        test_fail
 }
-else {
-       test_pass
-}
 
 # 13. ...and check the entry wasn't recreated
 update control {
@@ -150,9 +114,6 @@ cache
 if (!notfound) {
        test_fail
 }
-else {
-       test_pass
-}
 
 # 14. This should still allow the creation of a new entry
 update control {
@@ -162,34 +123,22 @@ cache
 if (!ok) {
        test_fail
 }
-else {
-       test_pass
-}
 
 # 15.
 cache
 if (!updated) {
        test_fail
 }
-else {
-       test_pass
-}
 
 # 16.
 if (&Cache-TTL) {
        test_fail
 }
-else {
-       test_pass
-}
 
 # 17.
 if (&request.Tmp-String-1 != &control.Tmp-String-1) {
        test_fail
 }
-else {
-       test_pass
-}
 
 update control {
        &Tmp-String-1 := 'cache me2'
@@ -203,17 +152,11 @@ cache
 if (!updated) {
        test_fail
 }
-else {
-       test_pass
-}
 
 # 19. Request Tmp-String-1 shouldn't have been updated yet
 if (&request.Tmp-String-1 == &control.Tmp-String-1) {
        test_fail
 }
-else {
-       test_pass
-}
 
 # 20. Check that a new entry is created
 update control {
@@ -223,34 +166,22 @@ cache
 if (!updated) {
        test_fail
 }
-else {
-       test_pass
-}
 
 # 21. Request Tmp-String-1 still shouldn't have been updated yet
 if (&request.Tmp-String-1 == &control.Tmp-String-1) {
        test_fail
 }
-else {
-       test_pass
-}
 
 # 22.
 cache
 if (!updated) {
        test_fail
 }
-else {
-       test_pass
-}
 
 # 23. Request Tmp-String-1 should now have been updated
 if (&request.Tmp-String-1 != &control.Tmp-String-1) {
        test_fail
 }
-else {
-       test_pass
-}
 
 # 24. Check Cache-Merge = yes works as expected (should update current request)
 update control {
@@ -262,30 +193,20 @@ cache
 if (!updated) {
        test_fail
 }
-else {
-       test_pass
-}
 
 # 25. Request Tmp-String-1 should now have been updated
 if (&request.Tmp-String-1 != &control.Tmp-String-1) {
        test_fail
 }
-else {
-       test_pass
-}
 
 # 26. Check Cache-Entry-Hits is updated as we expect
 if (&request.Cache-Entry-Hits != 0) {
        test_fail
 }
-else {
-       test_pass
-}
 
 cache
 if (&request.Cache-Entry-Hits != 1) {
        test_fail
 }
-else {
-       test_pass
-}
+
+test_pass
index 493c21f542c8b84998364911a446ebbd1a8c3462..3adbfa97f022f8ab76b3545401e97b2d171a6d7f 100644 (file)
@@ -25,77 +25,47 @@ cache_update
 if (!ok) {
        test_fail
 }
-else {
-       test_pass
-}
 
 # Merge
 cache_update
-if (updated) {
-       test_pass
-}
-else {
+if (!updated) {
        test_fail
 }
 
 # session-state should now contain all the reply attributes
-if ("%{session-state[#]}" == 2) {
-       test_pass
-}
-else {
+if ("%{session-state[#]}" != 2) {
        test_fail
 }
 
-if (&session-state.Reply-Message[0] == 'hello') {
-       test_pass
-}
-else {
+if (&session-state.Reply-Message[0] != 'hello') {
        test_fail
 }
 
-if (&session-state.Reply-Message[1] == 'goodbye') {
-       test_pass
-}
-else {
+if (&session-state.Reply-Message[1] != 'goodbye') {
        test_fail
 }
 
 # Tmp-String-1 should hold the result of the exec
-if (&Tmp-String-1 == 'echo test') {
-       test_pass
-}
-else {
+if (&Tmp-String-1 != 'echo test') {
        test_fail
 }
 
 # Literal values should be foo, rad, baz
-if ("%{Tmp-String-2[#]}" == 3) {
-       test_pass
-}
-else {
+if ("%{Tmp-String-2[#]}" != 3) {
        test_fail
 }
 
-if (&Tmp-String-2[0] == 'foo') {
-       test_pass
-}
-else {
+if (&Tmp-String-2[0] != 'foo') {
        test_fail
 }
 
 debug_request
 
-if (&Tmp-String-2[1] == 'rab') {
-       test_pass
-}
-else {
+if (&Tmp-String-2[1] != 'rab') {
        test_fail
 }
 
-if (&Tmp-String-2[2] == 'baz') {
-       test_pass
-}
-else {
+if (&Tmp-String-2[2] != 'baz') {
        test_fail
 }
 
@@ -103,3 +73,5 @@ else {
 update {
     &reply !* ANY
 }
+
+test_pass
diff --git a/src/tests/modules/cache_rbtree/cache-xlat.attrs b/src/tests/modules/cache_rbtree/cache-xlat.attrs
new file mode 100644 (file)
index 0000000..2c3f3ac
--- /dev/null
@@ -0,0 +1,11 @@
+#
+#  Input packet
+#
+Packet-Type = Access-Request
+User-Name = "bob"
+User-Password = "olobobob"
+
+#
+#  Expected answer
+#
+Packet-Type == Access-Accept
diff --git a/src/tests/modules/cache_rbtree/cache-xlat.unlang b/src/tests/modules/cache_rbtree/cache-xlat.unlang
new file mode 100644 (file)
index 0000000..de6fb7d
--- /dev/null
@@ -0,0 +1,33 @@
+#
+#  PRE: cache-logic
+#
+update {
+        &request.Tmp-String-0 := 'testkey'
+}
+
+update control {
+        &Tmp-String-1 := 'cache me'
+}
+
+cache
+if (!ok) {
+        test_fail
+}
+
+update request {
+        &Tmp-String-2 := "%(cache:request.Tmp-String-1)"
+}
+
+if (&Tmp-String-2 != &control.Tmp-String-1) {
+        test_fail
+}
+
+update request {
+        &Tmp-String-3 := "%(cache:request.Tmp-String-4)"
+}
+
+if (&Tmp-String-3 != "") {
+        test_fail
+}
+
+test_pass
\ No newline at end of file