From 31eba58363531156c6d884e757d5fefcfa86878c Mon Sep 17 00:00:00 2001 From: "Alan T. DeKok" Date: Tue, 22 Apr 2025 12:48:31 -0400 Subject: [PATCH] allow "catch" after "timeout" --- .../modules/reference/pages/unlang/catch.adoc | 7 +-- .../reference/pages/unlang/timeout.adoc | 45 ++++++++++++++--- src/lib/unlang/catch.c | 31 ++++-------- src/lib/unlang/catch_priv.h | 1 + src/lib/unlang/compile.c | 48 +++++++++++++++---- src/lib/unlang/timeout.c | 33 +++++++++++-- src/lib/unlang/unlang_priv.h | 19 ++++++++ src/tests/keywords/timeout-catch | 13 +++++ 8 files changed, 150 insertions(+), 47 deletions(-) create mode 100644 src/tests/keywords/timeout-catch diff --git a/doc/antora/modules/reference/pages/unlang/catch.adoc b/doc/antora/modules/reference/pages/unlang/catch.adoc index 9f4d7b6aed..bee5a5159e 100644 --- a/doc/antora/modules/reference/pages/unlang/catch.adoc +++ b/doc/antora/modules/reference/pages/unlang/catch.adoc @@ -14,9 +14,10 @@ catch [ rcodes ] { The `catch` statement runs a series of substatements in a block, but only if the previous xref:unlang/try.adoc[try] failed. [ rcodes ]:: Zero or more module return codes. The return codes can be `disallow`, `fail`, `invalid`, or `reject`. ++ +If the `catch` statement is after a xref:unlang/timeout.adoc[timeout] statement, then the `catch` will run only if the `timeout` section fails. In this case, you must use `catch timeout { ... }`. The `catch timeout` syntax is not allowed in any other situation. -[ statements ]:: The `unlang` commands which will be executed. A -`catch` block can be empty. +[ statements ]:: The `unlang` commands which will be executed. A `catch` block can be empty. If no rcode is given the `catch` statement matches all of the codes listed above. Otherwise, the `catch` statement matches one of the listed rcodes. @@ -44,7 +45,7 @@ catch invalid { # skipped after "catch fail" is run. } ---- -There is some overlap in functionality between `try` / xref:unlang/catch.adoc[catch] and xref:unlang/redundant.adoc[redundant]. The main difference is that a xref:unlang/catch.adoc[catch] statement can catch specific failure codes. +There is some overlap in functionality between xref:unlang/try.adoc[try] / `catch` and xref:unlang/redundant.adoc[redundant]. The main difference is that a xref:unlang/catch.adoc[catch] statement can catch specific failure codes. The xref:unlang/redundant.adoc[redundant] statement should be used to run one of many similar modules. For example, the xref:unlang/redundant.adoc[redundant] statement could be used to choose one of four different `sql` modules in a fail-over fashion. diff --git a/doc/antora/modules/reference/pages/unlang/timeout.adoc b/doc/antora/modules/reference/pages/unlang/timeout.adoc index 0ae3a8079e..b98ecd50b8 100644 --- a/doc/antora/modules/reference/pages/unlang/timeout.adoc +++ b/doc/antora/modules/reference/pages/unlang/timeout.adoc @@ -20,6 +20,11 @@ attribute reference. The contents of __ are interpreted as a The time scale can be changed by appending `s`, `us`, `ms`, `ns`, etc. as with all uses of `time_delta` values. +As a special case, a `timeout` section can be immediately followed by +a xref:unlang/catch.adoc[catch] statement, as `catch timeout { ... }`. +In that case, the xref:unlang/catch.adoc[catch] section is run when +the `timeout` expires. + .Example [source,unlang] ---- @@ -29,23 +34,49 @@ timeout 1ms { } ---- -In general, the best way to use `timeout` is in conjunction with a -`redundant` block. In the following example, the configuration allows -the `proxy` module to run for `4` seconds. If the `proxy` module -takes more than `4` seconds to return, _or_ if the `proxy` module -fails, the `detail` module is called. +== Timeout with catch + +In the following example, the configuration allows the `sql` module to +run for `4` seconds. If the `sql` module takes more than `4` seconds +to return, _or_ if the `sql` module fails, then the `detail` module is +called. + +.Example using catch +[source,unlang] +---- +timeout 4s { + sql +} +catch timeout { + detail +} +---- + + + +== Timeout with redundant + +The `timeout` can also be used inside of a +xref:unlang/redundant.adoc[redundant] block. This example has +_almost_ the same behavior as above. The difference here is that the +`detail` file here is run on when _either_ the `sql` module fails, or +the timeout is reached. + +Whether you choose to use a xref:unlang/redundant.adoc[redundant] or a +xref:unlang/catch.adoc[catch] block after the `timeout` depends on +your local requirements. .Example using redundant [source,unlang] ---- redundant timeout 4s { - proxy + sql } detail } ---- -// Copyright (C) 2022 Network RADIUS SAS. Licenced under CC-by-NC 4.0. +// Copyright (C) 2025 Network RADIUS SAS. Licenced under CC-by-NC 4.0. // This documentation was developed by Network RADIUS SAS. diff --git a/src/lib/unlang/catch.c b/src/lib/unlang/catch.c index 1fd47f7ef5..48ead63597 100644 --- a/src/lib/unlang/catch.c +++ b/src/lib/unlang/catch.c @@ -27,26 +27,6 @@ RCSID("$Id$") #include "unlang_priv.h" #include "catch_priv.h" -static unlang_action_t cleanup(unlang_stack_frame_t *frame, unlang_t *unlang) -{ - - /* - * Clean up this frame now, so that stats, etc. will be - * processed using the correct frame. - */ - frame_cleanup(frame); - - /* - * frame_next() will call cleanup *before* resetting the frame->instruction. - * but since the instruction is NULL, no duplicate cleanups will happen. - * - * frame_next() will then set frame->instruction = frame->next, and everything will be OK. - */ - frame->instruction = NULL; - frame->next = unlang; - return UNLANG_ACTION_EXECUTE_NEXT; -} - static unlang_action_t catch_skip_to_next(UNUSED rlm_rcode_t *p_result, UNUSED request_t *request, unlang_stack_frame_t *frame) { unlang_t *unlang; @@ -61,7 +41,7 @@ static unlang_action_t catch_skip_to_next(UNUSED rlm_rcode_t *p_result, UNUSED r break; } - return cleanup(frame, unlang); + return frame_set_next(frame, unlang); } static unlang_action_t unlang_catch(rlm_rcode_t *p_result, request_t *request, unlang_stack_frame_t *frame) @@ -69,7 +49,7 @@ static unlang_action_t unlang_catch(rlm_rcode_t *p_result, request_t *request, u #ifndef NDEBUG unlang_catch_t const *c = unlang_generic_to_catch(frame->instruction); - fr_assert(c->catching[*p_result]); + fr_assert(c->timeout || c->catching[*p_result]); #endif /* @@ -90,6 +70,11 @@ unlang_action_t unlang_interpret_skip_to_catch(rlm_rcode_t *p_result, request_t fr_assert(frame->instruction->type == UNLANG_TYPE_TRY); + /* + * 'try' at the end of a block without 'catch' should have been caught by the compiler. + */ + fr_assert(frame->instruction->next); + for (unlang = frame->instruction->next; unlang != NULL; unlang = unlang->next) { @@ -108,7 +93,7 @@ unlang_action_t unlang_interpret_skip_to_catch(rlm_rcode_t *p_result, request_t fr_assert(unlang != NULL); - return cleanup(frame, unlang); + return frame_set_next(frame, unlang); } void unlang_catch_init(void) diff --git a/src/lib/unlang/catch_priv.h b/src/lib/unlang/catch_priv.h index c394348cfa..76133dfd14 100644 --- a/src/lib/unlang/catch_priv.h +++ b/src/lib/unlang/catch_priv.h @@ -32,6 +32,7 @@ extern "C" { typedef struct { unlang_group_t group; + bool timeout; //!< are we catching a timeout bool catching[RLM_MODULE_NUMCODES]; } unlang_catch_t; diff --git a/src/lib/unlang/compile.c b/src/lib/unlang/compile.c index 8db27b0bfa..228b4c0090 100644 --- a/src/lib/unlang/compile.c +++ b/src/lib/unlang/compile.c @@ -2488,6 +2488,7 @@ static unlang_t *compile_catch(unlang_t *parent, unlang_compile_t *unlang_ctx, C unlang_catch_t *ca; CONF_ITEM *prev; char const *name; + bool catching_timeout = false; static unlang_ext_t const ext = { .type = UNLANG_TYPE_CATCH, @@ -2507,11 +2508,37 @@ static unlang_t *compile_catch(unlang_t *parent, unlang_compile_t *unlang_ctx, C name = cf_section_name1(cf_item_to_section(prev)); fr_assert(name != NULL); - /* - * The previous thing has to be a section. And it has to - * be either a "try" or a "catch". - */ - if ((strcmp(name, "try") != 0) && (strcmp(name, "catch") != 0)) { + if (strcmp(name, "timeout") == 0) { + CONF_ITEM *next; + + name = cf_section_name2(cs); + if (!name || (strcmp(name, "timeout") != 0)) { + cf_log_err(cs, "Invalid 'catch' after a 'timeout' section"); + return NULL; + } + + /* + * Check that the next section is NOT a "catch". + */ + next = cf_item_next(cf_parent(ci), ci); + while (next && cf_item_is_data(next)) next = cf_item_next(cf_parent(ci), next); + + if (next && cf_item_is_section(next) && + (strcmp(cf_section_name1(cf_item_to_section(next)), "catch") == 0)) { + cf_log_err(next, "Cannot have two 'catch' statements after a 'timeout' section"); + return NULL; + } + + /* + * We comp + */ + catching_timeout = true; + + } else if ((strcmp(name, "try") != 0) && (strcmp(name, "catch") != 0)) { + /* + * The previous thing has to be a section. And it has to + * be either a "try" or a "catch". + */ goto fail; } @@ -2523,10 +2550,13 @@ static unlang_t *compile_catch(unlang_t *parent, unlang_compile_t *unlang_ctx, C ca = unlang_group_to_catch(g); - /* - * No arg2: catch all errors - */ - if (!cf_section_name2(cs)) { + if (catching_timeout) { + ca->timeout = catching_timeout; + + } else if (!cf_section_name2(cs)) { + /* + * No arg2: catch errors + */ ca->catching[RLM_MODULE_REJECT] = true; ca->catching[RLM_MODULE_FAIL] = true; ca->catching[RLM_MODULE_INVALID] = true; diff --git a/src/lib/unlang/timeout.c b/src/lib/unlang/timeout.c index 233115d99b..f72e545e8e 100644 --- a/src/lib/unlang/timeout.c +++ b/src/lib/unlang/timeout.c @@ -27,7 +27,7 @@ RCSID("$Id$") #include #include "group_priv.h" #include "timeout_priv.h" -#include "interpret_priv.h" +#include "unlang_priv.h" typedef struct { bool success; @@ -72,13 +72,36 @@ static void unlang_timeout_handler(UNUSED fr_timer_list_t *tl, UNUSED fr_time_t static unlang_action_t unlang_timeout_resume_done(UNUSED rlm_rcode_t *p_result, request_t *request, unlang_stack_frame_t *frame) { unlang_frame_state_timeout_t *state = talloc_get_type_abort(frame->state, unlang_frame_state_timeout_t); + unlang_t *unlang; - if (!state->success) { - RWDEBUG("Timeout exceeded"); - return UNLANG_ACTION_FAIL; + unlang = frame->instruction->next; + + /* + * No timeout, we go to the next instruction. + * + * Unless the next instruction is a "catch timeout", in which case we skip it. + */ + if (state->success) { + if (!unlang || (unlang->type != UNLANG_TYPE_CATCH)) { + return UNLANG_ACTION_CALCULATE_RESULT; + } + + /* + * We skip the "catch" section if there's no timeout. + */ + return frame_set_next(frame, unlang->next); + } + + RWDEBUG("Timeout exceeded"); + + /* + * If there's a next instruction, AND it's a "catch", then we catch the timeout. + */ + if (unlang && (unlang->type == UNLANG_TYPE_CATCH)) { + return frame_set_next(frame, unlang); } - return UNLANG_ACTION_CALCULATE_RESULT; + return UNLANG_ACTION_FAIL; } static unlang_action_t unlang_timeout_set(rlm_rcode_t *p_result, request_t *request, unlang_stack_frame_t *frame) diff --git a/src/lib/unlang/unlang_priv.h b/src/lib/unlang/unlang_priv.h index c5ae5eee7f..e72a1f4b4b 100644 --- a/src/lib/unlang/unlang_priv.h +++ b/src/lib/unlang/unlang_priv.h @@ -521,6 +521,25 @@ static inline void frame_repeat(unlang_stack_frame_t *frame, unlang_process_t pr frame->process = process; } +static inline unlang_action_t frame_set_next(unlang_stack_frame_t *frame, unlang_t *unlang) +{ + /* + * Clean up this frame now, so that stats, etc. will be + * processed using the correct frame. + */ + frame_cleanup(frame); + + /* + * frame_next() will call cleanup *before* resetting the frame->instruction. + * but since the instruction is NULL, no duplicate cleanups will happen. + * + * frame_next() will then set frame->instruction = frame->next, and everything will be OK. + */ + frame->instruction = NULL; + frame->next = unlang; + return UNLANG_ACTION_EXECUTE_NEXT; +} + /** @name Conversion functions for converting #unlang_t to its specialisations * * Simple conversions: #unlang_module_t and #unlang_group_t are subclasses of #unlang_t, diff --git a/src/tests/keywords/timeout-catch b/src/tests/keywords/timeout-catch new file mode 100644 index 0000000000..d08523295c --- /dev/null +++ b/src/tests/keywords/timeout-catch @@ -0,0 +1,13 @@ +# +# PRE: timeout +# + +# +# @todo - we have to add a leading '0' here, otherwise cf_file.c complains +# +timeout 0.1s { + delay_10s +} +catch timeout { + success +} -- 2.47.3