From: Nick Porter <nick@portercomputing.co.uk>
Date: Thu, 29 Apr 2021 14:13:28 +0000 (+0100)
Subject: v4: Convert %(cache: ) to new xlat api (#4043)
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=505d8dd183d0a23917f48a2f76cde9ec693f25c6;p=thirdparty%2Ffreeradius-server.git

v4: Convert %(cache: ) to new xlat api (#4043)

* Convert %(cache: ) to new xlat api

* Add test for %(cache: ) xlat

* Tidy rlm_cache tests
---

diff --git a/src/modules/rlm_cache/rlm_cache.c b/src/modules/rlm_cache/rlm_cache.c
index 44e2144825f..65a05c4f06e 100644
--- a/src/modules/rlm_cache/rlm_cache.c
+++ b/src/modules/rlm_cache/rlm_cache.c
@@ -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;
 }
diff --git a/src/modules/rlm_cache/rlm_cache.h b/src/modules/rlm_cache/rlm_cache.h
index 4119e4e815a..0e84875dc83 100644
--- a/src/modules/rlm_cache/rlm_cache.h
+++ b/src/modules/rlm_cache/rlm_cache.h
@@ -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
  *
  */
diff --git a/src/tests/modules/cache_rbtree/cache-bin.unlang b/src/tests/modules/cache_rbtree/cache-bin.unlang
index 9136bacf8e1..d4a407a5fe9 100644
--- a/src/tests/modules/cache_rbtree/cache-bin.unlang
+++ b/src/tests/modules/cache_rbtree/cache-bin.unlang
@@ -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
diff --git a/src/tests/modules/cache_rbtree/cache-logic.unlang b/src/tests/modules/cache_rbtree/cache-logic.unlang
index 44564b1e538..49aa41724c0 100644
--- a/src/tests/modules/cache_rbtree/cache-logic.unlang
+++ b/src/tests/modules/cache_rbtree/cache-logic.unlang
@@ -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
diff --git a/src/tests/modules/cache_rbtree/cache-update.unlang b/src/tests/modules/cache_rbtree/cache-update.unlang
index 493c21f542c..3adbfa97f02 100644
--- a/src/tests/modules/cache_rbtree/cache-update.unlang
+++ b/src/tests/modules/cache_rbtree/cache-update.unlang
@@ -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
index 00000000000..2c3f3ac57f0
--- /dev/null
+++ b/src/tests/modules/cache_rbtree/cache-xlat.attrs
@@ -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
index 00000000000..de6fb7d10a3
--- /dev/null
+++ b/src/tests/modules/cache_rbtree/cache-xlat.unlang
@@ -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