Skip to content

Commit

Permalink
more fixes and assertion checks for tmpl_require_enum_prefix=false
Browse files Browse the repository at this point in the history
  • Loading branch information
alandekok committed Jan 18, 2025
1 parent 23feab5 commit 01a7ca8
Show file tree
Hide file tree
Showing 6 changed files with 114 additions and 16 deletions.
4 changes: 3 additions & 1 deletion src/lib/server/map.c
Original file line number Diff line number Diff line change
Expand Up @@ -1170,6 +1170,8 @@ 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 != '&')) {
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;
Expand Down Expand Up @@ -2651,7 +2653,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;
Expand Down
4 changes: 3 additions & 1 deletion src/lib/server/tmpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
75 changes: 66 additions & 9 deletions src/lib/server/tmpl_tokenize.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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)) {
Expand All @@ -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:
Expand All @@ -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
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
*/
Expand Down Expand Up @@ -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);

/*
Expand Down
6 changes: 3 additions & 3 deletions src/lib/unlang/compile.c
Original file line number Diff line number Diff line change
Expand Up @@ -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>"));
Expand Down Expand Up @@ -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>"));
Expand Down Expand Up @@ -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>"));
Expand Down
2 changes: 1 addition & 1 deletion src/lib/unlang/xlat_expr.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
39 changes: 38 additions & 1 deletion src/lib/unlang/xlat_tokenize.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
*/
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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.
*
Expand All @@ -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:
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 01a7ca8

Please sign in to comment.