Skip to content

Commit

Permalink
create local variable list, and put local variables into it
Browse files Browse the repository at this point in the history
  • Loading branch information
alandekok committed Sep 23, 2023
1 parent 5896a28 commit e6ca65a
Show file tree
Hide file tree
Showing 9 changed files with 80 additions and 21 deletions.
1 change: 1 addition & 0 deletions share/dictionary/freeradius/dictionary.freeradius.internal
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions src/lib/server/request.c
Original file line number Diff line number Diff line change
Expand Up @@ -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[] = {
Expand All @@ -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 }
};

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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");
Expand Down
13 changes: 13 additions & 0 deletions src/lib/server/request.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down Expand Up @@ -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
*
*/
Expand All @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion src/lib/server/tmpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions src/lib/server/tmpl_eval.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down
36 changes: 28 additions & 8 deletions src/lib/server/tmpl_tokenize.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}

/*
Expand Down
5 changes: 0 additions & 5 deletions src/lib/unlang/edit.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
15 changes: 11 additions & 4 deletions src/lib/unlang/interpret.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
20 changes: 17 additions & 3 deletions src/tests/keywords/local-variable
Original file line number Diff line number Diff line change
@@ -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
}

Expand Down

0 comments on commit e6ca65a

Please sign in to comment.