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 6cef7ae08faea6b840cffea027755efb92859e04..0c10ff4ee378aa6ce938d90fb35f49bf13cf6e23 100644 (file)
--- 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 d384c2d6979b9d8a545ccfcc2619dc68f7cf17af..45130d94d08c0e4172256f9bca4438a8008ed757 100644 (file)
--- 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 44835d900ec9cbaa52fab9023ae6e28c9035c187..b7b6255892ce33a76e9c459e2b7eb460a1c768f1 100644 (file)
--- 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 1d51d5a96038db6a4d3c4321255ae37213b7c579..682c834fe6a43bc20a68c0fd44c9470a3c29120d 100644 (file)
--- 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 7dee0170b26b5a4161da798bf9ff594dbf24082b..a06aa1acea55081f079f0698055c6c803de68348 100644 (file)
--- 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 768de083be99b45c69eb244d56784e3caf4d8193..578067c264912d5ccd7065dee6f94212e01371f6 100644 (file)
--- 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