From: Alan T. DeKok Date: Fri, 17 Jan 2025 23:15:39 +0000 (-0500) Subject: more fixes and assertion checks for tmpl_require_enum_prefix=false X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=23eee669a64cd156ee0e6eb18a2ecb3b38751311;p=thirdparty%2Ffreeradius-server.git more fixes and assertion checks for tmpl_require_enum_prefix=false --- diff --git a/src/lib/server/map.c b/src/lib/server/map.c index 4f3cb949508..13092ec41f7 100644 --- a/src/lib/server/map.c +++ b/src/lib/server/map.c @@ -2652,7 +2652,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; diff --git a/src/lib/server/tmpl.h b/src/lib/server/tmpl.h index 35610942096..3dc3dec6451 100644 --- a/src/lib/server/tmpl.h +++ b/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 { diff --git a/src/lib/server/tmpl_tokenize.c b/src/lib/server/tmpl_tokenize.c index b8c6088f9d9..b4e974e408c 100644 --- a/src/lib/server/tmpl_tokenize.c +++ b/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 @@ do_suffix: 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); /* diff --git a/src/lib/unlang/compile.c b/src/lib/unlang/compile.c index a2a5402d956..7ed99b1cd71 100644 --- a/src/lib/unlang/compile.c +++ b/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, "")); @@ -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, "")); @@ -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, "")); diff --git a/src/lib/unlang/xlat_expr.c b/src/lib/unlang/xlat_expr.c index 3e86bea42c6..49e27941129 100644 --- a/src/lib/unlang/xlat_expr.c +++ b/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 diff --git a/src/lib/unlang/xlat_tokenize.c b/src/lib/unlang/xlat_tokenize.c index 105bca7b2b7..df3a068d34b 100644 --- a/src/lib/unlang/xlat_tokenize.c +++ b/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;