Produce better errors when we can't resolve attributes

Be more strict about only resolving attributes in the dictionary associated with the current virtual server

diff --git a/src/lib/server/cond.h b/src/lib/server/cond.h
index 245edbf7214c78d1bab3a6f03ab14c53364fa135..fb8a532bf042201e2e810829db695dd6b2d36c92 100644 (file)
--- a/src/lib/server/cond.h
+++ b/src/lib/server/cond.h
@@ -95,13 +95,18 @@ struct fr_cond_s {
        fr_cond_t               *next;
 };
 
+typedef struct {
+       fr_cond_t       *cond;
+} fr_cond_iter_t;
+
 ssize_t        fr_cond_tokenize(CONF_SECTION *cs, fr_cond_t **head, tmpl_rules_t const *rules, fr_sbuff_t *in) CC_HINT(nonnull(1,2,4));
 
 int    fr_cond_promote_types(fr_cond_t *c, fr_sbuff_t *in, fr_sbuff_marker_t *m_lhs, fr_sbuff_marker_t *m_rhs) CC_HINT(nonnull(1));
 
 ssize_t        cond_print(fr_sbuff_t *out, fr_cond_t const *c);
 
-bool fr_cond_walk(fr_cond_t *head, bool (*callback)(fr_cond_t *cond, void *uctx), void *uctx);
+fr_cond_t *fr_cond_iter_init(fr_cond_iter_t *iter, fr_cond_t *head);
+fr_cond_t *fr_cond_iter_next(fr_cond_iter_t *iter);
 
 void fr_cond_async_update(fr_cond_t *cond);
 

diff --git a/src/lib/server/cond_tokenize.c b/src/lib/server/cond_tokenize.c
index 0697bf10436706c17a8d46a232a6a1edd2903546..2445b44ae38cf0080bc0c28f7b8886a2afdcb603 100644 (file)
--- a/src/lib/server/cond_tokenize.c
+++ b/src/lib/server/cond_tokenize.c
@@ -1588,47 +1588,42 @@ ssize_t fr_cond_tokenize(CONF_SECTION *cs, fr_cond_t **head, tmpl_rules_t const
        return slen + diff;
 }
 
-/*
- *     Walk in order.
+/** Initialise a cond iterator
+ *
+ * Will return the first leaf condition node.
+ *
+ * @param[out] iter    to initialise.
+ * @param[in] head     the root of the condition structure.
+ * @return The first leaf condition node.
  */
-bool fr_cond_walk(fr_cond_t *c, bool (*callback)(fr_cond_t *cond, void *uctx), void *uctx)
+fr_cond_t *fr_cond_iter_init(fr_cond_iter_t *iter, fr_cond_t *head)
 {
-       while (c) {
-               /*
-                *      Process this one, exit on error.
-                */
-               if (!callback(c, uctx)) return false;
+       fr_cond_t *c;
 
-               switch (c->type) {
-               case COND_TYPE_INVALID:
-                       return false;
+       for (c = head; c->type == COND_TYPE_CHILD; c = c->data.child);  /* Deepest condition */
 
-               case COND_TYPE_RCODE:
-               case COND_TYPE_TMPL:
-               case COND_TYPE_MAP:
-               case COND_TYPE_AND:
-               case COND_TYPE_OR:
-               case COND_TYPE_TRUE:
-               case COND_TYPE_FALSE:
-                       break;
+       return iter->cond = c;
+}
 
-               case COND_TYPE_CHILD:
-                       /*
-                        *      Walk over the child.
-                        */
-                       if (!fr_cond_walk(c->data.child, callback, uctx)) {
-                               return false;
-                       }
-                       break;
-               }
+/** Get the next leaf condition node
+ *
+ * @param[in] iter     to iterate over.
+ * @return The next leaf condition node.
+ */
+fr_cond_t *fr_cond_iter_next(fr_cond_iter_t *iter)
+{
+       fr_cond_t *c;
 
-               /*
-                *      process the next sibling
-                */
-               c = c->next;
+       /*
+        *      Walk up the tree, maybe...
+        */
+       for (c = iter->cond; c; c = c->parent) {
+               if (!c->next) continue; /* Done with this level */
+               for (c = c->next; c->type == COND_TYPE_CHILD; c = c->data.child);       /* down we go... */
+               break;
        }
 
-       return true;
+       return iter->cond = c;
 }
 
 /** Update the condition with "is async required".

diff --git a/src/lib/server/tmpl.h b/src/lib/server/tmpl.h
index 5c1fde7f31daed413200427af2bf38b5bd02a2a6..8207356dbce1f4c8dc79fbd7a92162351467325f 100644 (file)
--- a/src/lib/server/tmpl.h
+++ b/src/lib/server/tmpl.h
@@ -873,6 +873,8 @@ void                        tmpl_set_name_shallow(tmpl_t *vpt, fr_token_t quote, char const *name, ss
 
 void                   tmpl_set_name(tmpl_t *vpt, fr_token_t quote, char const *name, ssize_t len);
 
+void                   tmpl_set_dict_def(tmpl_t *vpt, fr_dict_t const *dict);
+
 int                    tmpl_afrom_value_box(TALLOC_CTX *ctx, tmpl_t **out, fr_value_box_t *data, bool steal);
 
 void                   tmpl_attr_ref_debug(const tmpl_attr_t *ar, int idx);

diff --git a/src/lib/server/tmpl_tokenize.c b/src/lib/server/tmpl_tokenize.c
index 1ffe1b97f76c1446ce1852462e66a2012824de2a..6e19a5aec15f07eddf999707ac1eb6c88c1460ae 100644 (file)
--- a/src/lib/server/tmpl_tokenize.c
+++ b/src/lib/server/tmpl_tokenize.c
@@ -536,6 +536,16 @@ void tmpl_set_name(tmpl_t *vpt, fr_token_t quote, char const *name, ssize_t len)
        vpt->quote = quote;
 }
 
+/** Change the default dictionary in the tmpl's resolution rules
+ *
+ * @param[in] vpt      to alter.
+ * @param[in] dict     to set.
+ */
+void tmpl_set_dict_def(tmpl_t *vpt, fr_dict_t const *dict)
+{
+       vpt->rules.dict_def = dict;
+}
+
 /** Initialise a tmpl using a format string to create the name
  *
  * @param[in] vpt      to initialise.
@@ -1437,12 +1447,13 @@ static inline int tmpl_attr_afrom_attr_substr(TALLOC_CTX *ctx, tmpl_attr_error_t
                slen = fr_dict_attr_search_by_qualified_name_substr(&dict_err, &da,
                                                                    t_rules->dict_def,
                                                                    name, p_rules ? p_rules->terminals : NULL,
-                                                                   !t_rules->disallow_internal);
+                                                                   !t_rules->disallow_internal,
+                                                                   t_rules->allow_foreign);
                /*
                 *      We can't know which dictionary the
                 *      attribute will be resolved in, so the
-                *      only way of recording the parent is
-                *      by looking at the da.
+                *      only way of recording what the parent
+                *      is by looking at the da.
                 */
                if (da) our_parent = da->parent;
        /*
@@ -3054,7 +3065,8 @@ static inline CC_HINT(always_inline) int tmpl_attr_resolve(tmpl_t *vpt)
                                                         &FR_SBUFF_IN(ar->ar_unresolved,
                                                                      talloc_array_length(ar->ar_unresolved) - 1),
                                                         NULL,
-                                                        !vpt->rules.disallow_internal);
+                                                        !vpt->rules.disallow_internal,
+                                                        vpt->rules.allow_foreign);
                if (!da) return -2;     /* Can't resolve, maybe the caller can resolve later */
 
                ar->ar_type = TMPL_ATTR_TYPE_NORMAL;

diff --git a/src/lib/unlang/compile.c b/src/lib/unlang/compile.c
index 04ccc192b70787464bd897e298c38d59d419afbf..e2fefa226cccd19f580091e06facf0840b02b2f0 100644 (file)
--- a/src/lib/unlang/compile.c
+++ b/src/lib/unlang/compile.c
@@ -174,12 +174,18 @@ static inline CC_HINT(always_inline) int unlang_rules_verify(tmpl_rules_t const
 
 #define RULES_VERIFY(_rules) if (unlang_rules_verify(_rules) < 0) return NULL;
 
-static bool pass2_fixup_tmpl(TALLOC_CTX *ctx, CONF_ITEM const *ci, tmpl_t **vpt_p)
+static bool pass2_fixup_tmpl(TALLOC_CTX *ctx, tmpl_t **vpt_p, CONF_ITEM const *ci, fr_dict_t const *dict)
 {
        tmpl_t *vpt = *vpt_p;
 
        TMPL_VERIFY(vpt);
 
+       /*
+        *      We may now know the correct dictionary
+        *      where we didn't before...
+        */
+       if (!vpt->rules.dict_def) tmpl_set_dict_def(vpt, dict);
+
        /*
         *      Convert virtual &Attr-Foo to "%{Attr-Foo}"
         */
@@ -205,9 +211,9 @@ static bool pass2_fixup_tmpl(TALLOC_CTX *ctx, CONF_ITEM const *ci, tmpl_t **vpt_
        return true;
 }
 
-static bool pass2_fixup_cond_map(fr_cond_t *c, CONF_ITEM *ci)
+static bool pass2_fixup_cond_map(fr_cond_t *c, CONF_ITEM *ci, fr_dict_t const *dict)
 {
-       tmpl_t          *vpt;
+       tmpl_t  *vpt;
        map_t   *map;
 
        map = c->data.map;      /* shorter */
@@ -237,11 +243,11 @@ static bool pass2_fixup_cond_map(fr_cond_t *c, CONF_ITEM *ci)
                 *      Resolve the attribute references first
                 */
                if (tmpl_is_attr_unresolved(map->lhs)) {
-                       if (!pass2_fixup_tmpl(map, map->ci, &map->lhs)) return false;
+                       if (!pass2_fixup_tmpl(map, &map->lhs, map->ci, dict)) return false;
                }
 
                if (tmpl_is_attr_unresolved(map->rhs)) {
-                       if (!pass2_fixup_tmpl(map, map->ci, &map->rhs)) return false;
+                       if (!pass2_fixup_tmpl(map, &map->rhs, map->ci, dict)) return false;
                }
 
                c->pass2_fixup = PASS2_FIXUP_NONE;
@@ -273,11 +279,11 @@ static bool pass2_fixup_cond_map(fr_cond_t *c, CONF_ITEM *ci)
                 *      @todo - allow anything anywhere.
                 */
                if (!tmpl_is_unresolved(map->rhs)) {
-                       if (!pass2_fixup_tmpl(map, map->ci, &map->lhs)) {
+                       if (!pass2_fixup_tmpl(map, &map->lhs, map->ci, dict)) {
                                return false;
                        }
                } else {
-                       if (!pass2_fixup_tmpl(map, map->ci, &map->lhs)) {
+                       if (!pass2_fixup_tmpl(map, &map->lhs, map->ci, dict)) {
                                return false;
                        }
 
@@ -346,20 +352,20 @@ static bool pass2_fixup_cond_map(fr_cond_t *c, CONF_ITEM *ci)
                 *      forbids this.
                 */
                if (tmpl_is_attr(map->lhs)) {
-                       if (!pass2_fixup_tmpl(map, map->ci, &map->rhs)) return false;
+                       if (!pass2_fixup_tmpl(map, &map->rhs, map->ci, dict)) return false;
                } else {
-                       if (!pass2_fixup_tmpl(map, map->ci, &map->rhs)) return false;
+                       if (!pass2_fixup_tmpl(map, &map->rhs, map->ci, dict)) return false;
                }
        }
 
        if (tmpl_is_exec_unresolved(map->lhs)) {
-               if (!pass2_fixup_tmpl(map, map->ci, &map->lhs)) {
+               if (!pass2_fixup_tmpl(map, &map->lhs, map->ci, dict)) {
                        return false;
                }
        }
 
        if (tmpl_is_exec_unresolved(map->rhs)) {
-               if (!pass2_fixup_tmpl(map, map->ci, &map->rhs)) {
+               if (!pass2_fixup_tmpl(map, &map->rhs, map->ci, dict)) {
                        return false;
                }
        }
@@ -400,7 +406,7 @@ static bool pass2_fixup_cond_map(fr_cond_t *c, CONF_ITEM *ci)
 
 #ifdef HAVE_REGEX
        if (tmpl_is_regex_xlat_unresolved(map->rhs)) {
-               if (!pass2_fixup_tmpl(map, map->ci, &map->rhs)) {
+               if (!pass2_fixup_tmpl(map, &map->rhs, map->ci, dict)) {
                        return false;
                }
        }
@@ -470,50 +476,6 @@ static bool pass2_fixup_cond_map(fr_cond_t *c, CONF_ITEM *ci)
        return true;
 }
 
-static bool pass2_cond_callback(fr_cond_t *c, void *uctx)
-{
-       CONF_SECTION *cs = talloc_get_type_abort(uctx, CONF_SECTION);
-       CONF_ITEM *ci = cf_section_to_item(cs);
-
-       switch (c->type) {
-       /*
-        *      These don't get optimized.
-        */
-       case COND_TYPE_TRUE:
-       case COND_TYPE_FALSE:
-       case COND_TYPE_RCODE:
-       case COND_TYPE_AND:
-       case COND_TYPE_OR:
-               return true;
-
-       /*
-        *      Call children.
-        */
-       case COND_TYPE_CHILD:
-               return pass2_cond_callback(c->data.child, uctx);
-
-       /*
-        *      Fix up the template.
-        */
-       case COND_TYPE_TMPL:
-               fr_assert(!tmpl_is_regex_xlat_unresolved(c->data.vpt));
-               return pass2_fixup_tmpl(c, ci, &c->data.vpt);
-
-       /*
-        *      Fixup the map
-        */
-       case COND_TYPE_MAP:
-               return pass2_fixup_cond_map(c, uctx);
-
-       /*
-        *      Nothing else has pass2 fixups
-        */
-       default:
-               fr_assert(0);
-               return false;
-       }
-}
-
 static bool pass2_fixup_update_map(map_t *map, tmpl_rules_t const *rules, fr_dict_attr_t const *parent)
 {
        RULES_VERIFY(rules);
@@ -525,13 +487,13 @@ static bool pass2_fixup_update_map(map_t *map, tmpl_rules_t const *rules, fr_dic
                 *      FIXME: compile to attribute && handle
                 *      the conversion in map_to_vp().
                 */
-               if (!pass2_fixup_tmpl(map, map->ci, &map->lhs)) {
+               if (!pass2_fixup_tmpl(map, &map->lhs, map->ci, rules->dict_def)) {
                        return false;
                }
        }
 
        if (tmpl_is_exec(map->lhs)) {
-               if (!pass2_fixup_tmpl(map, map->ci, &map->lhs)) {
+               if (!pass2_fixup_tmpl(map, &map->lhs, map->ci, rules->dict_def)) {
                        return false;
                }
        }
@@ -540,7 +502,7 @@ static bool pass2_fixup_update_map(map_t *map, tmpl_rules_t const *rules, fr_dic
         *      Deal with undefined attributes now.
         */
        if (tmpl_is_attr_unresolved(map->lhs)) {
-               if (!pass2_fixup_tmpl(map, map->ci, &map->lhs)) return false;
+               if (!pass2_fixup_tmpl(map, &map->lhs, map->ci, rules->dict_def)) return false;
        }
 
        /*
@@ -563,7 +525,7 @@ static bool pass2_fixup_update_map(map_t *map, tmpl_rules_t const *rules, fr_dic
                         *      FIXME: compile to attribute && handle
                         *      the conversion in map_to_vp().
                         */
-                       if (!pass2_fixup_tmpl(map, map->ci, &map->rhs)) {
+                       if (!pass2_fixup_tmpl(map, &map->rhs, map->ci, rules->dict_def)) {
                                return false;
                        }
                }
@@ -571,11 +533,11 @@ static bool pass2_fixup_update_map(map_t *map, tmpl_rules_t const *rules, fr_dic
                fr_assert(!tmpl_is_regex_xlat_unresolved(map->rhs));
 
                if (tmpl_is_attr_unresolved(map->rhs)) {
-                       if (!pass2_fixup_tmpl(map, map->ci, &map->rhs)) return false;
+                       if (!pass2_fixup_tmpl(map, &map->rhs, map->ci, rules->dict_def)) return false;
                }
 
                if (tmpl_is_exec(map->rhs)) {
-                       if (!pass2_fixup_tmpl(map, map->ci, &map->rhs)) {
+                       if (!pass2_fixup_tmpl(map, &map->rhs, map->ci, rules->dict_def)) {
                                return false;
                        }
                }
@@ -646,7 +608,8 @@ static bool pass2_fixup_map_rhs(unlang_group_t *g, tmpl_rules_t const *rules)
         */
        if (!gext->vpt) return true;
 
-       return pass2_fixup_tmpl(fr_map_list_head(&gext->map)->ci, cf_section_to_item(g->cs), &gext->vpt);
+       return pass2_fixup_tmpl(fr_map_list_head(&gext->map)->ci, &gext->vpt,
+                               cf_section_to_item(g->cs), rules->dict_def);
 }
 
 static void unlang_dump(unlang_t *instruction, int depth)
@@ -1981,7 +1944,7 @@ static unlang_t *compile_switch(unlang_t *parent, unlang_compile_t *unlang_ctx,
         *      This is so that compile_case() can do attribute type
         *      checks / casts against us.
         */
-       if (!pass2_fixup_tmpl(g, cf_section_to_item(cs), &gext->vpt)) {
+       if (!pass2_fixup_tmpl(g, &gext->vpt, cf_section_to_item(cs), unlang_ctx->rules->dict_def)) {
                talloc_free(g);
                return NULL;
        }
@@ -2493,7 +2456,7 @@ static unlang_t *compile_if_subsection(unlang_t *parent, unlang_compile_t *unlan
        unlang_t                *c;
 
        unlang_group_t          *g;
-       unlang_cond_t   *gext;
+       unlang_cond_t           *gext;
 
        fr_cond_t               *cond;
 
@@ -2510,13 +2473,35 @@ static unlang_t *compile_if_subsection(unlang_t *parent, unlang_compile_t *unlan
                                    unlang_ops[ext->type].name);
                c = compile_empty(parent, unlang_ctx, cs, ext);
        } else {
-               /*
-                *      The condition may refer to attributes, xlats, or
-                *      Auth-Types which didn't exist when it was first
-                *      parsed.  Now that they are all defined, we need to fix
-                *      them up.
-                */
-               if (!fr_cond_walk(cond, pass2_cond_callback, cs)) return NULL;
+               fr_cond_iter_t  iter;
+               fr_cond_t       *leaf;
+
+               for (leaf = fr_cond_iter_init(&iter, cond);
+                    leaf;
+                    leaf = fr_cond_iter_next(&iter)) {
+                       switch (leaf->type) {
+                       /*
+                        *      Fix up the template.
+                        */
+                       case COND_TYPE_TMPL:
+                               fr_assert(!tmpl_is_regex_xlat_unresolved(leaf->data.vpt));
+                               if (!pass2_fixup_tmpl(leaf, &leaf->data.vpt, cf_section_to_item(cs),
+                                                     unlang_ctx->rules->dict_def)) return false;
+                               break;
+
+                       /*
+                        *      Fixup the map
+                        */
+                       case COND_TYPE_MAP:
+                               if (!pass2_fixup_cond_map(leaf, cf_section_to_item(cs),
+                                                         unlang_ctx->rules->dict_def)) return false;
+                               break;
+
+                       default:
+                               continue;
+                       }
+               }
+
                fr_cond_async_update(cond);
                c = compile_section(parent, unlang_ctx, cs, ext);
        }
@@ -2744,7 +2729,7 @@ static unlang_t *compile_load_balance_subsection(unlang_t *parent, unlang_compil
                /*
                 *      Fixup the templates
                 */
-               if (!pass2_fixup_tmpl(g, cf_section_to_item(cs), &gext->vpt)) {
+               if (!pass2_fixup_tmpl(g, &gext->vpt, cf_section_to_item(cs), unlang_ctx->rules->dict_def)) {
                        talloc_free(g);
                        return NULL;
                }
@@ -2857,7 +2842,7 @@ static unlang_t *compile_subrequest(unlang_t *parent, unlang_compile_t *unlang_c
        unlang_t                        *c;
 
        unlang_group_t                  *g;
-       unlang_subrequest_t     *gext;
+       unlang_subrequest_t             *gext;
 
        unlang_compile_t                unlang_ctx2;
 

diff --git a/src/lib/util/dict.h b/src/lib/util/dict.h
index 38a9cdc4aa14214c0f8d3a6c0503959eb3de7297..953d2e622964dd3db036e7b331bcfe72a80f8d2a 100644 (file)
--- a/src/lib/util/dict.h
+++ b/src/lib/util/dict.h
@@ -502,29 +502,30 @@ fr_dict_attr_t const      *fr_dict_vendor_da_by_num(fr_dict_attr_t const *vendor_root
 ssize_t                        fr_dict_attr_search_by_qualified_name_substr(fr_dict_attr_err_t *err, fr_dict_attr_t const **out,
                                                                     fr_dict_t const *dict_def,
                                                                     fr_sbuff_t *name, fr_sbuff_term_t const *tt,
-                                                                    bool fallback)
+                                                                    bool internal, bool foreign)
                                                                     CC_HINT(nonnull(2, 4));
 
 ssize_t                        fr_dict_attr_search_by_name_substr(fr_dict_attr_err_t *err, fr_dict_attr_t const **out,
                                                           fr_dict_t const *dict_def,
                                                           fr_sbuff_t *name, fr_sbuff_term_t const *tt,
-                                                          bool fallback)
+                                                          bool internal, bool foreign)
                                                           CC_HINT(nonnull(2, 4));
 
 ssize_t                        fr_dict_attr_search_by_qualified_oid_substr(fr_dict_attr_err_t *err, fr_dict_attr_t const **out,
                                                                    fr_dict_t const *dict_def,
                                                                    fr_sbuff_t *in, fr_sbuff_term_t const *tt,
-                                                                   bool fallback)
+                                                                   bool internal, bool foreign)
                                                                    CC_HINT(nonnull(2, 4));
 
 fr_dict_attr_t const   *fr_dict_attr_search_by_qualified_oid(fr_dict_attr_err_t *err,
-                                                             fr_dict_t const *dict_def, char const *attr, bool fallback)
+                                                             fr_dict_t const *dict_def, char const *attr,
+                                                             bool internal, bool foreign)
                                                              CC_HINT(nonnull(3));
 
 ssize_t                        fr_dict_attr_search_by_oid_substr(fr_dict_attr_err_t *err, fr_dict_attr_t const **out,
                                                          fr_dict_t const *dict_def,
                                                          fr_sbuff_t *in, fr_sbuff_term_t const *tt,
-                                                         bool fallback)
+                                                         bool internal, bool foreign)
                                                          CC_HINT(nonnull(2, 4));
 
 ssize_t                        fr_dict_attr_by_name_substr(fr_dict_attr_err_t *err, fr_dict_attr_t const **out,

diff --git a/src/lib/util/dict_util.c b/src/lib/util/dict_util.c
index 11909718ca8f32c744979acd11fd474ce245152f..7af2b06a8186f72db95c47631029f25aed5a3bb3 100644 (file)
--- a/src/lib/util/dict_util.c
+++ b/src/lib/util/dict_util.c
@@ -1902,7 +1902,7 @@ ssize_t dict_by_protocol_substr(fr_dict_attr_err_t *err,
 
        if (!dict) {
                fr_strerror_printf("Unknown protocol '%s'", root.name);
-               *out = NULL;
+               memcpy(out, &dict_def, sizeof(*out));
                return 0;
        }
 
@@ -2160,12 +2160,25 @@ typedef ssize_t (*dict_attr_resolve_func_t)(fr_dict_attr_err_t *err,
 
 /** Internal function for searching for attributes in multiple dictionaries
  *
+ * @param[out] err             Any errors that occurred searching.
+ * @param[out] out             The attribute we found.
+ * @param[in] dict_def         The default dictionary to search in.
+ * @param[in] in               string to resolve to an attribute.
+ * @param[in] tt               terminals that indicate the end of the string.
+ * @param[in] internal         Resolve the attribute in the internal dictionary.
+ * @param[in] foreign          Resolve attribute in a foreign dictionary,
+ *                             i.e. one other than dict_def.
+ * @param[in] func             to use for resolution.
+ * @return
+ *     - <=0 on error (the offset of the error).
+ *     - >0 on success.
  */
 static inline CC_HINT(always_inline)
 ssize_t dict_attr_search(fr_dict_attr_err_t *err, fr_dict_attr_t const **out,
                         fr_dict_t const *dict_def,
                         fr_sbuff_t *in, fr_sbuff_term_t const *tt,
-                        bool fallback, dict_attr_resolve_func_t func)
+                        bool internal, bool foreign,
+                        dict_attr_resolve_func_t func)
 {
        fr_dict_attr_err_t      our_err;
        fr_hash_iter_t          iter;
@@ -2174,6 +2187,14 @@ ssize_t dict_attr_search(fr_dict_attr_err_t *err, fr_dict_attr_t const **out,
        ssize_t                 slen;
        fr_sbuff_t              our_in = FR_SBUFF_NO_ADVANCE(in);
 
+       /*
+        *      Always going to fail...
+        */
+       if (unlikely(!internal && !foreign && !dict_def)) {
+               if (err) *err = FR_DICT_ATTR_EINVAL;
+               return 0;
+       }
+
        /*
         *      dict_def search in the specified dictionary
         */
@@ -2184,7 +2205,7 @@ ssize_t dict_attr_search(fr_dict_attr_err_t *err, fr_dict_attr_t const **out,
                        return fr_sbuff_set(in, &our_in);
 
                case FR_DICT_ATTR_NOTFOUND:
-                       if (!fallback) goto error;
+                       if (!internal && !foreign) goto error;
                        break;
 
                default:
@@ -2195,17 +2216,19 @@ ssize_t dict_attr_search(fr_dict_attr_err_t *err, fr_dict_attr_t const **out,
        /*
         *      Next in the internal dictionary
         */
-       slen = func(&our_err, out, fr_dict_root(dict_gctx->internal), &our_in, tt);
-       switch (our_err) {
-       case FR_DICT_ATTR_OK:
-               return fr_sbuff_set(in, &our_in);
+       if (internal) {
+               slen = func(&our_err, out, fr_dict_root(dict_gctx->internal), &our_in, tt);
+               switch (our_err) {
+               case FR_DICT_ATTR_OK:
+                       return fr_sbuff_set(in, &our_in);
 
-       case FR_DICT_ATTR_NOTFOUND:
-               if (!fallback) goto error;
-               break;
+               case FR_DICT_ATTR_NOTFOUND:
+                       if (!foreign) goto error;
+                       break;
 
-       default:
-               goto error;
+               default:
+                       goto error;
+               }
        }
 
        /*
@@ -2231,6 +2254,44 @@ ssize_t dict_attr_search(fr_dict_attr_err_t *err, fr_dict_attr_t const **out,
        }
 
 error:
+       if (our_err == FR_DICT_ATTR_NOTFOUND) {
+               fr_sbuff_marker_t       start;
+               char                    *list = NULL;
+
+               our_in = FR_SBUFF_NO_ADVANCE(in);
+               fr_sbuff_marker(&start, &our_in);
+
+               list = talloc_strdup(NULL, "");
+
+               if (dict_def) {
+                       list = talloc_strdup_append_buffer(list, fr_dict_root(dict_def)->name);
+                       list = talloc_strdup_append_buffer(list, ", ");
+               }
+               if (internal) {
+                       list = talloc_strdup_append_buffer(list, fr_dict_root(dict_gctx->internal)->name);
+                       list = talloc_strdup_append_buffer(list, ", ");
+               }
+
+               if (foreign) {
+                       for (dict = fr_hash_table_iter_init(dict_gctx->protocol_by_num, &iter);
+                            dict;
+                            dict = fr_hash_table_iter_next(dict_gctx->protocol_by_num, &iter)) {
+                               if (dict == dict_def) continue;
+                               if (dict == dict_gctx->internal) continue;
+
+                               list = talloc_strdup_append_buffer(list, fr_dict_root(dict)->name);
+                               list = talloc_strdup_append_buffer(list, ", ");
+                       }
+               }
+
+               fr_strerror_printf("Attribute '%pV' not found.  Searched in: %pV",
+                                  fr_box_strvalue_len(fr_sbuff_current(&start),
+                                                      fr_sbuff_adv_until(&our_in, SIZE_MAX, tt, '\0')),
+                                  fr_box_strvalue_len(list, talloc_array_length(list) - 3));
+
+               talloc_free(list);
+       }
+
        if (err) *err = our_err;
        *out = NULL;
 
@@ -2246,7 +2307,8 @@ static inline CC_HINT(always_inline)
 ssize_t dict_attr_search_qualified(fr_dict_attr_err_t *err, fr_dict_attr_t const **out,
                                   fr_dict_t const *dict_def,
                                   fr_sbuff_t *in, fr_sbuff_term_t const *tt,
-                                  bool fallback, dict_attr_resolve_func_t func)
+                                  bool internal, bool foreign,
+                                  dict_attr_resolve_func_t func)
 {
        fr_sbuff_t              our_in = FR_SBUFF_NO_ADVANCE(in);
        fr_dict_attr_err_t      our_err;
@@ -2277,10 +2339,10 @@ ssize_t dict_attr_search_qualified(fr_dict_attr_err_t *err, fr_dict_attr_t const
                        return 0;
                }
 
-               fallback = false;
+               internal = foreign = false;
        }
 
-       slen = dict_attr_search(&our_err, out, initial, &our_in, tt, fallback, func);
+       slen = dict_attr_search(&our_err, out, initial, &our_in, tt, internal, foreign, func);
        if (our_err != FR_DICT_ATTR_OK) goto error;
 
        return fr_sbuff_set(in, &our_in);
@@ -2303,7 +2365,8 @@ ssize_t dict_attr_search_qualified(fr_dict_attr_err_t *err, fr_dict_attr_t const
  * @param[in] dict_def         Default dictionary for non-qualified dictionaries.
  * @param[in] name             Dictionary/Attribute name.
  * @param[in] tt               Terminal strings.
- * @param[in] fallback         If true, fallback to the internal dictionary.
+ * @param[in] internal         If true, fallback to the internal dictionary.
+ * @param[in] foreign          If true, fallback to foreign dictionaries.
  * @return
  *     - <= 0 on failure.
  *     - The number of bytes of name consumed on success.
@@ -2311,9 +2374,10 @@ ssize_t dict_attr_search_qualified(fr_dict_attr_err_t *err, fr_dict_attr_t const
 ssize_t fr_dict_attr_search_by_qualified_name_substr(fr_dict_attr_err_t *err, fr_dict_attr_t const **out,
                                                     fr_dict_t const *dict_def,
                                                     fr_sbuff_t *name, fr_sbuff_term_t const *tt,
-                                                    bool fallback)
+                                                    bool internal, bool foreign)
 {
-       return dict_attr_search_qualified(err, out, dict_def, name, tt, fallback, fr_dict_attr_by_name_substr);
+       return dict_attr_search_qualified(err, out, dict_def, name, tt,
+                                         internal, foreign, fr_dict_attr_by_name_substr);
 }
 
 /** Locate a #fr_dict_attr_t by its name in the top level namespace of a dictionary
@@ -2330,7 +2394,8 @@ ssize_t fr_dict_attr_search_by_qualified_name_substr(fr_dict_attr_err_t *err, fr
  * @param[in] dict_def         Default dictionary for non-qualified dictionaries.
  * @param[in] name             Dictionary/Attribute name.
  * @param[in] tt               Terminal strings.
- * @param[in] fallback         If true, fallback to the internal dictionary.
+ * @param[in] internal         If true, fallback to the internal dictionary.
+ * @param[in] foreign          If true, fallback to foreign dictionaries.
  * @return
  *     - <= 0 on failure.
  *     - The number of bytes of name consumed on success.
@@ -2338,9 +2403,10 @@ ssize_t fr_dict_attr_search_by_qualified_name_substr(fr_dict_attr_err_t *err, fr
 ssize_t fr_dict_attr_search_by_name_substr(fr_dict_attr_err_t *err, fr_dict_attr_t const **out,
                                           fr_dict_t const *dict_def,
                                           fr_sbuff_t *name, fr_sbuff_term_t const *tt,
-                                          bool fallback)
+                                          bool internal, bool foreign)
 {
-       return dict_attr_search_qualified(err, out, dict_def, name, tt, fallback, fr_dict_attr_by_name_substr);
+       return dict_attr_search_qualified(err, out, dict_def, name, tt,
+                                         internal, foreign, fr_dict_attr_by_name_substr);
 }
 
 /** Locate a qualified #fr_dict_attr_t by a dictionary qualified OID string
@@ -2357,16 +2423,19 @@ ssize_t fr_dict_attr_search_by_name_substr(fr_dict_attr_err_t *err, fr_dict_attr
  * @param[in] dict_def         Default dictionary for non-qualified dictionaries.
  * @param[in] in               Dictionary/Attribute name.
  * @param[in] tt               Terminal strings.
- * @param[in] fallback         If true, fallback to the internal dictionary.
+ * @param[in] internal         If true, fallback to the internal dictionary.
+ * @param[in] foreign          If true, fallback to foreign dictionaries.
  * @return
  *     - <= 0 on failure.
  *     - The number of bytes of name consumed on success.
  */
 ssize_t fr_dict_attr_search_by_qualified_oid_substr(fr_dict_attr_err_t *err, fr_dict_attr_t const **out,
                                                    fr_dict_t const *dict_def,
-                                                   fr_sbuff_t *in, fr_sbuff_term_t const *tt, bool fallback)
+                                                   fr_sbuff_t *in, fr_sbuff_term_t const *tt,
+                                                   bool internal, bool foreign)
 {
-       return dict_attr_search_qualified(err, out, dict_def, in, tt, fallback, fr_dict_attr_by_oid_substr);
+       return dict_attr_search_qualified(err, out, dict_def, in, tt,
+                                         internal, foreign, fr_dict_attr_by_oid_substr);
 }
 
 /** Locate a qualified #fr_dict_attr_t by a dictionary using a non-qualified OID string
@@ -2383,16 +2452,19 @@ ssize_t fr_dict_attr_search_by_qualified_oid_substr(fr_dict_attr_err_t *err, fr_
  * @param[in] dict_def         Default dictionary for non-qualified dictionaries.
  * @param[in] in               Dictionary/Attribute name.
  * @param[in] tt               Terminal strings.
- * @param[in] fallback         If true, fallback to the internal dictionary.
+ * @param[in] internal         If true, fallback to the internal dictionary.
+ * @param[in] foreign          If true, fallback to foreign dictionaries.
  * @return
  *     - <= 0 on failure.
  *     - The number of bytes of name consumed on success.
  */
 ssize_t fr_dict_attr_search_by_oid_substr(fr_dict_attr_err_t *err, fr_dict_attr_t const **out,
                                          fr_dict_t const *dict_def,
-                                         fr_sbuff_t *in, fr_sbuff_term_t const *tt, bool fallback)
+                                         fr_sbuff_t *in, fr_sbuff_term_t const *tt,
+                                         bool internal, bool foreign)
 {
-       return dict_attr_search_qualified(err, out, dict_def, in, tt, fallback, fr_dict_attr_by_oid_substr);
+       return dict_attr_search_qualified(err, out, dict_def, in, tt,
+                                         internal, foreign, fr_dict_attr_by_oid_substr);
 }
 
 /** Locate a qualified #fr_dict_attr_t by its name and a dictionary qualifier
@@ -2401,11 +2473,13 @@ ssize_t fr_dict_attr_search_by_oid_substr(fr_dict_attr_err_t *err, fr_dict_attr_
  *                             @see fr_dict_attr_err_t.
  * @param[in] dict_def         Default dictionary for non-qualified dictionaries.
  * @param[in] name             Dictionary/Attribute name.
- * @param[in] fallback         If true, fallback to the internal dictionary.
+ * @param[in] internal         If true, fallback to the internal dictionary.
+ * @param[in] foreign          If true, fallback to foreign dictionaries.
  * @return an #fr_dict_attr_err_t value.
  */
 fr_dict_attr_t const *fr_dict_attr_search_by_qualified_oid(fr_dict_attr_err_t *err, fr_dict_t const *dict_def,
-                                                          char const *name, bool fallback)
+                                                          char const *name,
+                                                          bool internal, bool foreign)
 {
        ssize_t                 slen;
        fr_sbuff_t              our_name;
@@ -2413,7 +2487,7 @@ fr_dict_attr_t const *fr_dict_attr_search_by_qualified_oid(fr_dict_attr_err_t *e
 
        fr_sbuff_init(&our_name, name, strlen(name) + 1);
 
-       slen = fr_dict_attr_search_by_qualified_oid_substr(err, &da, dict_def, &our_name, NULL, fallback);
+       slen = fr_dict_attr_search_by_qualified_oid_substr(err, &da, dict_def, &our_name, NULL, internal, foreign);
        if (slen <= 0) return NULL;
 
        if ((size_t)slen != fr_sbuff_len(&our_name)) {

diff --git a/src/lib/util/pair_legacy.c b/src/lib/util/pair_legacy.c
index ecf2df65ae3595cdf409171d5b320f906e507c10..e9662ebc997cafd5d7da43d42a6bc69c72aba0d5 100644 (file)
--- a/src/lib/util/pair_legacy.c
+++ b/src/lib/util/pair_legacy.c
@@ -175,7 +175,7 @@ fr_pair_t *fr_pair_make(TALLOC_CTX *ctx, fr_dict_t const *dict, fr_pair_list_t *
         *      It's not found in the dictionary, so we use
         *      another method to create the attribute.
         */
-       da = fr_dict_attr_search_by_qualified_oid(NULL, dict, attrname, true);
+       da = fr_dict_attr_search_by_qualified_oid(NULL, dict, attrname, true, true);
        if (!da) {
                vp = fr_pair_make_unknown(ctx, dict, attrname, value, op);
                if (!vp) return NULL;

diff --git a/src/lib/util/sbuff.c b/src/lib/util/sbuff.c
index 28332168cd0fcf45f2c8ebd24f3fc8f54113638e..42b35361a8b17444a0f9dc7dd64dbab2928b4054 100644 (file)
--- a/src/lib/util/sbuff.c
+++ b/src/lib/util/sbuff.c
@@ -1624,12 +1624,12 @@ size_t fr_sbuff_adv_past_allowed(fr_sbuff_t *sbuff, size_t len, bool
        return total;
 }
 
-/** Wind position until we hit a character in the until set
+/** Wind position until we hit a character in the terminal set
  *
  * @param[in] sbuff            sbuff to search in.
  * @param[in] len              Maximum amount to advance by. Unconstrained if SIZE_MAX.
  * @param[in] tt               Token terminals in the encompassing grammar.
- * @param[in] escape_chr       If not '\0', ignore characters in the until set when
+ * @param[in] escape_chr       If not '\0', ignore characters in the tt set when
  *                             prefixed with this escape character.
  * @return how many bytes we advanced.
  */

diff --git a/src/modules/rlm_detail/rlm_detail.c b/src/modules/rlm_detail/rlm_detail.c
index 606d8352cab77f9c0b1933df1cbae1a10c4a7e5c..7e4ef3c519a7b792d0835fe842fde905a68b1d49 100644 (file)
--- a/src/modules/rlm_detail/rlm_detail.c
+++ b/src/modules/rlm_detail/rlm_detail.c
@@ -174,7 +174,7 @@ static int mod_instantiate(void *instance, CONF_SECTION *conf)
                        attr = cf_pair_attr(cf_item_to_pair(ci));
                        if (!attr) continue; /* pair-anoia */
 
-                       da = fr_dict_attr_search_by_qualified_oid(NULL, dict_radius, attr, false);
+                       da = fr_dict_attr_search_by_qualified_oid(NULL, dict_radius, attr, false, false);
                        if (!da) {
                                cf_log_perr(conf, "Failed resolving attribute");
                                return -1;

diff --git a/src/modules/rlm_passwd/rlm_passwd.c b/src/modules/rlm_passwd/rlm_passwd.c
index b024b8ed787eb3dff3e5e0cc0b517f2d9c305624..40bae43c4d3c7541be531b54262b5f1101956003 100644 (file)
--- a/src/modules/rlm_passwd/rlm_passwd.c
+++ b/src/modules/rlm_passwd/rlm_passwd.c
@@ -480,7 +480,7 @@ static int mod_instantiate(void *instance, CONF_SECTION *conf)
        }
 
        da = fr_dict_attr_search_by_qualified_oid(NULL, dict_freeradius,
-                                          inst->pwd_fmt->field[key_field], true);
+                                                 inst->pwd_fmt->field[key_field], true, true);
        if (!da) {
                PERROR("Unable to resolve attribute");
                release_ht(inst->ht);

diff --git a/src/modules/rlm_sql/rlm_sql.c b/src/modules/rlm_sql/rlm_sql.c
index 63717a1dec2d2011e4ccad96d397067039902267..256eeaa0ec913c1a71af2ac222c26709f9fe4b2f 100644 (file)
--- a/src/modules/rlm_sql/rlm_sql.c
+++ b/src/modules/rlm_sql/rlm_sql.c
@@ -1069,7 +1069,8 @@ static int mod_bootstrap(void *instance, CONF_SECTION *conf)
                        goto error;
                }
 
-               inst->group_da = fr_dict_attr_search_by_qualified_oid(NULL, dict_freeradius, group_attribute, false);
+               inst->group_da = fr_dict_attr_search_by_qualified_oid(NULL, dict_freeradius, group_attribute,
+                                                                    false, false);
                if (!inst->group_da) {
                        PERROR("Failed resolving group attribute");
                        goto error;

diff --git a/src/modules/rlm_sqlippool/rlm_sqlippool.c b/src/modules/rlm_sqlippool/rlm_sqlippool.c
index 70b3608645a0cdf3b4f5758cfd69b33b8e11e4fb..f689018e04077c8135b28dd5249bc01a09a578bc 100644 (file)
--- a/src/modules/rlm_sqlippool/rlm_sqlippool.c
+++ b/src/modules/rlm_sqlippool/rlm_sqlippool.c
@@ -427,7 +427,7 @@ static int mod_instantiate(void *instance, CONF_SECTION *conf)
        }
 
        inst->allocated_address_da = fr_dict_attr_search_by_qualified_oid(NULL, dict_freeradius,
-                                                                  inst->allocated_address_attr, false);
+                                                                         inst->allocated_address_attr, false, false);
        if (!inst->allocated_address_da) {
                cf_log_perr(conf, "Failed resolving attribute");
                return -1;

diff --git a/src/tests/keywords/dhcpv4.conf b/src/tests/keywords/dhcpv4.conf
new file mode 100644 (file)
index 0000000..3e3f95a
--- /dev/null
+++ b/src/tests/keywords/dhcpv4.conf
@@ -0,0 +1,58 @@
+
+modules {
+       $INCLUDE ${raddb}/mods-enabled/always
+
+       $INCLUDE ${raddb}/mods-enabled/expr
+
+       $INCLUDE ${raddb}/mods-enabled/escape
+
+       dhcpv4 {
+
+       }
+
+       delay reschedule {
+               force_reschedule = yes
+       }
+
+       delay delay_10s {
+               delay = 10
+       }
+
+       unpack {
+
+       }
+
+}
+
+policy {
+$INCLUDE policy.conf
+}
+
+instantiate {
+       #
+       #  Just check that this can be referred to as "virtual_instantiate.post-auth"
+       #
+       load-balance virtual_instantiate {
+               ok
+               ok
+       }
+}
+
+#
+#  Virtual server for the DHCPv4 protocol.
+#
+server default {
+       namespace = dhcpv4
+
+       listen {
+               type = Discover
+       }
+
+       recv Discover {
+               #
+               # Include the test file specified by the
+               # KEYWORD environment variable.
+               #
+               $INCLUDE ${keyword}/$ENV{KEYWORD}
+       }
+}

diff --git a/src/tests/keywords/policy.conf b/src/tests/keywords/policy.conf
index 7351de904be04f9a712f5d53e92392a160db6ae6..2fc6401b9f929a2a6929e7e9e5c729abf7173c25 100644 (file)
--- a/src/tests/keywords/policy.conf
+++ b/src/tests/keywords/policy.conf
@@ -59,7 +59,7 @@ success {
                }
        }
 
-       if (&parent.request.User-Name && !&parent.reply.Result-Status) {
+       if (&parent.request && !&parent.reply.Result-Status) {
                update parent.reply {
                        &Result-Status := "success"
                }
@@ -73,7 +73,7 @@ test_fail {
                &Result-Status += "Failure in test at line %(interpreter:...line)"
        }
 
-       if (&parent.request.User-Name) {
+       if (&parent.request) {
                update parent.reply {
                        &Result-Status += "Failure in test at line %(interpreter:...line)"
                }

diff --git a/src/tests/keywords/xlat-dhcpv4 b/src/tests/keywords/xlat-dhcpv4
index b18496a41df6c88c8e3ca675e758f6b9388b66ea..d87023000b17d9aae04460674a547c964a49a34d 100644 (file)
--- a/src/tests/keywords/xlat-dhcpv4
+++ b/src/tests/keywords/xlat-dhcpv4
@@ -1,5 +1,6 @@
 #
 # PRE: update
+# PROTOCOL: dhcpv4
 #
 
 update control {

diff --git a/src/tests/keywords/xlat-dhcpv4.attrs b/src/tests/keywords/xlat-dhcpv4.attrs
new file mode 100644 (file)
index 0000000..6065f63
--- /dev/null
+++ b/src/tests/keywords/xlat-dhcpv4.attrs
@@ -0,0 +1,4 @@
+Packet-Type = Discover
+
+Result-Status == success
+Packet-Type == Offer