map_cast_from_hex() does not produce error messages
[freeradius.git] / src / main / modcall.c
index ba4984a..b3dd3f0 100644 (file)
@@ -68,14 +68,13 @@ typedef struct {
        enum {
                GROUPTYPE_SIMPLE = 0,
                GROUPTYPE_REDUNDANT,
-               GROUPTYPE_APPEND,
                GROUPTYPE_COUNT
        } grouptype;                            /* after mc */
        modcallable             *children;
        modcallable             *tail;          /* of the children list */
        CONF_SECTION            *cs;
-       value_pair_map_t        *map;           /* update */
-       value_pair_tmpl_t       *vpt;           /* switch */
+       vp_map_t        *map;           /* update */
+       vp_tmpl_t       *vpt;           /* switch */
        fr_cond_t               *cond;          /* if/elsif */
        bool                    done_pass2;
 } modgroup;
@@ -173,8 +172,6 @@ const FR_NAME_NUMBER mod_rcode_table[] = {
 };
 
 
-static char const *group_name[];
-
 /*
  *     Compile action && rcode for later use.
  */
@@ -187,12 +184,6 @@ static int compile_action(modcallable *c, CONF_PAIR *cp)
        value = cf_pair_value(cp);
        if (!value) return 0;
 
-       if (c->type != MOD_SINGLE) {
-               ERROR("%s[%d] Invalid return code assigment inside of a %s section",
-                     cf_pair_filename(cp), cf_pair_lineno(cp), group_name[c->type]);
-               return 0;
-       }
-
        if (!strcasecmp(value, "return"))
                action = MOD_ACTION_RETURN;
 
@@ -292,9 +283,9 @@ static rlm_rcode_t CC_HINT(nonnull) call_modsingle(rlm_components_t component, m
        blocked = (request->master_state == REQUEST_STOP_PROCESSING);
        if (blocked) return RLM_MODULE_NOOP;
 
-       RDEBUG3("modsingle[%s]: calling %s (%s) for request %d",
+       RDEBUG3("modsingle[%s]: calling %s (%s)",
                comp2str[component], sp->modinst->name,
-               sp->modinst->entry->name, request->number);
+               sp->modinst->entry->name);
        request->log.indent = 0;
 
        if (sp->modinst->force) {
@@ -318,19 +309,19 @@ static rlm_rcode_t CC_HINT(nonnull) call_modsingle(rlm_components_t component, m
         */
        blocked = (request->master_state == REQUEST_STOP_PROCESSING);
        if (blocked) {
-               RWARN("Module %s became unblocked for request %u", sp->modinst->entry->name, request->number);
+               RWARN("Module %s became unblocked", sp->modinst->entry->name);
        }
 
  fail:
        request->log.indent = indent;
-       RDEBUG3("modsingle[%s]: returned from %s (%s) for request %d",
+       RDEBUG3("modsingle[%s]: returned from %s (%s)",
               comp2str[component], sp->modinst->name,
-              sp->modinst->entry->name, request->number);
+              sp->modinst->entry->name);
 
        return request->rcode;
 }
 
-static int default_component_results[RLM_COMPONENT_COUNT] = {
+static int default_component_results[MOD_COUNT] = {
        RLM_MODULE_REJECT,      /* AUTH */
        RLM_MODULE_NOTFOUND,    /* AUTZ */
        RLM_MODULE_NOOP,        /* PREACCT */
@@ -347,7 +338,9 @@ static int default_component_results[RLM_COMPONENT_COUNT] = {
 };
 
 
-static char const *group_name[] = {
+extern char const *unlang_keyword[];
+
+char const *unlang_keyword[] = {
        "",
        "single",
        "group",
@@ -366,7 +359,8 @@ static char const *group_name[] = {
 #endif
        "policy",
        "reference",
-       "xlat"
+       "xlat",
+       NULL
 };
 
 static char const modcall_spaces[] = "                                                                ";
@@ -386,14 +380,14 @@ typedef struct modcall_stack_entry_t {
 
 
 static bool modcall_recurse(REQUEST *request, rlm_components_t component, int depth,
-                           modcall_stack_entry_t *entry);
+                           modcall_stack_entry_t *entry, bool do_next_sibling);
 
 /*
  *     Call a child of a block.
  */
 static void modcall_child(REQUEST *request, rlm_components_t component, int depth,
                          modcall_stack_entry_t *entry, modcallable *c,
-                         rlm_rcode_t *result)
+                         rlm_rcode_t *result, bool do_next_sibling)
 {
        modcall_stack_entry_t *next;
 
@@ -412,7 +406,7 @@ static void modcall_child(REQUEST *request, rlm_components_t component, int dept
        next->unwind = 0;
 
        if (!modcall_recurse(request, component,
-                            depth, next)) {
+                            depth, next, do_next_sibling)) {
                *result = RLM_MODULE_FAIL;
                 return;
        }
@@ -434,7 +428,7 @@ static void modcall_child(REQUEST *request, rlm_components_t component, int dept
  *     Interpret the various types of blocks.
  */
 static bool modcall_recurse(REQUEST *request, rlm_components_t component, int depth,
-                           modcall_stack_entry_t *entry)
+                           modcall_stack_entry_t *entry, bool do_next_sibling)
 {
        bool if_taken, was_if;
        modcallable *c;
@@ -455,6 +449,12 @@ redo:
         */
        if (!c) goto finish;
 
+       if (fr_debug_lvl >= 3) {
+               VERIFY_REQUEST(request);
+       }
+
+       rad_assert(c->debug_name != NULL); /* if this happens, all bets are off. */
+
        /*
         *      We've been asked to stop.  Do so.
         */
@@ -478,7 +478,7 @@ redo:
                g = mod_callabletogroup(c);
                rad_assert(g->cond != NULL);
 
-               RDEBUG2("%s %s{", group_name[c->type], c->name);
+               RDEBUG2("%s %s{", unlang_keyword[c->type], c->name);
 
                condition = radius_evaluate_cond(request, result, 0, g->cond);
                if (condition < 0) {
@@ -486,7 +486,7 @@ redo:
                        REDEBUG("Failed retrieving values required to evaluate condition");
                } else {
                        RDEBUG2("%s %s -> %s",
-                               group_name[c->type],
+                               unlang_keyword[c->type],
                                c->name, condition ? "TRUE" : "FALSE");
                }
 
@@ -518,8 +518,8 @@ redo:
                 *      Like MOD_ELSE, but allow for a later "else"
                 */
                if (if_taken) {
-                       RDEBUG2("... skipping %s for request %d: Preceding \"if\" was taken",
-                               group_name[c->type], request->number);
+                       RDEBUG2("... skipping %s: Preceding \"if\" was taken",
+                               unlang_keyword[c->type]);
                        was_if = true;
                        if_taken = true;
                        goto next_sibling;
@@ -537,14 +537,14 @@ redo:
        if (c->type == MOD_ELSE) {
                if (!was_if) { /* error */
                elsif_error:
-                       RDEBUG2("... skipping %s for request %d: No preceding \"if\"",
-                               group_name[c->type], request->number);
+                       RDEBUG2("... skipping %s: No preceding \"if\"",
+                               unlang_keyword[c->type]);
                        goto next_sibling;
                }
 
                if (if_taken) {
-                       RDEBUG2("... skipping %s for request %d: Preceding \"if\" was taken",
-                               group_name[c->type], request->number);
+                       RDEBUG2("... skipping %s: Preceding \"if\" was taken",
+                               unlang_keyword[c->type]);
                        was_if = false;
                        if_taken = false;
                        goto next_sibling;
@@ -588,7 +588,7 @@ redo:
        if (c->type == MOD_UPDATE) {
                int rcode;
                modgroup *g = mod_callabletogroup(c);
-               value_pair_map_t *map;
+               vp_map_t *map;
 
                MOD_LOG_OPEN_BRACE;
                RINDENT();
@@ -663,7 +663,7 @@ redo:
                     vp != NULL;
                     vp = fr_cursor_next(&copy)) {
 #ifndef NDEBUG
-                       if (fr_debug_flag >= 2) {
+                       if (fr_debug_lvl >= 2) {
                                char buffer[1024];
 
                                vp_prints_value(buffer, sizeof(buffer), vp, '"');
@@ -686,15 +686,17 @@ redo:
                        next->priority = 0;
                        next->unwind = 0;
 
-                       if (!modcall_recurse(request, component, depth + 1, next)) {
+                       if (!modcall_recurse(request, component, depth + 1, next, true)) {
                                break;
                        }
 
                        /*
-                        *      We've unwound to the enclosing
-                        *      "foreach".  Stop the unwinding.
+                        *      We've been asked to unwind to the
+                        *      enclosing "foreach".  We're here, so
+                        *      we can stop unwinding.
                         */
-                       if (next->unwind == MOD_FOREACH) {
+                       if (next->unwind == MOD_BREAK) {
+                               entry->unwind = 0;
                                break;
                        }
 
@@ -712,7 +714,7 @@ redo:
                 *      If we don't remove the request data, something could call
                 *      the xlat outside of a foreach loop and trigger a segv.
                 */
-               pairfree(&vps);
+               fr_pair_list_free(&vps);
                request_data_get(request, (void *)radius_get_vp, foreach_depth);
 
                rad_assert(next != NULL);
@@ -730,7 +732,7 @@ redo:
                int i;
                VALUE_PAIR **copy_p;
 
-               RDEBUG2("%s", group_name[c->type]);
+               RDEBUG2("%s", unlang_keyword[c->type]);
 
                for (i = 8; i >= 0; i--) {
                        copy_p = request_data_get(request, (void *)radius_get_vp, i);
@@ -774,6 +776,11 @@ redo:
                 *      MOD_GROUP.
                 */
                if (!g->children) {
+                       if (c->type == MOD_CASE) {
+                               result = RLM_MODULE_NOOP;
+                               goto calculate_result;
+                       }
+
                        RDEBUG2("%s { ... } # empty sub-section is ignored", c->name);
                        goto next_sibling;
                }
@@ -781,7 +788,7 @@ redo:
                MOD_LOG_OPEN_BRACE;
                modcall_child(request, component,
                              depth + 1, entry, g->children,
-                             &result);
+                             &result, true);
                MOD_LOG_CLOSE_BRACE;
                goto calculate_result;
        } /* MOD_GROUP */
@@ -792,8 +799,8 @@ redo:
                modgroup *g, *h;
                fr_cond_t cond;
                value_data_t data;
-               value_pair_map_t map;
-               value_pair_tmpl_t vpt;
+               vp_map_t map;
+               vp_tmpl_t vpt;
 
                MOD_LOG_OPEN_BRACE;
 
@@ -841,10 +848,12 @@ redo:
                    (g->vpt->type == TMPL_TYPE_XLAT) ||
                    (g->vpt->type == TMPL_TYPE_EXEC)) {
                        char *p;
+                       ssize_t len;
 
-                       if (tmpl_expand(request, &p, request, g->vpt) < 0) goto find_null_case;
+                       len = tmpl_aexpand(request, &p, request, g->vpt, NULL, NULL);
+                       if (len < 0) goto find_null_case;
                        data.strvalue = p;
-                       tmpl_init(&vpt, TMPL_TYPE_LITERAL, data.strvalue, talloc_array_length(data.strvalue) - 1);
+                       tmpl_init(&vpt, TMPL_TYPE_LITERAL, data.strvalue, len);
                }
 
                /*
@@ -914,7 +923,7 @@ redo:
 
        do_null_case:
                talloc_free(data.ptr);
-               modcall_child(request, component, depth + 1, entry, found, &result);
+               modcall_child(request, component, depth + 1, entry, found, &result, true);
                MOD_LOG_CLOSE_BRACE;
                goto calculate_result;
        } /* MOD_SWITCH */
@@ -943,12 +952,10 @@ redo:
                        }
                }
 
-               MOD_LOG_OPEN_BRACE;
-
                if (c->type == MOD_LOAD_BALANCE) {
                        modcall_child(request, component,
                                      depth + 1, entry, found,
-                                     &result);
+                                     &result, false);
 
                } else {
                        this = found;
@@ -956,7 +963,7 @@ redo:
                        do {
                                modcall_child(request, component,
                                              depth + 1, entry, this,
-                                             &result);
+                                             &result, false);
                                if (this->actions[result] == MOD_ACTION_RETURN) {
                                        priority = -1;
                                        break;
@@ -1007,7 +1014,7 @@ redo:
                        radius_xlat(buffer, sizeof(buffer), request, mx->xlat_name, NULL, NULL);
                } else {
                        RDEBUG("`%s`", mx->xlat_name);
-                       radius_exec_program(NULL, 0, NULL, request, mx->xlat_name, request->packet->vps,
+                       radius_exec_program(request, NULL, 0, NULL, request, mx->xlat_name, request->packet->vps,
                                            false, true, EXEC_TIMEOUT);
                }
 
@@ -1079,7 +1086,6 @@ calculate_result:
         */
        if (entry->unwind == MOD_BREAK) {
                RDEBUG2("# unwind to enclosing foreach");
-               entry->unwind = 0;
                goto finish;
        }
 
@@ -1088,9 +1094,11 @@ calculate_result:
        }
 
 next_sibling:
-       entry->c = entry->c->next;
+       if (do_next_sibling) {
+               entry->c = entry->c->next;
 
-       if (entry->c) goto redo;
+               if (entry->c) goto redo;
+       }
 
 finish:
        /*
@@ -1123,7 +1131,7 @@ int modcall(rlm_components_t component, modcallable *c, REQUEST *request)
        /*
         *      Call the main handler.
         */
-       if (!modcall_recurse(request, component, 0, &stack[0])) {
+       if (!modcall_recurse(request, component, 0, &stack[0], true)) {
                return RLM_MODULE_FAIL;
        }
 
@@ -1161,7 +1169,7 @@ static void dump_mc(modcallable *c, int indent)
                modgroup *g = mod_callabletogroup(c);
                modcallable *p;
                DEBUG("%.*s%s {", indent, "\t\t\t\t\t\t\t\t\t\t\t",
-                     group_name[c->type]);
+                     unlang_keyword[c->type]);
                for(p = g->children;p;p = p->next)
                        dump_mc(p, indent+1);
        } /* else ignore it for now */
@@ -1185,10 +1193,10 @@ static void dump_tree(rlm_components_t comp, modcallable *c)
 #endif
 
 /* These are the default actions. For each component, the group{} block
- * behaves like the code from the old module_*() function. redundant{} and
- * append{} are based on my guesses of what they will be used for. --Pac. */
+ * behaves like the code from the old module_*() function. redundant{}
+ * are based on my guesses of what they will be used for. --Pac. */
 static const int
-defaultactions[RLM_COMPONENT_COUNT][GROUPTYPE_COUNT][RLM_MODULE_NUMCODES] =
+defaultactions[MOD_COUNT][GROUPTYPE_COUNT][RLM_MODULE_NUMCODES] =
 {
        /* authenticate */
        {
@@ -1215,18 +1223,6 @@ defaultactions[RLM_COMPONENT_COUNT][GROUPTYPE_COUNT][RLM_MODULE_NUMCODES] =
                        MOD_ACTION_RETURN,      /* notfound */
                        MOD_ACTION_RETURN,      /* noop     */
                        MOD_ACTION_RETURN       /* updated  */
-               },
-               /* append */
-               {
-                       MOD_ACTION_RETURN,      /* reject   */
-                       1,                      /* fail     */
-                       MOD_ACTION_RETURN,      /* ok       */
-                       MOD_ACTION_RETURN,      /* handled  */
-                       MOD_ACTION_RETURN,      /* invalid  */
-                       MOD_ACTION_RETURN,      /* userlock */
-                       2,                      /* notfound */
-                       MOD_ACTION_RETURN,      /* noop     */
-                       MOD_ACTION_RETURN       /* updated  */
                }
        },
        /* authorize */
@@ -1254,18 +1250,6 @@ defaultactions[RLM_COMPONENT_COUNT][GROUPTYPE_COUNT][RLM_MODULE_NUMCODES] =
                        MOD_ACTION_RETURN,      /* notfound */
                        MOD_ACTION_RETURN,      /* noop     */
                        MOD_ACTION_RETURN       /* updated  */
-               },
-               /* append */
-               {
-                       MOD_ACTION_RETURN,      /* reject   */
-                       1,                      /* fail     */
-                       MOD_ACTION_RETURN,      /* ok       */
-                       MOD_ACTION_RETURN,      /* handled  */
-                       MOD_ACTION_RETURN,      /* invalid  */
-                       MOD_ACTION_RETURN,      /* userlock */
-                       2,                      /* notfound */
-                       MOD_ACTION_RETURN,      /* noop     */
-                       MOD_ACTION_RETURN       /* updated  */
                }
        },
        /* preacct */
@@ -1293,18 +1277,6 @@ defaultactions[RLM_COMPONENT_COUNT][GROUPTYPE_COUNT][RLM_MODULE_NUMCODES] =
                        MOD_ACTION_RETURN,      /* notfound */
                        MOD_ACTION_RETURN,      /* noop     */
                        MOD_ACTION_RETURN       /* updated  */
-               },
-               /* append */
-               {
-                       MOD_ACTION_RETURN,      /* reject   */
-                       1,                      /* fail     */
-                       MOD_ACTION_RETURN,      /* ok       */
-                       MOD_ACTION_RETURN,      /* handled  */
-                       MOD_ACTION_RETURN,      /* invalid  */
-                       MOD_ACTION_RETURN,      /* userlock */
-                       2,                      /* notfound */
-                       MOD_ACTION_RETURN,      /* noop     */
-                       MOD_ACTION_RETURN       /* updated  */
                }
        },
        /* accounting */
@@ -1332,18 +1304,6 @@ defaultactions[RLM_COMPONENT_COUNT][GROUPTYPE_COUNT][RLM_MODULE_NUMCODES] =
                        1,                      /* notfound */
                        2,                      /* noop     */
                        4                       /* updated  */
-               },
-               /* append */
-               {
-                       MOD_ACTION_RETURN,      /* reject   */
-                       1,                      /* fail     */
-                       MOD_ACTION_RETURN,      /* ok       */
-                       MOD_ACTION_RETURN,      /* handled  */
-                       MOD_ACTION_RETURN,      /* invalid  */
-                       MOD_ACTION_RETURN,      /* userlock */
-                       2,                      /* notfound */
-                       MOD_ACTION_RETURN,      /* noop     */
-                       MOD_ACTION_RETURN       /* updated  */
                }
        },
        /* checksimul */
@@ -1371,18 +1331,6 @@ defaultactions[RLM_COMPONENT_COUNT][GROUPTYPE_COUNT][RLM_MODULE_NUMCODES] =
                        MOD_ACTION_RETURN,      /* notfound */
                        MOD_ACTION_RETURN,      /* noop     */
                        MOD_ACTION_RETURN       /* updated  */
-               },
-               /* append */
-               {
-                       MOD_ACTION_RETURN,      /* reject   */
-                       1,                      /* fail     */
-                       MOD_ACTION_RETURN,      /* ok       */
-                       MOD_ACTION_RETURN,      /* handled  */
-                       MOD_ACTION_RETURN,      /* invalid  */
-                       MOD_ACTION_RETURN,      /* userlock */
-                       MOD_ACTION_RETURN,      /* notfound */
-                       MOD_ACTION_RETURN,      /* noop     */
-                       MOD_ACTION_RETURN       /* updated  */
                }
        },
        /* pre-proxy */
@@ -1410,18 +1358,6 @@ defaultactions[RLM_COMPONENT_COUNT][GROUPTYPE_COUNT][RLM_MODULE_NUMCODES] =
                        MOD_ACTION_RETURN,      /* notfound */
                        MOD_ACTION_RETURN,      /* noop     */
                        MOD_ACTION_RETURN       /* updated  */
-               },
-               /* append */
-               {
-                       MOD_ACTION_RETURN,      /* reject   */
-                       1,                      /* fail     */
-                       MOD_ACTION_RETURN,      /* ok       */
-                       MOD_ACTION_RETURN,      /* handled  */
-                       MOD_ACTION_RETURN,      /* invalid  */
-                       MOD_ACTION_RETURN,      /* userlock */
-                       2,                      /* notfound */
-                       MOD_ACTION_RETURN,      /* noop     */
-                       MOD_ACTION_RETURN       /* updated  */
                }
        },
        /* post-proxy */
@@ -1449,18 +1385,6 @@ defaultactions[RLM_COMPONENT_COUNT][GROUPTYPE_COUNT][RLM_MODULE_NUMCODES] =
                        MOD_ACTION_RETURN,      /* notfound */
                        MOD_ACTION_RETURN,      /* noop     */
                        MOD_ACTION_RETURN       /* updated  */
-               },
-               /* append */
-               {
-                       MOD_ACTION_RETURN,      /* reject   */
-                       1,                      /* fail     */
-                       MOD_ACTION_RETURN,      /* ok       */
-                       MOD_ACTION_RETURN,      /* handled  */
-                       MOD_ACTION_RETURN,      /* invalid  */
-                       MOD_ACTION_RETURN,      /* userlock */
-                       2,                      /* notfound */
-                       MOD_ACTION_RETURN,      /* noop     */
-                       MOD_ACTION_RETURN       /* updated  */
                }
        },
        /* post-auth */
@@ -1488,18 +1412,6 @@ defaultactions[RLM_COMPONENT_COUNT][GROUPTYPE_COUNT][RLM_MODULE_NUMCODES] =
                        MOD_ACTION_RETURN,      /* notfound */
                        MOD_ACTION_RETURN,      /* noop     */
                        MOD_ACTION_RETURN       /* updated  */
-               },
-               /* append */
-               {
-                       MOD_ACTION_RETURN,      /* reject   */
-                       1,                      /* fail     */
-                       MOD_ACTION_RETURN,      /* ok       */
-                       MOD_ACTION_RETURN,      /* handled  */
-                       MOD_ACTION_RETURN,      /* invalid  */
-                       MOD_ACTION_RETURN,      /* userlock */
-                       2,                      /* notfound */
-                       MOD_ACTION_RETURN,      /* noop     */
-                       MOD_ACTION_RETURN       /* updated  */
                }
        }
 #ifdef WITH_COA
@@ -1529,18 +1441,6 @@ defaultactions[RLM_COMPONENT_COUNT][GROUPTYPE_COUNT][RLM_MODULE_NUMCODES] =
                        MOD_ACTION_RETURN,      /* notfound */
                        MOD_ACTION_RETURN,      /* noop     */
                        MOD_ACTION_RETURN       /* updated  */
-               },
-               /* append */
-               {
-                       MOD_ACTION_RETURN,      /* reject   */
-                       1,                      /* fail     */
-                       MOD_ACTION_RETURN,      /* ok       */
-                       MOD_ACTION_RETURN,      /* handled  */
-                       MOD_ACTION_RETURN,      /* invalid  */
-                       MOD_ACTION_RETURN,      /* userlock */
-                       2,                      /* notfound */
-                       MOD_ACTION_RETURN,      /* noop     */
-                       MOD_ACTION_RETURN       /* updated  */
                }
        },
        /* send-coa */
@@ -1568,37 +1468,53 @@ defaultactions[RLM_COMPONENT_COUNT][GROUPTYPE_COUNT][RLM_MODULE_NUMCODES] =
                        MOD_ACTION_RETURN,      /* notfound */
                        MOD_ACTION_RETURN,      /* noop     */
                        MOD_ACTION_RETURN       /* updated  */
-               },
-               /* append */
-               {
-                       MOD_ACTION_RETURN,      /* reject   */
-                       1,                      /* fail     */
-                       MOD_ACTION_RETURN,      /* ok       */
-                       MOD_ACTION_RETURN,      /* handled  */
-                       MOD_ACTION_RETURN,      /* invalid  */
-                       MOD_ACTION_RETURN,      /* userlock */
-                       2,                      /* notfound */
-                       MOD_ACTION_RETURN,      /* noop     */
-                       MOD_ACTION_RETURN       /* updated  */
                }
        }
 #endif
 };
 
+static const int authtype_actions[GROUPTYPE_COUNT][RLM_MODULE_NUMCODES] =
+{
+       /* group */
+       {
+               MOD_ACTION_RETURN,      /* reject   */
+               MOD_ACTION_RETURN,      /* fail     */
+               4,                      /* ok       */
+               MOD_ACTION_RETURN,      /* handled  */
+               MOD_ACTION_RETURN,      /* invalid  */
+               MOD_ACTION_RETURN,      /* userlock */
+               1,                      /* notfound */
+               2,                      /* noop     */
+               3                       /* updated  */
+       },
+       /* redundant */
+       {
+               MOD_ACTION_RETURN,      /* reject   */
+               1,                      /* fail     */
+               MOD_ACTION_RETURN,      /* ok       */
+               MOD_ACTION_RETURN,      /* handled  */
+               MOD_ACTION_RETURN,      /* invalid  */
+               MOD_ACTION_RETURN,      /* userlock */
+               MOD_ACTION_RETURN,      /* notfound */
+               MOD_ACTION_RETURN,      /* noop     */
+               MOD_ACTION_RETURN       /* updated  */
+       }
+};
+
 /** Validate and fixup a map that's part of an update section.
  *
  * @param map to validate.
  * @param ctx data to pass to fixup function (currently unused).
  * @return 0 if valid else -1.
  */
-int modcall_fixup_update(value_pair_map_t *map, UNUSED void *ctx)
+int modcall_fixup_update(vp_map_t *map, UNUSED void *ctx)
 {
        CONF_PAIR *cp = cf_item_to_pair(map->ci);
 
        /*
         *      Anal-retentive checks.
         */
-       if (DEBUG_ENABLED2) {
+       if (DEBUG_ENABLED3) {
                if ((map->lhs->type == TMPL_TYPE_ATTR) && (map->lhs->name[0] != '&')) {
                        WARN("%s[%d]: Please change attribute reference to '&%s %s ...'",
                             cf_pair_filename(cp), cf_pair_lineno(cp),
@@ -1623,7 +1539,7 @@ int modcall_fixup_update(value_pair_map_t *map, UNUSED void *ctx)
                             cf_pair_filename(cp), cf_pair_lineno(cp));
                }
 
-               tmpl_free(&map->rhs);
+               TALLOC_FREE(map->rhs);
 
                map->rhs = tmpl_alloc(map, TMPL_TYPE_NULL, NULL, 0);
        }
@@ -1644,22 +1560,11 @@ int modcall_fixup_update(value_pair_map_t *map, UNUSED void *ctx)
        /*
         *      Depending on the attribute type, some operators are disallowed.
         */
-       if (map->lhs->type == TMPL_TYPE_ATTR) {
-               switch (map->op) {
-               default:
-                       cf_log_err(map->ci, "Invalid operator for attribute");
-                       return -1;
-
-               case T_OP_EQ:
-               case T_OP_CMP_EQ:
-               case T_OP_ADD:
-               case T_OP_SUB:
-               case T_OP_LE:
-               case T_OP_GE:
-               case T_OP_CMP_FALSE:
-               case T_OP_SET:
-                       break;
-               }
+       if ((map->lhs->type == TMPL_TYPE_ATTR) && (!fr_assignment_op[map->op] && !fr_equality_op[map->op])) {
+               cf_log_err(map->ci, "Invalid operator \"%s\" in update section.  "
+                          "Only assignment or filter operators are allowed",
+                          fr_int2str(fr_tokens, map->op, "<INVALID>"));
+               return -1;
        }
 
        if (map->lhs->type == TMPL_TYPE_LIST) {
@@ -1699,7 +1604,7 @@ int modcall_fixup_update(value_pair_map_t *map, UNUSED void *ctx)
 
                case T_OP_SET:
                        if (map->rhs->type == TMPL_TYPE_EXEC) {
-                               WARN("%s[%d] Please change ':=' to '=' for list assignment",
+                               WARN("%s[%d]: Please change ':=' to '=' for list assignment",
                                     cf_pair_filename(cp), cf_pair_lineno(cp));
                        }
 
@@ -1745,8 +1650,26 @@ int modcall_fixup_update(value_pair_map_t *map, UNUSED void *ctx)
                    (map->lhs->tmpl_da->type == PW_TYPE_STRING) &&
                    (cf_pair_value_type(cp) == T_SINGLE_QUOTED_STRING)) {
                        tmpl_cast_in_place_str(map->rhs);
+
                } else {
-                       if (!tmpl_cast_in_place(map->rhs, map->lhs->tmpl_da->type, map->lhs->tmpl_da)) {
+                       /*
+                        *      RHS is hex, try to parse it as
+                        *      type-specific data.
+                        */
+                       if (map->lhs->auto_converted &&
+                           (map->rhs->name[0] == '0') && (map->rhs->name[1] == 'x') &&
+                           (map->rhs->len > 2) && ((map->rhs->len & 0x01) == 0)) {
+                               vp_tmpl_t *vpt = map->rhs;
+                               map->rhs = NULL;
+
+                               if (!map_cast_from_hex(map, T_BARE_WORD, vpt->name)) {
+                                       map->rhs = vpt;
+                                       cf_log_err(map->ci, "Cannot parse RHS hex as the data type of the attribute %s", map->lhs->tmpl_da->name);
+                                       return -1;
+                               }
+                               talloc_free(vpt);
+
+                       } else if (tmpl_cast_in_place(map->rhs, map->lhs->tmpl_da->type, map->lhs->tmpl_da) < 0) {
                                cf_log_err(map->ci, "%s", fr_strerror());
                                return -1;
                        }
@@ -1783,7 +1706,7 @@ static modcallable *do_compile_modupdate(modcallable *parent, rlm_components_t c
        modgroup *g;
        modcallable *csingle;
 
-       value_pair_map_t *head;
+       vp_map_t *head;
 
        /*
         *      This looks at cs->name2 to determine which list to update
@@ -1830,7 +1753,7 @@ static modcallable *do_compile_modswitch (modcallable *parent, rlm_components_t
        modcallable *csingle;
        modgroup *g;
        ssize_t slen;
-       value_pair_tmpl_t *vpt;
+       vp_tmpl_t *vpt;
 
        name2 = cf_section_name2(cs);
        if (!name2) {
@@ -1850,7 +1773,7 @@ static modcallable *do_compile_modswitch (modcallable *parent, rlm_components_t
         *      will fix it up.
         */
        type = cf_section_name2_type(cs);
-       slen = tmpl_afrom_str(cs, &vpt, name2, strlen(name2), type, REQUEST_CURRENT, PAIR_LIST_REQUEST);
+       slen = tmpl_afrom_str(cs, &vpt, name2, strlen(name2), type, REQUEST_CURRENT, PAIR_LIST_REQUEST, true);
        if ((slen < 0) && ((type != T_BARE_WORD) || (name2[0] != '&'))) {
                char *spaces, *text;
 
@@ -1871,6 +1794,12 @@ static modcallable *do_compile_modswitch (modcallable *parent, rlm_components_t
         *      by a module.  That is checked in pass 2.
         */
 
+       if (vpt->type == TMPL_TYPE_LIST) {
+               cf_log_err_cs(cs, "Syntax error: Cannot switch over list '%s'", name2);
+               return NULL;
+       }
+
+
        /*
         *      Walk through the children of the switch section,
         *      ensuring that they're all 'case' statements
@@ -1932,7 +1861,7 @@ static modcallable *do_compile_modcase(modcallable *parent, rlm_components_t com
        char const *name2;
        modcallable *csingle;
        modgroup *g;
-       value_pair_tmpl_t *vpt;
+       vp_tmpl_t *vpt;
 
        if (!parent || (parent->type != MOD_SWITCH)) {
                cf_log_err_cs(cs, "\"case\" statements may only appear within a \"switch\" section");
@@ -1950,7 +1879,7 @@ static modcallable *do_compile_modcase(modcallable *parent, rlm_components_t com
 
                type = cf_section_name2_type(cs);
 
-               slen = tmpl_afrom_str(cs, &vpt, name2, strlen(name2), type, REQUEST_CURRENT, PAIR_LIST_REQUEST);
+               slen = tmpl_afrom_str(cs, &vpt, name2, strlen(name2), type, REQUEST_CURRENT, PAIR_LIST_REQUEST, true);
                if ((slen < 0) && ((type != T_BARE_WORD) || (name2[0] != '&'))) {
                        char *spaces, *text;
 
@@ -1966,6 +1895,11 @@ static modcallable *do_compile_modcase(modcallable *parent, rlm_components_t com
                        return NULL;
                }
 
+               if (vpt->type == TMPL_TYPE_LIST) {
+                       cf_log_err_cs(cs, "Syntax error: Cannot match list '%s'", name2);
+                       return NULL;
+               }
+
                /*
                 *      Otherwise a NULL vpt may refer to an attribute defined
                 *      by a module.  That is checked in pass 2.
@@ -2014,7 +1948,7 @@ static modcallable *do_compile_modforeach(modcallable *parent,
        modcallable *csingle;
        modgroup *g;
        ssize_t slen;
-       value_pair_tmpl_t *vpt;
+       vp_tmpl_t *vpt;
 
        name2 = cf_section_name2(cs);
        if (!name2) {
@@ -2035,7 +1969,7 @@ static modcallable *do_compile_modforeach(modcallable *parent,
         *      will fix it up.
         */
        type = cf_section_name2_type(cs);
-       slen = tmpl_afrom_str(cs, &vpt, name2, strlen(name2), type, REQUEST_CURRENT, PAIR_LIST_REQUEST);
+       slen = tmpl_afrom_str(cs, &vpt, name2, strlen(name2), type, REQUEST_CURRENT, PAIR_LIST_REQUEST, true);
        if ((slen < 0) && ((type != T_BARE_WORD) || (name2[0] != '&'))) {
                char *spaces, *text;
 
@@ -2221,6 +2155,96 @@ static int all_children_are_modules(CONF_SECTION *cs, char const *name)
        return 1;
 }
 
+/** Load a named module from "instantiate" or "policy".
+ *
+ * If it's "foo.method", look for "foo", and return "method" as the method
+ * we wish to use, instead of the input component.
+ *
+ * @param[out] pcomponent Where to write the method we found, if any.  If no method is specified
+ *     will be set to MOD_COUNT.
+ * @param[in] real_name Complete name string e.g. foo.authorize.
+ * @param[in] virtual_name Virtual module name e.g. foo.
+ * @param[in] method_name Method override (may be NULL) or the method name e.g. authorize.
+ * @return the CONF_SECTION specifying the virtual module.
+ */
+static CONF_SECTION *virtual_module_find_cs(rlm_components_t *pcomponent,
+                                           char const *real_name, char const *virtual_name, char const *method_name)
+{
+       CONF_SECTION *cs, *subcs;
+       rlm_components_t method = *pcomponent;
+       char buffer[256];
+
+       /*
+        *      Turn the method name into a method enum.
+        */
+       if (method_name) {
+               rlm_components_t i;
+
+               for (i = MOD_AUTHENTICATE; i < MOD_COUNT; i++) {
+                       if (strcmp(comp2str[i], method_name) == 0) break;
+               }
+
+               if (i != MOD_COUNT) {
+                       method = i;
+               } else {
+                       method_name = NULL;
+                       virtual_name = real_name;
+               }
+       }
+
+       /*
+        *      Look for "foo" in the "instantiate" section.  If we
+        *      find it, AND there's no method name, we've found the
+        *      right thing.
+        *
+        *      Return it to the caller, with the updated method.
+        */
+       cs = cf_section_find("instantiate");
+       if (cs) {
+               /*
+                *      Found "foo".  Load it as "foo", or "foo.method".
+                */
+               subcs = cf_section_sub_find_name2(cs, NULL, virtual_name);
+               if (subcs) {
+                       *pcomponent = method;
+                       return subcs;
+               }
+       }
+
+       /*
+        *      Look for it in "policy".
+        *
+        *      If there's no policy section, we can't do anything else.
+        */
+       cs = cf_section_find("policy");
+       if (!cs) return NULL;
+
+       /*
+        *      "foo.authorize" means "load policy "foo" as method "authorize".
+        *
+        *      And bail out if there's no policy "foo".
+        */
+       if (method_name) {
+               subcs = cf_section_sub_find_name2(cs, NULL, virtual_name);
+               if (subcs) *pcomponent = method;
+
+               return subcs;
+       }
+
+       /*
+        *      "foo" means "look for foo.component" first, to allow
+        *      method overrides.  If that's not found, just look for
+        *      a policy "foo".
+        *
+        */
+       snprintf(buffer, sizeof(buffer), "%s.%s",
+                virtual_name, comp2str[method]);
+       subcs = cf_section_sub_find_name2(cs, NULL, buffer);
+       if (subcs) return subcs;
+
+       return cf_section_sub_find_name2(cs, NULL, virtual_name);
+}
+
 
 /*
  *     Compile one entry of a module call.
@@ -2230,12 +2254,14 @@ static modcallable *do_compile_modsingle(modcallable *parent,
                                         int grouptype,
                                         char const **modname)
 {
-       char const *modrefname;
+       char const *modrefname, *p;
        modsingle *single;
        modcallable *csingle;
        module_instance_t *this;
        CONF_SECTION *cs, *subcs, *modules;
+       CONF_SECTION *loop;
        char const *realname;
+       rlm_components_t method = component;
 
        if (cf_item_is_section(ci)) {
                char const *name2;
@@ -2268,12 +2294,6 @@ static modcallable *do_compile_modsingle(modcallable *parent,
                                                   GROUPTYPE_REDUNDANT,
                                                   grouptype, MOD_GROUP);
 
-               } else if (strcmp(modrefname, "append") == 0) {
-                       *modname = name2;
-                       return do_compile_modgroup(parent, component, cs,
-                                                  GROUPTYPE_APPEND,
-                                                  grouptype, MOD_GROUP);
-
                } else if (strcmp(modrefname, "load-balance") == 0) {
                        *modname = name2;
 
@@ -2380,7 +2400,6 @@ static modcallable *do_compile_modsingle(modcallable *parent,
                 *      codes.
                 */
        } else {
-               CONF_SECTION *loop;
                CONF_PAIR *cp = cf_item_to_pair(ci);
                modrefname = cf_pair_attr(cp);
 
@@ -2393,80 +2412,37 @@ static modcallable *do_compile_modsingle(modcallable *parent,
                        return NULL;
                }
 
+               /*
+                *      In-place xlat's via %{...}.
+                *
+                *      This should really be removed from the server.
+                */
                if (((modrefname[0] == '%') && (modrefname[1] == '{')) ||
                    (modrefname[0] == '`')) {
                        return do_compile_modxlat(parent, component,
                                                  modrefname);
                }
-
-               /*
-                *      See if the module is a virtual one.  If so,
-                *      return that, rather than doing anything here.
-                */
-               subcs = NULL;
-               cs = cf_section_find("instantiate");
-               if (cs) subcs = cf_section_sub_find_name2(cs, NULL,
-                                                         modrefname);
-               if (!subcs &&
-                   (cs = cf_section_find("policy")) != NULL) {
-                       char buffer[256];
-
-                       snprintf(buffer, sizeof(buffer), "%s.%s",
-                                modrefname, comp2str[component]);
-
-                       /*
-                        *      Prefer name.section, then name.
-                        */
-                       subcs = cf_section_sub_find_name2(cs, NULL,
-                                                         buffer);
-                       if (!subcs) {
-                               subcs = cf_section_sub_find_name2(cs, NULL,
-                                                                 modrefname);
-                       }
-               }
-
-               /*
-                *      Allow policies to over-ride module names.
-                *      i.e. the "sql" policy can do some extra things,
-                *      and then call the "sql" module.
-                */
-               for (loop = cf_item_parent(ci);
-                    loop && subcs;
-                    loop = cf_item_parent(cf_section_to_item(loop))) {
-                       if (loop == subcs) {
-                               subcs = NULL;
-                       }
-               }
-
-               if (subcs) {
-                       /*
-                        *      redundant foo {} is a single.
-                        */
-                       if (cf_section_name2(subcs)) {
-                               return do_compile_modsingle(parent,
-                                                           component,
-                                                           cf_section_to_item(subcs),
-                                                           grouptype,
-                                                           modname);
-                       } else {
-                               /*
-                                *      foo {} is a group.
-                                */
-                               return do_compile_modgroup(parent,
-                                                          component,
-                                                          subcs,
-                                                          GROUPTYPE_SIMPLE,
-                                                          grouptype, MOD_GROUP);
-                       }
-               }
        }
 
 #ifdef WITH_UNLANG
+       /*
+        *      These can't be over-ridden.
+        */
        if (strcmp(modrefname, "break") == 0) {
+               if (!cf_item_is_pair(ci)) {
+                       cf_log_err(ci, "Invalid use of 'break' as section name.");
+                       return NULL;
+               }
+
                return do_compile_modbreak(parent, component, ci);
        }
 
        if (strcmp(modrefname, "return") == 0) {
+               if (!cf_item_is_pair(ci)) {
+                       cf_log_err(ci, "Invalid use of 'return' as section name.");
+                       return NULL;
+               }
+
                return do_compile_modgroup(parent, component, NULL,
                                           GROUPTYPE_SIMPLE, GROUPTYPE_SIMPLE,
                                           MOD_RETURN);
@@ -2474,124 +2450,240 @@ static modcallable *do_compile_modsingle(modcallable *parent,
 #endif
 
        /*
-        *      Not a virtual module.  It must be a real module.
+        *      Run a virtual server.  This is really terrible and
+        *      should be deleted.
         */
-       modules = cf_section_find("modules");
-       this = NULL;
-       realname = modrefname;
+       if (strncmp(modrefname, "server[", 7) == 0) {
+               char buffer[256];
+
+               if (!cf_item_is_pair(ci)) {
+                       cf_log_err(ci, "Invalid syntax");
+                       return NULL;
+               }
+
+               strlcpy(buffer, modrefname + 7, sizeof(buffer));
+               p = strrchr(buffer, ']');
+               if (!p || p[1] != '\0' || (p == buffer)) {
+                       cf_log_err(ci, "Invalid server reference in \"%s\".", modrefname);
+                       return NULL;
+               }
+
+               buffer[p - buffer] = '\0';
+
+               cs = cf_section_sub_find_name2(NULL, "server", buffer);
+               if (!cs) {
+                       cf_log_err(ci, "No such server \"%s\".", buffer);
+                       return NULL;
+               }
 
-       if (modules) {
                /*
-                *      Try to load the optional module.
+                *      Ignore stupid attempts to over-ride the return
+                *      code.
                 */
-               if (realname[0] == '-') realname++;
+               return do_compile_modserver(parent, component, ci,
+                                           modrefname, cs, buffer);
+       }
+
+       /*
+        *      We now have a name.  It can be one of two forms.  A
+        *      bare module name, or a section named for the module,
+        *      with over-rides for the return codes.
+        *
+        *      The name can refer to a real module, in the "modules"
+        *      section.  In that case, the name will be either the
+        *      first or second name of the sub-section of "modules".
+        *
+        *      Or, the name can refer to a policy, in the "policy"
+        *      section.  In that case, the name will be first name of
+        *      the sub-section of "policy".  Unless it's a "redudant"
+        *      block...
+        *
+        *      Or, the name can refer to a "module.method", in which
+        *      case we're calling a different method than normal for
+        *      this section.
+        *
+        *      Or, the name can refer to a virtual module, in the
+        *      "instantiate" section.  In that case, the name will be
+        *      the first of the sub-section of "instantiate".  Unless
+        *      it's a "redudant" block...
+        *
+        *      We try these in sequence, from the bottom up.  This is
+        *      so that things in "instantiate" and "policy" can
+        *      over-ride calls to real modules.
+        */
+
+
+       /*
+        *      Try:
+        *
+        *      instantiate { ... name { ...} ... }
+        *      instantiate { ... name.method { ...} ... }
+        *      policy { ... name { .. } .. }
+        *      policy { ... name.method { .. } .. }
+        *
+        *      The only difference between things in "instantiate"
+        *      and "policy" is that "instantiate" will cause modules
+        *      to be instantiated in a particular order.
+        */
+       subcs = NULL;
+       p = strrchr(modrefname, '.');
+       if (!p) {
+               subcs = virtual_module_find_cs(&method, modrefname, modrefname, NULL);
+       } else {
+               char buffer[256];
 
+               strlcpy(buffer, modrefname, sizeof(buffer));
+               buffer[p - modrefname] = '\0';
+
+               subcs = virtual_module_find_cs(&method, modrefname, buffer, buffer + (p - modrefname) + 1);
+       }
+
+       /*
+        *      Check that we're not creating a loop.  We may
+        *      be compiling an "sql" module reference inside
+        *      of an "sql" policy.  If so, we allow the
+        *      second "sql" to refer to the module.
+        */
+       for (loop = cf_item_parent(ci);
+            loop && subcs;
+            loop = cf_item_parent(cf_section_to_item(loop))) {
+               if (loop == subcs) {
+                       subcs = NULL;
+               }
+       }
+
+       /*
+        *      We've found the relevant entry.  It MUST be a
+        *      sub-section.
+        *
+        *      However, it can be a "redundant" block, or just a
+        *      section name.
+        */
+       if (subcs) {
                /*
-                *      As of v3, only known modules are in the
-                *      "modules" section.
+                *      modules.c takes care of ensuring that this is:
+                *
+                *      group foo { ...
+                *      load-balance foo { ...
+                *      redundant foo { ...
+                *      redundant-load-balance foo { ...
+                *
+                *      We can just recurs to compile the section as
+                *      if it was found here.
                 */
-               if (cf_section_sub_find_name2(modules, NULL, realname)) {
-                       this = find_module_instance(modules, realname, true);
-                       if (!this && (realname != modrefname)) {
-                               return NULL;
-                       }
-
+               if (cf_section_name2(subcs)) {
+                       csingle = do_compile_modsingle(parent,
+                                                      method,
+                                                      cf_section_to_item(subcs),
+                                                      grouptype,
+                                                      modname);
                } else {
                        /*
-                        *      We were asked to MAYBE load it and it
-                        *      doesn't exist.  Return a soft error.
+                        *      We have:
+                        *
+                        *      foo { ...
+                        *
+                        *      So we compile it like it was:
+                        *
+                        *      group foo { ...
                         */
-                       if (realname != modrefname) {
-                               *modname = modrefname;
-                               return NULL;
-                       }
+                       csingle = do_compile_modgroup(parent,
+                                                     method,
+                                                     subcs,
+                                                     GROUPTYPE_SIMPLE,
+                                                     grouptype, MOD_GROUP);
                }
-       }
-
-       if (!this) do {
-               int i;
-               char *p;
 
                /*
-                *      Maybe it's module.method
+                *      Return the compiled thing if we can.
                 */
-               p = strrchr(modrefname, '.');
-               if (p) for (i = RLM_COMPONENT_AUTH;
-                           i < RLM_COMPONENT_COUNT;
-                           i++) {
-                       if (strcmp(p + 1, comp2str[i]) == 0) {
-                               char buffer[256];
-
-                               strlcpy(buffer, modrefname, sizeof(buffer));
-                               buffer[p - modrefname] = '\0';
-                               component = i;
-
-                               this = find_module_instance(modules, buffer, true);
-                               if (this && !this->entry->module->methods[i]) {
-                                       *modname = NULL;
-                                       cf_log_err(ci, "Module %s has no such method %s", buffer, comp2str[i]);
-                                       return NULL;
-                               }
-                               break;
-                       }
-               }
-               if (this) break;
+               if (!csingle) return NULL;
+               if (cf_item_is_pair(ci)) return csingle;
 
                /*
-                *      Call a server.  This should really be deleted...
+                *      Else we have a reference to a policy, and that reference
+                *      over-rides the return codes for the policy!
                 */
-               if (strncmp(modrefname, "server[", 7) == 0) {
-                       char buffer[256];
+               goto action_override;
+       }
 
-                       strlcpy(buffer, modrefname + 7, sizeof(buffer));
-                       p = strrchr(buffer, ']');
-                       if (!p || p[1] != '\0' || (p == buffer)) {
-                               cf_log_err(ci, "Invalid server reference in \"%s\".", modrefname);
-                               return NULL;
-                       }
-                       *p = '\0';
+       /*
+        *      Not a virtual module.  It must be a real module.
+        */
+       modules = cf_section_find("modules");
+       this = NULL;
+       realname = modrefname;
 
-                       cs = cf_section_sub_find_name2(NULL, "server", buffer);
-                       if (!cs) {
-                               cf_log_err(ci, "No such server \"%s\".", buffer);
-                               return NULL;
-                       }
+       if (modules) {
+               /*
+                *      Try to load the optional module.
+                */
+               if (realname[0] == '-') realname++;
+
+               /*
+                *      As of v3, the "modules" section contains
+                *      modules we use.  Configuration for other
+                *      modules belongs in raddb/mods-available/,
+                *      which isn't loaded into the "modules" section.
+                */
+               this = module_instantiate_method(modules, realname, &method);
+               if (this) goto allocate_csingle;
 
-                       return do_compile_modserver(parent, component, ci,
-                                                   modrefname, cs, buffer);
+               /*
+                *      We were asked to MAYBE load it and it
+                *      doesn't exist.  Return a soft error.
+                */
+               if (realname != modrefname) {
+                       *modname = modrefname;
+                       return NULL;
                }
+       }
 
-               *modname = NULL;
-               cf_log_err(ci, "Failed to find \"%s\" in the \"modules\" section.", modrefname);
-               cf_log_err(ci, "Please verify that the configuration exists in the file %s/mods-enabled/%s.", get_radius_dir(), modrefname);
-               return NULL;
-       } while (0);
+       /*
+        *      Can't de-reference it to anything.  Ugh.
+        */
+       *modname = NULL;
+       cf_log_err(ci, "Failed to find \"%s\" as a module or policy.", modrefname);
+       cf_log_err(ci, "Please verify that the configuration exists in %s/mods-enabled/%s.", get_radius_dir(), modrefname);
+       return NULL;
 
        /*
         *      We know it's all OK, allocate the structures, and fill
         *      them in.
         */
+allocate_csingle:
+       /*
+        *      Check if the module in question has the necessary
+        *      component.
+        */
+       if (!this->entry->module->methods[method]) {
+               cf_log_err(ci, "\"%s\" modules aren't allowed in '%s' sections -- they have no such method.", this->entry->module->name,
+                          comp2str[method]);
+               return NULL;
+       }
+
        single = talloc_zero(parent, modsingle);
+       single->modinst = this;
+       *modname = this->entry->module->name;
+
        csingle = mod_singletocallable(single);
        csingle->parent = parent;
        csingle->next = NULL;
-       if (!parent || (component != RLM_COMPONENT_AUTH)) {
+       if (!parent || (component != MOD_AUTHENTICATE)) {
                memcpy(csingle->actions, defaultactions[component][grouptype],
                       sizeof csingle->actions);
        } else { /* inside Auth-Type has different rules */
-               memcpy(csingle->actions, defaultactions[RLM_COMPONENT_AUTZ][grouptype],
+               memcpy(csingle->actions, authtype_actions[grouptype],
                       sizeof csingle->actions);
        }
        rad_assert(modrefname != NULL);
        csingle->name = realname;
        csingle->type = MOD_SINGLE;
-       csingle->method = component;
+       csingle->method = method;
 
+action_override:
        /*
-        *      Singles can override the actions, virtual modules cannot.
-        *
-        *      FIXME: We may want to re-visit how to do this...
-        *      maybe a csingle as a ref?
+        *      Over-ride the default return codes of the module.
         */
        if (cf_item_is_section(ci)) {
                CONF_ITEM *csi;
@@ -2616,19 +2708,6 @@ static modcallable *do_compile_modsingle(modcallable *parent,
                }
        }
 
-       /*
-        *      Bail out if the module in question does not supply the
-        *      wanted component
-        */
-       if (!this->entry->module->methods[component]) {
-               cf_log_err(ci, "\"%s\" modules aren't allowed in '%s' sections -- they have no such method.", this->entry->module->name,
-                      comp2str[component]);
-               talloc_free(csingle);
-               return NULL;
-       }
-
-       single->modinst = this;
-       *modname = this->entry->module->name;
        return csingle;
 }
 
@@ -2735,7 +2814,7 @@ static modcallable *do_compile_modgroup(modcallable *parent,
                check_if:
                        if (g->cond->type == COND_TYPE_FALSE) {
                                INFO(" # Skipping contents of '%s' as it is always 'false' -- %s:%d",
-                                    group_name[g->mc.type],
+                                    unlang_keyword[g->mc.type],
                                     cf_section_filename(g->cs), cf_section_lineno(g->cs));
                                goto set_codes;
                        }
@@ -2748,11 +2827,21 @@ static modcallable *do_compile_modgroup(modcallable *parent,
                        rad_assert(parent != NULL);
                        p = mod_callabletogroup(parent);
 
-                       rad_assert(p->tail != NULL);
+                       if (!p->tail) goto elsif_fail;
 
+                       /*
+                        *      We're in the process of compiling the
+                        *      section, so the parent's tail is the
+                        *      previous "if" statement.
+                        */
                        f = mod_callabletogroup(p->tail);
-                       rad_assert((f->mc.type == MOD_IF) ||
-                                  (f->mc.type == MOD_ELSIF));
+                       if ((f->mc.type != MOD_IF) &&
+                           (f->mc.type != MOD_ELSIF)) {
+                       elsif_fail:
+                               cf_log_err_cs(g->cs, "Invalid location for 'elsif'.  There is no preceding 'if' statement");
+                               talloc_free(g);
+                               return NULL;
+                       }
 
                        /*
                         *      If we took the previous condition, we
@@ -2765,8 +2854,8 @@ static modcallable *do_compile_modgroup(modcallable *parent,
                        if (f->cond->type == COND_TYPE_TRUE) {
                        skip_true:
                                INFO(" # Skipping contents of '%s' as previous '%s' is always  'true' -- %s:%d",
-                                    group_name[g->mc.type],
-                                    group_name[f->mc.type],
+                                    unlang_keyword[g->mc.type],
+                                    unlang_keyword[f->mc.type],
                                     cf_section_filename(g->cs), cf_section_lineno(g->cs));
                                g->cond = f->cond;
                                goto set_codes;
@@ -2779,11 +2868,16 @@ static modcallable *do_compile_modgroup(modcallable *parent,
                        rad_assert(parent != NULL);
                        p = mod_callabletogroup(parent);
 
-                       rad_assert(p->tail != NULL);
+                       if (!p->tail) goto else_fail;
 
                        f = mod_callabletogroup(p->tail);
-                       rad_assert((f->mc.type == MOD_IF) ||
-                                  (f->mc.type == MOD_ELSIF));
+                       if ((f->mc.type != MOD_IF) &&
+                           (f->mc.type != MOD_ELSIF)) {
+                       else_fail:
+                               cf_log_err_cs(g->cs, "Invalid location for 'else'.  There is no preceding 'if' statement");
+                               talloc_free(g);
+                               return NULL;
+                       }
 
                        /*
                         *      If we took the previous condition, we
@@ -2879,10 +2973,10 @@ set_codes:
         */
        for (i = 0; i < RLM_MODULE_NUMCODES; i++) {
                if (!c->actions[i]) {
-                       if (!parent || (component != RLM_COMPONENT_AUTH)) {
+                       if (!parent || (component != MOD_AUTHENTICATE)) {
                                c->actions[i] = defaultactions[component][parentgrouptype][i];
                        } else { /* inside Auth-Type has different rules */
-                               c->actions[i] = defaultactions[RLM_COMPONENT_AUTZ][parentgrouptype][i];
+                               c->actions[i] = authtype_actions[parentgrouptype][i];
                        }
                }
        }
@@ -2918,7 +3012,7 @@ modcallable *compile_modgroup(modcallable *parent,
                                               GROUPTYPE_SIMPLE,
                                               GROUPTYPE_SIMPLE, MOD_GROUP);
 
-       if (debug_flag > 3) {
+       if (rad_debug_lvl > 3) {
                modcall_debug(ret, 2);
        }
 
@@ -2939,14 +3033,14 @@ void add_to_modcallable(modcallable *parent, modcallable *this)
 
 
 #ifdef WITH_UNLANG
-static bool pass2_xlat_compile(CONF_ITEM const *ci, value_pair_tmpl_t **pvpt, bool convert,
+static bool pass2_xlat_compile(CONF_ITEM const *ci, vp_tmpl_t **pvpt, bool convert,
                               DICT_ATTR const *da)
 {
        ssize_t slen;
        char *fmt;
        char const *error;
        xlat_exp_t *head;
-       value_pair_tmpl_t *vpt;
+       vp_tmpl_t *vpt;
 
        vpt = *pvpt;
 
@@ -2973,7 +3067,7 @@ static bool pass2_xlat_compile(CONF_ITEM const *ci, value_pair_tmpl_t **pvpt, bo
         *      Convert %{Attribute-Name} to &Attribute-Name
         */
        if (convert) {
-               value_pair_tmpl_t *attr;
+               vp_tmpl_t *attr;
 
                attr = xlat_to_tmpl_attr(talloc_parent(vpt), head);
                if (attr) {
@@ -2998,13 +3092,13 @@ static bool pass2_xlat_compile(CONF_ITEM const *ci, value_pair_tmpl_t **pvpt, bo
                        if (cf_item_is_pair(ci)) {
                                CONF_PAIR *cp = cf_item_to_pair(ci);
 
-                               WARN("%s[%d] Please change %%{%s} to &%s",
+                               WARN("%s[%d]: Please change \"%%{%s}\" to &%s",
                                       cf_pair_filename(cp), cf_pair_lineno(cp),
                                       attr->name, attr->name);
                        } else {
                                CONF_SECTION *cs = cf_item_to_section(ci);
 
-                               WARN("%s[%d] Please change %%{%s} to &%s",
+                               WARN("%s[%d]: Please change \"%%{%s}\" to &%s",
                                       cf_section_filename(cs), cf_section_lineno(cs),
                                       attr->name, attr->name);
                        }
@@ -3025,7 +3119,7 @@ static bool pass2_xlat_compile(CONF_ITEM const *ci, value_pair_tmpl_t **pvpt, bo
 
 
 #ifdef HAVE_REGEX
-static bool pass2_regex_compile(CONF_ITEM const *ci, value_pair_tmpl_t *vpt)
+static bool pass2_regex_compile(CONF_ITEM const *ci, vp_tmpl_t *vpt)
 {
        ssize_t slen;
        regex_t *preg;
@@ -3072,7 +3166,7 @@ static bool pass2_regex_compile(CONF_ITEM const *ci, value_pair_tmpl_t *vpt)
 }
 #endif
 
-static bool pass2_fixup_undefined(CONF_ITEM const *ci, value_pair_tmpl_t *vpt)
+static bool pass2_fixup_undefined(CONF_ITEM const *ci, vp_tmpl_t *vpt)
 {
        DICT_ATTR const *da;
 
@@ -3089,10 +3183,27 @@ static bool pass2_fixup_undefined(CONF_ITEM const *ci, value_pair_tmpl_t *vpt)
        return true;
 }
 
-static bool pass2_callback(UNUSED void *ctx, fr_cond_t *c)
+static bool pass2_callback(void *ctx, fr_cond_t *c)
 {
-       value_pair_map_t *map;
+       vp_map_t *map;
+       vp_tmpl_t *vpt;
 
+       /*
+        *      These don't get optimized.
+        */
+       if ((c->type == COND_TYPE_TRUE) ||
+           (c->type == COND_TYPE_FALSE)) {
+               return true;
+       }
+
+       /*
+        *      Call children.
+        */
+       if (c->type == COND_TYPE_CHILD) return pass2_callback(ctx, c->data.child);
+
+       /*
+        *      A few simple checks here.
+        */
        if (c->type == COND_TYPE_EXISTS) {
                if (c->data.vpt->type == TMPL_TYPE_XLAT) {
                        return pass2_xlat_compile(c->ci, &c->data.vpt, true, NULL);
@@ -3108,22 +3219,24 @@ static bool pass2_callback(UNUSED void *ctx, fr_cond_t *c)
                        if (!pass2_fixup_undefined(c->ci, c->data.vpt)) return false;
                        c->pass2_fixup = PASS2_FIXUP_NONE;
                }
-               return true;
-       }
 
-       /*
-        *      Maps have a paircompare fixup applied to them.
-        *      Others get ignored.
-        */
-       if (c->pass2_fixup == PASS2_FIXUP_NONE) {
-               if (c->type == COND_TYPE_MAP) {
-                       map = c->data.map;
-                       goto check_paircmp;
+               /*
+                *      Convert virtual &Attr-Foo to "%{Attr-Foo}"
+                */
+               vpt = c->data.vpt;
+               if ((vpt->type == TMPL_TYPE_ATTR) && vpt->tmpl_da->flags.virtual) {
+                       vpt->tmpl_xlat = xlat_from_tmpl_attr(vpt, vpt);
+                       vpt->type = TMPL_TYPE_XLAT_STRUCT;
                }
 
                return true;
        }
 
+       /*
+        *      And tons of complicated checks.
+        */
+       rad_assert(c->type == COND_TYPE_MAP);
+
        map = c->data.map;      /* shorter */
 
        /*
@@ -3160,7 +3273,6 @@ static bool pass2_callback(UNUSED void *ctx, fr_cond_t *c)
                c->pass2_fixup = PASS2_FIXUP_NONE;
        }
 
-check_paircmp:
        /*
         *      Just in case someone adds a new fixup later.
         */
@@ -3172,13 +3284,88 @@ check_paircmp:
         */
        if (map->lhs->type == TMPL_TYPE_XLAT) {
                /*
-                *      Don't compile the LHS to an attribute
-                *      reference for now.  When we do that, we've got
-                *      to check the RHS for type-specific data, and
-                *      parse it to a TMPL_TYPE_DATA.
+                *      Compile the LHS to an attribute reference only
+                *      if the RHS is a literal.
+                *
+                *      @todo v3.1: allow anything anywhere.
                 */
-               if (!pass2_xlat_compile(map->ci, &map->lhs, false, NULL)) {
-                       return false;
+               if (map->rhs->type != TMPL_TYPE_LITERAL) {
+                       if (!pass2_xlat_compile(map->ci, &map->lhs, false, NULL)) {
+                               return false;
+                       }
+               } else {
+                       if (!pass2_xlat_compile(map->ci, &map->lhs, true, NULL)) {
+                               return false;
+                       }
+
+                       /*
+                        *      Attribute compared to a literal gets
+                        *      the literal cast to the data type of
+                        *      the attribute.
+                        *
+                        *      The code in parser.c did this for
+                        *
+                        *              &Attr == data
+                        *
+                        *      But now we've just converted "%{Attr}"
+                        *      to &Attr, so we've got to do it again.
+                        */
+                       if ((map->lhs->type == TMPL_TYPE_ATTR) &&
+                           (map->rhs->type == TMPL_TYPE_LITERAL)) {
+                               /*
+                                *      RHS is hex, try to parse it as
+                                *      type-specific data.
+                                */
+                               if (map->lhs->auto_converted &&
+                                   (map->rhs->name[0] == '0') && (map->rhs->name[1] == 'x') &&
+                                   (map->rhs->len > 2) && ((map->rhs->len & 0x01) == 0)) {
+                                       vpt = map->rhs;
+                                       map->rhs = NULL;
+
+                                       if (!map_cast_from_hex(map, T_BARE_WORD, vpt->name)) {
+                                               map->rhs = vpt;
+                                               cf_log_err(map->ci, "Cannot parse RHS hex as the data type of the attribute %s", map->lhs->tmpl_da->name);
+                                               return -1;
+                                       }
+                                       talloc_free(vpt);
+
+                               } else if ((map->rhs->len > 0) ||
+                                          (map->op != T_OP_CMP_EQ) ||
+                                          (map->lhs->tmpl_da->type == PW_TYPE_STRING) ||
+                                          (map->lhs->tmpl_da->type == PW_TYPE_OCTETS)) {
+
+                                       if (tmpl_cast_in_place(map->rhs, map->lhs->tmpl_da->type, map->lhs->tmpl_da) < 0) {
+                                               cf_log_err(map->ci, "Failed to parse data type %s from string: %s",
+                                                          fr_int2str(dict_attr_types, map->lhs->tmpl_da->type, "<UNKNOWN>"),
+                                                          map->rhs->name);
+                                               return false;
+                                       } /* else the cast was successful */
+
+                               } else {        /* RHS is empty, it's just a check for empty / non-empty string */
+                                       vpt = talloc_steal(c, map->lhs);
+                                       map->lhs = NULL;
+                                       talloc_free(c->data.map);
+
+                                       /*
+                                        *      "%{Foo}" == '' ---> !Foo
+                                        *      "%{Foo}" != '' ---> Foo
+                                        */
+                                       c->type = COND_TYPE_EXISTS;
+                                       c->data.vpt = vpt;
+                                       c->negate = !c->negate;
+
+                                       WARN("%s[%d]: Please change (\"%%{%s}\" %s '') to %c&%s",
+                                            cf_section_filename(cf_item_to_section(c->ci)),
+                                            cf_section_lineno(cf_item_to_section(c->ci)),
+                                            vpt->name, c->negate ? "==" : "!=",
+                                            c->negate ? '!' : ' ', vpt->name);
+
+                                       /*
+                                        *      No more RHS, so we can't do more optimizations
+                                        */
+                                       return true;
+                               }
+                       }
                }
        }
 
@@ -3216,11 +3403,10 @@ check_paircmp:
            (strncmp(map->lhs->name, "Foreach-Variable-", 17) == 0)) {
                char *fmt;
                ssize_t slen;
-               value_pair_tmpl_t *vpt;
 
                fmt = talloc_asprintf(map->lhs, "%%{%s}", map->lhs->name);
                slen = tmpl_afrom_str(map, &vpt, fmt, talloc_array_length(fmt) - 1,
-                                     T_DOUBLE_QUOTED_STRING, REQUEST_CURRENT, PAIR_LIST_REQUEST);
+                                     T_DOUBLE_QUOTED_STRING, REQUEST_CURRENT, PAIR_LIST_REQUEST, true);
                if (slen < 0) {
                        char *spaces, *text;
 
@@ -3250,6 +3436,32 @@ check_paircmp:
 #endif
 
        /*
+        *      Convert &Packet-Type to "%{Packet-Type}", because
+        *      these attributes don't really exist.  The code to
+        *      find an attribute reference doesn't work, but the
+        *      xlat code does.
+        */
+       vpt = c->data.map->lhs;
+       if ((vpt->type == TMPL_TYPE_ATTR) && vpt->tmpl_da->flags.virtual) {
+               if (!c->cast) c->cast = vpt->tmpl_da;
+               vpt->tmpl_xlat = xlat_from_tmpl_attr(vpt, vpt);
+               vpt->type = TMPL_TYPE_XLAT_STRUCT;
+       }
+
+       /*
+        *      Convert RHS to expansions, too.
+        */
+       vpt = c->data.map->rhs;
+       if ((vpt->type == TMPL_TYPE_ATTR) && vpt->tmpl_da->flags.virtual) {
+               vpt->tmpl_xlat = xlat_from_tmpl_attr(vpt, vpt);
+               vpt->type = TMPL_TYPE_XLAT_STRUCT;
+       }
+
+       /*
+        *      @todo v3.1: do the same thing for the RHS...
+        */
+
+       /*
         *      Only attributes can have a paircompare registered, and
         *      they can only be with the current REQUEST, and only
         *      with the request pairs.
@@ -3288,7 +3500,7 @@ check_paircmp:
 
        /*
         *      Mark it as requiring a paircompare() call, instead of
-        *      paircmp().
+        *      fr_pair_cmp().
         */
        c->pass2_fixup = PASS2_PAIRCOMPARE;
 
@@ -3301,7 +3513,7 @@ check_paircmp:
  */
 static bool modcall_pass2_update(modgroup *g)
 {
-       value_pair_map_t *map;
+       vp_map_t *map;
 
        for (map = g->map; map != NULL; map = map->next) {
                if (map->rhs->type == TMPL_TYPE_XLAT) {
@@ -3353,11 +3565,11 @@ bool modcall_pass2(modcallable *mc)
 #ifdef WITH_UNLANG
                case MOD_UPDATE:
                        g = mod_callabletogroup(c);
-                       if (g->done_pass2) return true;
+                       if (g->done_pass2) goto do_next;
 
                        name2 = cf_section_name2(g->cs);
                        if (!name2) {
-                               c->debug_name = group_name[c->type];
+                               c->debug_name = unlang_keyword[c->type];
                        } else {
                                c->debug_name = talloc_asprintf(c, "update %s", name2);
                        }
@@ -3382,10 +3594,10 @@ bool modcall_pass2(modcallable *mc)
                case MOD_IF:
                case MOD_ELSIF:
                        g = mod_callabletogroup(c);
-                       if (g->done_pass2) return true;
+                       if (g->done_pass2) goto do_next;
 
                        name2 = cf_section_name2(g->cs);
-                       c->debug_name = talloc_asprintf(c, "%s %s", group_name[c->type], name2);
+                       c->debug_name = talloc_asprintf(c, "%s %s", unlang_keyword[c->type], name2);
 
                        /*
                         *      The compilation code takes care of
@@ -3409,10 +3621,10 @@ bool modcall_pass2(modcallable *mc)
 #ifdef WITH_UNLANG
                case MOD_SWITCH:
                        g = mod_callabletogroup(c);
-                       if (g->done_pass2) return true;
+                       if (g->done_pass2) goto do_next;
 
                        name2 = cf_section_name2(g->cs);
-                       c->debug_name = talloc_asprintf(c, "%s %s", group_name[c->type], name2);
+                       c->debug_name = talloc_asprintf(c, "%s %s", unlang_keyword[c->type], name2);
 
                        /*
                         *      We had &Foo-Bar, where Foo-Bar is
@@ -3425,7 +3637,7 @@ bool modcall_pass2(modcallable *mc)
 
                                slen = tmpl_afrom_str(g->cs, &g->vpt, c->name, strlen(c->name),
                                                      cf_section_name2_type(g->cs),
-                                                     REQUEST_CURRENT, PAIR_LIST_REQUEST);
+                                                     REQUEST_CURRENT, PAIR_LIST_REQUEST, true);
                                if (slen < 0) {
                                        char *spaces, *text;
 
@@ -3458,6 +3670,14 @@ bool modcall_pass2(modcallable *mc)
                        }
 
                        /*
+                        *      Convert virtual &Attr-Foo to "%{Attr-Foo}"
+                        */
+                       if ((g->vpt->type == TMPL_TYPE_ATTR) && g->vpt->tmpl_da->flags.virtual) {
+                               g->vpt->tmpl_xlat = xlat_from_tmpl_attr(g->vpt, g->vpt);
+                               g->vpt->type = TMPL_TYPE_XLAT_STRUCT;
+                       }
+
+                       /*
                         *      We may have: switch Foo-Bar {
                         *
                         *      where Foo-Bar is an attribute defined
@@ -3467,10 +3687,10 @@ bool modcall_pass2(modcallable *mc)
                         *      switch to using that.
                         */
                        if (g->vpt->type == TMPL_TYPE_LITERAL) {
-                               value_pair_tmpl_t *vpt;
+                               vp_tmpl_t *vpt;
 
                                slen = tmpl_afrom_str(g->cs, &vpt, c->name, strlen(c->name), cf_section_name2_type(g->cs),
-                                                     REQUEST_CURRENT, PAIR_LIST_REQUEST);
+                                                     REQUEST_CURRENT, PAIR_LIST_REQUEST, true);
                                if (slen < 0) goto parse_error;
                                if (vpt->type == TMPL_TYPE_ATTR) {
                                        talloc_free(g->vpt);
@@ -3489,9 +3709,9 @@ bool modcall_pass2(modcallable *mc)
                        if ((g->vpt->type == TMPL_TYPE_ATTR) &&
                            (c->name[0] != '&')) {
                                WARN("%s[%d]: Please change %s to &%s",
-                                      cf_section_filename(g->cs),
-                                      cf_section_lineno(g->cs),
-                                      c->name, c->name);
+                                    cf_section_filename(g->cs),
+                                    cf_section_lineno(g->cs),
+                                    c->name, c->name);
                        }
 
                do_children:
@@ -3501,13 +3721,13 @@ bool modcall_pass2(modcallable *mc)
 
                case MOD_CASE:
                        g = mod_callabletogroup(c);
-                       if (g->done_pass2) return true;
+                       if (g->done_pass2) goto do_next;
 
                        name2 = cf_section_name2(g->cs);
                        if (!name2) {
-                               c->debug_name = group_name[c->type];
+                               c->debug_name = unlang_keyword[c->type];
                        } else {
-                               c->debug_name = talloc_asprintf(c, "%s %s", group_name[c->type], name2);
+                               c->debug_name = talloc_asprintf(c, "%s %s", unlang_keyword[c->type], name2);
                        }
 
                        rad_assert(c->parent != NULL);
@@ -3524,7 +3744,7 @@ bool modcall_pass2(modcallable *mc)
                            (cf_section_name2_type(g->cs) == T_BARE_WORD)) {
                                slen = tmpl_afrom_str(g->cs, &g->vpt, c->name, strlen(c->name),
                                                      cf_section_name2_type(g->cs),
-                                                     REQUEST_CURRENT, PAIR_LIST_REQUEST);
+                                                     REQUEST_CURRENT, PAIR_LIST_REQUEST, true);
                                if (slen < 0) goto parse_error;
                        }
 
@@ -3552,7 +3772,7 @@ bool modcall_pass2(modcallable *mc)
                                if (f->vpt->type == TMPL_TYPE_ATTR) {
                                        rad_assert(f->vpt->tmpl_da != NULL);
 
-                                       if (!tmpl_cast_in_place(g->vpt, f->vpt->tmpl_da->type, f->vpt->tmpl_da)) {
+                                       if (tmpl_cast_in_place(g->vpt, f->vpt->tmpl_da->type, f->vpt->tmpl_da) < 0) {
                                                cf_log_err_cs(g->cs, "Invalid argument for case statement: %s",
                                                              fr_strerror());
                                                return false;
@@ -3562,6 +3782,12 @@ bool modcall_pass2(modcallable *mc)
                                goto do_children;
                        }
 
+                       if (g->vpt->type == TMPL_TYPE_ATTR_UNDEFINED) {
+                               if (!pass2_fixup_undefined(cf_section_to_item(g->cs), g->vpt)) {
+                                       return false;
+                               }
+                       }
+
                        /*
                         *      Compile and sanity check xlat
                         *      expansions.
@@ -3589,16 +3815,24 @@ bool modcall_pass2(modcallable *mc)
                                }
                        }
 
+                       /*
+                        *      Virtual attribute fixes for "case" statements, too.
+                        */
+                       if ((g->vpt->type == TMPL_TYPE_ATTR) && g->vpt->tmpl_da->flags.virtual) {
+                               g->vpt->tmpl_xlat = xlat_from_tmpl_attr(g->vpt, g->vpt);
+                               g->vpt->type = TMPL_TYPE_XLAT_STRUCT;
+                       }
+
                        if (!modcall_pass2(g->children)) return false;
                        g->done_pass2 = true;
                        break;
 
                case MOD_FOREACH:
                        g = mod_callabletogroup(c);
-                       if (g->done_pass2) return true;
+                       if (g->done_pass2) goto do_next;
 
                        name2 = cf_section_name2(g->cs);
-                       c->debug_name = talloc_asprintf(c, "%s %s", group_name[c->type], name2);
+                       c->debug_name = talloc_asprintf(c, "%s %s", unlang_keyword[c->type], name2);
 
                        /*
                         *      Already parsed, handle the children.
@@ -3620,7 +3854,7 @@ bool modcall_pass2(modcallable *mc)
                         *      Check for that now.
                         */
                        slen = tmpl_afrom_str(g->cs, &g->vpt, c->name, strlen(c->name), cf_section_name2_type(g->cs),
-                                             REQUEST_CURRENT, PAIR_LIST_REQUEST);
+                                             REQUEST_CURRENT, PAIR_LIST_REQUEST, true);
                        if (slen < 0) goto parse_error;
 
                check_children:
@@ -3634,19 +3868,19 @@ bool modcall_pass2(modcallable *mc)
                        break;
 
                case MOD_ELSE:
-                       c->debug_name = group_name[c->type];
+                       c->debug_name = unlang_keyword[c->type];
                        goto do_recurse;
 
                case MOD_POLICY:
                        g = mod_callabletogroup(c);
-                       c->debug_name = talloc_asprintf(c, "%s %s", group_name[c->type], cf_section_name1(g->cs));
+                       c->debug_name = talloc_asprintf(c, "%s %s", unlang_keyword[c->type], cf_section_name1(g->cs));
                        goto do_recurse;
 #endif
 
                case MOD_GROUP:
                case MOD_LOAD_BALANCE:
                case MOD_REDUNDANT_LOAD_BALANCE:
-                       c->debug_name = group_name[c->type];
+                       c->debug_name = unlang_keyword[c->type];
 
 #ifdef WITH_UNLANG
                do_recurse:
@@ -3658,16 +3892,25 @@ bool modcall_pass2(modcallable *mc)
                        } else if (c->type == MOD_GROUP) { /* for Auth-Type, etc. */
                                char const *name1 = cf_section_name1(g->cs);
 
-                               if (strcmp(name1, group_name[c->type]) != 0) {
-                                       c->debug_name = talloc_asprintf(c, "%s %s", name1, cf_section_name2(g->cs));
+                               if (strcmp(name1, unlang_keyword[c->type]) != 0) {
+                                       name2 = cf_section_name2(g->cs);
+
+                                       if (!name2) {
+                                               c->debug_name = name1;
+                                       } else {
+                                               c->debug_name = talloc_asprintf(c, "%s %s", name1, name2);
+                                       }
                                }
                        }
 
-                       if (g->done_pass2) return true;
+                       if (g->done_pass2) goto do_next;
                        if (!modcall_pass2(g->children)) return false;
                        g->done_pass2 = true;
                        break;
                }
+
+       do_next:
+               rad_assert(c->debug_name != NULL);
        }
 
        return true;
@@ -3677,7 +3920,7 @@ void modcall_debug(modcallable *mc, int depth)
 {
        modcallable *this;
        modgroup *g;
-       value_pair_map_t *map;
+       vp_map_t *map;
        char buffer[1024];
 
        for (this = mc; this != NULL; this = this->next) {
@@ -3697,7 +3940,7 @@ void modcall_debug(modcallable *mc, int depth)
                case MOD_UPDATE:
                        g = mod_callabletogroup(this);
                        DEBUG("%.*s%s {", depth, modcall_spaces,
-                               group_name[this->type]);
+                               unlang_keyword[this->type]);
 
                        for (map = g->map; map != NULL; map = map->next) {
                                map_prints(buffer, sizeof(buffer), map);
@@ -3710,7 +3953,7 @@ void modcall_debug(modcallable *mc, int depth)
                case MOD_ELSE:
                        g = mod_callabletogroup(this);
                        DEBUG("%.*s%s {", depth, modcall_spaces,
-                               group_name[this->type]);
+                               unlang_keyword[this->type]);
                        modcall_debug(g->children, depth + 1);
                        DEBUG("%.*s}", depth, modcall_spaces);
                        break;
@@ -3720,7 +3963,7 @@ void modcall_debug(modcallable *mc, int depth)
                        g = mod_callabletogroup(this);
                        fr_cond_sprint(buffer, sizeof(buffer), g->cond);
                        DEBUG("%.*s%s (%s) {", depth, modcall_spaces,
-                               group_name[this->type], buffer);
+                               unlang_keyword[this->type], buffer);
                        modcall_debug(g->children, depth + 1);
                        DEBUG("%.*s}", depth, modcall_spaces);
                        break;
@@ -3730,7 +3973,7 @@ void modcall_debug(modcallable *mc, int depth)
                        g = mod_callabletogroup(this);
                        tmpl_prints(buffer, sizeof(buffer), g->vpt, NULL);
                        DEBUG("%.*s%s %s {", depth, modcall_spaces,
-                               group_name[this->type], buffer);
+                               unlang_keyword[this->type], buffer);
                        modcall_debug(g->children, depth + 1);
                        DEBUG("%.*s}", depth, modcall_spaces);
                        break;
@@ -3739,7 +3982,7 @@ void modcall_debug(modcallable *mc, int depth)
                case MOD_FOREACH:
                        g = mod_callabletogroup(this);
                        DEBUG("%.*s%s %s {", depth, modcall_spaces,
-                               group_name[this->type], this->name);
+                               unlang_keyword[this->type], this->name);
                        modcall_debug(g->children, depth + 1);
                        DEBUG("%.*s}", depth, modcall_spaces);
                        break;
@@ -3752,7 +3995,7 @@ void modcall_debug(modcallable *mc, int depth)
                case MOD_GROUP:
                        g = mod_callabletogroup(this);
                        DEBUG("%.*s%s {", depth, modcall_spaces,
-                             group_name[this->type]);
+                             unlang_keyword[this->type]);
                        modcall_debug(g->children, depth + 1);
                        DEBUG("%.*s}", depth, modcall_spaces);
                        break;
@@ -3762,10 +4005,17 @@ void modcall_debug(modcallable *mc, int depth)
                case MOD_REDUNDANT_LOAD_BALANCE:
                        g = mod_callabletogroup(this);
                        DEBUG("%.*s%s {", depth, modcall_spaces,
-                               group_name[this->type]);
+                               unlang_keyword[this->type]);
                        modcall_debug(g->children, depth + 1);
                        DEBUG("%.*s}", depth, modcall_spaces);
                        break;
                }
        }
 }
+
+int modcall_pass2_condition(fr_cond_t *c)
+{
+       if (!fr_condition_walk(c, pass2_callback, NULL)) return -1;
+
+       return 0;
+}