From: Alan T. DeKok Date: Fri, 11 Apr 2025 17:50:27 +0000 (-0400) Subject: better errors X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7415940fa2be65be7e9f2cd2e7ea1c9127ecc150;p=thirdparty%2Ffreeradius-server.git better errors print which brace is missing ')' or '}' double-check and fix error locations --- diff --git a/src/lib/unlang/xlat_tokenize.c b/src/lib/unlang/xlat_tokenize.c index 6cef7ae08fa..0c10ff4ee37 100644 --- a/src/lib/unlang/xlat_tokenize.c +++ b/src/lib/unlang/xlat_tokenize.c @@ -389,7 +389,7 @@ static CC_HINT(nonnull) int xlat_tokenize_function_args(xlat_exp_head_t *head, f fr_sbuff_next(in); if (!fr_sbuff_next_if_char(in, ')')) { - fr_strerror_const("Missing ')'"); + fr_strerror_const("Missing closing brace ')'"); return -1; } @@ -458,6 +458,8 @@ static CC_HINT(nonnull) int xlat_tokenize_function_args(xlat_exp_head_t *head, f node->call.input_type = func->input_type; } + fr_sbuff_marker_release(&m_s); + (void) fr_sbuff_next(in); /* skip the '(' */ /* @@ -476,7 +478,7 @@ static CC_HINT(nonnull) int xlat_tokenize_function_args(xlat_exp_head_t *head, f */ if (xlat_tokenize_argv(node, &node->call.args, in, func, &xlat_function_arg_rules, t_rules, false) < 0) { -error: + error: talloc_free(node); return -1; } @@ -484,7 +486,7 @@ error: xlat_flags_merge(&node->flags, &node->call.args->flags); if (!fr_sbuff_next_if_char(in, ')')) { - fr_strerror_const("Missing closing brace"); + fr_strerror_const("Missing closing brace ')'"); goto error; } @@ -644,7 +646,7 @@ static CC_HINT(nonnull(1,2)) int xlat_tokenize_expansion(xlat_exp_head_t *head, } if (!fr_sbuff_extend(in)) { - fr_strerror_const("Missing closing brace"); + fr_strerror_const("Missing closing brace '}'"); fr_sbuff_marker_release(&m_s); return -1; } @@ -675,7 +677,7 @@ static CC_HINT(nonnull(1,2)) int xlat_tokenize_expansion(xlat_exp_head_t *head, } if (!fr_sbuff_is_char(in, '}')) { - fr_strerror_const("Missing closing brace"); + fr_strerror_const("Missing closing brace '}'"); return -1; } @@ -716,7 +718,7 @@ check_for_attr: * e.g. '%{myfirstxlat' */ if (!fr_sbuff_extend(in)) { - fr_strerror_const("Missing closing brace"); + fr_strerror_const("Missing closing brace '}'"); fr_sbuff_marker_release(&m_s); return -1; } @@ -755,7 +757,7 @@ check_for_attr: if (xlat_tokenize_attribute(head, in, &attr_p_rules, t_rules) < 0) return -1; if (!fr_sbuff_next_if_char(in, '}')) { - fr_strerror_const("Missing closing brace"); + fr_strerror_const("Missing closing brace '}'"); return -1; } @@ -1343,6 +1345,8 @@ fr_slen_t xlat_tokenize_word(TALLOC_CTX *ctx, xlat_exp_t **out, fr_sbuff_t *in, switch (quote) { case T_SOLIDUS_QUOTED_STRING: fr_strerror_const("Unexpected regular expression"); + fr_sbuff_advance(in, -1); /* to the actual '/' */ + our_in = FR_SBUFF(in); FR_SBUFF_ERROR_RETURN(&our_in); default: @@ -1350,6 +1354,30 @@ fr_slen_t xlat_tokenize_word(TALLOC_CTX *ctx, xlat_exp_t **out, fr_sbuff_t *in, FR_SBUFF_ERROR_RETURN(&our_in); case T_BARE_WORD: +#if 0 + /* + * Avoid a bounce through tmpls for %{...} and %func() + */ + if (fr_sbuff_is_char(&our_in, '%')) { + xlat_exp_head_t *head = NULL; + + MEM(head = xlat_exp_head_alloc(ctx)); + + slen = xlat_tokenize_input(head, &our_in, p_rules, t_rules); + if (slen <= 0) { + talloc_free(head); + FR_SBUFF_ERROR_RETURN(&our_in); + } + + fr_assert(fr_dlist_num_elements(&head->dlist) == 1); + + node = fr_dlist_pop_head(&head->dlist); + fr_assert(node != NULL); + (void) talloc_steal(ctx, node); + talloc_free(head); + goto done; + } +#endif break; case T_DOUBLE_QUOTED_STRING: @@ -1390,7 +1418,7 @@ fr_slen_t xlat_tokenize_word(TALLOC_CTX *ctx, xlat_exp_t **out, fr_sbuff_t *in, */ slen = tmpl_afrom_substr(node, &node->vpt, &our_in, quote, p_rules, t_rules); if (slen <= 0) { - fr_sbuff_advance(&our_in, -slen); /* point to the correct offset */ + fr_sbuff_advance(&our_in, -slen - 1); /* point to the correct offset */ error: talloc_free(node); @@ -1585,7 +1613,7 @@ fr_slen_t xlat_tokenize_argv(TALLOC_CTX *ctx, xlat_exp_head_t **out, fr_sbuff_t /* * skip spaces at the beginning as we don't want them to become a whitespace literal. */ - fr_sbuff_adv_past_whitespace(in, SIZE_MAX, NULL); + fr_sbuff_adv_past_whitespace(&our_in, SIZE_MAX, NULL); fr_sbuff_marker(&m, &our_in); argc = 1; @@ -1594,7 +1622,6 @@ fr_slen_t xlat_tokenize_argv(TALLOC_CTX *ctx, xlat_exp_head_t **out, fr_sbuff_t fr_token_t quote; size_t len; - fr_sbuff_set(&m, &our_in); /* Record start of argument */ arg_t_rules.literals_safe_for = arg->safe_for; /* @@ -1602,6 +1629,8 @@ fr_slen_t xlat_tokenize_argv(TALLOC_CTX *ctx, xlat_exp_head_t **out, fr_sbuff_t */ if (!spaces) fr_sbuff_adv_past_whitespace(&our_in, SIZE_MAX, NULL); + fr_sbuff_set(&m, &our_in); /* Record start of argument */ + if (!spaces && !xlat_func_bare_words) { quote = T_BARE_WORD; } else { @@ -1682,6 +1711,7 @@ fr_slen_t xlat_tokenize_argv(TALLOC_CTX *ctx, xlat_exp_head_t **out, fr_sbuff_t if (arg->type == FR_TYPE_NULL) { fr_strerror_printf("Too many arguments, expected %zu, got %d", (size_t) (arg - arg_start), argc); + fr_sbuff_set(&our_in, &m); goto error; } @@ -1787,7 +1817,6 @@ fr_slen_t xlat_tokenize(TALLOC_CTX *ctx, xlat_exp_head_t **out, fr_sbuff_t *in, fr_sbuff_t our_in = FR_SBUFF(in); xlat_exp_head_t *head; - MEM(head = xlat_exp_head_alloc(ctx)); fr_strerror_clear(); /* Clear error buffer */ diff --git a/src/tests/unit/condition/base.txt b/src/tests/unit/condition/base.txt index d384c2d6979..45130d94d08 100644 --- a/src/tests/unit/condition/base.txt +++ b/src/tests/unit/condition/base.txt @@ -14,10 +14,10 @@ match (request.User-Name == reply.User-Name) # All IP address literals should be parsed as prefixes condition ("foo\ -match ERROR offset 1: Unterminated string +match ERROR offset 2: Unterminated string condition ("foo -match ERROR offset 1: Unterminated string +match ERROR offset 2: Unterminated string condition () match ERROR offset 1: Empty expressions are invalid diff --git a/src/tests/unit/xlat/alternation.txt b/src/tests/unit/xlat/alternation.txt index 44835d900ec..b7b6255892c 100644 --- a/src/tests/unit/xlat/alternation.txt +++ b/src/tests/unit/xlat/alternation.txt @@ -49,13 +49,13 @@ match ERROR offset 18: Empty attribute reference # Discuss - Not sure the offset/message is correct here, but not sure if we can determine the correct offset either xlat %{%{User-Name} || 'foo' -match ERROR offset 23: Missing closing brace +match ERROR offset 23: Missing closing brace '}' xlat %{%{User-Name}:} match %{%{User-Name}:} xlat %{%{User-Name}:-"foo"} -match ERROR offset 15: Old style alternation of %{...:-...} is no longer supported +match ERROR offset 14: Old style alternation of %{...:-...} is no longer supported count match 35 diff --git a/src/tests/unit/xlat/base.txt b/src/tests/unit/xlat/base.txt index 1d51d5a9603..682c834fe6a 100644 --- a/src/tests/unit/xlat/base.txt +++ b/src/tests/unit/xlat/base.txt @@ -59,7 +59,7 @@ match ERROR offset 2: Unexpected text after attribute reference #match ERROR offset 3: Unresolved attributes not allowed in expansions here xlat %{3 -match ERROR offset 3: Missing closing brace +match ERROR offset 3: Missing closing brace '}' # # Tests for xlat expansion @@ -68,11 +68,14 @@ xlat %{Tunnel-Password} match %{Tunnel-Password} xlat %test_no_args('foo') -match ERROR offset 19: Too many arguments, expected 0, got 1 +match ERROR offset 14: Too many arguments, expected 0, got 1 xlat %test('bar') match %test('bar') +xlat %test() +match ERROR offset 6: Missing required arg 1 + xlat %{Tunnel-Password} match %{Tunnel-Password} @@ -125,7 +128,7 @@ xlat %length() match %length() xlat %length( -match ERROR offset 8: Missing closing brace +match ERROR offset 8: Missing closing brace ')' xlat %length(1 + 2 match ERROR offset 13: Missing ')' after argument 1 @@ -152,7 +155,7 @@ match ERROR offset 20: Attribute 'bar' not found. Searched in: RADIUS, internal # Empty and malformed expansions # xlat %{ -match ERROR offset 2: Missing closing brace +match ERROR offset 2: Missing closing brace '}' xlat %{} match ERROR offset 2: Empty expressions are invalid @@ -188,21 +191,20 @@ xlat %{foo bar} match ERROR offset 2: Attribute 'foo' not found. Searched in: RADIUS, internal: Unresolved attributes are not allowed here xlat %test( -match ERROR offset 6: Missing closing brace +match ERROR offset 6: Missing closing brace ')' xlat %test(%{User-Name) -match ERROR offset 17: Missing closing brace +match ERROR offset 17: Missing closing brace '}' -# Discuss - Not sure the offset/message is correct here, but not sure if we can determine the correct offset either xlat %test(%{User-Name} match ERROR offset 18: Missing ')' after argument 1 xlat %{myfirstxlat -match ERROR offset 13: Missing closing brace +match ERROR offset 13: Missing closing brace '}' # Issue seen in the wild that caused an SEGV during pass2 xlat %{%{control.IP-Pool.Name}:%{reply.IP-Pool.Range} -match ERROR offset 48: Missing closing brace +match ERROR offset 48: Missing closing brace '}' # # API to split xlat strings into argv-style arguments. @@ -235,7 +237,7 @@ match [0]{ /bin/sh }, [1]{ "foo bar" }, [2]{ %{User-Name} %{Filter-Id} } # to xlat_tokenize_expansion() # xlat_argv /bin/sh "foo bar" "%{User-Name} %{Filter-Id" -match ERROR offset 45: Unexpected text after attribute reference +match ERROR offset 44: Unexpected text after attribute reference # and text immediately after a variable expansion xlat_argv echo hello %{Filter-Id}:1234 world @@ -263,7 +265,7 @@ match %rpad(User-Name, 5, 'x') # so the "offset" is wrong. It also doesn't say *which* string is wrong. We'll fix that later. # xlat %rpad(User-Name, 'foo', 'x') -match ERROR offset 16: Invalid argument 2 - Failed parsing string as type 'uint64' +match ERROR offset 17: Invalid argument 2 - Failed parsing string as type 'uint64' # # Argument quoting @@ -295,4 +297,4 @@ xlat %md5('arg"') match %md5(0x61726722) count -match 157 +match 159 diff --git a/src/tests/unit/xlat/cond_base.txt b/src/tests/unit/xlat/cond_base.txt index 7dee0170b26..a06aa1acea5 100644 --- a/src/tests/unit/xlat/cond_base.txt +++ b/src/tests/unit/xlat/cond_base.txt @@ -9,10 +9,10 @@ proto-dictionary radius # All IP address literals should be parsed as prefixes xlat_purify ("foo\ -match ERROR offset 1: Unterminated string +match ERROR offset 2: Unterminated string xlat_purify ("foo -match ERROR offset 1: Unterminated string +match ERROR offset 2: Unterminated string xlat_purify () match ERROR offset 1: Empty expressions are invalid diff --git a/src/tests/xlat/expr.txt b/src/tests/xlat/expr.txt index 768de083be9..578067c2649 100644 --- a/src/tests/xlat/expr.txt +++ b/src/tests/xlat/expr.txt @@ -5,7 +5,7 @@ # @todo - this offset is way wrong # xlat_expr %file.exists() -match ERROR offset 25 'Missing required arg 1' +match ERROR offset 11 'Missing required arg 1' # # Only one argument