MINOR: actions: Use ACT_RET_CONT code to ignore an error from a custom action

Some custom actions are just ignored and skipped when an error is encoutered. In
that case, we jump to the next rule. To do so, most of them use the return code
ACT_RET_ERR. Currently, for http rules and tcp content rules, it is not a
problem because this code is handled the same way than ACT_RET_CONT. But, it
means there is no way to handle the error as other actions. The custom actions
must handle the error and return ACT_RET_DONE. For instance, when http-request
rules are processed, an error when we try to replace a header value leads to a
bad request and an error 400 is returned to the client. But when we fail to
replace the URI, the error is silently ignored. This difference between the
custom actions and the others is an obstacle to write new custom actions.

So, in this first patch, ACT_RET_CONT is now returned from custom actions
instead of ACT_RET_ERR when an error is encoutered if it should be ignored. The
behavior remains the same but it is now possible to handle true errors using the
return code ACT_RET_ERR. Some actions will probably be reviewed to determine if
an error is fatal or not. Other patches will be pushed to trigger an error when
a custom action returns the ACT_RET_ERR code.

This patch is not tagged as a bug because it is just a design issue. But others
will depends on it. So be careful during backports, if so.
This commit is contained in:
Christopher Faulet 2019-12-13 09:01:57 +01:00
parent cb9106b3e3
commit 13403761d5
4 changed files with 13 additions and 18 deletions

View File

@ -2247,7 +2247,7 @@ enum act_return dns_action_do_resolve(struct act_rule *rule, struct proxy *px,
fqdn = smp->data.u.str.area;
if (action_prepare_for_resolution(s, fqdn) == -1)
return ACT_RET_ERR;
return ACT_RET_CONT; /* on error, ignore the action */
s->dns_ctx.parent = rule;
dns_link_resolution(s, OBJ_TYPE_STREAM, 0);

View File

@ -4493,9 +4493,8 @@ parse_spoe_flt(char **args, int *cur_arg, struct proxy *px,
/* Send message of a SPOE group. This is the action_ptr callback of a rule
* associated to a "send-spoe-group" action.
*
* It returns ACT_RET_CONT is processing is finished without error, it returns
* ACT_RET_YIELD if the action is in progress. Otherwise it returns
* ACT_RET_ERR. */
* It returns ACT_RET_CONT if processing is finished (with error or not), it returns
* ACT_RET_YIELD if the action is in progress. */
static enum act_return
spoe_send_group(struct act_rule *rule, struct proxy *px,
struct session *sess, struct stream *s, int flags)
@ -4515,7 +4514,7 @@ spoe_send_group(struct act_rule *rule, struct proxy *px,
}
}
if (agent == NULL || group == NULL || ctx == NULL)
return ACT_RET_ERR;
return ACT_RET_CONT;
if (ctx->state == SPOE_CTX_ST_NONE)
return ACT_RET_CONT;
@ -4552,7 +4551,7 @@ spoe_send_group(struct act_rule *rule, struct proxy *px,
return ACT_RET_YIELD;
}
else
return ACT_RET_ERR;
return ACT_RET_CONT;
}
/* Check an "send-spoe-group" action. Here, we'll try to find the real SPOE

View File

@ -6225,7 +6225,7 @@ static enum act_return hlua_action(struct act_rule *rule, struct proxy *px,
case HLUA_E_OK:
if (!consistency_check(s, dir, &s->hlua->cons)) {
si_retnclose(&s->si[0], &msg);
return ACT_RET_ERR;
return ACT_RET_DONE;
}
if (s->hlua->flags & HLUA_STOP)
return ACT_RET_DONE;
@ -6257,7 +6257,7 @@ static enum act_return hlua_action(struct act_rule *rule, struct proxy *px,
case HLUA_E_ERRMSG:
if (!consistency_check(s, dir, &s->hlua->cons)) {
si_retnclose(&s->si[0], &msg);
return ACT_RET_ERR;
return ACT_RET_DONE;
}
/* Display log. */
SEND_ERR(px, "Lua function '%s': %s.\n",
@ -6268,7 +6268,7 @@ static enum act_return hlua_action(struct act_rule *rule, struct proxy *px,
case HLUA_E_ETMOUT:
if (!consistency_check(s, dir, &s->hlua->cons)) {
si_retnclose(&s->si[0], &msg);
return ACT_RET_ERR;
return ACT_RET_DONE;
}
SEND_ERR(px, "Lua function '%s': execution timeout.\n", rule->arg.hlua_rule->fcn.name);
return 0;
@ -6276,7 +6276,7 @@ static enum act_return hlua_action(struct act_rule *rule, struct proxy *px,
case HLUA_E_NOMEM:
if (!consistency_check(s, dir, &s->hlua->cons)) {
si_retnclose(&s->si[0], &msg);
return ACT_RET_ERR;
return ACT_RET_DONE;
}
SEND_ERR(px, "Lua function '%s': out of memory error.\n", rule->arg.hlua_rule->fcn.name);
return 0;
@ -6284,7 +6284,7 @@ static enum act_return hlua_action(struct act_rule *rule, struct proxy *px,
case HLUA_E_YIELD:
if (!consistency_check(s, dir, &s->hlua->cons)) {
si_retnclose(&s->si[0], &msg);
return ACT_RET_ERR;
return ACT_RET_DONE;
}
SEND_ERR(px, "Lua function '%s': aborting Lua processing on expired timeout.\n",
rule->arg.hlua_rule->fcn.name);
@ -6293,7 +6293,7 @@ static enum act_return hlua_action(struct act_rule *rule, struct proxy *px,
case HLUA_E_ERR:
if (!consistency_check(s, dir, &s->hlua->cons)) {
si_retnclose(&s->si[0], &msg);
return ACT_RET_ERR;
return ACT_RET_DONE;
}
/* Display log. */
SEND_ERR(px, "Lua function '%s' return an unknown error.\n",

View File

@ -51,7 +51,7 @@ static enum act_return http_action_set_req_line(struct act_rule *rule, struct pr
struct session *sess, struct stream *s, int flags)
{
struct buffer *replace;
enum act_return ret = ACT_RET_ERR;
enum act_return ret = ACT_RET_CONT;
replace = alloc_trash_chunk();
if (!replace)
@ -67,8 +67,6 @@ static enum act_return http_action_set_req_line(struct act_rule *rule, struct pr
http_req_replace_stline(rule->arg.http.action, replace->area,
replace->data, px, s);
ret = ACT_RET_CONT;
leave:
free_trash_chunk(replace);
return ret;
@ -143,7 +141,7 @@ static enum act_parse_ret parse_set_req_line(const char **args, int *orig_arg, s
static enum act_return http_action_replace_uri(struct act_rule *rule, struct proxy *px,
struct session *sess, struct stream *s, int flags)
{
enum act_return ret = ACT_RET_ERR;
enum act_return ret = ACT_RET_CONT;
struct buffer *replace, *output;
struct ist uri;
int len;
@ -171,8 +169,6 @@ static enum act_return http_action_replace_uri(struct act_rule *rule, struct pro
http_req_replace_stline((long)rule->arg.act.p[0], output->area, len, px, s);
ret = ACT_RET_CONT;
leave:
free_trash_chunk(output);
free_trash_chunk(replace);