if (failed expansion) --> fail

unless it's followed by an "else" or "elsif".

With documentation updates and fixed tests

diff --git a/doc/antora/modules/reference/pages/unlang/if.adoc b/doc/antora/modules/reference/pages/unlang/if.adoc
index add4e13cccedfdd65c65ac5f461d4862699fc519..9c40c826d95454dfb2054576fce130ccca0ba1d4 100644 (file)
--- a/doc/antora/modules/reference/pages/unlang/if.adoc
+++ b/doc/antora/modules/reference/pages/unlang/if.adoc
@@ -25,6 +25,40 @@ if (User-Name == "bob") {
 }
 ----
 
+== Expansions in an "if" condition
+
+An `if` condition may contain a xref:xlat/index.adoc[dynamic
+expansion], such as a call to the SQL module in the following example:
+
+.Example of Dynamic Expansion in Condition
+[source,unlang]
+----
+if %sql("SELECT reply_message FROM users where username = '%{User-Name}'") {
+    reject
+}
+----
+
+Unlike a simple check as with `if 1==2`, this dynamic expansion may fail.
+This failure can happen when the SQL database is unreachable, for
+example.  Different failures are possible for different functions.
+
+When a xref:xlat/index.adoc[dynamic expansion] fails, and `if`
+statement is followed by a trailing xref:unlang/else.adoc[else] or
+xref:unlang/elsif.adoc[elsif] statement, then `if` condition is treated
+as "false", and the `else` or `elsif statement is processed.
+
+Otherwise, the failed `if` statement returns the `fail`
+xref:unlang/return_codes.adoc[rcode], which is processed according to
+the rules for the current section.
+
+This distinction between "condition which evaluates to `false`" and
+"condition which fails to be processed" allow policies to distinguish
+those two cases.
+
+In previous versions of the server, a failed dynamic expansion was
+treated as a condition which returned `false`, which was not always
+optimal.
+
 == Practical Suggestions
 
 There are a number of practical suggestions which make it easier to work with conditions.
@@ -72,5 +106,5 @@ if (User-Name ==
 
 The last entry in an `if` section can also be an xref:unlang/actions.adoc[actions] subsection.
 
-// Copyright (C) 2021 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.

diff --git a/src/lib/unlang/compile.c b/src/lib/unlang/compile.c
index 780edef03df828ba86507c5c7bf4b291ac63e9db..dad3a4e95f8bed88e39e5f7fe8d0d615b744b48a 100644 (file)
--- a/src/lib/unlang/compile.c
+++ b/src/lib/unlang/compile.c
@@ -3615,6 +3615,7 @@ static unlang_t *compile_if_subsection(unlang_t *parent, unlang_compile_t *unlan
 
        unlang_group_t          *g;
        unlang_cond_t           *gext;
+       CONF_ITEM               *ci;
 
        xlat_exp_head_t         *head = NULL;
        bool                    is_truthy = false, value = false;
@@ -3644,7 +3645,7 @@ static unlang_t *compile_if_subsection(unlang_t *parent, unlang_compile_t *unlan
                                .allow_unresolved = false,
                                .allow_unknown = false,
                                .allow_wildcard = true,
-                       },
+               },
                        .literals_safe_for = unlang_ctx->rules->literals_safe_for,
                };
 
@@ -3708,6 +3709,25 @@ static unlang_t *compile_if_subsection(unlang_t *parent, unlang_compile_t *unlan
        gext->is_truthy = is_truthy;
        gext->value = value;
 
+       ci = cf_section_to_item(cs);
+       while ((ci = cf_item_next(parent->ci, ci)) != NULL) {
+               if (cf_item_is_data(ci)) continue;
+
+               break;
+       }
+
+       /*
+        *      If there's an 'if' without an 'else', then remember that.
+        */
+       if (ci && cf_item_is_section(ci)) {
+               char const *name;
+
+               name = cf_section_name1(cf_item_to_section(ci));
+               fr_assert(name != NULL);
+
+               gext->has_else = (strcmp(name, "else") == 0) || (strcmp(name, "elsif") == 0);
+       }
+
        return c;
 }
 

diff --git a/src/lib/unlang/condition.c b/src/lib/unlang/condition.c
index f711a30c21684f110a157a5d5a79261617a903b5..365b10abea43a1a4dd556846381d49ea98e4fcc7 100644 (file)
--- a/src/lib/unlang/condition.c
+++ b/src/lib/unlang/condition.c
@@ -40,6 +40,19 @@ static unlang_action_t unlang_if_resume(rlm_rcode_t *p_result, request_t *reques
        fr_value_box_t                  *box = fr_value_box_list_head(&state->out);
        bool                            value;
 
+       /*
+        *      Something in the conditional evaluation failed.
+        */
+       if (!state->success) {
+               unlang_group_t *g = unlang_generic_to_group(frame->instruction);
+               unlang_cond_t  *gext = unlang_group_to_cond(g);
+
+               REDEBUG2("... failed to evaluate condition ...");
+
+               if (!gext->has_else) RETURN_MODULE_FAIL;
+               return UNLANG_ACTION_EXECUTE_NEXT;
+       }
+
        if (!box) {
                value = false;
 

diff --git a/src/lib/unlang/condition_priv.h b/src/lib/unlang/condition_priv.h
index a8e8dd23d2c392c6bd661bafafc1bc4f162b10d9..414062cd73660884fb861776a694f5de48d872af 100644 (file)
--- a/src/lib/unlang/condition_priv.h
+++ b/src/lib/unlang/condition_priv.h
@@ -33,6 +33,7 @@ typedef struct {
        xlat_exp_head_t *head;
        bool            is_truthy;
        bool            value;
+       bool            has_else;
 } unlang_cond_t;
 
 /** Cast a group structure to the cond keyword extension

diff --git a/src/lib/unlang/xlat_expr.c b/src/lib/unlang/xlat_expr.c
index c2da1069ba59b7c1e6b72fafa8c9157bd1a88048..43cae3aea08cdd071eca94f59bbe1a1d1aa2848b 100644 (file)
--- a/src/lib/unlang/xlat_expr.c
+++ b/src/lib/unlang/xlat_expr.c
@@ -315,12 +315,16 @@ static xlat_action_t xlat_binary_op(TALLOC_CTX *ctx, fr_dcursor_t *out,
         *      Each argument is a FR_TYPE_GROUP, with one or more elements in a list.
         */
        a = fr_value_box_list_head(in);
-       b = fr_value_box_list_next(in, a);
-
-       if (!a && !b) return XLAT_ACTION_FAIL;
+       if (!a) {
+               REDEBUG("Left argument to %s is missing", fr_tokens[op]);
+               return XLAT_ACTION_FAIL;
+       }
 
-       fr_assert(!a || (a->type == FR_TYPE_GROUP));
-       fr_assert(!b || (b->type == FR_TYPE_GROUP));
+       b = fr_value_box_list_next(in, a);
+       if (!b) {
+               REDEBUG("Right argument to %s is missing", fr_tokens[op]);
+               return XLAT_ACTION_FAIL;
+       }
 
        fr_assert(!fr_comparison_op[op]);
 
@@ -452,9 +456,16 @@ static xlat_action_t xlat_cmp_op(TALLOC_CTX *ctx, fr_dcursor_t *out,
         *      Each argument is a FR_TYPE_GROUP, with one or more elements in a list.
         */
        a = fr_value_box_list_head(in);
-       b = fr_value_box_list_next(in, a);
+       if (!a) {
+               REDEBUG("Left argument to %s is missing", fr_tokens[op]);
+               return XLAT_ACTION_FAIL;
+       }
 
-       if (!a || !b) return XLAT_ACTION_FAIL;
+       b = fr_value_box_list_next(in, a);
+       if (!b) {
+               REDEBUG("Right argument to %s is missing", fr_tokens[op]);
+               return XLAT_ACTION_FAIL;
+       }
 
        fr_assert(a->type == FR_TYPE_GROUP);
        fr_assert(b->type == FR_TYPE_GROUP);

diff --git a/src/tests/keywords/expr b/src/tests/keywords/expr
index b79581f7ad385493647de336752ac4f3a925baf4..8943965b97019e5bf17e5c464922937e476ba2f3 100644 (file)
--- a/src/tests/keywords/expr
+++ b/src/tests/keywords/expr
@@ -98,7 +98,10 @@ if !(((1 << 2) | 1) == 5) {  # needs extra () to resolve precedence
 
 if ("%{ test_date}" <= 0) {
        test_fail
+} else {
+  # nothing!  Previous expansion has failed, so it MUST have an "else"
 }
+
 if (test_date <= 0) {
        test_fail
 }

diff --git a/src/tests/keywords/unpack b/src/tests/keywords/unpack
index 07c23dbf3792bd19d673d45551f6067ea6b6138e..49d4a193e9637c9fcdf457f4c666d30266e4075b 100644 (file)
--- a/src/tests/keywords/unpack
+++ b/src/tests/keywords/unpack
@@ -25,7 +25,7 @@ test_octets := 0x000001020304
 result_integer := %unpack(%{test_octets}, 4, 'uint16')
 
 # Octets 4 and 5 == 0x0304 == 772
-if ~(result_integer == 772) {
+if (result_integer != 772) {
        test_fail
 }