update / fix restrictions on retries.

diff --git a/src/lib/server/module.c b/src/lib/server/module.c
index 169d95a6e13c47de3a1f8989dce41f836e237943..e6781a0407f46eefec5d8f3cb2cb985594a41ad5 100644 (file)
--- a/src/lib/server/module.c
+++ b/src/lib/server/module.c
@@ -1567,16 +1567,7 @@ module_instance_t *module_bootstrap(module_instance_t const *parent, CONF_SECTIO
         *      Compile the default "actions" subsection, which includes retries.
         */
        actions = cf_section_find(cs, "actions", NULL);
-       if (actions && !unlang_compile_actions(&mi->actions, actions)) {
-               talloc_free(mi);
-               return NULL;
-       }
-
-       /*
-        *      If the module isn't marked as "retry safe", then disallow retries.
-        */
-       if (mi->actions.retry.irt && ((mi->module->type & RLM_TYPE_RETRY) != 0)) {
-               cf_log_err(cs, "Cannot do retries for module \"%s\" - it does not support them", mi->name);
+       if (actions && unlang_compile_actions(&mi->actions, actions, (mi->module->type & RLM_TYPE_RETRY) != 0)) {
                talloc_free(mi);
                return NULL;
        }

diff --git a/src/lib/unlang/compile.c b/src/lib/unlang/compile.c
index dc79b86a22b63d5574a5ac1c7f5918a0296b9207..e17137d3d52963866e67e292569e2a0abb3841a5 100644 (file)
--- a/src/lib/unlang/compile.c
+++ b/src/lib/unlang/compile.c
@@ -1575,24 +1575,15 @@ static bool compile_retry_section(unlang_actions_t *actions, CONF_ITEM *ci)
                        cf_log_err(csi, "Invalid item '%s' in 'retry' configuration.", name);
                        return false;
                }
-
-               /*
-                *      Sanity check the values.  If IRT is zero, then
-                *      the rest MUST be zero.
-                */
-               if (!actions->retry.irt &&
-                   (actions->retry.mrt || actions->retry.mrc || actions->retry.mrd)) {
-                       cf_log_err(csi, "If 'initial_rtx_time' is zero, then the other fields MUST be zero");
-                       return false;
-               }
        }
 
        return true;
 }
 
-bool unlang_compile_actions(unlang_actions_t *actions, CONF_SECTION *action_cs)
+bool unlang_compile_actions(unlang_actions_t *actions, CONF_SECTION *action_cs, bool module_retry)
 {
        int i;
+       bool disallow_retry_action = false;
        CONF_ITEM *csi;
        CONF_SECTION *cs;
 
@@ -1650,6 +1641,22 @@ bool unlang_compile_actions(unlang_actions_t *actions, CONF_SECTION *action_cs)
                }
        }
 
+       if (module_retry) {
+               if (!actions->retry.irt) {
+                       cf_log_err(csi, "initial_rtx_time MUST be non-zero for modules which support retries.");
+                       return false;
+               }
+       } else {
+               if (actions->retry.irt) {
+                       cf_log_err(csi, "initial_rtx_time MUST be zero, as only max_rtx_count and max_rtx_duration are used.");
+                       return false;
+               }
+
+               if (!actions->retry.mrc && !actions->retry.mrd) {
+                       disallow_retry_action = true;
+               }
+       }
+
        /*
         *      Sanity check that "fail = retry", we actually have a
         *      retry section.
@@ -1657,7 +1664,19 @@ bool unlang_compile_actions(unlang_actions_t *actions, CONF_SECTION *action_cs)
        for (i = 0; i < RLM_MODULE_NUMCODES; i++) {
                if (actions->actions[i] != MOD_ACTION_RETRY) continue;
 
-               if (!actions->retry.irt) {
+               if (module_retry) {
+                       cf_log_err(csi, "Cannot use a '%s = retry' action for a module which has its own retries",
+                                  fr_table_str_by_value(mod_rcode_table, i, "???"));
+                       return false;
+               }
+
+               if (disallow_retry_action) {
+                       cf_log_err(csi, "max_rtx_count and max_rtx_duration cannot both be zero when using '%s = retry'",
+                                  fr_table_str_by_value(mod_rcode_table, i, "???"));
+                       return false;
+               }
+
+               if (!actions->retry.irt && !actions->retry.mrc && !actions->retry.mrd) {
                        cf_log_err(csi, "Cannot use a '%s = retry' action without a 'retry { ... }' section.",
                                   fr_table_str_by_value(mod_rcode_table, i, "???"));
                        return false;
@@ -1750,7 +1769,7 @@ static bool compile_action_subsection(unlang_t *c, CONF_SECTION *cs, CONF_SECTIO
                return false;
        }
 
-       return unlang_compile_actions(&c->actions, subcs);
+       return unlang_compile_actions(&c->actions, subcs, false);
 }
 
 
@@ -3452,7 +3471,7 @@ static unlang_t *compile_function(unlang_t *parent, unlang_compile_t *unlang_ctx
         *      Else we have a reference to a policy, and that reference
         *      over-rides the return codes for the policy!
         */
-       if (!unlang_compile_actions(&c->actions, cf_item_to_section(ci))) {
+       if (!unlang_compile_actions(&c->actions, cf_item_to_section(ci), false)) {
                talloc_free(c);
                return NULL;
        }
@@ -3645,25 +3664,11 @@ static unlang_t *compile_module(unlang_t *parent, unlang_compile_t *unlang_ctx,
         *      If a module reference is a section, then the section
         *      should contain action over-rides.  We add those here.
         */
-       if (cf_item_is_section(ci)) {
-               if (!unlang_compile_actions(&c->actions, cf_item_to_section(ci))) {
-                       talloc_free(c);
-                       return NULL;
-               }
-       }
-
-       /*
-        *      If we're retrying this section, then all modules in
-        *      the section have to be marked as retry-safe.
-        *
-        *      src/lib/server/module.c already checks if the default
-        *      module actions are retry safe, so we don't need to
-        *      check that here.
-        */
-       if (unlang_ctx->actions.retry.irt && ((inst->module->type & RLM_TYPE_RETRY) != 0)) {
-               cf_log_err(ci, "Cannot do retries for module \"%s\" - it does not support them", inst->module->name);
-               talloc_free(c);
-               return NULL;
+       if (cf_item_is_section(ci) &&
+           !unlang_compile_actions(&c->actions, cf_item_to_section(ci),
+                                   (inst->module->type & RLM_TYPE_RETRY) != 0)) {
+                   talloc_free(c);
+                   return NULL;
        }
 
        return c;

diff --git a/src/lib/unlang/compile.h b/src/lib/unlang/compile.h
index 25995cf677c3cea75177f4ee980610782856736a..1e7b579ee107b505832b6ad124c08d7637f341c6 100644 (file)
--- a/src/lib/unlang/compile.h
+++ b/src/lib/unlang/compile.h
@@ -42,7 +42,7 @@ int           unlang_compile(CONF_SECTION *cs, rlm_components_t component, tmpl_rules_t c
 
 bool           unlang_compile_is_keyword(const char *name);
 
-bool           unlang_compile_actions(unlang_actions_t *actions, CONF_SECTION *parent);
+bool           unlang_compile_actions(unlang_actions_t *actions, CONF_SECTION *parent, bool module_retry);
 
 #ifdef __cplusplus
 }