From: Alan T. DeKok Date: Sat, 23 Sep 2023 11:55:45 +0000 (-0400) Subject: create local variable list, and put local variables into it X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e6ca65a093133f5f130d66283b7ef38f372229dd;p=thirdparty%2Ffreeradius-server.git 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 d322469ba8c..e8c303ad057 100644 --- 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 05b4aecc751..745281f6104 100644 --- 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 295bac5880b..c46a4d3d311 100644 --- 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 83a17ea01e9..4d00e544b66 100644 --- 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 d71a887d526..d930082fcd9 100644 --- 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, "")); 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 4fe8b40cfd4..98c5fbc6c35 100644 --- 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 790c507ea8a..1ba66c6a9fb 100644 --- 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 3caf49ba04f..1fa2101c8ec 100644 --- 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 c92c1f6f80a..1009f99de60 100644 --- 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 }