From: Nick Porter <nick@portercomputing.co.uk>
Date: Thu, 18 Mar 2021 16:40:26 +0000 (+0000)
Subject: v4: Define arguments for %(sub: ) xlat (#3998)
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=54bdb9317c3b425ee5b11b6d94ef58115d5fde8e;p=thirdparty%2Ffreeradius-server.git

v4: Define arguments for %(sub: ) xlat (#3998)

* Define arguments for sub xlat

* Don't free twice

* Update sub xlat tests to reflect new syntax

* Update documentation to reflect new sub xlat syntax

* Use escape rules we're provided with
---

diff --git a/doc/antora/modules/reference/pages/xlat/builtin.adoc b/doc/antora/modules/reference/pages/xlat/builtin.adoc
index adafeeea490..2ca2c5983ad 100644
--- a/doc/antora/modules/reference/pages/xlat/builtin.adoc
+++ b/doc/antora/modules/reference/pages/xlat/builtin.adoc
@@ -961,7 +961,7 @@ if (&control.Tmp-String-0 =~ /^(?<login>(.*))@(?<domain>(.*))$/) {
 The user@example.com { login=user, domain=example.com }
 ```
 
-### %{sub:/<regex>/[flags] <replace> <subject>}
+### %(sub:/<regex>/[flags] <replace> <subject>)
 
 Substitute text just as easily as it can match it, even using regex patterns.
 
@@ -975,7 +975,7 @@ update control {
     &Tmp-String-0 := "Caipirinha is a light and refreshing drink!"
 }
 update reply {
-    &Reply-Message := "%{sub:/ / , %{control.Tmp-String-0}}"
+    &Reply-Message := "%(sub:/ / , %{control.Tmp-String-0})"
 }
 ----
 
diff --git a/src/lib/unlang/xlat_builtin.c b/src/lib/unlang/xlat_builtin.c
index e312a737bb7..8d8cfff7195 100644
--- a/src/lib/unlang/xlat_builtin.c
+++ b/src/lib/unlang/xlat_builtin.c
@@ -2829,15 +2829,15 @@ static xlat_action_t xlat_func_strlen(TALLOC_CTX *ctx, fr_dcursor_t *out,
 #ifdef HAVE_REGEX_PCRE2
 /** Perform regex substitution TODO CHECK
  *
- * Called when %{sub:} pattern begins with "/"
+ * Called when %(sub:) pattern begins with "/"
  *
 @verbatim
-%{sub:/<regex>/[flags] <replace> <subject>}
+%(sub:/<regex>/[flags] <replace> <subject>)
 @endverbatim
  *
  * Example: (User-Name = "foo")
 @verbatim
-"%{sub:/oo.*$/ un %{User-Name}}" == "fun"
+"%(sub:/oo.*$/ un %{User-Name})" == "fun"
 @endverbatim
  *
  * @see #xlat_func_sub
@@ -2849,49 +2849,27 @@ static xlat_action_t xlat_func_sub_regex(TALLOC_CTX *ctx, fr_dcursor_t *out,
 					 fr_value_box_list_t *in)
 {
 	char const		*p, *q, *end;
-	char const		*regex, *rep, *subject;
+	char const		*regex;
 	char			*buff;
-	size_t			regex_len, rep_len, subject_len;
+	size_t			regex_len;
 	ssize_t			slen;
 	regex_t			*pattern;
 	fr_regex_flags_t	flags;
 	fr_value_box_t		*vb;
-	fr_value_box_t		*in_head = fr_dlist_head(in);
-
+	fr_value_box_t		*regex_vb = fr_dlist_head(in);
+	fr_value_box_t		*rep_vb = fr_dlist_next(in, regex_vb);
+	fr_value_box_t		*subject_vb = fr_dlist_next(in, rep_vb);
 
-	/*
-	 *	If there's no input, there's no output
-	 */
-	if (!in_head) {
-		REDEBUG("No input arguments");
-		return XLAT_ACTION_FAIL;
-	}
-
-	/*
-	 *	Concatenate all input
-	 */
-	if (fr_value_box_list_concat(ctx, in_head, in, FR_TYPE_STRING, true) < 0) {
-		RPEDEBUG("Failed concatenating input");
-		return XLAT_ACTION_FAIL;
-	}
 
-	p = in_head->vb_strvalue;
-	end = p + in_head->vb_length;
+	p = regex_vb->vb_strvalue;
+	end = p + regex_vb->vb_length;
 
 	if (p == end) {
 		REDEBUG("Regex must not be empty");
 		return XLAT_ACTION_FAIL;
 	}
 
-	/*
-	 *	Parse '/<regex>/'
-	 */
-	if (*p != '/') {
-		REDEBUG("Regex must start with '/'");
-		return XLAT_ACTION_FAIL;
-	}
-	p++;
-
+	p++;	/* Advance past '/' */
 	regex = p;
 
 	q = memchr(p, '/', end - p);
@@ -2906,45 +2884,14 @@ static xlat_action_t xlat_func_sub_regex(TALLOC_CTX *ctx, fr_dcursor_t *out,
 	/*
 	 *	Parse '[flags]'
 	 */
-	q = memchr(p, ' ', end - p);
-	if (!q) {
-		REDEBUG("Missing replacement");
-		return XLAT_ACTION_FAIL;
-	}
-
 	memset(&flags, 0, sizeof(flags));
 
-	slen = regex_flags_parse(NULL, &flags, &FR_SBUFF_IN(p, q), NULL, true);
+	slen = regex_flags_parse(NULL, &flags, &FR_SBUFF_IN(p, end), NULL, true);
 	if (slen < 0) {
 		RPEDEBUG("Failed parsing regex flags");
 		return XLAT_ACTION_FAIL;
 	}
 
-	/*
-	 *	Parse ' <replace>'
-	 */
-	p += slen;
-
-	fr_assert(*p == ' ');
-
-	p++;	/* Skip space */
-	rep = p;
-
-	/*
-	 *	Parse ' <subject>'
-	 */
-	q = memchr(p, ' ', end - p);
-	if (!q) {
-		REDEBUG("Missing subject");
-		return XLAT_ACTION_FAIL;
-	}
-	rep_len = q - p;
-
-	p = q + 1;
-
-	subject = p;
-	subject_len = end - p;
-
 	/*
 	 *	Process the substitution
 	 */
@@ -2955,13 +2902,14 @@ static xlat_action_t xlat_func_sub_regex(TALLOC_CTX *ctx, fr_dcursor_t *out,
 
 	MEM(vb = fr_value_box_alloc_null(ctx));
 	if (regex_substitute(vb, &buff, 0, pattern, &flags,
-			     subject, subject_len, rep, rep_len, NULL) < 0) {
+			     subject_vb->vb_strvalue, subject_vb->vb_length,
+			     rep_vb->vb_strvalue, rep_vb->vb_length, NULL) < 0) {
 		RPEDEBUG("Failed performing substitution");
 		talloc_free(vb);
 		talloc_free(pattern);
 		return XLAT_ACTION_FAIL;
 	}
-	fr_value_box_bstrdup_buffer_shallow(NULL, vb, NULL, buff, in_head->tainted);
+	fr_value_box_bstrdup_buffer_shallow(NULL, vb, NULL, buff, subject_vb->tainted);
 
 	fr_dcursor_append(out, vb);
 
@@ -2972,15 +2920,22 @@ static xlat_action_t xlat_func_sub_regex(TALLOC_CTX *ctx, fr_dcursor_t *out,
 #endif
 
 
+static xlat_arg_parser_t const xlat_func_sub_args[] = {
+	{ .required = true, .concat = true, .type = FR_TYPE_STRING },
+	{ .required = true, .concat = true, .type = FR_TYPE_STRING },
+	{ .required = true, .concat = true, .type = FR_TYPE_STRING },
+	XLAT_ARG_PARSER_TERMINATOR
+};
+
 /** Perform regex substitution
  *
 @verbatim
-%{sub:<pattern> <replace> <subject>}
+%(sub:<pattern> <replace> <subject>)
 @endverbatim
  *
  * Example: (User-Name = "foobar")
 @verbatim
-"%{sub:oo un %{User-Name}}" == "funbar"
+"%(sub:oo un %{User-Name})" == "funbar"
 @endverbatim
  *
  * @see xlat_func_sub_regex
@@ -3002,34 +2957,12 @@ static xlat_action_t xlat_func_sub(TALLOC_CTX *ctx, fr_dcursor_t *out,
 	char const		*pattern, *rep;
 	size_t			pattern_len, rep_len;
 
-	fr_value_box_t		*vb;
-	fr_value_box_t		*in_head = fr_dlist_head(in);
-
-	/*
-	 *	If there's no input, there's no output
-	 */
-	if (!in_head) {
-		REDEBUG("No input arguments");
-		return XLAT_ACTION_FAIL;
-	}
-
-	/*
-	 *	Concatenate all input
-	 */
-	if (fr_value_box_list_concat(ctx, in_head, in, FR_TYPE_STRING, true) < 0) {
-		RPEDEBUG("Failed concatenating input");
-		return XLAT_ACTION_FAIL;
-	}
-
-	p = in_head->vb_strvalue;
-	end = p + in_head->vb_length;
+	fr_value_box_t		*rep_vb, *subject_vb, *vb;
+	fr_value_box_t		*pattern_vb = fr_dlist_head(in);
 
-	if (p == end) {
-		REDEBUG("Substitution arguments must not be empty");
-		return XLAT_ACTION_FAIL;
-	}
+	pattern = pattern_vb->vb_strvalue;
 
-	if (*p == '/') {
+	if (*pattern == '/') {
 #ifdef HAVE_REGEX_PCRE2
 		return xlat_func_sub_regex(ctx, out, request, xlat_inst, xlat_thread_inst, in);
 #else
@@ -3040,33 +2973,22 @@ static xlat_action_t xlat_func_sub(TALLOC_CTX *ctx, fr_dcursor_t *out,
 	}
 
 	/*
-	 *	Parse '<pattern> '
+	 *	Check for empty pattern
 	 */
-	q = memchr(p, ' ', end - p);
-	if (!q || (q == p)) {
-		REDEBUG("Missing pattern");
+	pattern_len = pattern_vb->vb_length;
+	if (pattern_len == 0) {
+		REDEBUG("Empty pattern");
 		return XLAT_ACTION_FAIL;
 	}
 
-	pattern = p;
-	pattern_len = q - p;
-	p = q + 1;
+	rep_vb = fr_dlist_next(in, pattern_vb);
+	rep = rep_vb->vb_strvalue;
+	rep_len = rep_vb->vb_length;
 
-	/*
-	 *	Parse '<replacement> '
-	 */
-	q = memchr(p, ' ', end - p);
-	if (!q) {
-		REDEBUG("Missing subject");
-		return XLAT_ACTION_FAIL;
-	}
-	rep = p;
-	rep_len = q - p;
-	p = q + 1;
+	subject_vb = fr_dlist_next(in, rep_vb);
+	p = subject_vb->vb_strvalue;
+	end = p + subject_vb->vb_length;
 
-	/*
-	 *	Parse '<subject>'
-	 */
 	MEM(vb = fr_value_box_alloc_null(ctx));
 	vb_str = talloc_bstrndup(vb, "", 0);
 
@@ -3082,7 +3004,7 @@ static xlat_action_t xlat_func_sub(TALLOC_CTX *ctx, fr_dcursor_t *out,
 		p = q + pattern_len;
 	}
 
-	if (fr_value_box_bstrdup_buffer_shallow(vb, vb, NULL, vb_str, in_head->vb_strvalue) < 0) {
+	if (fr_value_box_bstrdup_buffer_shallow(vb, vb, NULL, vb_str, subject_vb->tainted) < 0) {
 		RPEDEBUG("Failed creating output box");
 		talloc_free(vb);
 		return XLAT_ACTION_FAIL;
@@ -3094,6 +3016,7 @@ static xlat_action_t xlat_func_sub(TALLOC_CTX *ctx, fr_dcursor_t *out,
 	return XLAT_ACTION_DONE;
 }
 
+
 /** Change case of a string
  *
  * If upper is true, change to uppercase, otherwise, change to lowercase
@@ -3421,7 +3344,7 @@ do { \
 
 	XLAT_REGISTER_MONO("string", xlat_func_string, xlat_func_string_arg);
 	XLAT_REGISTER_MONO("strlen", xlat_func_strlen, xlat_func_strlen_arg);
-	xlat_register(NULL, "sub", xlat_func_sub, false);
+	XLAT_REGISTER_ARGS("sub", xlat_func_sub, xlat_func_sub_args);
 	XLAT_REGISTER_MONO("tolower", xlat_func_tolower, xlat_change_case_arg);
 	XLAT_REGISTER_MONO("toupper", xlat_func_toupper, xlat_change_case_arg);
 	XLAT_REGISTER_MONO("urlquote", xlat_func_urlquote, xlat_func_urlquote_arg);
diff --git a/src/lib/unlang/xlat_tokenize.c b/src/lib/unlang/xlat_tokenize.c
index d2e4b2f1e07..839ccd2cbc1 100644
--- a/src/lib/unlang/xlat_tokenize.c
+++ b/src/lib/unlang/xlat_tokenize.c
@@ -1337,7 +1337,7 @@ ssize_t xlat_tokenize_argv(TALLOC_CTX *ctx, xlat_exp_t **head, xlat_flags_t *fla
 		tmp_p_rules = (fr_sbuff_parse_rules_t){	/* Stack allocated due to CL scope */
 			.terminals = fr_sbuff_terminals_amerge(NULL, p_rules->terminals,
 							       tmpl_parse_rules_bareword_quoted.terminals),
-			.escapes = tmpl_parse_rules_bareword_quoted.escapes
+			.escapes = (p_rules->escapes ? p_rules->escapes : tmpl_parse_rules_bareword_quoted.escapes)
 		};
 		our_p_rules = &tmp_p_rules;
 	} else {
diff --git a/src/lib/util/regex.c b/src/lib/util/regex.c
index b949385102f..cb07390cd1a 100644
--- a/src/lib/util/regex.c
+++ b/src/lib/util/regex.c
@@ -530,8 +530,6 @@ again:
 				fr_strerror_const("libpcre2 out of memory");
 				return -1;
 			}
-
-			talloc_free(buff);
 			buff_len = actual_len;	/* The length we get passed back includes the \0 */
 			buff = talloc_array(ctx, char, buff_len);
 			goto again;
diff --git a/src/tests/keywords/xlat-sub b/src/tests/keywords/xlat-sub
index b240b71da9c..382625dd77b 100644
--- a/src/tests/keywords/xlat-sub
+++ b/src/tests/keywords/xlat-sub
@@ -12,68 +12,68 @@ update request {
 #
 
 # Global substitution
-if ("%{sub:a b %{Tmp-String-0}}" != 'bbb') {
+if ("%(sub:a b %{Tmp-String-0})" != 'bbb') {
 	test_fail
 }
 
 # No match
-if ("%{sub:c b %{Tmp-String-0}}" != 'aaa') {
+if ("%(sub:c b %{Tmp-String-0})" != 'aaa') {
 	test_fail
 }
 
 # Line ending rewrite
-if ("%{sub:\n \r %{Tmp-String-1}}" != "\r\r\r") {
+if ("%(sub:\n \r %{Tmp-String-1})" != "\r\r\r") {
 	test_fail
 }
 
 # Removal
-if ("%{sub:a  %{Tmp-String-0}}" != "") {
+if ("%(sub:a '' %{Tmp-String-0})" != "") {
 	test_fail
 }
 
 # Removal of last word only
-if ("%{sub:dog  %{Tmp-String-2}}" != "the quick brown fox jumped over the lazy ") {
+if ("%(sub:dog '' %{Tmp-String-2})" != "the quick brown fox jumped over the lazy ") {
 	test_fail
 }
 
 # Removal of first and subsequent word
-if ("%{sub:the  %{Tmp-String-2}}" != " quick brown fox jumped over  lazy dog") {
+if ("%(sub:the '' %{Tmp-String-2})" != " quick brown fox jumped over  lazy dog") {
 	test_fail
 }
 
 # Removal of middle word
-if ("%{sub:jumped  %{Tmp-String-2}}" != "the quick brown fox  over the lazy dog") {
+if ("%(sub:jumped '' %{Tmp-String-2})" != "the quick brown fox  over the lazy dog") {
 	test_fail
 }
 
 # Replacement of last word only
-if ("%{sub:dog cat %{Tmp-String-2}}" != "the quick brown fox jumped over the lazy cat") {
+if ("%(sub:dog cat %{Tmp-String-2})" != "the quick brown fox jumped over the lazy cat") {
 	test_fail
 }
 
 # Replacement of first and subsequent word
-if ("%{sub:the cat %{Tmp-String-2}}" != "cat quick brown fox jumped over cat lazy dog") {
+if ("%(sub:the cat %{Tmp-String-2})" != "cat quick brown fox jumped over cat lazy dog") {
 	test_fail
 }
 
 # Replacement of middle word
-if ("%{sub:jumped cat %{Tmp-String-2}}" != "the quick brown fox cat over the lazy dog") {
+if ("%(sub:jumped cat %{Tmp-String-2})" != "the quick brown fox cat over the lazy dog") {
 	test_fail
 }
 
 if ("${feature.regex-pcre2}" == 'yes') {
 # Basic substitutions
-if ("%{sub:/a/ b %{Tmp-String-0}}" != 'baa') {
+if ("%(sub:/a/ b %{Tmp-String-0})" != 'baa') {
 	test_fail
 }
 
 # Global substitution
-if ("%{sub:/a/g b %{Tmp-String-0}}" != 'bbb') {
+if ("%(sub:/a/g b %{Tmp-String-0})" != 'bbb') {
 	test_fail
 }
 
 # No match
-if ("%{sub:/z/ b %{Tmp-String-0}}" != 'aaa') {
+if ("%(sub:/z/ b %{Tmp-String-0})" != 'aaa') {
 	test_fail
 }
 
@@ -87,26 +87,26 @@ if ("%{length:%{Tmp-String-1}}" != 3) {
 }
 
 # Strip out just the first newline
-if ("%{sub:/^./s  %{Tmp-String-1}}" != "\n\n") {
+if ("%(sub:/^./s '' %{Tmp-String-1})" != "\n\n") {
 	test_fail
 }
 
-if ("%{sub:/\n/  %{Tmp-String-1}}" != "\n\n") {
+if ("%(sub:/\n/ '' %{Tmp-String-1})" != "\n\n") {
 	test_fail
 }
 
 # Strip out all the newlines
-if ("%{sub:/\n/g  %{Tmp-String-1}}" != '') {
+if ("%(sub:/\n/g '' %{Tmp-String-1})" != '') {
 	test_fail
 }
 
 # Line ending switch
-if ("%{sub:/\n/g \r %{Tmp-String-1}}" != "\r\r\r") {
+if ("%(sub:/\n/g \r %{Tmp-String-1})" != "\r\r\r") {
 	test_fail
 }
 
 # Bad regex
-if ("%{sub:/***/g . %{Tmp-String-0}}" != '') {
+if ("%(sub:/***/g . %{Tmp-String-0})" != '') {
 	test_fail
 }
 
@@ -115,7 +115,7 @@ if (&Module-Failure-Message[1] != 'Failed compiling regex: quantifier does not f
 }
 
 # Empty regex
-if ("%{sub://g . %{Tmp-String-0}}" != '') {
+if ("%(sub://g . %{Tmp-String-0})" != '') {
 	test_fail
 }