]> git.ipfire.org Git - thirdparty/freeradius-server.git/commitdiff
catch many many more corner cases with 'try'
authorAlan T. DeKok <aland@freeradius.org>
Mon, 28 Jul 2025 00:27:42 +0000 (20:27 -0400)
committerAlan T. DeKok <aland@freeradius.org>
Mon, 28 Jul 2025 00:37:28 +0000 (20:37 -0400)
if the input compile_ctx has REJECT or RETURN for an action, AND
there's a "try / catch" for it, we do NOT have the try actions
stay "REJECT" or "RETURN"

also change the behavior so catch { ... } means "catch ALL" of the
rcodes.

the error rcodes are special, too :(

doc/antora/modules/reference/pages/unlang/catch.adoc
doc/antora/modules/reference/pages/unlang/try.adoc
src/lib/unlang/catch.c
src/lib/unlang/try.c

index 70f43d0977db4367414f2cc29092f2037c0ba742..9539366951c1620f2bb5d46c43fe41353aae60bd 100644 (file)
@@ -11,16 +11,17 @@ catch [ <return-codes> ] {
 }
 ----
 
-The `catch` statement runs a series of substatements in a block, but only if the previous xref:unlang/try.adoc[try] failed.
+The `catch` statement runs a series of substatements in a block, but
+only if the previous xref:unlang/try.adoc[try] failed.
 
-<return codes>:: Zero or more xref:unlang/condition/return_codes.adoc[return codes].  Multiple return codes are separated by commas.
+<return codes>:: Zero or more xref:unlang/condition/return_codes.adoc[return codes].  Multiple return codes are separated by spaces.
 
 [ statements ]:: The `unlang` commands which will be executed.  A `catch` block can be empty.
 
-If no return code is given, then the `catch` statement will match all of the return codes.  Otherwise, the `catch` statement matches only the return codes which were listed.
-
 Multiple `catch` statements can be placed one after the other, to `catch` different errors.  Only one of the statements will be executed.  Once a `catch` statement is finished, the interpreter will skip all trailing `catch` statements, and continue execution with the next statement.
 
+As a special case, the final 'catch' statement can list no return codes.  i.e `catch { ... }`.  In that case, it will match all return codes which were not listed in previous 'catch' statements.
+
 .Example
 
 [source,unlang]
@@ -43,6 +44,8 @@ catch invalid {   # skipped after "catch fail" is run.
 }
 ----
 
+## try / catch versus redundant
+
 There is some overlap in functionality between xref:unlang/try.adoc[try] / `catch` and xref:unlang/redundant.adoc[redundant].  The main difference is that a xref:unlang/catch.adoc[catch] statement can catch specific failure codes.
 
 The xref:unlang/redundant.adoc[redundant] statement should be used to run
@@ -52,5 +55,5 @@ In contrast, the `try` / `catch` statements should be used for more complex poli
 
 The `try` / `catch` statements can also run different statements for each failure code, which is not possible with xref:unlang/redundant.adoc[redundant].
 
-// Copyright (C) 2023 Network RADIUS SAS.  Licenced under CC-by-NC 4.0.
+// Copyright (C) 2025 Network RADIUS SAS.  Licenced under CC-by-NC 4.0.
 // This documentation was developed by Network RADIUS SAS.
index 31224dfeffd390ce507c53bd137a204f30b52184..bbe171375efe3f0265be990b633d9090d2246b72 100644 (file)
@@ -6,21 +6,24 @@
 try {
     [ statements ]
 }
-catch {
+catch [ <return-codes> ] {
+    ...
+}
+catch [ <return-codes> ] {
     ...
 }
 ----
 
 The `try` statement runs a series of substatements in a block.  If the
-block returns an error such as `fail`, `reject`, `invalid`, or
-`disallow`, a subsequent xref:unlang/catch.adoc[catch] block is
-executed.
+block returns an error such as `fail`, `reject`, `invalid`,
+`disallow`, or `timeout`, a subsequent xref:unlang/catch.adoc[catch]
+block is executed.
 
 [ statements ]:: The `unlang` commands which will be executed.  A
 `try` block cannot be empty.
 
-Every `try` block must be followed by a xref:unlang/catch.adoc[catch]
-block.
+Every `try` block must be followed by at least one
+xref:unlang/catch.adoc[catch] block.
 
 .Example
 
@@ -29,14 +32,14 @@ block.
 try {
        sql
 }
-catch {
+catch fail {
        # ... run only if sql failed
 
        ok      # over-ride the "fail" code
 }
 ----
 
-There is some overlap in functionality between `try` / xref:unlang/catch.adoc[catch] and xref:unlang/redundant.adoc[redundant].  The main difference (TODO) is that a xref:unlang/catch.adoc[catch] statement can catch specific actions.
+There is some overlap in functionality between `try` / xref:unlang/catch.adoc[catch] and xref:unlang/redundant.adoc[redundant].  See the xref:unlang/catch.adoc[catch] documentation for more details.
 
-// Copyright (C) 2023 Network RADIUS SAS.  Licenced under CC-by-NC 4.0.
+// Copyright (C) 2025 Network RADIUS SAS.  Licenced under CC-by-NC 4.0.
 // This documentation was developed by Network RADIUS SAS.
index 102526e3eb46b52eceb1b4a8f59854bc875931ef..d2673ba3b6cedded9b57cd6c6755229a41ed3791 100644 (file)
@@ -34,24 +34,18 @@ static unlang_action_t unlang_catch(UNUSED unlang_result_t *p_result, request_t
 }
 
 
-static int catch_argv(CONF_SECTION *cs, unlang_try_t *gext, char const *name, unlang_t *c)
+/*
+ *     Sanity checks have already been done by compile_try().
+ */
+static void catch_argv(unlang_try_t *gext, char const *name, unlang_t *c)
 {
-       int rcode;
+       rlm_rcode_t rcode;
 
-       rcode = fr_table_value_by_str(mod_rcode_table, name, -1);
-       if (rcode < 0) {
-               cf_log_err(cs, "Unknown rcode '%s'.", name);
-               return -1;
-       }
-
-       if (gext->catch[rcode]) {
-               cf_log_err(cs, "Duplicate rcode '%s'.", name);
-               return -1;
-       }
+       rcode = fr_table_value_by_str(mod_rcode_table, name, RLM_MODULE_NOT_SET);
+       fr_assert(rcode != RLM_MODULE_NOT_SET);
+       fr_assert(!gext->catch[rcode]);
 
        gext->catch[rcode] = c;
-
-       return 0;
 }
 
 static unlang_t *unlang_compile_catch(unlang_t *parent, unlang_compile_ctx_t *unlang_ctx, CONF_ITEM const *ci)
@@ -90,30 +84,28 @@ static unlang_t *unlang_compile_catch(unlang_t *parent, unlang_compile_ctx_t *un
         */
        c->debug_name = c->name = talloc_typed_asprintf(c, "%s %s", cf_section_name1(cs), cf_section_name2(cs));
 
+       /*
+        *      catch { ... } has to be the last one, and will catch _all_ rcodes that weren't mentioned
+        *      before.
+        */
        if (!cf_section_name2(cs)) {
-               /*
-                *      No arg2: catch errors
-                */
-               gext->catch[RLM_MODULE_REJECT] = c;
-               gext->catch[RLM_MODULE_FAIL] = c;
-               gext->catch[RLM_MODULE_INVALID] = c;
-               gext->catch[RLM_MODULE_DISALLOW] = c;
+               int i;
+
+               for (i = 0; i < RLM_MODULE_NUMCODES; i++) {
+                       if (gext->catch[i]) continue;
+
+                       gext->catch[i] = c;
+               }
 
        } else {
                int i;
 
                name = cf_section_name2(cs);
 
-               if (catch_argv(cs, gext, name, c) < 0) {
-                       talloc_free(c);
-                       return NULL;
-               }
+               catch_argv(gext, name, c);
 
                for (i = 0; (name = cf_section_argv(cs, i)) != NULL; i++) {
-                       if (catch_argv(cs, gext, name, c) < 0) {
-                               talloc_free(c);
-                               return NULL;
-                       }
+                       catch_argv(gext, name, c);
                }
        }
 
index 4f47f0a45a56f97cfc781e0023a85718c7fdb480..3d60ced7fdaebfe3c0b4ec05aa8adbd0f6e103c1 100644 (file)
@@ -74,6 +74,10 @@ static unlang_t *unlang_compile_try(unlang_t *parent, unlang_compile_ctx_t *unla
        unlang_group_t  *g;
        unlang_t        *c;
        CONF_SECTION    *parent_cs, *next;
+       int             i;
+       CONF_SECTION    *default_catch = NULL;
+       unlang_compile_ctx_t unlang_ctx2;
+       CONF_SECTION    *catcher[RLM_MODULE_NUMCODES] = {};
 
        /*
         *      The transaction is empty, ignore it.
@@ -105,7 +109,144 @@ static unlang_t *unlang_compile_try(unlang_t *parent, unlang_compile_ctx_t *unla
        c = unlang_group_to_generic(g);
        c->debug_name = c->name = cf_section_name1(cs);
 
-       return unlang_compile_children(g, unlang_ctx);
+       /*
+        *      Whatever the input compile ctx is, we over-ride the default actions.
+        */
+       unlang_compile_ctx_copy(&unlang_ctx2, unlang_ctx);
+
+       /*
+        *      Loop over all of the "catch" sections, figuring out which rcodes are being "catch"ed.
+        */
+       for (default_catch = NULL;
+            next && (strcmp(cf_section_name1(next), "catch") == 0);
+            next = cf_section_next(parent_cs, next)) {
+               rlm_rcode_t rcode;
+               char const *name;
+
+               /*
+                *      catch { ... } is the default, and we can't
+                *      have anything after it.
+                */
+               if (default_catch) {
+                       cf_log_err(default_catch, "Invalid 'catch' - cannot have another 'catch' after a default 'catch { ... }'");
+                       cs = default_catch;
+               print_catch_url:
+                       cf_log_err(cs, DOC_KEYWORD_REF(catch));
+                       talloc_free(g);
+                       return NULL;
+               }
+
+               name = cf_section_name2(next);
+               if (!name) {
+                       default_catch = next;
+                       continue;
+               }
+
+               rcode = fr_table_value_by_str(mod_rcode_table, name, RLM_MODULE_NOT_SET);
+               if (rcode == RLM_MODULE_NOT_SET) {
+                       cf_log_err(cs, "Invalid argument to 'catch' - unknown rcode '%s'.", name);
+                       goto print_catch_url;
+               }
+
+               if (catcher[rcode]) {
+                       cf_log_err(next, "Duplicate rcode '%s'", name);
+                       cf_log_err(catcher[rcode], "First instance is here");
+                       goto print_catch_url;
+               }
+               catcher[rcode] = next;
+
+               for (i = 0; (name = cf_section_argv(next, i)) != NULL; i++) {
+                       rcode = fr_table_value_by_str(mod_rcode_table, name, RLM_MODULE_NOT_SET);
+                       if (rcode == RLM_MODULE_NOT_SET) {
+                               cf_log_err(cs, "Invalid argument to 'catch' - unknown rcode '%s'.", name);
+                               goto print_catch_url;
+                       }
+
+                       if (catcher[rcode]) {
+                               cf_log_err(next, "Duplicate rcode '%s'", name);
+                               cf_log_err(catcher[rcode], "First instance is here");
+                               goto print_catch_url;
+                       }
+
+                       catcher[rcode] = next;
+               }
+       }
+
+       /*
+        *      Check that the default will be used.
+        *
+        *      Note that we do NOT change the priorities for the defaults.
+        */
+       if (default_catch) {
+               bool set = false;
+
+               for (i = 0; i < RLM_MODULE_NUMCODES; i++) {
+                       if (!catcher[i]) {
+                               set = true;
+                               break;
+                       }
+               }
+
+               if (!set) {
+                       cf_log_err(default_catch, "Invalide 'catch { ... }' - all rcodes had previously been used");
+                       goto print_catch_url;
+               }
+
+               /*
+                *      Errors are still "return", even in a default "catch".
+                *
+                *      Normal rcodes will run to the end of the try section, and then be "catch"ed.
+                */
+               if (!catcher[RLM_MODULE_REJECT]) catcher[RLM_MODULE_REJECT] = default_catch;
+               if (!catcher[RLM_MODULE_FAIL]) catcher[RLM_MODULE_FAIL] = default_catch;
+               if (!catcher[RLM_MODULE_INVALID]) catcher[RLM_MODULE_INVALID] = default_catch;
+               if (!catcher[RLM_MODULE_DISALLOW]) catcher[RLM_MODULE_DISALLOW] = default_catch;
+               if (!catcher[RLM_MODULE_TIMEOUT]) catcher[RLM_MODULE_TIMEOUT] = default_catch;
+       }
+
+       /*
+        *      Loop again over the rcodes, setting the child actions to RETURN if necessary.
+        *
+        *      If the child is returning for that action, ensure that _we_ aren't returning.
+        *
+        *      Note that as above, reject / fail / invalid / disallow / timeout are errors, and cause the
+        *      child to immediately return.  All other rcodes 
+        */
+       for (i = 0; i < RLM_MODULE_NUMCODES; i++) {
+               /*
+                *      No one cares about this rcode.  It can return, reject, etc.
+                */
+               if (!catcher[i]) continue;
+
+               /*
+                *      Error rcodes cause the child section to bail immediately, but the "try" instruction
+                *      does not bail.
+                */
+               if ((i == RLM_MODULE_REJECT) ||
+                   (i == RLM_MODULE_FAIL) ||
+                   (i == RLM_MODULE_INVALID) ||
+                   (i == RLM_MODULE_DISALLOW) ||
+                   (i == RLM_MODULE_TIMEOUT)) {
+                       unlang_ctx2.actions.actions[i] = MOD_ACTION_RETURN;
+                       c->actions.actions[i] = MOD_ACTION_NOT_SET;
+                       continue;
+               }
+
+               /*
+                *      Normal rcodes cause the child section to run to completion, and the "try" section does
+                *      not bail.
+                */
+               if ((unlang_ctx2.actions.actions[i] > MOD_ACTION_NOT_SET) &&
+                   (unlang_ctx2.actions.actions[i] < MOD_PRIORITY_MIN)) {
+                       unlang_ctx2.actions.actions[i] = MOD_ACTION_NOT_SET;
+                       c->actions.actions[i] = MOD_ACTION_NOT_SET;
+               }
+       }
+
+       /*
+        *      Compile the children using the new compile ctx.
+        */
+       return unlang_compile_children(g, &unlang_ctx2);
 }