From: Alan T. DeKok Date: Thu, 14 Dec 2023 02:02:53 +0000 (-0500) Subject: set default action return codes to "fail=1" X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2fb5d7d4bc4986383ab18b8447515661eedb3a45;p=thirdparty%2Ffreeradius-server.git set default action return codes to "fail=1" because otherwise most tests will have to manually set an "actions" block --- diff --git a/doc/antora/modules/reference/pages/unlang/transaction.adoc b/doc/antora/modules/reference/pages/unlang/transaction.adoc index e2b34fd7987..09c3d09c7a4 100644 --- a/doc/antora/modules/reference/pages/unlang/transaction.adoc +++ b/doc/antora/modules/reference/pages/unlang/transaction.adoc @@ -17,21 +17,13 @@ been made will be reverted. == Caveats -The `transaction` keyword currently has some major limitations. +The `transaction` sets its own action defaults for return codes +`fail`, `invalid`, and `disallow`. The priority for those return +codes is set to `1`, instead of the default `return`. This behavior +allows for the caller to edit attributes, but continue processing if +something goes wrong. -The first limitation is that most processing sections have a default -action of `return` for failures. This setting means that failures -cause the server to stop processing the entire section. The -`transaction` keyword is no different. - -If instead the server should continue processing the next statement -after the `transaction`, then the `transaction` section must finish -with an xref:unlang/actions.adoc[actions] subsection, as given in the -example below. This subsection over-ride sthe default action of -`return` for failures, and allows processing to continue after the -`transaction`. - -The second limitation of `transaction` is that it can only revert +The main limitation of `transaction` is that it can only revert attribute editing which is done via the xref:unlang/edit.adoc[edit] statements. If a module performs attribute editing (e.g. `sql`, `files`, etc.), then those edits are not reverted. @@ -40,21 +32,21 @@ Similarly, a `transaction` cannot undo operations on external databases. For example, any data which was inserted into `sql` during a `transaction` statement will remain in `sql`. -.Examples +.Example + +In this example, if the SQL `select` statement fails, then the +`&reply.Framed-IP-Address` attribute is _not_ updated, and the +`transaction` section returns `fail`. [source,unlang] ---- transaction { - &reply.Filter-Id := %exec - sql - - actions { - fail = 1 - } + &reply.Filter-Id := %sql("SELECT ...") + &reply.Framed-IP-Address := 192.0.2.1 } ---- -The last entry in a `transaction` section can also be an xref:unlang/actions.adoc[actions] subsection. +The last entry in a `transaction` section can also be an xref:unlang/actions.adoc[actions] subsection. If set, those actions over-ride any previously set defaults. == Grouping Edits @@ -63,7 +55,7 @@ xref:unlang/edit.adoc[edit] instructions. When edit instructions are grouped, then the edits are done in an atomic transaction. That is, either all of the edits succeed, or none of them do. -For now, the only purpose of `transaction` is to group edits. +For now, the only purpose of `transaction` is to group edits. If a module is used inside of // Copyright (C) 2023 Network RADIUS SAS. Licenced under CC-by-NC 4.0. // This documentation was developed by Network RADIUS SAS. diff --git a/src/lib/unlang/compile.c b/src/lib/unlang/compile.c index b03e31caaa1..b3af9d0f815 100644 --- a/src/lib/unlang/compile.c +++ b/src/lib/unlang/compile.c @@ -2183,7 +2183,7 @@ static bool compile_action_subsection(unlang_t *c, CONF_SECTION *cs, CONF_SECTIO } -static unlang_t *compile_children(unlang_group_t *g, unlang_compile_t *unlang_ctx_in) +static unlang_t *compile_children(unlang_group_t *g, unlang_compile_t *unlang_ctx_in, bool set_action_defaults) { CONF_ITEM *ci = NULL; unlang_t *c, *single; @@ -2439,7 +2439,7 @@ static unlang_t *compile_children(unlang_group_t *g, unlang_compile_t *unlang_ct * Set the default actions, if they haven't already been * set by an "actions" section above. */ - compile_action_defaults(c, unlang_ctx); + if (set_action_defaults) compile_action_defaults(c, unlang_ctx); return c; } @@ -2487,7 +2487,7 @@ static unlang_t *compile_section(unlang_t *parent, unlang_compile_t *unlang_ctx, MEM(c->debug_name = talloc_typed_asprintf(c, "%s %s", name1, name2)); } - return compile_children(g, unlang_ctx); + return compile_children(g, unlang_ctx, true); } @@ -2629,6 +2629,7 @@ static bool transaction_ok(CONF_SECTION *cs, bool *all_edits) static unlang_t *compile_transaction(unlang_t *parent, unlang_compile_t *unlang_ctx, CONF_SECTION *cs) { + unlang_group_t *g; unlang_t *c; bool all_edits = true; unlang_compile_t unlang_ctx2; @@ -2639,6 +2640,11 @@ static unlang_t *compile_transaction(unlang_t *parent, unlang_compile_t *unlang_ .type_name = "unlang_transaction_t", }; + /* + * The transaction is empty, ignore it. + */ + if (!cf_item_next(cs, NULL)) return UNLANG_IGNORE; + if (!transaction_ok(cs, &all_edits)) return NULL; /* @@ -2653,10 +2659,27 @@ static unlang_t *compile_transaction(unlang_t *parent, unlang_compile_t *unlang_ unlang_ctx2.all_edits = all_edits; - c = compile_section(parent, &unlang_ctx2, cs, &transaction); - if (!c || (c == UNLANG_IGNORE)) return c; + /* + * We always create a group, even if the section is empty. + */ + g = group_allocate(parent, cs, &transaction); + if (!g) return NULL; + + c = unlang_group_to_generic(g); + c->debug_name = c->name = cf_section_name1(cs); + + if (!compile_children(g, &unlang_ctx2, false)) return NULL; + + /* + * The default for a failed transaction is to continue on + * failure. + */ + if (!c->actions.actions[RLM_MODULE_FAIL]) c->actions.actions[RLM_MODULE_FAIL] = 1; + if (!c->actions.actions[RLM_MODULE_INVALID]) c->actions.actions[RLM_MODULE_INVALID] = 1; + if (!c->actions.actions[RLM_MODULE_DISALLOW]) c->actions.actions[RLM_MODULE_DISALLOW] = 1; + + compile_action_defaults(c, unlang_ctx); - c->type = UNLANG_TYPE_TRANSACTION; return c; } diff --git a/src/tests/keywords/transaction b/src/tests/keywords/transaction index f26ec8c3be0..222e10b9e0b 100644 --- a/src/tests/keywords/transaction +++ b/src/tests/keywords/transaction @@ -10,10 +10,6 @@ transaction { fail &bar := "nope" - - actions { - fail = 1 - } } #