From: Alan T. DeKok Date: Sun, 26 Jan 2025 19:17:12 +0000 (-0500) Subject: complain on empty conditions X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4a3d9366277e8bee2d97cd25a65dfa3b9385497d;p=thirdparty%2Ffreeradius-server.git complain on empty conditions update error messages for spelling mistake batten down the rules for parsing enum names --- diff --git a/src/lib/server/tmpl_tokenize.c b/src/lib/server/tmpl_tokenize.c index 630ec3b92f5..3f37d7aa37d 100644 --- a/src/lib/server/tmpl_tokenize.c +++ b/src/lib/server/tmpl_tokenize.c @@ -3114,6 +3114,17 @@ static ssize_t tmpl_afrom_enum(TALLOC_CTX *ctx, tmpl_t **out, fr_sbuff_t *in, if (tmpl_require_enum_prefix) return 0; if (!t_rules->enumv) return 0; + + } else if (t_rules->enumv && + ((t_rules->enumv->type == FR_TYPE_IPV6_ADDR) || + ((t_rules->enumv->type == FR_TYPE_IPV6_PREFIX)))) { + + /* + * We can't have enumerated names for IPv6 addresses. + * + * @todo - allow them ONLY if the RHS string is a valid enum name. + */ + return 0; } vpt = tmpl_alloc_null(ctx); @@ -3279,6 +3290,15 @@ fr_slen_t tmpl_afrom_substr(TALLOC_CTX *ctx, tmpl_t **out, return tmpl_afrom_value_substr(ctx, out, in, quote, &my_t_rules, true, p_rules); } + /* + * Prefer enum names to IPv6 addresses. + */ + if (t_rules->enumv && fr_sbuff_is_str_literal(&our_in, "::")) { + slen = tmpl_afrom_enum(ctx, out, &our_in, p_rules, t_rules); + if (slen > 0) goto done_bareword; + fr_assert(!*out); + } + /* * See if it's a boolean value */ @@ -3366,8 +3386,10 @@ fr_slen_t tmpl_afrom_substr(TALLOC_CTX *ctx, tmpl_t **out, * We can't parse it as anything, that's an error. */ if (tmpl_require_enum_prefix) { - fr_strerror_const("Failed parsing input"); - FR_SBUFF_ERROR_RETURN(&our_in); + if (!fr_sbuff_is_str_literal(&our_in, "::")) { + fr_strerror_const("Failed parsing input"); + FR_SBUFF_ERROR_RETURN(&our_in); + } } /* diff --git a/src/lib/unlang/xlat_expr.c b/src/lib/unlang/xlat_expr.c index b2a03af5614..73994683121 100644 --- a/src/lib/unlang/xlat_expr.c +++ b/src/lib/unlang/xlat_expr.c @@ -2179,7 +2179,7 @@ static fr_slen_t tokenize_unary(xlat_exp_head_t *head, xlat_exp_t **out, fr_sbuf unary->flags = func->flags; unary->flags.impure_func = !func->flags.pure; - if (tokenize_field(unary->call.args, &node, &our_in, p_rules, t_rules, bracket_rules, out_c, (c == '!')) < 0) { + if (tokenize_field(unary->call.args, &node, &our_in, p_rules, t_rules, bracket_rules, out_c, (c == '!')) <= 0) { talloc_free(unary); FR_SBUFF_ERROR_RETURN(&our_in); } @@ -2460,7 +2460,18 @@ static fr_slen_t tokenize_field(xlat_exp_head_t *head, xlat_exp_t **out, fr_sbuf } our_t_rules.cast = FR_TYPE_NULL; - our_t_rules.enumv = NULL; +// our_t_rules.enumv = NULL; + + /* + * As a special case, we allow + * + * &reply = "foo = bar" + * + * and then we don't parse the RHS as any enum. + */ + if ( our_t_rules.enumv && !fr_type_is_leaf(our_t_rules.enumv->type)) { + our_t_rules.enumv = enumv = NULL; + } /* * If we still have '(', then recurse for other expressions @@ -2476,11 +2487,17 @@ static fr_slen_t tokenize_field(xlat_exp_head_t *head, xlat_exp_t **out, fr_sbuf our_t_rules.cast = FR_TYPE_NULL; our_t_rules.enumv = NULL; + fr_sbuff_skip_whitespace(&our_in); + if (fr_sbuff_is_char(&our_in, ')')) { + fr_strerror_printf("Empty expressions are invalid."); + FR_SBUFF_ERROR_RETURN(&our_in); + } + /* * No input rules means "ignore external terminal sequences, as we're expecting a ')' as * our terminal sequence. */ - if (tokenize_expression(head, &node, &our_in, bracket_rules, &our_t_rules, T_INVALID, bracket_rules, NULL, cond) < 0) { + if (tokenize_expression(head, &node, &our_in, bracket_rules, &our_t_rules, T_INVALID, bracket_rules, NULL, cond) <= 0) { FR_SBUFF_ERROR_RETURN(&our_in); } @@ -2567,7 +2584,8 @@ static fr_slen_t tokenize_field(xlat_exp_head_t *head, xlat_exp_t **out, fr_sbuf * our_t_rules, and will try to parse any data there as * of the correct type. */ - if (tmpl_afrom_substr(node, &vpt, &our_in, quote, p_rules, &our_t_rules) < 0) { + slen = tmpl_afrom_substr(node, &vpt, &our_in, quote, p_rules, &our_t_rules); + if ((slen < 0) || ((slen == 0) && (quote == T_BARE_WORD))) { error: FR_SBUFF_ERROR_RETURN(&our_in); } @@ -2626,17 +2644,15 @@ static fr_slen_t tokenize_field(xlat_exp_head_t *head, xlat_exp_t **out, fr_sbuf fr_sbuff_skip_whitespace(&our_in); -#if 0 /* * A bare word which is NOT a known attribute. That's an error. */ - if (tmpl_is_data_unresolved(vpt)) { + if (tmpl_require_enum_prefix && tmpl_is_data_unresolved(vpt)) { fr_assert(quote == T_BARE_WORD); fr_strerror_const("Failed parsing input"); fr_sbuff_set(&our_in, &opand_m); goto error; } -#endif /* * The tmpl has a cast, and it's the same as the explicit cast we were given, we can sometimes @@ -2711,10 +2727,12 @@ static fr_slen_t tokenize_field(xlat_exp_head_t *head, xlat_exp_t **out, fr_sbuf cast_type = FR_TYPE_NULL; } + } else if (tmpl_is_attr_unresolved(vpt)) { + fr_assert(t_rules->attr.allow_unresolved); + } else if (tmpl_is_data_unresolved(vpt)) { - /* - * A bare word which is NOT a known attribute. That's an error. - */ + fr_assert(!tmpl_require_enum_prefix); + fr_assert(quote == T_BARE_WORD); fr_strerror_const("Failed parsing input"); fr_sbuff_set(&our_in, &opand_m); @@ -2886,7 +2904,7 @@ static fr_slen_t tokenize_expression(xlat_exp_head_t *head, xlat_exp_t **out, fr * Get the LHS of the operation. */ slen = tokenize_unary(head, &lhs, &our_in, p_rules, t_rules, bracket_rules, &c, cond); - if (slen < 0) FR_SBUFF_ERROR_RETURN(&our_in); + if (slen <= 0) FR_SBUFF_ERROR_RETURN(&our_in); if (slen == 0) { fr_assert(lhs == NULL); @@ -3015,9 +3033,25 @@ redo: slen = tokenize_regex_rhs(head, &rhs, &our_in, t_rules, bracket_rules); } else { - slen = tokenize_expression(head, &rhs, &our_in, p_rules, t_rules, op, bracket_rules, input_rules, cond); + tmpl_rules_t our_t_rules = *t_rules; + + /* + * The terminal rules for expressions includes "-" and "+", both of which are allowed in + * enum names. If we pass the enumv down to the next function, it will see + * "Access-Accept", and then only parse "Access". Which is wrong. + * + * For now, if we _don't_ have tmpl_require_enum_prefix, then we don't pass the enumv, + * and we somehow skip the entire enum name. The name is then resolved later by + * something... + */ + if (tmpl_require_enum_prefix && (lhs->type == XLAT_TMPL) && tmpl_is_attr(lhs->vpt) && + fr_sbuff_is_str_literal(&our_in, "::")) { + our_t_rules.enumv = tmpl_attr_tail_da(lhs->vpt); + } + + slen = tokenize_expression(head, &rhs, &our_in, p_rules, &our_t_rules, op, bracket_rules, input_rules, cond); } - if (slen < 0) { + if (slen <= 0) { talloc_free(lhs); FR_SBUFF_ERROR_RETURN(&our_in); } @@ -3175,7 +3209,7 @@ static fr_slen_t xlat_tokenize_expression_internal(TALLOC_CTX *ctx, xlat_exp_hea talloc_free(bracket_rules); talloc_free(terminal_rules); - if (slen < 0) { + if (slen <= 0) { talloc_free(head); FR_SBUFF_ERROR_RETURN(in); } @@ -3190,7 +3224,11 @@ static fr_slen_t xlat_tokenize_expression_internal(TALLOC_CTX *ctx, xlat_exp_hea * error. */ if ((node->type == XLAT_TMPL) && tmpl_is_data_unresolved(node->vpt)) { - fr_strerror_const("Unexpected text - attribute names must prefixed with '&'"); + if (!tmpl_require_enum_prefix) { + fr_strerror_const("Unexpected text - attribute names must be prefixed with '&'"); + } else { + fr_strerror_const("Unknown attribute"); + } return -1; } diff --git a/src/tests/unit/condition/base.txt b/src/tests/unit/condition/base.txt index 5904d27c64f..a78ff32e751 100644 --- a/src/tests/unit/condition/base.txt +++ b/src/tests/unit/condition/base.txt @@ -18,7 +18,7 @@ condition ("foo match ERROR offset 2: Unterminated string condition () -match ERROR offset 2: No operand found. Expected &ref, literal, 'quoted literal', "%{expansion}", or enum value +match ERROR offset 2: Empty expressions are invalid. condition (!) match ERROR offset 3: No operand found. Expected &ref, literal, 'quoted literal', "%{expansion}", or enum value @@ -386,7 +386,7 @@ condition ('a') match 'a' condition (a) -match ERROR offset 1: Unexpected text - attribute names must prefixed with '&' +match ERROR offset 1: Unexpected text - attribute names must be prefixed with '&' # # Module return codes are OK @@ -678,6 +678,11 @@ match ((&User-Name == "bob") && ((&User-Password == "bob") || &EAP-Message)) condition (&User-Name == """bob "quoted" """) match (&User-Name == "bob \"quoted\" ") +# +# And enums inside of a sub-expression. +# +condition !(&Service-Type == ::Framed-User) +match !(&Service-Type == ::Framed-User) count -match 305 +match 307 diff --git a/src/tests/unit/xlat/base.txt b/src/tests/unit/xlat/base.txt index 95a4cdba138..276407f870a 100644 --- a/src/tests/unit/xlat/base.txt +++ b/src/tests/unit/xlat/base.txt @@ -182,7 +182,7 @@ xlat %{\n} match ERROR offset 3: No operand found. Expected &ref, literal, 'quoted literal', "%{expansion}", or enum value xlat %{foo } -match ERROR offset 7: Unexpected text - attribute names must prefixed with '&' +match ERROR offset 7: Unexpected text - attribute names must be prefixed with '&' xlat %{foo bar} match ERROR offset 7: Invalid operator @@ -191,7 +191,7 @@ xlat %test( match ERROR offset 7: Missing closing brace xlat %test(%{User-Name) -match ERROR offset 18: Unexpected text - attribute names must prefixed with '&' +match ERROR offset 18: Unexpected text - attribute names must be prefixed with '&' # Discuss - Not sure the offset/message is correct here, but not sure if we can determine the correct offset either xlat %test(%{User-Name} diff --git a/src/tests/unit/xlat/cond_base.txt b/src/tests/unit/xlat/cond_base.txt index 4177ae67ce1..39503b183fb 100644 --- a/src/tests/unit/xlat/cond_base.txt +++ b/src/tests/unit/xlat/cond_base.txt @@ -15,7 +15,7 @@ xlat_purify ("foo match ERROR offset 2: Unterminated string xlat_purify () -match ERROR offset 2: No operand found. Expected &ref, literal, 'quoted literal', "%{expansion}", or enum value +match ERROR offset 2: Empty expressions are invalid. xlat_purify (!) match ERROR offset 3: No operand found. Expected &ref, literal, 'quoted literal', "%{expansion}", or enum value