call_env: Write out an explicit result from evaluation so that module calls fail...

diff --git a/src/lib/unlang/call_env.c b/src/lib/unlang/call_env.c
index 3c2529ae95acae0042c8233d2df2ab4ab2a61c98..ecb4d20bf8a28c4b06b724ee77df13686fca4798 100644 (file)
--- a/src/lib/unlang/call_env.c
+++ b/src/lib/unlang/call_env.c
@@ -192,9 +192,10 @@ size_t call_env_count(size_t *vallen, CONF_SECTION const *cs, call_env_t const *
 
 /** Parse the result of call_env tmpl expansion
  */
-static inline CC_HINT(always_inline) int call_env_value_parse(TALLOC_CTX *ctx, request_t *request, void *out,
-                                                             void **tmpl_out, call_env_parsed_t const *env,
-                                                             fr_value_box_list_t *tmpl_expanded)
+static inline CC_HINT(always_inline)
+call_env_result_t call_env_value_parse(TALLOC_CTX *ctx, request_t *request, void *out,
+                                      void **tmpl_out, call_env_parsed_t const *env,
+                                      fr_value_box_list_t *tmpl_expanded)
 {
        fr_value_box_t  *vb;
 
@@ -205,7 +206,7 @@ static inline CC_HINT(always_inline) int call_env_value_parse(TALLOC_CTX *ctx, r
        if (!vb) {
                if (!env->rule->pair.nullable) {
                        RPEDEBUG("Failed to evaluate required module option %s", env->rule->name);
-                       return -1;
+                       return CALL_ENV_MISSING;
                }
                return 0;
        }
@@ -217,13 +218,13 @@ static inline CC_HINT(always_inline) int call_env_value_parse(TALLOC_CTX *ctx, r
            fr_value_box_list_concat_in_place(vb, vb, tmpl_expanded, FR_BASE_TYPE(env->rule->type),
                                              FR_VALUE_BOX_LIST_FREE, true, SIZE_MAX) < 0 ) {
                RPEDEBUG("Failed concatenating values for %s", env->rule->name);
-               return -1;
+               return CALL_ENV_INVALID;
        }
 
        if (env->rule->pair.single && (fr_value_box_list_num_elements(tmpl_expanded) > 1)) {
                RPEDEBUG("%d values found for %s.  Only one is allowed",
                         fr_value_box_list_num_elements(tmpl_expanded), env->rule->name);
-               return -1;
+               return CALL_ENV_INVALID;
        }
 
        while ((vb = fr_value_box_list_pop_head(tmpl_expanded))) {
@@ -243,17 +244,18 @@ static inline CC_HINT(always_inline) int call_env_value_parse(TALLOC_CTX *ctx, r
                }
        }
 
-       return 0;
+       return CALL_ENV_SUCCESS;
 }
 
 /** Context to keep track of expansion of call environments
  *
  */
 typedef struct {
-       call_env_parsed_head_t const    *call_env_parsed;       //!< Head of the parsed list of tmpls to expand.
+       call_env_result_t               *result;                //!< Where to write the return code of callenv expansion.
+       call_env_parsed_head_t const    *parsed;                //!< Head of the parsed list of tmpls to expand.
        call_env_parsed_t const         *last_expanded;         //!< The last expanded tmpl.
        fr_value_box_list_t             tmpl_expanded;          //!< List to write value boxes to as tmpls are expanded.
-       void                            **env_data;             //!< Final destination structure for value boxes.
+       void                            **data;                 //!< Final destination structure for value boxes.
 } call_env_ctx_t;
 
 static unlang_action_t call_env_expand_repeat(rlm_rcode_t *p_result, int *priority, request_t *request, void *uctx);
@@ -268,10 +270,13 @@ static unlang_action_t call_env_expand_start(UNUSED rlm_rcode_t *p_result, UNUSE
        call_env_parsed_t const *env;
        void            *out;
 
-       call_env_ctx->last_expanded = call_env_parsed_next(call_env_ctx->call_env_parsed, call_env_ctx->last_expanded);
-       if (!call_env_ctx->last_expanded) return UNLANG_ACTION_CALCULATE_RESULT;
+       call_env_ctx->last_expanded = call_env_parsed_next(call_env_ctx->parsed, call_env_ctx->last_expanded);
+       if (!call_env_ctx->last_expanded) {     /* No more! */
+               if (call_env_ctx->result) *call_env_ctx->result = CALL_ENV_SUCCESS;
+               return UNLANG_ACTION_CALCULATE_RESULT;
+       }
 
-       ctx = *call_env_ctx->env_data;
+       ctx = *call_env_ctx->data;
        env = call_env_ctx->last_expanded;
 
        /*
@@ -286,14 +291,14 @@ static unlang_action_t call_env_expand_start(UNUSED rlm_rcode_t *p_result, UNUSE
         *      Multi pair options should allocate boxes in the context of the array
         */
        if (env->rule->pair.multi) {
-               out = ((uint8_t *)(*call_env_ctx->env_data)) + env->rule->offset;
+               out = ((uint8_t *)(*call_env_ctx->data)) + env->rule->offset;
 
                /*
                 *      For multi pair options, allocate the array before expanding the first entry.
                 */
                if (env->multi_index == 0) {
                        void *array;
-                       MEM(array = _talloc_zero_array((*call_env_ctx->env_data), env->rule->pair.size,
+                       MEM(array = _talloc_zero_array((*call_env_ctx->data), env->rule->pair.size,
                                                       env->opt_count, env->rule->pair.type_name));
                        *(void **)out = array;
                }
@@ -316,6 +321,7 @@ static unlang_action_t call_env_expand_repeat(UNUSED rlm_rcode_t *p_result, UNUS
        void                    *out = NULL, *tmpl_out = NULL;
        call_env_ctx_t          *call_env_ctx = talloc_get_type_abort(uctx, call_env_ctx_t);
        call_env_parsed_t       const *env;
+       call_env_result_t       result;
 
        env = call_env_ctx->last_expanded;
        if (!env) return UNLANG_ACTION_CALCULATE_RESULT;
@@ -324,7 +330,7 @@ static unlang_action_t call_env_expand_repeat(UNUSED rlm_rcode_t *p_result, UNUS
        /*
         *      Find the location of the output
         */
-       out = ((uint8_t*)(*call_env_ctx->env_data)) + env->rule->offset;
+       out = ((uint8_t*)(*call_env_ctx->data)) + env->rule->offset;
 
        /*
         *      If this is a multi pair option, the output is an array.
@@ -336,12 +342,18 @@ static unlang_action_t call_env_expand_repeat(UNUSED rlm_rcode_t *p_result, UNUS
        }
 
 tmpl_only:
-       if (env->rule->pair.tmpl_offset) tmpl_out = ((uint8_t *)*call_env_ctx->env_data) + env->rule->pair.tmpl_offset;
+       if (env->rule->pair.tmpl_offset) tmpl_out = ((uint8_t *)*call_env_ctx->data) + env->rule->pair.tmpl_offset;
 
-       if (call_env_value_parse(*call_env_ctx->env_data, request, out, tmpl_out, env,
-                                &call_env_ctx->tmpl_expanded) < 0) return UNLANG_ACTION_FAIL;
+       result = call_env_value_parse(*call_env_ctx->data, request, out, tmpl_out, env, &call_env_ctx->tmpl_expanded);
+       if (result != CALL_ENV_SUCCESS) {
+               if (call_env_ctx->result) *call_env_ctx->result = result;
+               return UNLANG_ACTION_FAIL;
+       }
 
-       if (!call_env_parsed_next(call_env_ctx->call_env_parsed, env)) return UNLANG_ACTION_CALCULATE_RESULT;
+       if (!call_env_parsed_next(call_env_ctx->parsed, env)) {
+               if (call_env_ctx->result) *call_env_ctx->result = CALL_ENV_SUCCESS;
+               return UNLANG_ACTION_CALCULATE_RESULT;
+       }
 
        return unlang_function_push(request, call_env_expand_start, call_env_expand_repeat, NULL, 0, UNLANG_SUB_FRAME,
                                    call_env_ctx);
@@ -351,11 +363,12 @@ tmpl_only:
  *
  * @param[in] ctx              in which to allocate destination structure for resulting value boxes.
  * @param[in] request          Current request.
+ * @param[out] env_result      Where to write the result of the callenv expansion.  May be NULL
  * @param[in,out] env_data     Where the destination structure should be created.
  * @param[in] call_env         Call environment being expanded.
  * @param[in] call_env_parsed  Parsed tmpls for the call environment.
  */
-unlang_action_t call_env_expand(TALLOC_CTX *ctx, request_t *request, void **env_data, call_method_env_t const *call_env,
+unlang_action_t call_env_expand(TALLOC_CTX *ctx, request_t *request, call_env_result_t *env_result, void **env_data, call_method_env_t const *call_env,
                                call_env_parsed_head_t const *call_env_parsed)
 {
        call_env_ctx_t  *call_env_ctx;
@@ -363,8 +376,10 @@ unlang_action_t call_env_expand(TALLOC_CTX *ctx, request_t *request, void **env_
        MEM(call_env_ctx = talloc_zero(ctx, call_env_ctx_t));
        MEM(*env_data = talloc_zero_array(ctx, uint8_t, call_env->inst_size));
        talloc_set_name_const(*env_data, call_env->inst_type);
-       call_env_ctx->env_data = env_data;
-       call_env_ctx->call_env_parsed = call_env_parsed;
+       call_env_ctx->result = env_result;
+       if (env_result) *env_result = CALL_ENV_INVALID; /* Make sure we ran to completion*/
+       call_env_ctx->data = env_data;
+       call_env_ctx->parsed = call_env_parsed;
        fr_value_box_list_init(&call_env_ctx->tmpl_expanded);
 
        return unlang_function_push(request, call_env_expand_start, call_env_expand_repeat, NULL, 0, UNLANG_SUB_FRAME,

diff --git a/src/lib/unlang/call_env.h b/src/lib/unlang/call_env.h
index 3051d74399f51f0e94cd3c965dba1d9155668196..8b93b89bcc76bfe6cfed893236fd92155a5a83cf 100644 (file)
--- a/src/lib/unlang/call_env.h
+++ b/src/lib/unlang/call_env.h
@@ -30,7 +30,6 @@ extern "C" {
 #endif
 
 #include <freeradius-devel/util/dlist.h>
-#include <freeradius-devel/server/cf_parse.h>
 
 typedef struct call_env_s              call_env_t;
 typedef struct call_env_parsed_s       call_env_parsed_t;
@@ -39,6 +38,17 @@ typedef struct call_method_env_s     call_method_env_t;
 FR_DLIST_TYPES(call_env_parsed)
 FR_DLIST_TYPEDEFS(call_env_parsed, call_env_parsed_head_t, call_env_parsed_entry_t)
 
+#include <freeradius-devel/unlang/action.h>
+#include <freeradius-devel/server/cf_parse.h>
+#include <freeradius-devel/server/request.h>
+#include <freeradius-devel/server/tmpl.h>
+
+typedef enum {
+       CALL_ENV_SUCCESS = 0,
+       CALL_ENV_MISSING = -1,
+       CALL_ENV_INVALID = -2
+} call_env_result_t;
+
 typedef enum {
        CALL_ENV_TYPE_VALUE_BOX = 1,
        CALL_ENV_TYPE_VALUE_BOX_LIST,
@@ -211,7 +221,7 @@ int call_env_parse(TALLOC_CTX *ctx, call_env_parsed_head_t *parsed, char const *
 
 size_t call_env_count(size_t *vallen, CONF_SECTION const *cs, call_env_t const *call_env);
 
-unlang_action_t call_env_expand(TALLOC_CTX *ctx, request_t *request, void **env_data, call_method_env_t const *call_env,
+unlang_action_t call_env_expand(TALLOC_CTX *ctx, request_t *request, call_env_result_t *result, void **env_data, call_method_env_t const *call_env,
                                call_env_parsed_head_t const *call_env_parsed);
 
 #ifdef __cplusplus

diff --git a/src/lib/unlang/module.c b/src/lib/unlang/module.c
index 4c758064308fd1bc2c93e4227032e94af67f6659..721bae353187dda9736af96f5f1aa65c600431a6 100644 (file)
--- a/src/lib/unlang/module.c
+++ b/src/lib/unlang/module.c
@@ -29,6 +29,8 @@ RCSID("$Id$")
 #include <freeradius-devel/server/cond.h>
 #include <freeradius-devel/server/modpriv.h>
 #include <freeradius-devel/server/request_data.h>
+#include <freeradius-devel/server/rcode.h>
+#include <freeradius-devel/unlang/call_env.h>
 
 #include "module_priv.h"
 #include "subrequest_priv.h"
@@ -910,19 +912,26 @@ static unlang_action_t unlang_module(rlm_rcode_t *p_result, request_t *request,
                goto done;
        }
 
-       if (mc->method_env && !state->env_data) {
-               ua = call_env_expand(state, request, &state->env_data, mc->method_env, &mc->call_env_parsed);
-               switch (ua) {
-               case UNLANG_ACTION_FAIL:
-                       goto fail;
+       if (mc->method_env) {
+               if (!state->env_data) {
+                       ua = call_env_expand(state, request, &state->env_result, &state->env_data, mc->method_env, &mc->call_env_parsed);
+                       switch (ua) {
+                       case UNLANG_ACTION_FAIL:
+                               goto fail;
 
-               case UNLANG_ACTION_PUSHED_CHILD:
-                       frame_repeat(frame, unlang_module);
-                       return UNLANG_ACTION_PUSHED_CHILD;
+                       case UNLANG_ACTION_PUSHED_CHILD:
+                               frame_repeat(frame, unlang_module);
+                               return UNLANG_ACTION_PUSHED_CHILD;
 
-               default:
-                       break;
+                       default:
+                               break;
+                       }
                }
+
+               /*
+                *      Fail the module call on callenv failure
+                */
+               if (state->env_result != CALL_ENV_SUCCESS) RETURN_MODULE_FAIL;
        }
 
        /*

diff --git a/src/lib/unlang/module_priv.h b/src/lib/unlang/module_priv.h
index 74ce79bc33ba3ef84e1e10e323a4678d258b441b..5a465661b77d323a4cb0707db7badadd725c3065 100644 (file)
--- a/src/lib/unlang/module_priv.h
+++ b/src/lib/unlang/module_priv.h
@@ -53,7 +53,7 @@ typedef struct {
                                                                ///< structure because the #unlang_t tree is
                                                                ///< shared between all threads, so we can't
                                                                ///< cache thread-specific data in the #unlang_t.
-
+       call_env_result_t               env_result;             //!< Result of the previous call environment expansion.
        void                            *env_data;              //!< Expanded per call "call environment" tmpls.
 
 #ifndef NDEBUG

diff --git a/src/lib/unlang/xlat.c b/src/lib/unlang/xlat.c
index ce663ca251fd147110e9d82c60d228a8279a9032..9dad880ca8221f9c9075487d676c067aabd944a9 100644 (file)
--- a/src/lib/unlang/xlat.c
+++ b/src/lib/unlang/xlat.c
@@ -309,7 +309,7 @@ static unlang_action_t unlang_xlat_repeat(rlm_rcode_t *p_result, request_t *requ
         *      If the xlat is a function with a method_env, expand it before calling the function.
         */
        if ((state->exp->type == XLAT_FUNC) && state->exp->call.func->call_env && !state->env_data) {
-               unlang_action_t ua = call_env_expand(state, request, &state->env_data,
+               unlang_action_t ua = call_env_expand(state, request, NULL, &state->env_data,
                                                     state->exp->call.func->call_env,
                                                     &state->exp->call.inst->call_env_parsed);
                switch (ua) {