create local variable list, and put local variables into it

diff --git a/share/dictionary/freeradius/dictionary.freeradius.internal b/share/dictionary/freeradius/dictionary.freeradius.internal
index d322469ba8cd6d31d5188b2802faa0dee9a82520..e8c303ad057635a8d93a5ceb4d36a2e19f0e58bb 100644 (file)
--- a/share/dictionary/freeradius/dictionary.freeradius.internal
+++ b/share/dictionary/freeradius/dictionary.freeradius.internal
@@ -50,6 +50,7 @@ ATTRIBUTE     request                                 2       group
 ATTRIBUTE      reply                                   3       group
 ATTRIBUTE      control                                 4       group
 ATTRIBUTE      session-State                           5       group
+ATTRIBUTE      local-variables                         6       group
 
 #
 #      Range:  11 - 19

diff --git a/src/lib/server/request.c b/src/lib/server/request.c
index 05b4aecc751af1cee52a1a4ce94ea87753806a5b..745281f61048ea55843b12c692c92e49361c4626 100644 (file)
--- a/src/lib/server/request.c
+++ b/src/lib/server/request.c
@@ -42,6 +42,7 @@ fr_dict_attr_t const *request_attr_request;
 fr_dict_attr_t const *request_attr_reply;
 fr_dict_attr_t const *request_attr_control;
 fr_dict_attr_t const *request_attr_state;
+fr_dict_attr_t const *request_attr_local;
 
 extern fr_dict_attr_autoload_t request_dict_attr[];
 fr_dict_attr_autoload_t request_dict_attr[] = {
@@ -50,6 +51,7 @@ fr_dict_attr_autoload_t request_dict_attr[] = {
        { .out = &request_attr_reply, .name = "reply", .type = FR_TYPE_GROUP, .dict = &dict_freeradius },
        { .out = &request_attr_control, .name = "control", .type = FR_TYPE_GROUP, .dict = &dict_freeradius },
        { .out = &request_attr_state, .name = "session-state", .type = FR_TYPE_GROUP, .dict = &dict_freeradius },
+       { .out = &request_attr_local, .name = "local-variables", .type = FR_TYPE_GROUP, .dict = &dict_freeradius },
        { NULL }
 };
 
@@ -247,6 +249,7 @@ static inline CC_HINT(always_inline) int request_init(char const *file, int line
                if (!request->pair_list.request) list_init(request->pair_root, request);
                if (!request->pair_list.reply) list_init(request->pair_root, reply);
                if (!request->pair_list.control) list_init(request->pair_root, control);
+               if (!request->pair_list.local) list_init(request->pair_root, local);
                if (!request->pair_list.state) {
                        list_init(NULL, state);
 #ifndef NDEBUG
@@ -728,6 +731,7 @@ void request_verify(char const *file, int line, request_t const *request)
 #endif
 
        fr_pair_list_verify(file, line, request->session_state_ctx, &request->session_state_pairs);
+       fr_pair_list_verify(file, line, request->local_ctx, &request->local_pairs);
 
        if (request->packet) {
                packet_verify(file, line, request, request->packet, &request->request_pairs, "request");

diff --git a/src/lib/server/request.h b/src/lib/server/request.h
index 295bac5880be00555b3c9f7f38266645904f1498..c46a4d3d311486c308a8c698de38a8d92dac7934 100644 (file)
--- a/src/lib/server/request.h
+++ b/src/lib/server/request.h
@@ -80,6 +80,7 @@ extern fr_dict_attr_t const *request_attr_request;
 extern fr_dict_attr_t const *request_attr_reply;
 extern fr_dict_attr_t const *request_attr_control;
 extern fr_dict_attr_t const *request_attr_state;
+extern fr_dict_attr_t const *request_attr_local;
 
 /** Convenience macro for accessing the request list
  *
@@ -125,6 +126,17 @@ extern fr_dict_attr_t const *request_attr_state;
  */
 #define session_state_ctx      pair_list.state
 
+/** Convenience macro for accessing the state list
+ *
+ * This should be used in the form `&request->local_pairs`
+ * to get a pointer to the head of the local list.
+ */
+#define local_pairs    pair_list.local->children
+
+/** Talloc ctx for allocating local variagbles
+ */
+#define local_ctx      pair_list.local
+
 /** Pair lists accessible from the request
  *
  */
@@ -133,6 +145,7 @@ typedef struct {
        fr_pair_t               *reply;         //!< Pair containing the reply list.
        fr_pair_t               *control;       //!< Pair containing the control list.
        fr_pair_t               *state;         //!< Pair containing the state list.
+       fr_pair_t               *local;         //!< Pair containing local variables
 } request_pair_lists_t;
 
 typedef enum {

diff --git a/src/lib/server/tmpl.h b/src/lib/server/tmpl.h
index 83a17ea01e96fd27feada3ffd1c55dcb762717c8..4d00e544b669b6de3665dccb4bb132c1ab791f39 100644 (file)
--- a/src/lib/server/tmpl.h
+++ b/src/lib/server/tmpl.h
@@ -670,7 +670,8 @@ static inline bool tmpl_attr_is_list_attr(tmpl_attr_t const *ar)
        return (ar->ar_da == request_attr_request) ||
               (ar->ar_da == request_attr_reply) ||
               (ar->ar_da == request_attr_control) ||
-              (ar->ar_da == request_attr_state);
+              (ar->ar_da == request_attr_state) ||
+              (ar->ar_da == request_attr_local);
 }
 
 /** Return true if the head attribute reference is a list reference

diff --git a/src/lib/server/tmpl_eval.c b/src/lib/server/tmpl_eval.c
index d71a887d52659ab2347e4d6ce1ba64c88403c1f9..d930082fcd9de7805cf3f3001821f400e21a0d4f 100644 (file)
--- a/src/lib/server/tmpl_eval.c
+++ b/src/lib/server/tmpl_eval.c
@@ -87,6 +87,8 @@ fr_pair_list_t *tmpl_list_head(request_t *request, fr_dict_attr_t const *list)
 
        if (list == request_attr_state) return &request->session_state_pairs;
 
+       if (list == request_attr_local) return &request->local_pairs;
+
        RWDEBUG2("List \"%s\" is not available", tmpl_list_name(list, "<INVALID>"));
 
        return NULL;
@@ -119,6 +121,8 @@ TALLOC_CTX *tmpl_list_ctx(request_t *request, fr_dict_attr_t const *list)
 
        if (list == request_attr_state) return request->session_state_ctx;
 
+       if (list == request_attr_local) return request->local_ctx;
+
        return NULL;
 }
 

diff --git a/src/lib/server/tmpl_tokenize.c b/src/lib/server/tmpl_tokenize.c
index 4fe8b40cfd42941b22da901841b2334989a9426f..98c5fbc6c3591be33070a51bdf4ce1cf2a608ddb 100644 (file)
--- a/src/lib/server/tmpl_tokenize.c
+++ b/src/lib/server/tmpl_tokenize.c
@@ -401,8 +401,9 @@ fr_slen_t tmpl_attr_list_from_substr(fr_dict_attr_t const **da_p, fr_sbuff_t *in
             (da = request_attr_reply)) ||
            ((fr_sbuff_adv_past_strcase(&our_in, request_attr_control->name, request_attr_control->name_len)) &&
             (da = request_attr_control)) ||
-           ((fr_sbuff_adv_past_strcase(&our_in, request_attr_state->name, request_attr_state->name_len)) &&
+           ((fr_sbuff_adv_past_strcase(&our_in, request_attr_state->name, request_attr_state->name_len)) &&         
             (da = request_attr_state))) {
+               /* note: no local variables */
                *da_p = da;
                FR_SBUFF_SET_RETURN(in, &our_in);
        }
@@ -2128,14 +2129,33 @@ ssize_t tmpl_afrom_attr_substr(TALLOC_CTX *ctx, tmpl_attr_error_t *err,
 
        /*
         *      Local variables cannot be given an explicit parent or list modifier.
-        *
-        *      @todo - maybe allow parent references for local variables?  But that's just weird.
         */
-       if (tmpl_is_attr(vpt) && tmpl_attr_tail_da(vpt) && tmpl_attr_tail_da(vpt)->flags.local && (tmpl_attr_list_num_elements(tmpl_attr(vpt)) > 1)) {
-               fr_strerror_printf("Local attributes cannot be used in any list");
-               if (err) *err = TMPL_ATTR_ERROR_FOREIGN_NOT_ALLOWED;
-               fr_sbuff_set(&our_name, &m_l);
-               goto error;
+       if (tmpl_is_attr(vpt) && tmpl_attr_tail_da(vpt) && tmpl_attr_tail_da(vpt)->flags.local) {
+               tmpl_attr_t *ar;
+
+               if (tmpl_attr_list_num_elements(tmpl_attr(vpt)) > 1) {
+                       fr_strerror_printf("Local attributes cannot be used in any list");
+                       if (err) *err = TMPL_ATTR_ERROR_FOREIGN_NOT_ALLOWED;
+                       fr_sbuff_set(&our_name, &m_l);
+                       goto error;
+               }
+
+               /*
+                *      That being said, local variables are named "foo", but are always put into the local list.
+                */
+               MEM(ar = talloc(vpt, tmpl_attr_t));
+               *ar = (tmpl_attr_t){
+                       .ar_type = TMPL_ATTR_TYPE_NORMAL,
+                       .ar_da = request_attr_local,
+                       .ar_parent = fr_dict_root(fr_dict_internal())
+               };
+
+
+               /*
+                *      Prepend the local list ref so it gets evaluated
+                *      first.
+                */
+               tmpl_attr_list_insert_head(tmpl_attr(vpt), ar);
        }
 
        /*

diff --git a/src/lib/unlang/edit.c b/src/lib/unlang/edit.c
index 790c507ea8a020c5110975b73987f7c30a0737a4..1ba66c6a9fbb6ea13e8fdd9ce22f6d8f6757c54d 100644 (file)
--- a/src/lib/unlang/edit.c
+++ b/src/lib/unlang/edit.c
@@ -375,11 +375,6 @@ static int apply_edits_to_list(request_t *request, unlang_frame_state_edit_t *st
                                continue;
                        }
 
-                       if (vp->da->flags.local) {
-                               RWDEBUG("Not removing local variable %pP", vp);
-                               continue;
-                       }
-
                        if (fr_edit_list_pair_delete(current->el, list, vp) < 0) {
                                tmpl_dcursor_clear(&cc);
                                return -1;

diff --git a/src/lib/unlang/interpret.c b/src/lib/unlang/interpret.c
index 3caf49ba04f936f23e0c89eec8238fa1971c8c27..1fa2101c8ec7a5a8137582e7f612de239b6987ea 100644 (file)
--- a/src/lib/unlang/interpret.c
+++ b/src/lib/unlang/interpret.c
@@ -217,10 +217,12 @@ typedef struct {
 
 static int _local_variables_free(unlang_variable_ref_t *ref)
 {
-       fr_pair_list_foreach(&ref->request->request_pairs, vp) {
-               if (vp->da->dict == ref->dict) {
-                       (void) fr_pair_delete(&ref->request->request_pairs, vp);
-               }
+       fr_pair_t *vp;
+
+       while ((vp = fr_pair_list_tail(&ref->request->local_pairs)) != NULL) {
+               if (vp->da->dict != ref->dict) break;
+
+               (void) fr_pair_delete(&ref->request->local_pairs, vp);
        }
 
        return 0;
@@ -266,6 +268,11 @@ unlang_action_t unlang_interpret_push_children(rlm_rcode_t *p_result, request_t
 
        if (!g->variables) return UNLANG_ACTION_PUSHED_CHILD;
 
+       /*
+        *      Note that we do NOT create the variables, This way we don't have to worry about any
+        *      uninitialized values.  If the admin tries to use the variable without initializing it, they
+        *      will get a "no such attribute" error.
+        */
        if (!frame->state) {
                MEM(ref = talloc(stack, unlang_variable_ref_t));
                frame->state = ref;

diff --git a/src/tests/keywords/local-variable b/src/tests/keywords/local-variable
index c92c1f6f80a2ef120355c0ae7dc9bf1726e91209..1009f99de608b3f24dda4375f49d6b75b7407218 100644 (file)
--- a/src/tests/keywords/local-variable
+++ b/src/tests/keywords/local-variable
@@ -1,25 +1,39 @@
 &Filter-Id := 2
 
-if !(&request.[#] == 4) {
+if !(&request.[#] == 5) {
+       debug_request
        test_fail
 }
 
 group {
        uint32 foo
 
+       #
+       #  We declared it, but it does *not* exist until we assign
+       #  a value to it.
+       #
+       if &foo {
+               test_fail
+       }
+
        &foo := 1
 
        &Filter-Id := &foo
 
+       if !(&Filter-Id == &foo) {
+               test_fail
+       }
+
        #
-       #  For now, local variables are in the request.
+       #  Local variables are *not* in the request.
        #
-       if !(&request.[#] == 6) {
+       if !(&request.[#] == 5) {
                test_fail
        }
 } # leaving this scope automatically deletes &foo
 
 if !(&request.[#] == 5) {
+       debug_request
        test_fail
 }