clean up use of safe_for

remove explicit safe_for argument from xlat_tokenize().  It is
already being passed in the tmpl_rules_t.   Passing a separate
and explicit 0 breaks all kinds of things.  e.g.. "..." gets
marked correctly, but %{"..."} does not.

require xlat_tokenize() to be passed a tmpl_rules_t.

update trigger code to allow the use of attributes from the
local dictionary.  Which is usually internal, but should arguably
also be allowed to be a protocol dictionary.  So that we can send
protocol-specfic triggers

add commented out function to double-check safe_for after parsing.

update callers of xlat_tokenize() to create a local tmpl_rules_t,
and pass the correct safe_for

Author: alandekok

src/bin/unit_test_attribute.c

@@ -2864,8 +2864,7 @@ static size_t command_xlat_normalise(command_result_t *result, command_file_ctx_
 						.allow_unresolved = cc->tmpl_rules.attr.allow_unresolved
 					},
 					.xlat = cc->tmpl_rules.xlat,
-				},
-				0);
+				});
 	if (dec_len <= 0) {
 		fr_strerror_printf_push_head("ERROR offset %d", (int) -dec_len);
 

src/bin/unit_test_module.c

@@ -486,7 +486,7 @@ static bool do_xlats(fr_event_list_t *el, request_t *request, char const *filena
 								};
 
 
-			slen = xlat_tokenize(xlat_ctx, &head, &line, &p_rules, &t_rules, 0);
+			slen = xlat_tokenize(xlat_ctx, &head, &line, &p_rules, &t_rules);
 			if (slen <= 0) {
 				talloc_free(xlat_ctx);
 				fr_sbuff_in_sprintf(&out, "ERROR offset %d '%s'", (int) -slen, fr_strerror());

src/lib/server/cf_parse.c

@@ -1402,7 +1402,7 @@ int cf_section_parse_pass2(void *base, CONF_SECTION *cs)
 							     .allow_unresolved = false,
 							     .allow_foreign = (dict == NULL)
 						     },
-					     }, 0);
+					     });
 			if (slen < 0) {
 				char *spaces, *text;
 

src/lib/server/tmpl_tokenize.c

@@ -3262,7 +3262,7 @@ fr_slen_t tmpl_afrom_substr(TALLOC_CTX *ctx, tmpl_t **out,
 			xlat_exp_head_t	*head = NULL;
 
 			vpt = tmpl_alloc_null(ctx);
-			slen = xlat_tokenize(vpt, &head, &our_in, p_rules, t_rules, t_rules->literals_safe_for);
+			slen = xlat_tokenize(vpt, &head, &our_in, p_rules, t_rules);
 			if (slen <= 0) FR_SBUFF_ERROR_RETURN(&our_in);
 
 			if (xlat_needs_resolving(head)) UNRESOLVED_SET(&type);
@@ -3471,7 +3471,7 @@ fr_slen_t tmpl_afrom_substr(TALLOC_CTX *ctx, tmpl_t **out,
 
 		vpt = tmpl_alloc_null(ctx);
 
-		slen = xlat_tokenize(vpt, &head, &our_in, p_rules, t_rules, t_rules->literals_safe_for);
+		slen = xlat_tokenize(vpt, &head, &our_in, p_rules, t_rules);
 		if (slen < 0) FR_SBUFF_ERROR_RETURN(&our_in);
 
 		/*
@@ -3555,6 +3555,9 @@ fr_slen_t tmpl_afrom_substr(TALLOC_CTX *ctx, tmpl_t **out,
 	{
 		xlat_exp_head_t		*head = NULL;
 		tmpl_type_t		type = TMPL_TYPE_REGEX_XLAT;
+		tmpl_rules_t		arg_t_rules = *t_rules;
+
+		arg_t_rules.literals_safe_for = FR_REGEX_SAFE_FOR;
 
 		if (!fr_type_is_null(t_rules->cast)) {
 			fr_strerror_const("Casts cannot be used with regular expressions");
@@ -3564,7 +3567,7 @@ fr_slen_t tmpl_afrom_substr(TALLOC_CTX *ctx, tmpl_t **out,
 
 		vpt = tmpl_alloc_null(ctx);
 
-		slen = xlat_tokenize(vpt, &head, &our_in, p_rules, t_rules, FR_REGEX_SAFE_FOR);
+		slen = xlat_tokenize(vpt, &head, &our_in, p_rules, &arg_t_rules);
 		if (slen < 0) FR_SBUFF_ERROR_RETURN(&our_in);
 
 		/*
@@ -4003,6 +4006,7 @@ int tmpl_cast_in_place(tmpl_t *vpt, fr_type_t type, fr_dict_attr_t const *enumv)
 		 *	i.e. TMPL_TYPE_DATA_UNRESOLVED != TMPL_TYPE_DATA(FR_TYPE_STRING)
 		 */
 		if (fr_value_box_cast_in_place(vpt, &vpt->data.literal, type, NULL) < 0) return -1;
+//		fr_value_box_mark_safe_for(&vpt->data.literal, vpt->rules.literals_safe_for); ??? is this necessary?
 
 		/*
 		 *	Strings get quoted, everything else is a bare

src/lib/server/trigger.c

@@ -380,11 +380,17 @@ int trigger_exec(unlang_interpret_t *intp,
 	MEM(trigger = talloc_zero(request, fr_trigger_t));
 	fr_value_box_list_init(&trigger->args);
 	trigger->command = talloc_strdup(trigger, value);
-	trigger->timeout = fr_time_delta_from_sec(5);	/* FIXME - Should be configurable? */
+	trigger->timeout = fr_time_delta_from_sec(5);	/* @todo - Should be configurable? */
 
 	slen = xlat_tokenize_argv(trigger, &trigger->xlat,
 				  &FR_SBUFF_IN(trigger->command, talloc_array_length(trigger->command) - 1),
-				  NULL, NULL, NULL, true);
+				  NULL, NULL, &(tmpl_rules_t) {
+					  .attr = {
+						  .dict_def = request->dict,
+						  .list_def = request_attr_request,
+						  .allow_unresolved = false,
+					  },
+				  }, true);
 	if (slen <= 0) {
 		char *spaces, *text;
 

src/lib/unlang/xlat.h

@@ -405,12 +405,11 @@ fr_slen_t	xlat_tokenize_condition(TALLOC_CTX *ctx, xlat_exp_head_t **head, fr_sb
 					fr_sbuff_parse_rules_t const *p_rules, tmpl_rules_t const *t_rules);
 
 fr_slen_t 	xlat_tokenize_argv(TALLOC_CTX *ctx, xlat_exp_head_t **head, fr_sbuff_t *in,
-				   xlat_t const *xlat,
-				   fr_sbuff_parse_rules_t const *p_rules, tmpl_rules_t const *t_rules, bool spaces);
+				   xlat_t const *xlat, fr_sbuff_parse_rules_t const *p_rules, tmpl_rules_t const *t_rules,
+				   bool spaces) CC_HINT(nonnull(1,2,3,6));
 
 fr_slen_t	xlat_tokenize(TALLOC_CTX *ctx, xlat_exp_head_t **head, fr_sbuff_t *in,
-			      fr_sbuff_parse_rules_t const *p_rules, tmpl_rules_t const *t_rules,
-			      fr_value_box_safe_for_t literals_safe_for);
+			      fr_sbuff_parse_rules_t const *p_rules, tmpl_rules_t const *t_rules);
 
 fr_slen_t	xlat_print(fr_sbuff_t *in, xlat_exp_head_t const *node, fr_sbuff_escape_rules_t const *e_rules);
 

src/lib/unlang/xlat_eval.c

@@ -1509,7 +1509,7 @@ ssize_t _xlat_eval(TALLOC_CTX *ctx, char **out, size_t outlen, request_t *reques
 					    .runtime_el = unlang_interpret_event_list(request),
 				    },
 				    .at_runtime = true,
-			    }, 0);
+			    });
 	if (len == 0) {
 		if (*out) {
 			**out = '\0';

src/lib/unlang/xlat_tokenize.c

@@ -220,9 +220,9 @@ static int xlat_validate_function_arg(xlat_arg_parser_t const *arg_p, xlat_exp_t
 			fr_assert(tmpl_rules_cast(vpt) == FR_TYPE_NULL);
 
 			fr_value_box_steal(node, &node->data, tmpl_value(vpt));
-
 			talloc_free(vpt);
 			xlat_exp_set_type(node, XLAT_BOX);
+			fr_value_box_mark_safe_for(&node->data, arg_p->safe_for);
 
 		} else {
 			fr_assert(!tmpl_is_data_unresolved(node->vpt));
@@ -310,7 +310,7 @@ fr_slen_t xlat_validate_function_args(xlat_exp_t *node)
  *	- 0 if the string was parsed into a function.
  *	- <0 on parse error.
  */
-static int xlat_tokenize_function_args(xlat_exp_head_t *head, fr_sbuff_t *in, tmpl_rules_t const *t_rules)
+static CC_HINT(nonnull) int xlat_tokenize_function_args(xlat_exp_head_t *head, fr_sbuff_t *in, tmpl_rules_t const *t_rules)
 {
 	char c;
 	xlat_exp_t *node;
@@ -824,15 +824,12 @@ int xlat_tokenize_expansion(xlat_exp_head_t *head, fr_sbuff_t *in,
  * @param[in] in		sbuff to parse.
  * @param[in] p_rules		that control parsing.
  * @param[in] t_rules		that control attribute reference and xlat function parsing.
- * @param[in] safe_for		mark up literal values as being pre-escaped.  May be merged
- *				with t_rules in future.
  * @return
  *	- <0 on failure
  *	- >=0 for number of bytes parsed
  */
-static ssize_t xlat_tokenize_input(xlat_exp_head_t *head, fr_sbuff_t *in,
-				   fr_sbuff_parse_rules_t const *p_rules, tmpl_rules_t const *t_rules,
-				   fr_value_box_safe_for_t safe_for)
+static CC_HINT(nonnull(1,2,4)) ssize_t xlat_tokenize_input(xlat_exp_head_t *head, fr_sbuff_t *in,
+							   fr_sbuff_parse_rules_t const *p_rules, tmpl_rules_t const *t_rules)
 {
 	xlat_exp_t			*node = NULL;
 	fr_slen_t			slen;
@@ -883,7 +880,7 @@ static ssize_t xlat_tokenize_input(xlat_exp_head_t *head, fr_sbuff_t *in,
 		do_value_box:
 			xlat_exp_set_name_buffer_shallow(node, str);
 			fr_value_box_strdup(node, &node->data, NULL, str, false);
-			fr_value_box_mark_safe_for(&node->data, safe_for);
+			fr_value_box_mark_safe_for(&node->data, t_rules->literals_safe_for);
 			node->flags.constant = true;
 			fr_assert(node->flags.pure);
 
@@ -1348,6 +1345,35 @@ ssize_t xlat_print(fr_sbuff_t *out, xlat_exp_head_t const *head, fr_sbuff_escape
 	return fr_sbuff_used_total(out) - at_in;
 }
 
+#if 0
+static void xlat_safe_for(xlat_exp_head_t *head, fr_value_box_safe_for_t safe_for)
+{
+	xlat_exp_foreach(head, node) {
+		switch (node->type) {
+		case XLAT_BOX:
+			if (node->data.safe_for != safe_for) {
+				ERROR("FAILED %lx %lx - %s", node->data.safe_for, safe_for, node->fmt);
+			}
+			fr_assert(node->data.safe_for == safe_for);
+			break;
+
+		case XLAT_GROUP:
+			xlat_safe_for(node->group, safe_for);
+			break;
+
+		case XLAT_TMPL:
+			if (!tmpl_is_xlat(node->vpt)) break;
+
+			xlat_safe_for(tmpl_xlat(node->vpt), safe_for);
+			break;
+
+		default:
+			break;
+		}
+	}
+}
+#endif
+
 
 /** Tokenize an xlat expansion into a series of XLAT_TYPE_CHILD arguments
  *
@@ -1378,8 +1404,7 @@ fr_slen_t xlat_tokenize_argv(TALLOC_CTX *ctx, xlat_exp_head_t **out, fr_sbuff_t
 	fr_sbuff_parse_rules_t		tmp_p_rules;
 	xlat_exp_head_t			*head;
 	xlat_arg_parser_t const		*arg = NULL, *arg_start;
-	tmpl_rules_t const		*our_t_rules = t_rules;
-	tmpl_rules_t			tmp_t_rules;
+	tmpl_rules_t			arg_t_rules;
 
 	if (xlat && xlat->args) {
 		arg_start = arg = xlat->args;	/* Track the arguments as we parse */
@@ -1388,6 +1413,7 @@ fr_slen_t xlat_tokenize_argv(TALLOC_CTX *ctx, xlat_exp_head_t **out, fr_sbuff_t
 								  XLAT_ARG_PARSER_TERMINATOR };
 		arg_start = arg = &default_arg[0];
 	}
+	arg_t_rules = *t_rules;
 
 	if (spaces) {
 		fr_assert(p_rules != &xlat_function_arg_rules);
@@ -1421,15 +1447,12 @@ fr_slen_t xlat_tokenize_argv(TALLOC_CTX *ctx, xlat_exp_head_t **out, fr_sbuff_t
 		 *	expression to be just about anything.
 		 */
 		if (!xlat_func_bare_words) {
-			tmp_t_rules = *t_rules;
-			our_t_rules = &tmp_t_rules;
-
-			tmp_t_rules.enumv = NULL;
-			tmp_t_rules.cast = FR_TYPE_NULL;
-			tmp_t_rules.attr.namespace = NULL;
-			tmp_t_rules.attr.request_def = NULL;
-			tmp_t_rules.attr.list_def = request_attr_request;
-			tmp_t_rules.attr.list_presence = TMPL_ATTR_LIST_ALLOW;
+			arg_t_rules.enumv = NULL;
+			arg_t_rules.cast = FR_TYPE_NULL;
+			arg_t_rules.attr.namespace = NULL;
+			arg_t_rules.attr.request_def = NULL;
+			arg_t_rules.attr.list_def = request_attr_request;
+			arg_t_rules.attr.list_presence = TMPL_ATTR_LIST_ALLOW;
 		}
 	}
 
@@ -1449,6 +1472,7 @@ fr_slen_t xlat_tokenize_argv(TALLOC_CTX *ctx, xlat_exp_head_t **out, fr_sbuff_t
 		size_t		len;
 
 		fr_sbuff_set(&m, &our_in);	/* Record start of argument */
+		arg_t_rules.literals_safe_for = arg->safe_for;
 
 		/*
 		 *	Whitespace isn't significant for comma-separated argvs
@@ -1497,7 +1521,7 @@ fr_slen_t xlat_tokenize_argv(TALLOC_CTX *ctx, xlat_exp_head_t **out, fr_sbuff_t
 				 *
 				 *	No spaces - each arugment is an expression, which can have embedded spaces.
 				 */
-				slen = xlat_tokenize_input(node->group, &our_in, our_p_rules, t_rules, arg->safe_for);
+				slen = xlat_tokenize_input(node->group, &our_in, our_p_rules, &arg_t_rules);
 
 			} else {
 			tokenize_expression:
@@ -1508,7 +1532,7 @@ fr_slen_t xlat_tokenize_argv(TALLOC_CTX *ctx, xlat_exp_head_t **out, fr_sbuff_t
 					slen = 0;
 
 				} else {
-					slen = xlat_tokenize_expression(node, &node->group, &our_in, our_p_rules, our_t_rules);
+					slen = xlat_tokenize_expression(node, &node->group, &our_in, our_p_rules, &arg_t_rules);
 				}
 			}
 			if (slen < 0) {
@@ -1551,7 +1575,7 @@ fr_slen_t xlat_tokenize_argv(TALLOC_CTX *ctx, xlat_exp_head_t **out, fr_sbuff_t
 			XLAT_DEBUG("ARGV double quotes <-- %.*s", (int) fr_sbuff_remaining(&our_in), fr_sbuff_current(&our_in));
 
 			if (xlat_tokenize_input(node->group, &our_in,
-						&value_parse_rules_double_quoted, t_rules, arg->safe_for) < 0) goto error;
+						&value_parse_rules_double_quoted, &arg_t_rules) < 0) goto error;
 			break;
 
 		/*
@@ -1600,6 +1624,13 @@ fr_slen_t xlat_tokenize_argv(TALLOC_CTX *ctx, xlat_exp_head_t **out, fr_sbuff_t
 		fmt = talloc_bstrndup(node, fr_sbuff_current(&m), fr_sbuff_behind(&m));
 		xlat_exp_set_name_buffer_shallow(node, fmt);
 
+		/*
+		 *	Assert that the parser has created things which are safe for the current argument.
+		 *
+		 *	@todo - function should be marked up with safe_for, and not each individual argument.
+		 */
+//		xlat_safe_for(node->group, arg->safe_for);
+
 		node->flags = node->group->flags;
 
 		xlat_exp_insert_tail(head, node);
@@ -1682,20 +1713,14 @@ fr_slen_t xlat_tokenize_argv(TALLOC_CTX *ctx, xlat_exp_head_t **out, fr_sbuff_t
  * @param[in] p_rules			controlling how the string containing the xlat
  *					expansions should be parsed.
  * @param[in] t_rules			controlling how attribute references are parsed.
- *					Do NOT alter this function to take tmpl_rules_t
- *					as this provides another value for literals_safe_for
- *					and this gets very confusing.
- * @param[in] literals_safe_for		the safe_for value to assign to any literals occurring at the
- *					top level of the expansion.
  * @return
  *	- >0 on success.
  *	- 0 and *head == NULL - Parse failure on first char.
  *	- 0 and *head != NULL - Zero length expansion
  *	- < 0 the negative offset of the parse failure.
  */
 fr_slen_t xlat_tokenize(TALLOC_CTX *ctx, xlat_exp_head_t **out, fr_sbuff_t *in,
-			fr_sbuff_parse_rules_t const *p_rules, tmpl_rules_t const *t_rules,
-			fr_value_box_safe_for_t literals_safe_for)
+			fr_sbuff_parse_rules_t const *p_rules, tmpl_rules_t const *t_rules)
 {
 	fr_sbuff_t	our_in = FR_SBUFF(in);
 	xlat_exp_head_t	*head;
@@ -1704,7 +1729,7 @@ fr_slen_t xlat_tokenize(TALLOC_CTX *ctx, xlat_exp_head_t **out, fr_sbuff_t *in,
 	MEM(head = xlat_exp_head_alloc(ctx));
 	fr_strerror_clear();	/* Clear error buffer */
 
-	if (xlat_tokenize_input(head, &our_in, p_rules, t_rules, literals_safe_for) < 0) {
+	if (xlat_tokenize_input(head, &our_in, p_rules, t_rules) < 0) {
 		talloc_free(head);
 		FR_SBUFF_ERROR_RETURN(&our_in);
 	}

src/modules/rlm_sqlcounter/rlm_sqlcounter.c

@@ -463,7 +463,7 @@ static int call_env_query_parse(TALLOC_CTX *ctx, void *out, tmpl_rules_t const *
 					['%'] = '%',
 					['\\'] = '\\',
 				},
-		  }}, t_rules, 0) < 0) {
+		  }}, t_rules) < 0) {
 		talloc_free(query);
 		return -1;
 	}