Switch to using tmpl_from_attr_substr in xlat_tokenize_expansion instead of duplicati...
authorArran Cudbard-Bell <a.cudbardb@freeradius.org>
Wed, 19 Nov 2014 17:05:53 +0000 (12:05 -0500)
committerArran Cudbard-Bell <a.cudbardb@freeradius.org>
Wed, 19 Nov 2014 19:23:09 +0000 (14:23 -0500)
* fixup radius_request_name/radius_list_name to have signature/behaviour consistent with other substr type parsing functions.
* pass through 'allow_unknown' to tmpl_from_attr_substr which determines whether an unknown attribute produces a parse failure or not.

src/include/tmpl.h
src/main/map.c
src/main/pair.c
src/main/tmpl.c
src/main/xlat.c
src/modules/rlm_cache/rlm_cache.c
src/modules/rlm_exec/rlm_exec.c
src/modules/rlm_expr/rlm_expr.c
src/modules/rlm_rest/rest.c
src/tests/unit/xlat.txt

index c86e9c5..d42dda4 100644 (file)
@@ -191,11 +191,11 @@ VALUE_PAIR                **radius_list(REQUEST *request, pair_lists_t list);
 
 TALLOC_CTX             *radius_list_ctx(REQUEST *request, pair_lists_t list_name);
 
-pair_lists_t           radius_list_name(char const **name, pair_lists_t unknown);
+size_t                 radius_list_name(pair_lists_t *out, char const *name, pair_lists_t default_list);
 
 int                    radius_request(REQUEST **request, request_refs_t name);
 
-request_refs_t         radius_request_name(char const **name, request_refs_t unknown);
+size_t                 radius_request_name(request_refs_t *out, char const *name, request_refs_t unknown);
 
 
 /* Template manipulation and execution */
@@ -213,15 +213,15 @@ value_pair_tmpl_t *tmpl_alloc(TALLOC_CTX *ctx, tmpl_type_t type, char const *nam
  *
  */
 ssize_t                        tmpl_from_attr_substr(value_pair_tmpl_t *vpt, char const *name,
-                                             request_refs_t request_def, pair_lists_t list_def);
+                                             request_refs_t request_def, pair_lists_t list_def, bool allow_unknown);
 
 ssize_t                        tmpl_from_attr_str(value_pair_tmpl_t *vpt, char const *name,
                                           request_refs_t request_def,
-                                          pair_lists_t list_def);
+                                          pair_lists_t list_def, bool allow_unknown);
 
 ssize_t                        tmpl_afrom_attr_str(TALLOC_CTX *ctx, value_pair_tmpl_t **out, char const *name,
                                            request_refs_t request_def,
-                                           pair_lists_t list_def);
+                                           pair_lists_t list_def, bool allow_unknown);
 
 /*
  *     Parses any type of string into a template
index d9d4a7f..0e87569 100644 (file)
@@ -185,7 +185,7 @@ int map_afrom_cp(TALLOC_CTX *ctx, value_pair_map_t **out, CONF_PAIR *cp,
        /*
         *      LHS must always be an attribute reference.
         */
-       slen = tmpl_afrom_attr_str(ctx, &map->lhs, attr, dst_request_def, dst_list_def);
+       slen = tmpl_afrom_attr_str(ctx, &map->lhs, attr, dst_request_def, dst_list_def, true);
        if (slen <= 0) {
                char *spaces, *text;
 
@@ -297,10 +297,9 @@ int map_afrom_cs(value_pair_map_t **out, CONF_SECTION *cs,
 
        cs_list = p = cf_section_name2(cs);
        if (cs_list) {
-               request_def = radius_request_name(&p, REQUEST_CURRENT);
+               p += radius_request_name(&request_def, p, REQUEST_CURRENT);
                if (request_def == REQUEST_UNKNOWN) {
-                       cf_log_err(ci, "Default request specified "
-                                  "in mapping section is invalid");
+                       cf_log_err(ci, "Default request specified in mapping section is invalid");
                        return -1;
                }
 
index 18ba6ad..f2e0201 100644 (file)
@@ -766,7 +766,7 @@ int radius_get_vp(VALUE_PAIR **out, REQUEST *request, char const *name)
 
        *out = NULL;
 
-       if (tmpl_from_attr_str(&vpt, name, REQUEST_CURRENT, PAIR_LIST_REQUEST) <= 0) {
+       if (tmpl_from_attr_str(&vpt, name, REQUEST_CURRENT, PAIR_LIST_REQUEST, false) <= 0) {
                return -4;
        }
 
@@ -792,7 +792,7 @@ int radius_copy_vp(TALLOC_CTX *ctx, VALUE_PAIR **out, REQUEST *request, char con
 
        *out = NULL;
 
-       if (tmpl_from_attr_str(&vpt, name, REQUEST_CURRENT, PAIR_LIST_REQUEST) <= 0) {
+       if (tmpl_from_attr_str(&vpt, name, REQUEST_CURRENT, PAIR_LIST_REQUEST, false) <= 0) {
                return -4;
        }
 
index e043f74..1a741a0 100644 (file)
@@ -59,106 +59,86 @@ const FR_NAME_NUMBER request_refs[] = {
 
 /** Resolve attribute name to a list.
  *
- * Check the name string for qualifiers that specify a list and return
- * an pair_lists_t value for that list. This value may be passed to
+ * Check the name string for qualifiers that specify a list and write
+ * a pair_lists_t value for that list to out. This value may be passed to
  * radius_list, along with the current request, to get a pointer to the
  * actual list in the request.
  *
- * If qualifiers were consumed, write a new pointer into name to the
- * char after the last qualifier to be consumed.
+ * If we're sure we've definitely found a list qualifier token delimiter
+ * but the string doesn't match a list qualifier, return 0 and write
+ * PAIR_LIST_UNKNOWN to out.
  *
  * radius_list_name should be called before passing a name string that
  * may contain qualifiers to dict_attrbyname.
  *
  * @see dict_attrbyname
  *
- * @param[in,out] name of attribute.
- * @param[in] default_list the list to return if no qualifiers were found.
- * @return PAIR_LIST_UNKOWN if qualifiers couldn't be resolved to a list.
+ * @param[out] out Where to write the list qualifier.
+ * @param[in] name String containing list qualifiers to parse.
+ * @param[in] def the list to return if no qualifiers were found.
+ * @return 0 if no valid list qualifier could be found, else the number of
+ *     bytes consumed.
  */
-pair_lists_t radius_list_name(char const **name, pair_lists_t default_list)
+size_t radius_list_name(pair_lists_t *out, char const *name, pair_lists_t def)
 {
-       char const *p = *name;
+       char const *p = name;
        char const *q;
-       pair_lists_t output;
 
-       /* This should never be a NULL pointer or zero length string */
-       rad_assert(name && *name);
+       /* This should never be a NULL pointer */
+       rad_assert(name);
 
        /*
-        *      Unfortunately, ':' isn't a definitive separator for
-        *      the list name.  We may have numeric tags, too.
+        *      Try and determine the end of the token
         */
-       q = strchr(p, ':');
-       if (q) {
-               /*
-                *      Check for tagged attributes.  They have
-                *      "name:tag", where tag is a decimal number.
-                *      Valid tags are invalid attributes, so that's
-                *      OK.
-                *
-                *      Also allow "name:tag[#]" as a tag.
-                *
-                *      However, "request:" is allowed, too, and
-                *      shouldn't be interpreted as a tag.
-                *
-                *      We do this check first rather than just
-                *      looking up the request name, because this
-                *      check is cheap, and looking up the request
-                *      name is expensive.
-                */
-               if (isdigit((int) q[1])) {
-                       char const *d = q + 1;
+       for (q = p; dict_attr_allowed_chars[(uint8_t) *q]; q++);
 
-                       while (isdigit((int) *d)) {
-                               d++;
-                       }
+       switch (*q) {
+       /*
+        *      It's a bareword made up entirely of dictionary chars
+        *      check and see if it's a list qualifier, and if it's
+        *      not, return the def and say we couldn't parse
+        *      anything.
+        */
+       case '\0':
+               *out = fr_substr2int(pair_lists, p, PAIR_LIST_UNKNOWN, (q - p));
+               if (*out != PAIR_LIST_UNKNOWN) return q - p;
+               *out = def;
+               return 0;
+
+       /*
+        *      It may be a list qualifier delimiter. Because of tags
+        *      We need to check that it doesn't look like a tag suffix.
+        *      We do this by looking at the chars between ':' and the
+        *      next token delimiter, and seeing if they're all digits.
+        */
+       case ':':
+       {
+               char const *d = q + 1;
+
+               if (isdigit((int) *d)) {
+                       while (isdigit((int) *d)) d++;
 
                        /*
-                        *      Return the DEFAULT list as supplied by
-                        *      the caller.  This is usually
-                        *      PAIRLIST_REQUEST.
+                        *      Char after the number string
+                        *      was a token delimiter, so this is a
+                        *      tag, not a list qualifier.
                         */
-                       if (!*d || (*d == '[')) {
-                               return default_list;
+                       if (!dict_attr_allowed_chars[(uint8_t) *d]) {
+                               *out = def;
+                               return 0;
                        }
                }
 
-               /*
-                *      If the first part is a list name, then treat
-                *      it as a list.  This means that we CANNOT have
-                *      an attribute which is named "request",
-                *      "reply", etc.  Allowing a tagged attribute
-                *      "request:3" would just be insane.
-                */
-               output = fr_substr2int(pair_lists, p, PAIR_LIST_UNKNOWN, (q - p));
-               if (output != PAIR_LIST_UNKNOWN) {
-                       *name = (q + 1);        /* Consume the list and delimiter */
-                       return output;
-               }
+               *out = fr_substr2int(pair_lists, p, PAIR_LIST_UNKNOWN, (q - p));
+               if (*out == PAIR_LIST_UNKNOWN) return 0;
 
-               /*
-                *      It's not a known list, say so.
-                */
-               return PAIR_LIST_UNKNOWN;
+               return (q + 1) - name; /* Consume the list and delimiter */
        }
 
-       /*
-        *      The input string may be just a list name,
-        *      e.g. "request".  Check for that.
-        */
-       q = (p + strlen(p));
-       output = fr_substr2int(pair_lists, p, PAIR_LIST_UNKNOWN, (q - p));
-       if (output != PAIR_LIST_UNKNOWN) {
-               *name = q;
-               return output;
+       default:
+               *out = def;
+               return 0;
        }
-
-       /*
-        *      It's just an attribute name.  Return the default list
-        *      as supplied by the caller.
-        */
-       return default_list;
 }
 
 /** Resolve attribute pair_lists_t value to an attribute list.
@@ -308,47 +288,46 @@ TALLOC_CTX *radius_list_ctx(REQUEST *request, pair_lists_t list_name)
 
 /** Resolve attribute name to a request.
  *
- * Check the name string for qualifiers that reference a parent request and
- * write the pointer to this request to 'request'.
+ * Check the name string for qualifiers that reference a parent request.
  *
- * If qualifiers were consumed, write a new pointer into name to the
- * char after the last qualifier to be consumed.
+ * If we find a string that matches a request, then return the number of
+ * chars we consumed.
  *
- * radius_ref_request should be called before radius_list_name.
+ * If we find a string that looks like a request qualifier but isn't,
+ * return 0 and set out to REQUEST_UNKNOWN.
+ *
+ * If we can't find a string that looks like a request qualifier, set out
+ * to def, and return 0.
  *
  * @see radius_list_name
- * @param[in,out] name of attribute.
+ * @param[out] out request ref.
+ * @param[in] name of attribute.
  * @param[in] def default request ref to return if no request qualifier is present.
- * @return one of the REQUEST_* definitions or REQUEST_UNKOWN
+ * @return 0 if no valid request qualifier could be found, else the number of
+ *     bytes consumed.
  */
-request_refs_t radius_request_name(char const **name, request_refs_t def)
+size_t radius_request_name(request_refs_t *out, char const *name, request_refs_t def)
 {
-       char *p;
-       int request;
-
-       p = strchr(*name, '.');
-       if (!p) {
-               return def;
-       }
+       char const *p, *q;
 
+       p = name;
        /*
-        *      We may get passed "127.0.0.1".
+        *      Try and determine the end of the token
         */
-       request = fr_substr2int(request_refs, *name, REQUEST_UNKNOWN, p - *name);
+       for (q = p; dict_attr_allowed_chars[(uint8_t) *q] && (*q != '.') && (*q != '-'); q++);
 
        /*
-        *      If we get a valid name, skip it.
+        *      First token delimiter wasn't a '.'
         */
-       if (request != REQUEST_UNKNOWN) {
-               *name = p + 1;
-               return request;
+       if (*q != '.') {
+               *out = def;
+               return 0;
        }
 
-       /*
-        *      Otherwise leave it alone, and return the caller's
-        *      default.
-        */
-       return def;
+       *out = fr_substr2int(request_refs, name, REQUEST_UNKNOWN, q - p);
+       if (*out == REQUEST_UNKNOWN) return 0;
+
+       return (q + 1) - p;
 }
 
 /** Resolve request to a request.
@@ -721,12 +700,13 @@ value_pair_tmpl_t *tmpl_alloc(TALLOC_CTX *ctx, tmpl_type_t type, char const *nam
  * @param[in] name of attribute including qualifiers.
  * @param[in] request_def The default request to insert unqualified attributes into.
  * @param[in] list_def The default list to insert unqualified attributes into.
+ * @param[in] allow_unknown If true, we don't generate a parse error on unknown
+ *     attributes, and instead set type to TMPL_TYPE_ATTR_UNKNOWN.
  * @return <= 0 on error (offset as negative integer), > 0 on success (number of bytes parsed)
  */
 ssize_t tmpl_from_attr_substr(value_pair_tmpl_t *vpt, char const *name,
-                             request_refs_t request_def, pair_lists_t list_def)
+                             request_refs_t request_def, pair_lists_t list_def, bool allow_unknown)
 {
-       bool force_attr = false;
        char const *p;
        long num;
        char *q;
@@ -738,18 +718,19 @@ ssize_t tmpl_from_attr_substr(value_pair_tmpl_t *vpt, char const *name,
        memset(&attr, 0, sizeof(attr));
 
        p = name;
-       if (*p == '&') {
-               force_attr = true;
-               p++;
-       }
 
-       attr.request = radius_request_name(&p, request_def);
+       if (*p == '&') p++;
+
+       p += radius_request_name(&attr.request, p, request_def);
        if (attr.request == REQUEST_UNKNOWN) {
                fr_strerror_printf("Invalid request qualifier");
                return -(p - name);
        }
 
-       attr.list = radius_list_name(&p, list_def);
+       /*
+        *      Finding a list qualifier is optional
+        */
+       p += radius_list_name(&attr.list, p, list_def);
        if (attr.list == PAIR_LIST_UNKNOWN) {
                fr_strerror_printf("Invalid list qualifier");
                return -(p - name);
@@ -776,14 +757,13 @@ ssize_t tmpl_from_attr_substr(value_pair_tmpl_t *vpt, char const *name,
                /*
                 *      Can't parse it as an attribute, it must be a literal string.
                 */
-               if (!force_attr) {
-                       fr_strerror_printf("Should be re-parsed as bare word (shouldn't see me)");
+               if (!allow_unknown) {
+                       fr_strerror_printf("Unknown attribute");
                        return -(p - name);
                }
 
-
                /*
-                *      Copy the name to a field for later evaluation
+                *      Copy the name to a field for later resolution
                 */
                type = TMPL_TYPE_ATTR_UNKNOWN;
                for (q = attr.unknown.name; dict_attr_allowed_chars[(int) *p]; *q++ = *p++) {
@@ -823,7 +803,18 @@ skip_tag:
        if (*p == '[') {
                p++;
 
-               if (*p != '*') {
+               switch (*p) {
+               case '#':
+                       attr.num = NUM_COUNT;
+                       p++;
+                       break;
+
+               case '*':
+                       attr.num = NUM_ALL;
+                       p++;
+                       break;
+
+               default:
                        num = strtol(p, &q, 10);
                        if (p == q) {
                                fr_strerror_printf("Array index is not an integer");
@@ -836,9 +827,7 @@ skip_tag:
                        }
                        attr.num = num;
                        p = q;
-               } else {
-                       attr.num = NUM_ALL;
-                       p++;
+                       break;
                }
 
                if (*p != ']') {
@@ -880,14 +869,16 @@ finish:
  * @param[in] name attribute name including qualifiers.
  * @param[in] request_def The default request to insert unqualified attributes into.
  * @param[in] list_def The default list to insert unqualified attributes into.
+ * @param[in] allow_unknown If true, we don't generate a parse error on unknown
+ *     attributes, and instead set type to TMPL_TYPE_ATTR_UNKNOWN.
  * @return <= 0 on error (offset as negative integer), > 0 on success (number of bytes parsed)
  */
 ssize_t tmpl_from_attr_str(value_pair_tmpl_t *vpt, char const *name, request_refs_t request_def,
-                          pair_lists_t list_def)
+                          pair_lists_t list_def, bool allow_unknown)
 {
        ssize_t slen;
 
-       slen = tmpl_from_attr_substr(vpt, name, request_def, list_def);
+       slen = tmpl_from_attr_substr(vpt, name, request_def, list_def, allow_unknown);
        if (slen <= 0) return slen;
        if (name[slen] != '\0') {
                fr_strerror_printf("Unexpected text after attribute name");
@@ -910,17 +901,19 @@ ssize_t tmpl_from_attr_str(value_pair_tmpl_t *vpt, char const *name, request_ref
  * @param[in] request_def The default request to insert unqualified
  *     attributes into.
  * @param[in] list_def The default list to insert unqualified attributes into.
+ * @param[in] allow_unknown If true, we don't generate a parse error on unknown
+ *     attributes, and instead set type to TMPL_TYPE_ATTR_UNKNOWN.
  * @return <= 0 on error (offset as negative integer), > 0 on success (number of bytes parsed)
  */
 ssize_t tmpl_afrom_attr_str(TALLOC_CTX *ctx, value_pair_tmpl_t **out, char const *name,
-                           request_refs_t request_def, pair_lists_t list_def)
+                           request_refs_t request_def, pair_lists_t list_def, bool allow_unknown)
 {
        ssize_t slen;
        value_pair_tmpl_t *vpt;
 
        MEM(vpt = talloc(ctx, value_pair_tmpl_t)); /* tmpl_from_attr_substr zeros it */
 
-       slen = tmpl_from_attr_substr(vpt, name, request_def, list_def);
+       slen = tmpl_from_attr_substr(vpt, name, request_def, list_def, allow_unknown);
        if (slen <= 0) {
                tmpl_free(&vpt);
                return slen;
@@ -1199,7 +1192,7 @@ ssize_t tmpl_afrom_str(TALLOC_CTX *ctx, value_pair_tmpl_t **out, char const *nam
                 *      If we can parse it as an attribute, it's an attribute.
                 *      Otherwise, treat it as a literal.
                 */
-               slen = tmpl_afrom_attr_str(ctx, &vpt, name, request_def, list_def);
+               slen = tmpl_afrom_attr_str(ctx, &vpt, name, request_def, list_def, (name[0] == '&'));
                if ((name[0] == '&') && (slen <= 0)) return slen;
                if (slen > 0) break;
                /* FALL-THROUGH */
index 88fb4de..6096376 100644 (file)
@@ -318,7 +318,7 @@ static ssize_t xlat_debug_attr(UNUSED void *instance, REQUEST *request, char con
 
        while (isspace((int) *fmt)) fmt++;
 
-       if (tmpl_from_attr_str(&vpt, fmt, REQUEST_CURRENT, PAIR_LIST_REQUEST) <= 0) {
+       if (tmpl_from_attr_str(&vpt, fmt, REQUEST_CURRENT, PAIR_LIST_REQUEST, false) <= 0) {
                RDEBUG("%s", fr_strerror());
                return -1;
        }
@@ -800,7 +800,7 @@ void xlat_free(void)
 static ssize_t xlat_tokenize_expansion(TALLOC_CTX *ctx, char *fmt, xlat_exp_t **head,
                                       char const **error);
 static ssize_t xlat_tokenize_literal(TALLOC_CTX *ctx, char *fmt, xlat_exp_t **head,
-                                    int brace, char const **error);
+                                    bool brace, char const **error);
 static size_t xlat_process(char **out, REQUEST *request, xlat_exp_t const * const head,
                           RADIUS_ESCAPE_STRING escape, void *escape_ctx);
 
@@ -879,7 +879,7 @@ static ssize_t xlat_tokenize_expansion(TALLOC_CTX *ctx, char *fmt, xlat_exp_t **
 {
        ssize_t slen;
        char *p, *q, *brace;
-       char const *attrname;
+       bool is_attr = false;
        xlat_exp_t *node;
 
        rad_assert(fmt[0] == '%');
@@ -888,13 +888,11 @@ static ssize_t xlat_tokenize_expansion(TALLOC_CTX *ctx, char *fmt, xlat_exp_t **
        /*
         *      %{%{...}:-bar}
         */
-       if ((fmt[2] == '%') && (fmt[3] == '{')) {
-               return xlat_tokenize_alternation(ctx, fmt, head, error);
-       }
+       if ((fmt[2] == '%') && (fmt[3] == '{')) return xlat_tokenize_alternation(ctx, fmt, head, error);
 
        XLAT_DEBUG("EXPANSION <-- %s", fmt);
        node = talloc_zero(ctx, xlat_exp_t);
-       attrname = node->fmt = fmt + 2;
+       node->fmt = fmt + 2;
        node->len = 0;
 
 #ifdef HAVE_REGEX
@@ -928,250 +926,125 @@ static ssize_t xlat_tokenize_expansion(TALLOC_CTX *ctx, char *fmt, xlat_exp_t **
         *      %{request:Tunnel-Password:1[#]}
         *      %{mod:foo}
         */
-        brace = NULL;
-       for (p = fmt + 2; *p != '\0'; p++) {
-               if (*p == ':') break;
+       brace = NULL;
 
-               if (isspace((int) *p)) break;
+       /*
+        *      This is for efficiency, so we don't search for an xlat,
+        *      when what's being referenced is obviously an attribute.
+        */
+       p = fmt + 2;
+       for (q = p; *q != '\0'; q++) {
+               if (*q == ':') break;
 
-               if (*p == '[') break;
+               if (isspace((int) *q)) break;
+
+               if (*q == '[') {
+                       is_attr = true;
+                       continue;
+               }
 
-               if (*p == '}') break;
+               if (*q == '}') break;
        }
 
-       if (*p != ':') p = NULL;
+       /*
+        *      Check for empty expressions %{}
+        */
+       if ((*q == '}') && (q == p)) {
+               *error = "Empty expression is invalid";
+               return -(q - fmt);
+       }
 
        /*
         *      Might be a module name reference.
+        *
+        *      If it's not, it's an attribute or parse error.
         */
-       if (p) {
-               *p = '\0';
-
-               /*
-                *      %{mod:foo}
-                */
+       if (*q == ':') {
+               *q = '\0';
                node->xlat = xlat_find(node->fmt);
                if (node->xlat) {
+                       /*
+                        *      %{mod:foo}
+                        */
                        node->type = XLAT_MODULE;
 
-                       XLAT_DEBUG("MOD <-- %s ... %s", node->fmt, p + 1);
+                       p = q + 1;
+                       XLAT_DEBUG("MOD <-- %s ... %s", node->fmt, p);
 
-                       slen = xlat_tokenize_literal(node, p + 1, &node->child, true, error);
+                       slen = xlat_tokenize_literal(node, p, &node->child, true, error);
                        if (slen <= 0) {
                                talloc_free(node);
                                return slen - (p - fmt);
                        }
-                       p += slen + 1;
+                       p += slen;
 
                        *head = node;
                        rad_assert(node->next == NULL);
+
                        return p - fmt;
                }
+               *q = ':';       /* Avoids a strdup */
+       }
 
+       /*
+        *      The first token ends with:
+        *      - '[' - Which is an attribute index, so it must be an attribute.
+        *      - '}' - The end of the expansion, which means it was a bareword.
+        */
+       slen = tmpl_from_attr_substr(&node->attr, p, REQUEST_CURRENT, PAIR_LIST_REQUEST, true);
+       if (slen <= 0) {
                /*
-                *      Modules can have '}' in their RHS, so we
-                *      didn't check for that until now.
-                *
-                *      As of now, node->fmt MUST be a reference to an
-                *      attribute, however complicated.  So it MUST have a closing brace.
-                */
-               brace = strchr(p + 1, '}');
-               if (!brace) goto no_brace;
-               *brace = '\0';
-
-               /*
-                *      %{User-Name}
-                *      %{User-Name[1]}
-                *      %{Tunnel-Password:1}
-                *      %{request:Tunnel-Password:1}
-                *
-                *      <sigh>  The syntax is fairly poor.
-                */
-               XLAT_DEBUG("Looking for list in '%s'", attrname);
-
-               /*
-                *      Not a module.  Has to be an attribute
-                *      reference.
-                *
-                *      As of v3, we've removed %{request: ..>} as
-                *      internally registered xlats.
+                *      If the parse error occurred before the ':'
+                *      then the error is changed to 'Unknown module',
+                *      as it was more likely to be a bad module name,
+                *      than a request qualifier.
                 */
-               *p = ':';
-               node->attr.tmpl_request = radius_request_name(&attrname, REQUEST_CURRENT);
-               rad_assert(node->attr.tmpl_request != REQUEST_UNKNOWN);
-
-               node->attr.tmpl_list = radius_list_name(&attrname, PAIR_LIST_REQUEST);
-               if (node->attr.tmpl_list == PAIR_LIST_UNKNOWN) {
-                       talloc_free(node);
+               if ((*q == ':') && ((p + (slen * -1)) < q)) {
                        *error = "Unknown module";
-                       return -2;
-               }
-
-               /*
-                *      Check for a trailing tag.
-                */
-               p = strchr(attrname, ':');
-               if (p) *p = '\0';
-
-       } else {
-               brace = strchr(attrname, '}');
-               if (!brace) {
-               no_brace:
-                       talloc_free(node);
-                       *error = "No matching closing brace";
-                       return -1;      /* second character of format string */
+               } else {
+                       *error = fr_strerror();
                }
-               *brace = '\0';
-
-               node->attr.tmpl_request = REQUEST_CURRENT;
-               node->attr.tmpl_list = PAIR_LIST_REQUEST;
-       }
-
-       *brace = '\0';
-
-       XLAT_DEBUG("Looking for attribute name in %s", attrname);
-
-       /*
-        *      Allow for an array reference.  They come AFTER the
-        *      tag, if the tag exists.  Otherwise, they come after
-        *      the attribute name.
-        */
-       if (p) {
-               q = strchr(p + 1, '[');
-       } else {
-               q = strchr(attrname, '[');
-       }
-       if (q) *(q++) = '\0';
-
-       if (!*attrname) {
-               talloc_free(node);
-               *error = "Empty expression is invalid";
-               return -(attrname - fmt);
+               return slen - (p - fmt);
        }
 
        /*
-        *      It's either an attribute name, or a Tunnel-Password:TAG
-        *      with the ':' already set to NULL.
+        *      Might be a virtual XLAT attribute
         */
-       node->attr.tmpl_da = dict_attrbyname(attrname);
-       if (!node->attr.tmpl_da) {
-               /*
-                *      Foreach.  Maybe other stuff, too.
-                */
-               node->xlat = xlat_find(attrname);
+       if (node->attr.type == TMPL_TYPE_ATTR_UNKNOWN) {
+               node->xlat = xlat_find(node->attr.tmpl_unknown_name);
                if (node->xlat) {
                        node->type = XLAT_VIRTUAL;
-                       node->fmt = attrname;
+                       node->fmt = node->attr.tmpl_unknown_name;
 
                        XLAT_DEBUG("VIRTUAL <-- %s", node->fmt);
                        *head = node;
                        rad_assert(node->next == NULL);
-                       brace++;
-                       return brace - fmt;
+                       q++;
+                       return q - fmt;
                }
 
                talloc_free(node);
                *error = "Unknown attribute";
-               return -(attrname - fmt);
+               return -(node->fmt - fmt);
        }
 
-       /*
-        *      Parse the tag.
-        */
-       if (p) {
-               unsigned long tag;
-               char *end;
-
-               if (!node->attr.tmpl_da->flags.has_tag) {
-                       talloc_free(node);
-                       *error = "Attribute cannot have a tag";
-                       return - (p - fmt);
-               }
-
-               tag = strtoul(p + 1, &end, 10);
-               p++;
-
-               if (tag == ULONG_MAX) {
-                       talloc_free(node);
-                       *error = "Invalid tag value";
-                       return - (p - fmt);
-               }
-
-               node->attr.tmpl_tag = tag;
-               p = end;
-
-               if (*p) {
-                       talloc_free(node);
-                       *error = "Unexpected text after tag";
-                       return - (p - fmt);
-               }
-
-       } else {
-               node->attr.tmpl_tag = TAG_ANY;
-               /* leave p alone */
-       }
-
-       /*
-        *      Check for array reference
-        */
-       if (q) {
-               unsigned long num;
-               char *end;
-
-               p = q;
-               if (*p== '#') {
-                       node->attr.tmpl_num = NUM_COUNT;
-                       p++;
-
-               } else if (*p == '*') {
-                       node->attr.tmpl_num = NUM_ALL;
-                       p++;
-
-               } else if (isdigit((int) *p)) {
-                       num = strtoul(p, &end, 10);
-                       if (num > 1000) {
-                               talloc_free(node);
-                               *error = "Invalid array index";
-                               return - (p - fmt);
-                       }
-                       p = end;
-                       node->attr.tmpl_num = num;
-
-               } else {
-                       talloc_free(node);
-                       *error = "Invalid array index";
-                       return - (p - fmt);
-               }
-
-               if (*p != ']') {
-                       talloc_free(node);
-                       *error = "Expected ']'";
-                       return - (p - fmt);
-               }
-
-               p++;
-               if (*p) {
-                       talloc_free(node);
-                       *error = "Unexpected text after array reference";
-                       return - (p - fmt);
-               }
-       } else {
-               node->attr.tmpl_num = NUM_ANY;
-       }
-
-       rad_assert(!p || (p == brace));
-
        node->type = XLAT_ATTRIBUTE;
-       p = brace + 1;
-
+       p += slen;
+       if (*p != '}') {
+               talloc_free(node);
+               *error = "No matching closing brace";
+               return -1;      /* second character of format string */
+       }
+       p++;
        *head = node;
        rad_assert(node->next == NULL);
+
        return p - fmt;
 }
 
 
 static ssize_t xlat_tokenize_literal(TALLOC_CTX *ctx, char *fmt, xlat_exp_t **head,
-                                    int brace, char const **error)
+                                    bool brace, char const **error)
 {
        char *p;
        xlat_exp_t *node;
index 36f7910..f4f4c8c 100644 (file)
@@ -484,7 +484,11 @@ static ssize_t cache_xlat(void *instance, REQUEST *request,
        size_t                  len;
        int                     ret = 0;
 
-       list = radius_list_name(&p, PAIR_LIST_REQUEST);
+       p += radius_list_name(&list, p, PAIR_LIST_REQUEST);
+       if (list == PAIR_LIST_UNKNOWN) {
+               REDEBUG("Unknown list qualifier in \"%s\"", fmt);
+               return -1;
+       }
 
        target = dict_attrbyname(p);
        if (!target) {
@@ -514,11 +518,6 @@ static ssize_t cache_xlat(void *instance, REQUEST *request,
                vps = c->control;
                break;
 
-       case PAIR_LIST_UNKNOWN:
-               PTHREAD_MUTEX_UNLOCK(&inst->cache_mutex);
-               REDEBUG("Unknown list qualifier in \"%s\"", fmt);
-               return -1;
-
        default:
                PTHREAD_MUTEX_UNLOCK(&inst->cache_mutex);
                REDEBUG("Unsupported list \"%s\"",
index 49452ed..84f9d2e 100644 (file)
@@ -220,7 +220,7 @@ static int mod_instantiate(CONF_SECTION *conf, void *instance)
 
        if (inst->input) {
                p = inst->input;
-               inst->input_list = radius_list_name(&p, PAIR_LIST_UNKNOWN);
+               p += radius_list_name(&inst->input_list, p, PAIR_LIST_UNKNOWN);
                if ((inst->input_list == PAIR_LIST_UNKNOWN) || (*p != '\0')) {
                        cf_log_err_cs(conf, "Invalid input list '%s'", inst->input);
                        return -1;
@@ -229,7 +229,7 @@ static int mod_instantiate(CONF_SECTION *conf, void *instance)
 
        if (inst->output) {
                p = inst->output;
-               inst->output_list = radius_list_name(&p, PAIR_LIST_UNKNOWN);
+               p += radius_list_name(&inst->output_list, p, PAIR_LIST_UNKNOWN);
                if ((inst->output_list == PAIR_LIST_UNKNOWN) || (*p != '\0')) {
                        cf_log_err_cs(conf, "Invalid output list '%s'", inst->output);
                        return -1;
@@ -240,8 +240,7 @@ static int mod_instantiate(CONF_SECTION *conf, void *instance)
         *      Sanity check the config.  If we're told to NOT wait,
         *      then the output pairs must not be defined.
         */
-       if (!inst->wait &&
-           (inst->output != NULL)) {
+       if (!inst->wait && (inst->output != NULL)) {
                cf_log_err_cs(conf, "Cannot read output pairs if wait = no");
                return -1;
        }
index 4f1014d..25ce454 100644 (file)
@@ -221,10 +221,11 @@ static bool get_number(REQUEST *request, char const **string, int64_t *answer)
                VALUE_PAIR *vp;
                value_pair_tmpl_t vpt;
 
-               slen = tmpl_from_attr_substr(&vpt, p, REQUEST_CURRENT, PAIR_LIST_REQUEST);
+               p += 1;
+
+               slen = tmpl_from_attr_substr(&vpt, p, REQUEST_CURRENT, PAIR_LIST_REQUEST, false);
                if (slen < 0) {
-                       RDEBUG("Failed parsing attribute name '%s': %s",
-                              p, fr_strerror());
+                       RDEBUG("Failed parsing attribute name '%s': %s", p, fr_strerror());
                        return false;
                }
 
@@ -1144,7 +1145,7 @@ static ssize_t pairs_xlat(UNUSED void *instance, UNUSED REQUEST *request,
 
        VALUE_PAIR *vp;
 
-       if (tmpl_from_attr_str(&vpt, fmt, REQUEST_CURRENT, PAIR_LIST_REQUEST) <= 0) {
+       if (tmpl_from_attr_str(&vpt, fmt, REQUEST_CURRENT, PAIR_LIST_REQUEST, false) <= 0) {
                REDEBUG("%s", fr_strerror());
                return -1;
        }
index e074e1e..bc93efb 100644 (file)
@@ -1007,7 +1007,7 @@ static int rest_decode_post(UNUSED rlm_rest_t *instance, UNUSED rlm_rest_section
                 *  the string after the list qualifier.
                 */
                attribute = name;
-               request_name = radius_request_name(&attribute, REQUEST_CURRENT);
+               attribute += radius_request_name(&request_name, attribute, REQUEST_CURRENT);
                if (request_name == REQUEST_UNKNOWN) {
                        RWDEBUG("Invalid request qualifier, skipping");
 
@@ -1024,10 +1024,9 @@ static int rest_decode_post(UNUSED rlm_rest_t *instance, UNUSED rlm_rest_section
                        continue;
                }
 
-               list_name = radius_list_name(&attribute, PAIR_LIST_REPLY);
+               attribute += radius_list_name(&list_name, attribute, PAIR_LIST_REPLY);
                if (list_name == PAIR_LIST_UNKNOWN) {
                        RWDEBUG("Invalid list qualifier, skipping");
-
                        curl_free(name);
 
                        continue;
@@ -1294,7 +1293,7 @@ static int json_pairmake(rlm_rest_t *instance, UNUSED rlm_rest_section_t *sectio
                 */
                RDEBUG2("Parsing attribute \"%s\"", name);
 
-               if (tmpl_from_attr_str(&dst, name, REQUEST_CURRENT, PAIR_LIST_REPLY) <= 0) {
+               if (tmpl_from_attr_str(&dst, name, REQUEST_CURRENT, PAIR_LIST_REPLY, false) <= 0) {
                        RWDEBUG("Failed parsing attribute: %s, skipping...", fr_strerror());
                        continue;
                }
index e66d96a..1e82e57 100644 (file)
@@ -102,7 +102,7 @@ xlat %{}
 data ERROR offset 2 'Empty expression is invalid'
 
 xlat %{ }
-data ERROR offset 2 'Unknown attribute'
+data ERROR offset 2 'Invalid attribute name'
 
 xlat %{%{User-Name}:-}
 data %{%{User-Name}:-}