From: Alan T. DeKok Date: Sun, 1 Sep 2024 12:35:15 +0000 (-0400) Subject: add, test, and document &atrr-foo[&attr-num] X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9578a1bf849bffdf39fa2b7ae3db73c92a2eda07;p=thirdparty%2Ffreeradius-server.git add, test, and document &atrr-foo[&attr-num] --- diff --git a/doc/antora/modules/reference/pages/unlang/attr.adoc b/doc/antora/modules/reference/pages/unlang/attr.adoc index 7a15b58dac..bb6080fef1 100644 --- a/doc/antora/modules/reference/pages/unlang/attr.adoc +++ b/doc/antora/modules/reference/pages/unlang/attr.adoc @@ -42,22 +42,51 @@ looks in the input packet list for the named attribute. .Syntax [source,unlang] ---- -&Attribute-Name[] +&Attribute-Name[] ---- When an attribute appears multiple times in a list, this syntax allows you to address the attributes as if they were array entries. The -`` value defines which attribute to address. The `[0]` value +`` value defines which attribute to address. The `[0]` value refers to the first attributes, `[1]` refers to the second attribute, etc. -.Examples +The `` can be an integer (0..1000), as in the example below. + +The indexes are limited to 1000, because there are essentially no +protocols which have more than 1000 attributes. + +.Integer Array index [source,unlang] ---- &EAP-Message[1] &reply.NAS-IP-Address[2] ---- +The `` can also be a reference to a numerical attribute, as in the example below. + +The reference *must* be to an attribute of numerical data type. Structural data types and `string` or `octets` types are not allowed. If the index is out of bounds (e.g. negative), then the reference fails. + +The main utility of attribute indexes is in a xref:unlang/foreach.adoc[foreach] loop. + +.Attribute reference as Array index +[source,unlang] +---- +uint32 foo + +&foo = 2 + +&EAP-Message[&foo] +---- + +The `` can also be a special value `n`, which means "the last attribute in the list. + +.Last attribute in a list +[source,unlang] +---- +&EAP-Message[n] +---- + === Array References in lists It is sometimes useful to refer to children of a list, without diff --git a/doc/antora/modules/reference/pages/xlat/attribute.adoc b/doc/antora/modules/reference/pages/xlat/attribute.adoc index 38469bec36..3b91077f7a 100644 --- a/doc/antora/modules/reference/pages/xlat/attribute.adoc +++ b/doc/antora/modules/reference/pages/xlat/attribute.adoc @@ -40,17 +40,21 @@ Examples of using references inside of a string: Returns an integer containing the number of named attributes -`%{Attribute-Name[0]}`:: - -When an attribute appears multiple times in a list, this syntax allows -you to address the attributes as with array entries. `[0]` refers to -the first attributes, `[1]` refers to the second attribute, etc. - `%{Attribute-Name[*]}`:: Returns a comma-separated string containing all values for the named attributes. +`%{Attribute-Name[n]}`:: + +Returns the last attribute in the list. + +`%{Attribute-Name[]}`:: + +When an attribute appears multiple times in a list, this syntax allows +you to address the attributes as with array entries. `[0]` refers to +the first attributes, `[1]` refers to the second attribute, etc. See the xref:unlang/attr.adoc[attribute reference page for more information. + == Lists and Grouping attributes There is similar syntax for referencing children of a list, without @@ -59,15 +63,17 @@ addressing the children by name. `%{request.[#]}`:: Returns an integer containing the number of attributes contained in the `request` list. Any list name can be used. -`%{request.[0]}`:: - -Returns the value of the first attribute in the `request` list. - `%{request.[*]}`:: Returns a comma-separated string containing all values of attributes in the `request` list. -See also xref:unlang/attr.adoc[unlang attribute references]. +`%{request.[n]}`:: + +Returns the last attribute in the list. + +`%{request.[0]}`:: + +Returns the value of the first attribute in the `request` list. See the xref:unlang/attr.adoc[attribute reference page for more information. Note that the old syntax of `%{request[*]}` will instead refer to array entries of the `request` list. This is a change from previous diff --git a/src/lib/server/tmpl.h b/src/lib/server/tmpl.h index 7c4dc72848..8531d582e0 100644 --- a/src/lib/server/tmpl.h +++ b/src/lib/server/tmpl.h @@ -413,12 +413,22 @@ typedef enum { TMPL_ATTR_FILTER_TYPE_NONE = 0, //!< No filter present. TMPL_ATTR_FILTER_TYPE_INDEX, //!< Filter is an index type. TMPL_ATTR_FILTER_TYPE_CONDITION, //!< Filter is a condition + TMPL_ATTR_FILTER_TYPE_TMPL, //!< Filter is a tmpl } tmpl_attr_filter_type_t; typedef struct { tmpl_attr_filter_type_t _CONST type; //!< Type of filter this is. int16_t _CONST num; //!< For array references. - xlat_exp_head_t _CONST *cond; //!< xlat condition + + /* + * These are "union" because they are disjoint. The "num" field is arguably disjoint, too, but + * there is currently a lot of code in tmpl_tokenize.c which directly references ar->ar_num + * without checking the type. + */ + union { + xlat_exp_head_t _CONST *cond; //!< xlat condition + tmpl_t _CONST *tmpl; //!< tmpl + }; } tmpl_attr_filter_t; /** An element in a list of nested attribute references @@ -507,11 +517,13 @@ FR_DLIST_FUNCS(tmpl_request_list, tmpl_request_t, entry) #define ar_num filter.num #define ar_cond filter.cond +#define ar_tmpl filter.tmpl #define ar_filter_type filter.type #define ar_filter_is_none(_ar) ((_ar)->ar_filter_type == TMPL_ATTR_FILTER_TYPE_NONE) #define ar_filter_is_num(_ar) ((_ar)->ar_filter_type == TMPL_ATTR_FILTER_TYPE_INDEX) #define ar_filter_is_cond(_ar) ((_ar)->ar_filter_type == TMPL_ATTR_FILTER_TYPE_CONDITION) +#define ar_filter_is_tmpl(_ar) ((_ar)->ar_filter_type == TMPL_ATTR_FILTER_TYPE_TMPL) /** @} */ /** A source or sink of value data. diff --git a/src/lib/server/tmpl_dcursor.c b/src/lib/server/tmpl_dcursor.c index 5ddd1f4992..b67f5261f3 100644 --- a/src/lib/server/tmpl_dcursor.c +++ b/src/lib/server/tmpl_dcursor.c @@ -128,14 +128,58 @@ fr_pair_t *_tmpl_cursor_eval(fr_pair_t *curr, tmpl_dcursor_ctx_t *cc) tmpl_dcursor_nested_t *ns; fr_pair_t *iter = curr, *vp; bool pop = false; + int16_t num = NUM_ALL; ns = fr_dlist_tail(&cc->nested); ar = ns->ar; vp = fr_dcursor_current(&ns->cursor); - fr_assert(!ar || ar_filter_is_none(ar) || ar_filter_is_num(ar)); /* @todo - add evaluation of conditions */ + if (!ar) goto all_inst; - if (ar) switch (ar->ar_num) { + /* + * Array indexes can be attribute references. In which case they must be castable to a uint8_t. + * + * i.e. there's likly no point in allowing the array ref to specify "none", or "any", or "count". + * + * Arguably it's useful to specify "all", but why? The main utility of the array reference is to + * index a particular attribute when looping over a list of attributes. + */ + if (ar_filter_is_tmpl(ar)) { + uint8_t ref; + + fr_assert(ar_filter_is_tmpl(ar)); + fr_assert(tmpl_is_attr(ar->ar_tmpl)); + + /* + * Can't cast it, we're done. + */ + if (tmpl_expand(&ref, NULL, 0, cc->request, ar->ar_tmpl, NULL, NULL) < 0) { + vp = NULL; + pop = true; + goto done; + } + + num = ref; + goto find_num; + } + + /* + * @todo - add dynamic evaluation of conditions. But that would work _only_ if the conditions + * aren't blocking, AND we somehow have a way for the conditions to reference a "self" attribute. + */ + + /* + * No filter means "first one", unless the "foreach" code called tmpl_attr_rewrite_leaf_num(), + * which rewrites are_ + */ + if (ar_filter_is_none(ar)) { + num = 0; + } else { + fr_assert(ar_filter_is_num(ar)); + num = ar->ar_num; + } + + switch (num) { /* * Get the first instance */ @@ -149,6 +193,9 @@ fr_pair_t *_tmpl_cursor_eval(fr_pair_t *curr, tmpl_dcursor_ctx_t *cc) case NUM_ALL: case NUM_COUNT: all_inst: + /* + * @todo - arguably we shouldn't try building things here. + */ if (!vp) pop = true; /* pop only when we're done */ fr_dcursor_next(&ns->cursor); break; @@ -167,20 +214,21 @@ fr_pair_t *_tmpl_cursor_eval(fr_pair_t *curr, tmpl_dcursor_ctx_t *cc) * Get the n'th instance */ default: + find_num: { int16_t i = 0; - while ((i++ < ar->ar_num) && vp) vp = fr_dcursor_next(&ns->cursor); + while ((i++ < num) && vp) vp = fr_dcursor_next(&ns->cursor); pop = true; } break; - } else goto all_inst; + } /* * If no pair was found and there is a fill * callback, call that, depending on the suffix */ - if (!vp && cc->build && ar) switch (ar->ar_num) { + if (!vp && cc->build && ar) switch (num) { case NUM_UNSPEC: case NUM_LAST: case 0: @@ -191,6 +239,7 @@ fr_pair_t *_tmpl_cursor_eval(fr_pair_t *curr, tmpl_dcursor_ctx_t *cc) break; } +done: if (pop) tmpl_cursor_nested_pop(cc); return vp; diff --git a/src/lib/server/tmpl_tokenize.c b/src/lib/server/tmpl_tokenize.c index 88b80d17f7..fee0924eb4 100644 --- a/src/lib/server/tmpl_tokenize.c +++ b/src/lib/server/tmpl_tokenize.c @@ -1362,55 +1362,64 @@ static fr_slen_t tmpl_attr_parse_filter(tmpl_attr_error_t *err, tmpl_attr_t *ar, fr_sbuff_next(&our_name); break; - case 'n': - ar->ar_num = NUM_LAST; - fr_sbuff_next(&our_name); + case '0': + case '1': + case '2': + case '3': + case '4': + case '5': + case '6': + case '7': + case '8': + case '9': + { + ssize_t rcode; + fr_sbuff_parse_error_t sberr = FR_SBUFF_PARSE_OK; + fr_sbuff_t tmp = FR_SBUFF(&our_name); + + /* + * All digits (not hex). + */ + rcode = fr_sbuff_out(&sberr, &ar->ar_num, &tmp); + if ((rcode < 0) || !fr_sbuff_is_char(&tmp, ']')) goto parse_tmpl; + + if ((ar->ar_num > 1000) || (ar->ar_num < 0)) { + fr_strerror_printf("Invalid array index '%hi' (should be between 0-1000)", ar->ar_num); + ar->ar_num = 0; + goto error; + } + + fr_sbuff_set(&our_name, &tmp); /* Advance name _AFTER_ doing checks */ break; + } + + case '"': + case '\'': + case '`': + case '/': + fr_strerror_const("Invalid data type for array index"); + goto error; /* Used as EOB here */ missing_closing: case '\0': fr_strerror_const("No closing ']' for array index"); - if (err) *err = TMPL_ATTR_ERROR_INVALID_ARRAY_INDEX; error: + if (err) *err = TMPL_ATTR_ERROR_INVALID_ARRAY_INDEX; FR_SBUFF_ERROR_RETURN(&our_name); - default: + case '(': /* (...) expression */ { - fr_sbuff_parse_error_t sberr = FR_SBUFF_PARSE_OK; fr_sbuff_t tmp = FR_SBUFF(&our_name); - ssize_t rcode; fr_slen_t slen; tmpl_rules_t t_rules; fr_sbuff_parse_rules_t p_rules; fr_sbuff_term_t const filter_terminals = FR_SBUFF_TERMS(L("]")); - rcode = fr_sbuff_out(&sberr, &ar->ar_num, &tmp); - if ((rcode > 0) && (fr_sbuff_is_char(&tmp, ']'))) { - if ((ar->ar_num > 1000) || (ar->ar_num < 0)) { - fr_strerror_printf("Invalid array index '%hi' (should be between 0-1000)", ar->ar_num); - ar->ar_num = 0; - if (err) *err = TMPL_ATTR_ERROR_INVALID_ARRAY_INDEX; - goto error; - } - - fr_sbuff_set(&our_name, &tmp); /* Advance name _AFTER_ doing checks */ - break; - } - - /* - * Temporary parsing hack: &User-Name[a] does _not_ match a condition 'a'. - */ - if (!fr_sbuff_is_char(&tmp, '&')) { - fr_strerror_const("Invalid array index"); - if (err) *err = TMPL_ATTR_ERROR_INVALID_ARRAY_INDEX; - goto error; - } - /* * For now, we don't allow filtering on leaf values. e.g. * - * &User-Name[&User-Name == foo] + * &User-Name[(&User-Name == foo)] * * @todo - find some sane way of allowing this, without mangling the xlat expression * parser too badly. The simplest way is likely to just parse the expression, and then @@ -1434,10 +1443,9 @@ static fr_slen_t tmpl_attr_parse_filter(tmpl_attr_error_t *err, tmpl_attr_t *ar, * In order to fix that, we have to */ if (!fr_type_is_structural(ar->ar_da->type)) { - fr_strerror_printf("Invalid filter - cannot use filter on leaf attributes"); - ar->ar_num = 0; - if (err) *err = TMPL_ATTR_ERROR_INVALID_ARRAY_INDEX; - goto error; + fr_strerror_printf("Invalid filter - cannot use filter on leaf attributes"); + ar->ar_num = 0; + goto error; } fr_assert(ar->ar_da != NULL); @@ -1446,7 +1454,7 @@ static fr_slen_t tmpl_attr_parse_filter(tmpl_attr_error_t *err, tmpl_attr_t *ar, tmp = FR_SBUFF(&our_name); t_rules = (tmpl_rules_t) {}; t_rules.attr = *at_rules; - t_rules.attr.namespace = ar->ar_da; + t_rules.attr.namespace = ar->ar_da; /* @todo - parent? */ p_rules = (fr_sbuff_parse_rules_t) { .terminals = &filter_terminals, @@ -1457,11 +1465,74 @@ static fr_slen_t tmpl_attr_parse_filter(tmpl_attr_error_t *err, tmpl_attr_t *ar, * Check if it's a condition. */ slen = xlat_tokenize_condition(ar, &ar->ar_cond, &tmp, &p_rules, &t_rules); - if (slen < 0) { + if (slen < 0) goto error; + + ar->ar_filter_type = TMPL_ATTR_FILTER_TYPE_CONDITION; + fr_sbuff_set(&our_name, &tmp); /* Advance name _AFTER_ doing checks */ + break; + } + + case 'n': + /* + * [n] is the last one + * + * [nope] is a reference to "nope". + */ + if (fr_sbuff_is_str(&our_name, "n]", 2)) { + ar->ar_num = NUM_LAST; + fr_sbuff_next(&our_name); + break; + } + FALL_THROUGH; + + default: + parse_tmpl: + { + fr_sbuff_t tmp = FR_SBUFF(&our_name); + ssize_t slen; + tmpl_rules_t t_rules; + fr_sbuff_parse_rules_t p_rules; + fr_sbuff_term_t const filter_terminals = FR_SBUFF_TERMS(L("]")); + + tmp = FR_SBUFF(&our_name); + t_rules = (tmpl_rules_t) {}; + t_rules.attr = *at_rules; + + /* + * Don't reset namespace, we always want to start searching from the top level of the + * dictionaries. + */ + + p_rules = (fr_sbuff_parse_rules_t) { + .terminals = &filter_terminals, + .escapes = NULL + }; + + /* + * @todo - for some reason, the tokenize_condition code allows for internal + * vs protocol vs local attributes, whereas the tmpl function only accepts + * internal ones. + */ + slen = tmpl_afrom_substr(ar, &ar->ar_tmpl, &tmp, T_BARE_WORD, &p_rules, &t_rules); + if (slen <= 0) goto error; + + if (!tmpl_is_attr(ar->ar_tmpl)) { + fr_strerror_printf("Invalid array index"); goto error; } - ar->ar_filter_type = TMPL_ATTR_FILTER_TYPE_CONDITION; + /* + * Arguably we _could_ say &User-Name["foo"] matches all user-name with value "foo", + * but that would confuse the issue for &Integer-Thing[4]. + * + * For matching therefore, we really need to have a way to define "self". + */ + if (!fr_type_numeric[tmpl_attr_tail_da(ar->ar_tmpl)->type]) { + fr_strerror_printf("Invalid data type for array index (must be numeric)"); + goto error; + } + + ar->ar_filter_type = TMPL_ATTR_FILTER_TYPE_TMPL; fr_sbuff_set(&our_name, &tmp); /* Advance name _AFTER_ doing checks */ break; } diff --git a/src/tests/unit/condition/base.txt b/src/tests/unit/condition/base.txt index 208c215945..b8066a972a 100644 --- a/src/tests/unit/condition/base.txt +++ b/src/tests/unit/condition/base.txt @@ -538,7 +538,7 @@ condition &User-Name[1001] == 'bob' match ERROR offset 12: Invalid array index '1001' (should be between 0-1000) condition &User-Name[-1] == 'bob' -match ERROR offset 12: Invalid array index '-1' (should be between 0-1000) +match ERROR offset 12: Invalid array index # # Sometimes the attribute/condition parser needs to fallback to bare words diff --git a/src/tests/unit/condition/filter.txt b/src/tests/unit/condition/filter.txt index feb22356c8..6b3f89aec9 100644 --- a/src/tests/unit/condition/filter.txt +++ b/src/tests/unit/condition/filter.txt @@ -25,7 +25,7 @@ condition &User-Name[n] == "foo" match (&User-Name[n] == "foo") condition &User-Name[-1] == "foo" -match ERROR offset 12: Invalid array index '-1' (should be between 0-1000) +match ERROR offset 12: Invalid array index # # tmpl_dcursor.c has an assert which fails if you try to use @@ -35,7 +35,7 @@ match ERROR offset 12: Invalid array index '-1' (should be between 0-1000) # # @todo - feature - allow expressions in tmpl_attr_parse_filter(() # -condition &User-Name[&User-Name == 'bar'] == "foo" +condition &User-Name[(&User-Name == 'bar')] == "foo" match ERROR offset 12: Invalid filter - cannot use filter on leaf attributes # @@ -51,7 +51,7 @@ match ERROR offset 12: Invalid filter - cannot use filter on leaf attributes # # @todo - feature - this error is misleading and wrong. # -condition &TLS-Certificate[&Common-Name == 'user@example.com'] == bar +condition &TLS-Certificate[(&Common-Name == 'user@example.com')] == bar match ERROR offset 1: Cannot use list references in condition count diff --git a/src/tests/unit/xlat/cond_base.txt b/src/tests/unit/xlat/cond_base.txt index 958e4d4b01..ea2f1d6447 100644 --- a/src/tests/unit/xlat/cond_base.txt +++ b/src/tests/unit/xlat/cond_base.txt @@ -539,7 +539,7 @@ xlat_purify &User-Name[1001] == 'bob' match ERROR offset 12: Invalid array index '1001' (should be between 0-1000) xlat_purify &User-Name[-1] == 'bob' -match ERROR offset 12: Invalid array index '-1' (should be between 0-1000) +match ERROR offset 12: Invalid array index # # attributes MUST be prefixed with '&'.