From: Arran Cudbard-Bell Date: Sun, 7 Apr 2019 21:44:47 +0000 (-0400) Subject: Rework subrequest code to allow subrequests that persist and run through multiple... X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=420717593cee7df3ff68e6890179bbf478886690;p=thirdparty%2Ffreeradius-server.git Rework subrequest code to allow subrequests that persist and run through multiple sections --- diff --git a/src/lib/server/virtual_servers.c b/src/lib/server/virtual_servers.c index 5a1025c0cd2..a44cef7b3c2 100644 --- a/src/lib/server/virtual_servers.c +++ b/src/lib/server/virtual_servers.c @@ -648,6 +648,18 @@ CONF_SECTION *virtual_server_find(char const *name) return cf_section_find(virtual_server_root, "server", name); } +/** Find a virtual server using one of its sections + * + * @param[in] section to find parent virtual server for. + * @return + * - The virtual server section on success. + * - NULL if the child isn't associated with any virtual server section. + */ +CONF_SECTION *virtual_server_by_child(CONF_SECTION *section) +{ + return cf_section_find_in_parent(section, "server", CF_IDENT_ANY); +} + /** Free a virtual namespace callback * */ diff --git a/src/lib/server/virtual_servers.h b/src/lib/server/virtual_servers.h index a82a5f3e057..cf2989ed06d 100644 --- a/src/lib/server/virtual_servers.h +++ b/src/lib/server/virtual_servers.h @@ -62,6 +62,8 @@ typedef int (*fr_virtual_server_compile_t)(CONF_SECTION *server); CONF_SECTION *virtual_server_find(char const *name); +CONF_SECTION *virtual_server_by_child(CONF_SECTION *section); + int virtual_server_namespace_register(char const *namespace, fr_virtual_server_compile_t func); fr_dict_t *virtual_server_namespace(char const *virtual_server); diff --git a/src/lib/unlang/interpret.c b/src/lib/unlang/interpret.c index d0a88230396..e0414628061 100644 --- a/src/lib/unlang/interpret.c +++ b/src/lib/unlang/interpret.c @@ -1336,7 +1336,7 @@ int unlang_init(void) unlang_module_init(); unlang_parallel_init(); unlang_return_init(); - if (unlang_subrequest_init() < 0) return -1; + if (unlang_subrequest_op_init() < 0) return -1; unlang_switch_init(); return 0; @@ -1344,5 +1344,5 @@ int unlang_init(void) void unlang_free(void) { - unlang_subrequest_free(); + unlang_subrequest_op_free(); } diff --git a/src/lib/unlang/module.c b/src/lib/unlang/module.c index 5a6e5c60bde..fd57f411921 100644 --- a/src/lib/unlang/module.c +++ b/src/lib/unlang/module.c @@ -324,9 +324,20 @@ int unlang_module_fd_delete(REQUEST *request, void const *ctx, int fd) /** Yield, spawning a child request, and resuming once the child request is complete * - * @param[out] out Final rcode from when evaluation of the child request finishes. - * @param[in] request The current request. - * @param[in] server_cs the subrequest will execute in. + * @param[in] out Final rcode from when evaluation of the child request finishes. + * @param[out] child - If not NULL, and points to a NULL pointer a pointer to the + * child will be provided. + * The caller can then manipulate the child, adding request data + * and/or attributes. + * The child pointer must be explicitly freed with + * #unlang_subrequest_free once it is no longer needed. + * - If not NULL, and points to a request, the request will be run + * through the section passed as section_cs. The request must + * have been allocated during a previous call to + * unlang_module_yield_to_subrequest. + * - If NULL the child will be automatically freed when the subrequest + * completes. + * @param[in] parent The current request. * @param[in] section_cs to execute. * @param[in] default_rcode The rcode the child starts executing its section with. * @param[in] resume function to call when the child has finished executing. @@ -335,40 +346,52 @@ int unlang_module_fd_delete(REQUEST *request, void const *ctx, int fd) * @return * - RLM_MODULE_YIELD. */ -rlm_rcode_t unlang_module_yield_to_subrequest(rlm_rcode_t *out, - REQUEST *request, - CONF_SECTION *server_cs, CONF_SECTION *section_cs, - rlm_rcode_t default_rcode, +rlm_rcode_t unlang_module_yield_to_subrequest(rlm_rcode_t *out, REQUEST **child, REQUEST *parent, + CONF_SECTION *section_cs, rlm_rcode_t default_rcode, fr_unlang_module_resume_t resume, fr_unlang_module_signal_t signal, void *rctx) { unlang_t *instruction = (unlang_t *)cf_data_value(cf_data_find(section_cs, unlang_group_t, NULL)); - fr_dict_t *dict; + rad_assert(instruction); /* - * Work out the dictionary from the server section's cf_data + * Push the resumption point */ - dict = virtual_server_namespace(cf_section_name2(server_cs)); + (void) unlang_module_yield(parent, resume, signal, rctx); - /* - * If this fires, fix the validation logic - * don't just set a default value. - * - * *ALL* virtual servers should have a namespace. - */ - rad_assert(dict); + if (!child || !*child) { + CONF_SECTION *server_cs; + fr_dict_t *dict; - /* - * Push the resumption point - */ - (void) unlang_module_yield(request, resume, signal, rctx); + server_cs = virtual_server_by_child(section_cs); + /* + * We don't support executing orphaned sections. + */ + rad_assert(server_cs); - /* - * Push the subrequest frame. - */ - unlang_subrequest_push(out, request, server_cs, instruction, dict, default_rcode, true); + /* + * Work out the dictionary from the server section's cf_data + */ + dict = virtual_server_namespace(cf_section_name2(server_cs)); + + /* + * If this asserts, fix the validation logic + * don't just set a default value. + * + * *ALL* virtual servers should have a namespace. + */ + rad_assert(dict); + + /* + * Push the subrequest frame. + */ + unlang_subrequest_push(out, child, parent, server_cs, instruction, dict, default_rcode, true); + } else { + unlang_subrequest_push_again(out, talloc_get_type_abort(*child, REQUEST), + parent, instruction, default_rcode, true); + } return RLM_MODULE_YIELD; /* This may allow us to do optimisations in future */ } diff --git a/src/lib/unlang/module.h b/src/lib/unlang/module.h index 8f0aa564e14..ddb098772a6 100644 --- a/src/lib/unlang/module.h +++ b/src/lib/unlang/module.h @@ -104,9 +104,8 @@ int unlang_module_fd_add(REQUEST *request, int unlang_module_fd_delete(REQUEST *request, void const *rctx, int fd); -rlm_rcode_t unlang_module_yield_to_subrequest(rlm_rcode_t *out, - REQUEST *request, - CONF_SECTION *server_cs, CONF_SECTION *section_cs, +rlm_rcode_t unlang_module_yield_to_subrequest(rlm_rcode_t *out, REQUEST **child, REQUEST *request, + CONF_SECTION *section_cs, rlm_rcode_t default_rcode, fr_unlang_module_resume_t resume, fr_unlang_module_signal_t signal, void *rctx); diff --git a/src/lib/unlang/subrequest.c b/src/lib/unlang/subrequest.c index 5444404689e..599911803f3 100644 --- a/src/lib/unlang/subrequest.c +++ b/src/lib/unlang/subrequest.c @@ -33,12 +33,9 @@ RCSID("$Id$") */ typedef struct { rlm_rcode_t *presult; //!< Where to store the result. - unlang_t *instruction; //!< Where the subrequest should start executing. - rlm_rcode_t default_rcode; //!< What the rcode should be when the subrequest - ///< enters the virtual server section. - CONF_SECTION *server_cs; //!< Server configuration section. - fr_dict_t const *namespace; //!< What protocol the subrequest should represent. - bool detachable; //!< Whether the request can be detached. + REQUEST *child; //!< Pre-allocated child request. + bool persist : 1; //!< Whether we should free the child after it completes. + bool detachable : 1; //!< Whether the request can be detached. } unlang_frame_state_subrequest_t; @@ -107,8 +104,8 @@ static unlang_action_t unlang_subrequest_resume(REQUEST *request, rlm_rcode_t *p *presult = rcode; frame->instruction->type = UNLANG_TYPE_SUBREQUEST; /* for debug purposes */ - request_detach(child); /* Doesn't actually detach the client, just does cleanups */ - talloc_free(child); + + if (!state->persist) unlang_subrequest_free(&child); /* * Pass the result back to the module @@ -132,7 +129,6 @@ static unlang_action_t unlang_subrequest_resume(REQUEST *request, rlm_rcode_t *p (void) talloc_get_type_abort(mr, unlang_resume_t); rad_assert(mr->resume == NULL); - rad_assert(mr->rctx == child); #endif /* @@ -173,13 +169,32 @@ static unlang_action_t unlang_subrequest(REQUEST *request, * should be filled out by unlang_push_subrequest. */ if (!state) { - /* - * This will be freed implicitly if the - * frame is popped, so we don't need to - * clean it up. - */ - frame->state = state = talloc_zero(stack, unlang_frame_state_subrequest_t); - state->default_rcode = frame->result; + frame->state = state = talloc(stack, unlang_frame_state_subrequest_t); + state->child = unlang_io_child_alloc(request, g->children, + request->server_cs, request->dict, + frame->result, + UNLANG_NEXT_SIBLING, UNLANG_DETACHABLE); + if (!state->child) { + rcode = RLM_MODULE_FAIL; + priority = instruction->actions[*presult]; + + calculate_result: + /* + * Pass the result back to the module + * that created the subrequest, or + * use it to modify the current section + * rcode. + */ + if (state->presult) { + *state->presult = rcode; + return UNLANG_ACTION_CALCULATE_RESULT; + } + + *presult = rcode; + *ppriority = priority; + + return UNLANG_ACTION_CALCULATE_RESULT; + } /* * Probably not a great idea to set this @@ -194,40 +209,9 @@ static unlang_action_t unlang_subrequest(REQUEST *request, * in to the currently executing function. */ state->presult = NULL; - state->server_cs = request->server_cs; - state->namespace = request->dict; - state->instruction = g->children; - state->detachable = UNLANG_DETACHABLE; - } - - /* - * Allocate the child request. - */ - child = unlang_io_child_alloc(request, state->instruction, - state->server_cs, state->namespace, - state->default_rcode, - UNLANG_NEXT_SIBLING, state->detachable); - if (!child) { - rcode = RLM_MODULE_FAIL; - priority = instruction->actions[*presult]; - - calculate_result: - /* - * Pass the result back to the module - * that created the subrequest, or - * use it to modify the current section - * rcode. - */ - if (state->presult) { - *state->presult = rcode; - return UNLANG_ACTION_CALCULATE_RESULT; - } - - *presult = rcode; - *ppriority = priority; - - return UNLANG_ACTION_CALCULATE_RESULT; + state->persist = false; } + child = state->child; RDEBUG2("- creating subrequest (%s)", child->name); @@ -243,8 +227,7 @@ static unlang_action_t unlang_subrequest(REQUEST *request, */ rcode = unlang_run(child); if (rcode != RLM_MODULE_YIELD) { - request_detach(child); - talloc_free(child); + if (!state->persist) unlang_subrequest_free(&child); priority = instruction->actions[*presult]; @@ -384,10 +367,76 @@ static unlang_t subrequest_instruction = { }, }; +/** Free a child request, detaching it from its parent and freeing allocated memory + * + * @param[in] child to free. + */ +void unlang_subrequest_free(REQUEST **child) +{ + request_detach(*child); /* Doesn't actually detach the client, just does cleanups */ + talloc_free(*child); + *child = NULL; +} + +/** Push a pre-existing child back onto the stack as a subrequest + * + * The child *MUST* have been allocated with unlang_subrequest_push. Instruction *MUST* belong to + * the same virtual server as the original request was executed in. + * + * After the child is no longer required it *MUST* be freed with #unlang_subrequest_free. + * It's not enough to free it with talloc_free. + * + * @param[in] out Where to write the result of the subrequest. + * @param[in] child to push. + * @param[in] parent of the child. + * @param[in] instruction to execute in the child. + * @param[in] default_rcode for instruction being executed. + * @param[in] top_frame Set to UNLANG_TOP_FRAME if the interpreter should return. + * Set to UNLANG_SUB_FRAME if the interprer should continue. + */ +void unlang_subrequest_push_again(rlm_rcode_t *out, REQUEST *child, REQUEST *parent, + unlang_t *instruction, rlm_rcode_t default_rcode, bool top_frame) +{ + unlang_frame_state_subrequest_t *state; + unlang_stack_t *stack = parent->stack; + unlang_stack_frame_t *frame; + + rad_assert(child->parent == parent); + + /* + * Push a new subrequest frame onto the stack + */ + unlang_push(stack, &subrequest_instruction, RLM_MODULE_UNKNOWN, UNLANG_NEXT_STOP, top_frame); + frame = &stack->frame[stack->depth]; + + /* + * Allocate a state for the subrequest + * This lets us override the normal request + * the subrequest instruction would alloc. + */ + MEM(frame->state = state = talloc_zero(stack, unlang_frame_state_subrequest_t)); + state->presult = out; + state->persist = true; + state->child = child; + + /* + * Push the instruction to execute onto the child's stack. + */ + stack = child->stack; + unlang_push(stack, instruction, default_rcode, UNLANG_NEXT_SIBLING, UNLANG_SUB_FRAME); + stack->frame[stack->depth].top_frame = true; +} + /** Allocate a child and set it up for execution * - * @param[out] out Where to write the result of the subrequest. - * @param[in] request to hang child request off of. + * After the child is no longer required it *MUST* be freed with #unlang_subrequest_free. + * It's not enough to free it with talloc_free. This will be done automatically if + * no child pointer is provided to store a pointer to the child. + * + * @param[in] out Where to write the result of the subrequest. + * @param[out] child The child we allocated. The caller can then add + * VALUE_PAIRs and request data before the child is executed. + * @param[in] parent to hang child request off of. * @param[in] server_cs Server to execute subrequest in. * @param[in] instruction Where to start the child. * @param[in] namespace to use for the subrequest. @@ -395,36 +444,42 @@ static unlang_t subrequest_instruction = { * @param[in] top_frame Set to UNLANG_TOP_FRAME if the interpreter should return. * Set to UNLANG_SUB_FRAME if the interprer should continue. */ -void unlang_subrequest_push(rlm_rcode_t *out, - REQUEST *request, +void unlang_subrequest_push(rlm_rcode_t *out, REQUEST **child, + REQUEST *parent, CONF_SECTION *server_cs, unlang_t *instruction, fr_dict_t const *namespace, rlm_rcode_t default_rcode, bool top_frame) { unlang_frame_state_subrequest_t *state; - unlang_stack_t *stack = request->stack; + unlang_stack_t *stack = parent->stack; unlang_stack_frame_t *frame; /* - * Push a new xlat eval frame onto the stack + * Push a new subrequest frame onto the stack */ unlang_push(stack, &subrequest_instruction, RLM_MODULE_UNKNOWN, UNLANG_NEXT_STOP, top_frame); frame = &stack->frame[stack->depth]; /* - * Allocate a state which serves to configure - * the subrequest. + * Allocate a state for the subrequest + * This lets us override the normal request + * the subrequest instruction would alloc. */ MEM(frame->state = state = talloc_zero(stack, unlang_frame_state_subrequest_t)); state->presult = out; - state->default_rcode = default_rcode; - state->instruction = instruction; - state->server_cs = server_cs; - state->namespace = namespace; + state->persist = child ? true : false; + + /* + * Pushes instruction onto child + */ + state->child = unlang_io_child_alloc(parent, instruction, + server_cs, namespace, + default_rcode, + UNLANG_NEXT_SIBLING, UNLANG_NORMAL_CHILD); } -int unlang_subrequest_init(void) +int unlang_subrequest_op_init(void) { if (fr_dict_autoload(subrequest_dict) < 0) { PERROR("%s", __FUNCTION__); @@ -453,7 +508,7 @@ int unlang_subrequest_init(void) return 0; } -void unlang_subrequest_free(void) +void unlang_subrequest_op_free(void) { fr_dict_autofree(subrequest_dict); } diff --git a/src/lib/unlang/subrequest_priv.h b/src/lib/unlang/subrequest_priv.h index a773d9aeab2..891dcae7e1f 100644 --- a/src/lib/unlang/subrequest_priv.h +++ b/src/lib/unlang/subrequest_priv.h @@ -25,12 +25,15 @@ #ifdef __cplusplus extern "C" { #endif +void unlang_subrequest_free(REQUEST **child); -void unlang_subrequest_push(rlm_rcode_t *out, - REQUEST *request, - CONF_SECTION *server_cs, unlang_t *instruction, fr_dict_t const *namespace, - rlm_rcode_t default_rcode, - bool top_frame); +void unlang_subrequest_push_again(rlm_rcode_t *out, REQUEST *child, REQUEST *parent, + unlang_t *instruction, rlm_rcode_t default_rcode, bool top_frame); + +void unlang_subrequest_push(rlm_rcode_t *out, REQUEST **child, REQUEST *parent, + CONF_SECTION *server_cs, unlang_t *instruction, fr_dict_t const *namespace, + rlm_rcode_t default_rcode, + bool top_frame); #ifdef __cplusplus } diff --git a/src/lib/unlang/unlang_priv.h b/src/lib/unlang/unlang_priv.h index 941f3d2e101..f61ff7f32ff 100644 --- a/src/lib/unlang/unlang_priv.h +++ b/src/lib/unlang/unlang_priv.h @@ -396,9 +396,9 @@ void unlang_return_init(void); void unlang_parallel_init(void); -int unlang_subrequest_init(void); +int unlang_subrequest_op_init(void); -void unlang_subrequest_free(void); +void unlang_subrequest_op_free(void); void unlang_switch_init(void); /* @} **/