more fixes and assertion checks for tmpl_require_enum_prefix=false

Author: alandekok

src/lib/server/map.c

@@ -1170,6 +1170,10 @@ static int map_value_afrom_cp(TALLOC_CTX *ctx, map_t **out, map_t *parent, CONF_
 		 *	Don't bother trying things which we know aren't attributes.
 		 */
 		if ((t_rules->attr.prefix == TMPL_ATTR_REF_PREFIX_YES) && (*attr != '&')) {
+			extern bool tmpl_require_enum_prefix;
+
+			fr_assert(!tmpl_require_enum_prefix); /* @todo - fix this later */
+
 			slen = tmpl_afrom_substr(ctx, &map->lhs, &FR_SBUFF_IN(attr, talloc_array_length(attr) - 1), T_BARE_WORD, NULL, t_rules);
 			if (slen <= 0) goto marker;
 			break;
@@ -2651,7 +2655,7 @@ int map_afrom_fields(TALLOC_CTX *ctx, map_t **out, map_t **parent_p, request_t *
 		/*
 		 *	No enums here.
 		 */
-		fr_assert(my_rules.attr.prefix == TMPL_ATTR_REF_PREFIX_YES);
+		fr_assert(my_rules.attr.prefix != TMPL_ATTR_REF_PREFIX_NO);
 		fr_assert(my_rules.attr.list_def == request_attr_request);
 
 		my_rules.enumv = NULL;

src/lib/server/tmpl.h

@@ -264,7 +264,7 @@ typedef struct tmpl_s tmpl_t;
 typedef enum {
 	TMPL_ATTR_REF_PREFIX_YES = 0,			//!< Attribute refs must have '&' prefix.
 	TMPL_ATTR_REF_PREFIX_NO,			//!< Attribute refs have no '&' prefix.
-	TMPL_ATTR_REF_PREFIX_AUTO 			//!< Attribute refs may have a '&' prefix.
+	TMPL_ATTR_REF_PREFIX_AUTO			//!< Attribute refs may have a '&' prefix.
 } tmpl_attr_prefix_t;
 
 /** Specify whether attribute references can have a list (or parent) reference
@@ -327,6 +327,8 @@ struct tmpl_attr_rules_s {
 	uint8_t			allow_foreign:1;	//!< Allow arguments not found in dict_def.
 
 	uint8_t			disallow_filters:1;	//!< disallow filters.
+
+	uint8_t			xlat:1	;		//!< for %{User-Name}
 };
 
 struct tmpl_xlat_rules_s {

src/lib/server/tmpl_tokenize.c

@@ -103,7 +103,18 @@ TMPL_REQUEST_REF_DEF(tmpl_request_def_parent, REQUEST_PARENT);
  *
  * Defaults are used if a NULL rules pointer is passed to the parsing function.
  */
-#define DEFAULT_RULES tmpl_rules_t const default_rules = { .attr = { .list_def = request_attr_request }}
+#define DEFAULT_RULES tmpl_rules_t default_rules = { .attr = { .list_def = request_attr_request }}
+
+#define CHECK_T_RULES do { \
+	if (!t_rules) { \
+		t_rules = &default_rules; \
+		if (tmpl_require_enum_prefix) default_rules.attr.prefix = TMPL_ATTR_REF_PREFIX_AUTO; \
+	} else if (tmpl_require_enum_prefix && (t_rules->attr.prefix == TMPL_ATTR_REF_PREFIX_YES)) { \
+		default_rules = *t_rules; \
+		default_rules.attr.prefix = TMPL_ATTR_REF_PREFIX_AUTO; \
+		t_rules = &default_rules; \
+	} \
+  } while (0)
 
 
 /* clang-format off */
@@ -541,11 +552,8 @@ static fr_slen_t tmpl_request_ref_list_from_substr(TALLOC_CTX *ctx, tmpl_attr_er
 	tmpl_attr_rules_t const	*at_rules;
 	DEFAULT_RULES;
 
-	if (!t_rules) {
-		at_rules = &default_rules.attr;
-	} else {
-		at_rules = &t_rules->attr;
-	}
+	CHECK_T_RULES;
+	at_rules = &t_rules->attr;
 
 	/*
 	 *	The caller wants to know the default namespace for
@@ -1076,6 +1084,11 @@ int tmpl_attr_copy(tmpl_t *dst, tmpl_t const *src)
 	tmpl_request_list_talloc_reverse_free(&dst->data.attribute.rr);
 	tmpl_request_ref_list_copy(dst, &dst->data.attribute.rr, &src->data.attribute.rr);
 
+	/*
+	 *	Ensure that we copy over any parsing rules, defaults, etc.
+	 */
+	dst->rules = src->rules;
+
 	TMPL_ATTR_VERIFY(dst);
 
 	return 0;
@@ -1773,6 +1786,8 @@ static inline int tmpl_attr_afrom_attr_substr(TALLOC_CTX *ctx, tmpl_attr_error_t
 	fr_dict_attr_err_t	dict_err;
 	fr_dict_attr_t const	*our_parent = parent;
 
+	fr_assert(!tmpl_require_enum_prefix || (vpt->rules.attr.prefix != TMPL_ATTR_REF_PREFIX_YES));
+
 	fr_sbuff_marker(&m_s, name);
 
 	if (depth > FR_DICT_MAX_TLV_STACK) {
@@ -2163,6 +2178,8 @@ static inline int tmpl_attr_afrom_attr_substr(TALLOC_CTX *ctx, tmpl_attr_error_t
 	if (tmpl_is_attr(vpt) && tmpl_attr_tail_is_normal(vpt) &&
 	    (tmpl_rules_cast(vpt) == tmpl_attr_tail_da(vpt)->type)) vpt->rules.cast = FR_TYPE_NULL;
 
+	TMPL_VERIFY(vpt);
+
 	fr_sbuff_marker_release(&m_s);
 	return 0;
 }
@@ -2218,13 +2235,17 @@ ssize_t tmpl_afrom_attr_substr(TALLOC_CTX *ctx, tmpl_attr_error_t *err,
 	fr_sbuff_t			our_name = FR_SBUFF(name);	/* Take a local copy in case we need to back track */
 	bool				is_raw = false;
 	tmpl_attr_rules_t const		*at_rules;
+	tmpl_attr_rules_t	       	our_at_rules;
 	fr_sbuff_marker_t		m_l;
 	fr_dict_attr_t const		*namespace;
 	DEFAULT_RULES;
 
-	if (!t_rules) t_rules = &default_rules;
+	CHECK_T_RULES;
+
 	at_rules = &t_rules->attr;
 
+	fr_assert(!tmpl_require_enum_prefix || (at_rules->prefix != TMPL_ATTR_REF_PREFIX_YES));
+
 	if (err) *err = TMPL_ATTR_ERROR_NONE;
 
 	if (!fr_sbuff_extend(&our_name)) {
@@ -2246,6 +2267,14 @@ ssize_t tmpl_afrom_attr_substr(TALLOC_CTX *ctx, tmpl_attr_error_t *err,
 			}
 			break;
 		}
+
+		/*
+		 *	Rewrite the prefix parsing to "auto", which affects the printing.
+		 */
+		our_at_rules = *at_rules;
+		our_at_rules.prefix = TMPL_ATTR_REF_PREFIX_AUTO;
+		at_rules = &our_at_rules;
+
 		FALL_THROUGH;	/* if we do require enum prefixes, then the '&' is optional */
 
 	case TMPL_ATTR_REF_PREFIX_AUTO:
@@ -2270,6 +2299,7 @@ ssize_t tmpl_afrom_attr_substr(TALLOC_CTX *ctx, tmpl_attr_error_t *err,
 	 */
 	MEM(vpt = tmpl_alloc(ctx, TMPL_TYPE_ATTR, T_BARE_WORD, NULL, 0));
 	vpt->data.attribute.ref_prefix = TMPL_ATTR_REF_PREFIX_YES;
+	vpt->rules.attr.prefix = at_rules->prefix;
 
 	/*
 	 *	The "raw." prefix marks up the leaf attribute
@@ -2433,6 +2463,9 @@ ssize_t tmpl_afrom_attr_substr(TALLOC_CTX *ctx, tmpl_attr_error_t *err,
 		fr_assert(ar != NULL);
 
 		if (tmpl_attr_is_list_attr(ar)) vpt->rules.attr.list_def = ar->ar_da;
+
+		fr_assert(!tmpl_require_enum_prefix || (vpt->rules.attr.prefix != TMPL_ATTR_REF_PREFIX_YES));
+
 	}
 
 	if (!tmpl_substr_terminal_check(&our_name, p_rules)) {
@@ -2478,7 +2511,7 @@ ssize_t tmpl_afrom_attr_str(TALLOC_CTX *ctx, tmpl_attr_error_t *err,
 	ssize_t slen, name_len;
 	DEFAULT_RULES;
 
-	if (!t_rules) t_rules = &default_rules;	/* Use the defaults */
+	CHECK_T_RULES;
 
 	name_len = strlen(name);
 	slen = tmpl_afrom_attr_substr(ctx, err, out, &FR_SBUFF_IN(name, name_len), NULL, t_rules);
@@ -3188,7 +3221,7 @@ fr_slen_t tmpl_afrom_substr(TALLOC_CTX *ctx, tmpl_t **out,
 	tmpl_t			*vpt = NULL;
 	DEFAULT_RULES;
 
-	if (!t_rules) t_rules = &default_rules;	/* Use the defaults */
+	CHECK_T_RULES;
 
 	*out = NULL;
 
@@ -3318,6 +3351,8 @@ fr_slen_t tmpl_afrom_substr(TALLOC_CTX *ctx, tmpl_t **out,
 		if (slen > 0) goto done_bareword;
 		fr_assert(!*out);
 
+		fr_assert(!tmpl_require_enum_prefix || (t_rules->attr.prefix != TMPL_ATTR_REF_PREFIX_YES));
+
 		/*
 		 *	See if it's an attribute reference
 		 *	without the prefix.
@@ -3596,6 +3631,8 @@ tmpl_t *tmpl_copy(TALLOC_CTX *ctx, tmpl_t const *in)
 		if (unlikely(xlat_copy(vpt, vpt->data.xlat.ex, in->data.xlat.ex) < 0)) goto error;
 	}
 
+	TMPL_ATTR_VERIFY(vpt);
+
 	return vpt;
 }
 
@@ -4265,6 +4302,8 @@ int tmpl_resolve(tmpl_t *vpt, tmpl_res_rules_t const *tr_rules)
 		fr_assert(0);
 	}
 
+	TMPL_VERIFY(vpt);
+
 	return ret;
 }
 
@@ -4644,6 +4683,13 @@ fr_slen_t tmpl_attr_print(fr_sbuff_t *out, tmpl_t const *vpt, tmpl_attr_prefix_t
 		return 0;
 	}
 
+	/*
+	 *	Suppress the prefix on new syntax.
+	 */
+	if (tmpl_require_enum_prefix && (ar_prefix == TMPL_ATTR_REF_PREFIX_YES)) {
+		ar_prefix = TMPL_ATTR_REF_PREFIX_AUTO;
+	}
+
 	/*
 	 *	Handle printing the request reference
 	 *	prefix.
@@ -4837,6 +4883,13 @@ fr_slen_t tmpl_print(fr_sbuff_t *out, tmpl_t const *vpt,
 
 	TMPL_VERIFY(vpt);
 
+	/*
+	 *	Suppress the prefix on new syntax.
+	 */
+	if (tmpl_require_enum_prefix && (ar_prefix == TMPL_ATTR_REF_PREFIX_YES)) {
+		ar_prefix = TMPL_ATTR_REF_PREFIX_AUTO;
+	}
+
 	switch (vpt->type) {
 	case TMPL_TYPE_ATTR_UNRESOLVED:
 	case TMPL_TYPE_ATTR:
@@ -4993,6 +5046,8 @@ void tmpl_attr_verify(char const *file, int line, tmpl_t const *vpt)
 
 	fr_assert(tmpl_is_attr_unresolved(vpt) || tmpl_is_attr(vpt));
 
+	fr_assert(!tmpl_require_enum_prefix || (vpt->rules.attr.prefix != TMPL_ATTR_REF_PREFIX_YES));
+
 	/*
 	 *	Loop detection
 	 */
@@ -5781,6 +5836,8 @@ void tmpl_rules_child_init(TALLOC_CTX *ctx, tmpl_rules_t *out, tmpl_rules_t cons
 
 	if (!tmpl_is_attr(vpt)) return;
 
+	fr_assert(!tmpl_require_enum_prefix || (vpt->rules.attr.prefix != TMPL_ATTR_REF_PREFIX_YES));
+
 	da = tmpl_attr_tail_da(vpt);
 
 	/*

src/lib/unlang/compile.c

@@ -408,7 +408,7 @@ static int unlang_fixup_map(map_t *map, UNUSED void *ctx)
 	/*
 	 *	Anal-retentive checks.
 	 */
-	if (DEBUG_ENABLED3) {
+	if (!tmpl_require_enum_prefix && DEBUG_ENABLED3) {
 		if (tmpl_is_attr(map->lhs) && (map->lhs->name[0] != '&')) {
 			cf_log_warn(cp, "Please change attribute reference to '&%s %s ...'",
 				    map->lhs->name, fr_table_str_by_value(fr_tokens_table, map->op, "<INVALID>"));
@@ -474,7 +474,7 @@ int unlang_fixup_update(map_t *map, void *ctx)
 	/*
 	 *	Anal-retentive checks.
 	 */
-	if (DEBUG_ENABLED3) {
+	if (!tmpl_require_enum_prefix && DEBUG_ENABLED3) {
 		if (tmpl_is_attr(map->lhs) && (map->lhs->name[0] != '&')) {
 			cf_log_warn(cp, "Please change attribute reference to '&%s %s ...'",
 				    map->lhs->name, fr_table_str_by_value(fr_tokens_table, map->op, "<INVALID>"));
@@ -1306,7 +1306,7 @@ static int unlang_fixup_edit(map_t *map, void *ctx)
 	/*
 	 *	Anal-retentive checks.
 	 */
-	if (DEBUG_ENABLED3) {
+	if (!tmpl_require_enum_prefix && DEBUG_ENABLED3) {
 		if (tmpl_is_attr(map->lhs) && (map->lhs->name[0] != '&')) {
 			cf_log_warn(cp, "Please change attribute reference to '&%s %s ...'",
 				    map->lhs->name, fr_table_str_by_value(fr_tokens_table, map->op, "<INVALID>"));

src/lib/unlang/xlat_expr.c

@@ -2704,7 +2704,7 @@ static fr_slen_t tokenize_field(xlat_exp_head_t *head, xlat_exp_t **out, fr_sbuf
 #ifndef NDEBUG
 		if (vpt->name[0] == '%') {
 			fr_assert(vpt->rules.attr.prefix == TMPL_ATTR_REF_PREFIX_NO);
-		} else {
+		} else if (!tmpl_require_enum_prefix) {
 			fr_assert(vpt->rules.attr.prefix == TMPL_ATTR_REF_PREFIX_YES);
 		}
 #endif

src/lib/unlang/xlat_tokenize.c

@@ -45,6 +45,8 @@ RCSID("$Id$")
 #  define XLAT_HEXDUMP(...)
 #endif
 
+extern bool tmpl_require_enum_prefix;
+
 /** These rules apply to literal values and function arguments inside of an expansion
  *
  */
@@ -465,10 +467,17 @@ static int xlat_tokenize_attribute(xlat_exp_head_t *head, fr_sbuff_t *in,
 	xlat_exp_t		*node;
 
 	fr_sbuff_marker_t	m_s;
-	tmpl_rules_t		 our_t_rules;
+	tmpl_rules_t		our_t_rules;
 
 	XLAT_DEBUG("ATTRIBUTE <-- %.*s", (int) fr_sbuff_remaining(in), fr_sbuff_current(in));
 
+	/*
+	 *	Suppress the prefix on new syntax.
+	 */
+	if (tmpl_require_enum_prefix && (attr_prefix == TMPL_ATTR_REF_PREFIX_YES)) {
+		attr_prefix = TMPL_ATTR_REF_PREFIX_AUTO;
+	}
+
 	/*
 	 *	We need a local copy as we always allow unknowns.
 	 *	This is because not all attribute references
@@ -550,6 +559,14 @@ static int xlat_tokenize_attribute(xlat_exp_head_t *head, fr_sbuff_t *in,
 	}
 
 done:
+	/*
+	 *	Remember that it was %{User-Name}
+	 *
+	 *	This is a temporary hack until all of the unit tests
+	 *	pass without '&'.
+	 */
+	UNCONST(tmpl_attr_rules_t *, &vpt->rules.attr)->xlat = true;
+
 	/*
 	 *	Attributes and module calls aren't pure.
 	 */
@@ -1137,6 +1154,12 @@ ssize_t xlat_print_node(fr_sbuff_t *out, xlat_exp_head_t const *head, xlat_exp_t
 		fr_assert(talloc_parent(node->vpt) == node);
 		fr_assert(!node->flags.pure);
 
+		/*
+		 *	Can't have prefix YES if we're using the new flag.  The parser / tmpl alloc routines
+		 *	MUST have set this to prefix AUTO.
+		 */
+		fr_assert(!tmpl_require_enum_prefix || (node->vpt->rules.attr.prefix != TMPL_ATTR_REF_PREFIX_YES));
+
 		/*
 		 *	Parsing &User-Name or User-Name gets printed as &User-Name.
 		 *
@@ -1147,6 +1170,18 @@ ssize_t xlat_print_node(fr_sbuff_t *out, xlat_exp_head_t const *head, xlat_exp_t
 			FR_SBUFF_IN_STRCPY_RETURN(out, node->fmt);
 			goto done;
 		}
+
+		/*
+		 *	No '&', print the name, BUT without any attribute prefix.
+		 */
+		if (tmpl_require_enum_prefix && !node->vpt->rules.attr.xlat) {
+			char const *p = node->fmt;
+
+			if (*p == '&') p++;
+
+			FR_SBUFF_IN_STRCPY_RETURN(out, p);
+			goto done;
+		}
 		break;
 
 	case XLAT_ONE_LETTER:
@@ -1354,6 +1389,8 @@ fr_slen_t xlat_tokenize_argv(TALLOC_CTX *ctx, xlat_exp_head_t **out, fr_sbuff_t
 			 *	tmpl tokenizer, and then pass the tmpl to the function.  Which also means that
 			 *	we need to be able to have a fr_value_box_t which holds a ptr to a tmpl.  And
 			 *	update the function arguments to say "we want a tmpl, not a string".
+			 *
+			 *	@todo - tmpl_require_enum_prefix
 			 */
 			if (allow_attr && fr_sbuff_is_char(&our_in, '&')) {
 				if (xlat_tokenize_attribute(node->group, &our_in, our_p_rules, t_rules, TMPL_ATTR_REF_PREFIX_YES) < 0) goto error;