MINOR: http-rules/tcp-rules: Call the defined action function first if defined
When TCP and HTTP rules are evaluated, if an action function (action_ptr field
in the act_rule structure) is defined for a given action, it is now always
called in priority over the test on the action type. Concretly, for now, only
custom actions define it. Thus there is no change. It just let us the choice to
extend the action type beyond the existing ones in the enum.
diff --git a/include/types/action.h b/include/types/action.h
index 8f0961e..5b4ede7 100644
--- a/include/types/action.h
+++ b/include/types/action.h
@@ -59,6 +59,11 @@
ACT_FLAG_FIRST = 0x00000002, /* first call for this action */
};
+/* known actions to be used without any action function pointer. This enum is
+ * typically used in a switch case, iff .action_ptr is undefined. So if an
+ * action function is defined for one of following action types, the function
+ * have the priority over the switch.
+ */
enum act_name {
ACT_CUSTOM = 0,
@@ -99,6 +104,8 @@
ACT_ACTION_TRK_SCMAX = ACT_ACTION_TRK_SC0 + MAX_SESS_STKCTR - 1,
};
+/* NOTE: if <.action_ptr> is defined, the referenced function will always be
+ * called regardless the action type. */
struct act_rule {
struct list list;
struct acl_cond *cond; /* acl condition to meet */
diff --git a/src/http_ana.c b/src/http_ana.c
index 9d51349..7f14c4b 100644
--- a/src/http_ana.c
+++ b/src/http_ana.c
@@ -2973,6 +2973,43 @@
}
}
+ /* Always call the action function if defined */
+ if (rule->action_ptr) {
+ if ((s->req.flags & CF_READ_ERROR) ||
+ ((s->req.flags & (CF_SHUTR|CF_READ_NULL)) &&
+ (px->options & PR_O_ABRT_CLOSE)))
+ act_flags |= ACT_FLAG_FINAL;
+
+ switch (rule->action_ptr(rule, px, sess, s, act_flags)) {
+ case ACT_RET_CONT:
+ break;
+ case ACT_RET_STOP:
+ rule_ret = HTTP_RULE_RES_STOP;
+ goto end;
+ case ACT_RET_YIELD:
+ s->current_rule = rule;
+ rule_ret = HTTP_RULE_RES_YIELD;
+ goto end;
+ case ACT_RET_ERR:
+ rule_ret = HTTP_RULE_RES_ERROR;
+ goto end;
+ case ACT_RET_DONE:
+ rule_ret = HTTP_RULE_RES_DONE;
+ goto end;
+ case ACT_RET_DENY:
+ rule_ret = HTTP_RULE_RES_DENY;
+ goto end;
+ case ACT_RET_ABRT:
+ rule_ret = HTTP_RULE_RES_ABRT;
+ goto end;
+ case ACT_RET_INV:
+ rule_ret = HTTP_RULE_RES_BADREQ;
+ goto end;
+ }
+ continue; /* eval the next rule */
+ }
+
+ /* If not action function defined, check for known actions */
switch (rule->action) {
case ACT_ACTION_ALLOW:
rule_ret = HTTP_RULE_RES_STOP;
@@ -3221,40 +3258,6 @@
}
break;
- case ACT_CUSTOM:
- if ((s->req.flags & CF_READ_ERROR) ||
- ((s->req.flags & (CF_SHUTR|CF_READ_NULL)) &&
- (px->options & PR_O_ABRT_CLOSE)))
- act_flags |= ACT_FLAG_FINAL;
-
- switch (rule->action_ptr(rule, px, s->sess, s, act_flags)) {
- case ACT_RET_CONT:
- break;
- case ACT_RET_STOP:
- rule_ret = HTTP_RULE_RES_STOP;
- goto end;
- case ACT_RET_YIELD:
- s->current_rule = rule;
- rule_ret = HTTP_RULE_RES_YIELD;
- goto end;
- case ACT_RET_ERR:
- rule_ret = HTTP_RULE_RES_ERROR;
- goto end;
- case ACT_RET_DONE:
- rule_ret = HTTP_RULE_RES_DONE;
- goto end;
- case ACT_RET_DENY:
- rule_ret = HTTP_RULE_RES_DENY;
- goto end;
- case ACT_RET_ABRT:
- rule_ret = HTTP_RULE_RES_ABRT;
- goto end;
- case ACT_RET_INV:
- rule_ret = HTTP_RULE_RES_BADREQ;
- goto end;
- }
- break;
-
case ACT_ACTION_TRK_SC0 ... ACT_ACTION_TRK_SCMAX:
/* Note: only the first valid tracking parameter of each
* applies.
@@ -3299,7 +3302,7 @@
}
break;
- /* other flags exists, but normally, they never be matched. */
+ /* other flags exists, but normally, they never be matched. */
default:
break;
}
@@ -3375,6 +3378,44 @@
act_flags |= ACT_FLAG_FIRST;
resume_execution:
+
+ /* Always call the action function if defined */
+ if (rule->action_ptr) {
+ if ((s->req.flags & CF_READ_ERROR) ||
+ ((s->req.flags & (CF_SHUTR|CF_READ_NULL)) &&
+ (px->options & PR_O_ABRT_CLOSE)))
+ act_flags |= ACT_FLAG_FINAL;
+
+ switch (rule->action_ptr(rule, px, sess, s, act_flags)) {
+ case ACT_RET_CONT:
+ break;
+ case ACT_RET_STOP:
+ rule_ret = HTTP_RULE_RES_STOP;
+ goto end;
+ case ACT_RET_YIELD:
+ s->current_rule = rule;
+ rule_ret = HTTP_RULE_RES_YIELD;
+ goto end;
+ case ACT_RET_ERR:
+ rule_ret = HTTP_RULE_RES_ERROR;
+ goto end;
+ case ACT_RET_DONE:
+ rule_ret = HTTP_RULE_RES_DONE;
+ goto end;
+ case ACT_RET_DENY:
+ rule_ret = HTTP_RULE_RES_DENY;
+ goto end;
+ case ACT_RET_ABRT:
+ rule_ret = HTTP_RULE_RES_ABRT;
+ goto end;
+ case ACT_RET_INV:
+ rule_ret = HTTP_RULE_RES_BADREQ;
+ goto end;
+ }
+ continue; /* eval the next rule */
+ }
+
+ /* If not action function defined, check for known actions */
switch (rule->action) {
case ACT_ACTION_ALLOW:
rule_ret = HTTP_RULE_RES_STOP; /* "allow" rules are OK */
@@ -3638,41 +3679,7 @@
}
break;
- case ACT_CUSTOM:
- if ((s->req.flags & CF_READ_ERROR) ||
- ((s->req.flags & (CF_SHUTR|CF_READ_NULL)) &&
- (px->options & PR_O_ABRT_CLOSE)))
- act_flags |= ACT_FLAG_FINAL;
-
- switch (rule->action_ptr(rule, px, s->sess, s, act_flags)) {
- case ACT_RET_CONT:
- break;
- case ACT_RET_STOP:
- rule_ret = HTTP_RULE_RES_STOP;
- goto end;
- case ACT_RET_YIELD:
- s->current_rule = rule;
- rule_ret = HTTP_RULE_RES_YIELD;
- goto end;
- case ACT_RET_ERR:
- rule_ret = HTTP_RULE_RES_ERROR;
- goto end;
- case ACT_RET_DONE:
- rule_ret = HTTP_RULE_RES_DONE;
- goto end;
- case ACT_RET_DENY:
- rule_ret = HTTP_RULE_RES_DENY;
- goto end;
- case ACT_RET_ABRT:
- rule_ret = HTTP_RULE_RES_ABRT;
- goto end;
- case ACT_RET_INV:
- rule_ret = HTTP_RULE_RES_BADREQ;
- goto end;
- }
- break;
-
- /* other flags exists, but normally, they never be matched. */
+ /* other flags exists, but normally, they never be matched. */
default:
break;
}
diff --git a/src/tcp_rules.c b/src/tcp_rules.c
index 6027e12..988c8e9 100644
--- a/src/tcp_rules.c
+++ b/src/tcp_rules.c
@@ -153,9 +153,36 @@
if (ret) {
act_flags |= ACT_FLAG_FIRST;
resume_execution:
- /* we have a matching rule. */
+
+ /* Always call the action function if defined */
+ if (rule->action_ptr) {
+ if (partial & SMP_OPT_FINAL)
+ act_flags |= ACT_FLAG_FINAL;
+
+ switch (rule->action_ptr(rule, s->be, s->sess, s, act_flags)) {
+ case ACT_RET_CONT:
+ break;
+ case ACT_RET_STOP:
+ case ACT_RET_DONE:
+ goto end;
+ case ACT_RET_YIELD:
+ s->current_rule = rule;
+ goto missing_data;
+ case ACT_RET_DENY:
+ goto deny;
+ case ACT_RET_ABRT:
+ goto abort;
+ case ACT_RET_ERR:
+ goto internal;
+ case ACT_RET_INV:
+ goto invalid;
+ }
+ continue; /* eval the next rule */
+ }
+
+ /* If not action function defined, check for known actions */
if (rule->action == ACT_ACTION_ALLOW) {
- break;
+ goto end;
}
else if (rule->action == ACT_ACTION_DENY) {
goto deny;
@@ -210,37 +237,10 @@
len);
cap[h->index][len] = 0;
}
- else {
- /* Custom keywords. */
- if (!rule->action_ptr)
- continue;
-
- if (partial & SMP_OPT_FINAL)
- act_flags |= ACT_FLAG_FINAL;
-
- switch (rule->action_ptr(rule, s->be, s->sess, s, act_flags)) {
- case ACT_RET_CONT:
- continue;
- case ACT_RET_STOP:
- case ACT_RET_DONE:
- break;
- case ACT_RET_YIELD:
- s->current_rule = rule;
- goto missing_data;
- case ACT_RET_DENY:
- goto deny;
- case ACT_RET_ABRT:
- goto abort;
- case ACT_RET_ERR:
- goto internal;
- case ACT_RET_INV:
- goto invalid;
- }
- break; /* ACT_RET_STOP/DONE */
- }
}
}
+ end:
/* if we get there, it means we have no rule which matches, or
* we have an explicit accept, so we apply the default accept.
*/
@@ -356,9 +356,35 @@
if (ret) {
act_flags |= ACT_FLAG_FIRST;
resume_execution:
- /* we have a matching rule. */
+ /* Always call the action function if defined */
+ if (rule->action_ptr) {
+ if (partial & SMP_OPT_FINAL)
+ act_flags |= ACT_FLAG_FINAL;
+
+ switch (rule->action_ptr(rule, s->be, s->sess, s, act_flags)) {
+ case ACT_RET_CONT:
+ break;
+ case ACT_RET_STOP:
+ case ACT_RET_DONE:
+ goto end;
+ case ACT_RET_YIELD:
+ s->current_rule = rule;
+ goto missing_data;
+ case ACT_RET_DENY:
+ goto deny;
+ case ACT_RET_ABRT:
+ goto abort;
+ case ACT_RET_ERR:
+ goto internal;
+ case ACT_RET_INV:
+ goto invalid;
+ }
+ continue; /* eval the next rule */
+ }
+
+ /* If not action function defined, check for known actions */
if (rule->action == ACT_ACTION_ALLOW) {
- break;
+ goto end;
}
else if (rule->action == ACT_ACTION_DENY) {
goto deny;
@@ -368,41 +394,12 @@
si_must_kill_conn(chn_prod(rep));
si_shutr(chn_prod(rep));
si_shutw(chn_prod(rep));
- break;
- }
- else {
- /* Custom keywords. */
- if (!rule->action_ptr)
- continue;
-
- if (partial & SMP_OPT_FINAL)
- act_flags |= ACT_FLAG_FINAL;
-
- switch (rule->action_ptr(rule, s->be, s->sess, s, act_flags)) {
- case ACT_RET_CONT:
- continue;
- case ACT_RET_STOP:
- case ACT_RET_DONE:
- break;
- case ACT_RET_YIELD:
- channel_dont_close(rep);
- s->current_rule = rule;
- DBG_TRACE_DEVEL("waiting for more data", STRM_EV_STRM_ANA|STRM_EV_TCP_ANA, s);
- return 0;
- case ACT_RET_DENY:
- goto deny;
- case ACT_RET_ABRT:
- goto abort;
- case ACT_RET_ERR:
- goto internal;
- case ACT_RET_INV:
- goto invalid;
- }
- break; /* ACT_RET_STOP/DONE */
+ goto end;
}
}
}
+ end:
/* if we get there, it means we have no rule which matches, or
* we have an explicit accept, so we apply the default accept.
*/
@@ -411,6 +408,12 @@
DBG_TRACE_LEAVE(STRM_EV_STRM_ANA|STRM_EV_TCP_ANA, s);
return 1;
+ missing_data:
+ channel_dont_close(rep);
+ s->current_rule = rule;
+ DBG_TRACE_DEVEL("waiting for more data", STRM_EV_STRM_ANA|STRM_EV_TCP_ANA, s);
+ return 0;
+
deny:
_HA_ATOMIC_ADD(&s->sess->fe->fe_counters.denied_resp, 1);
_HA_ATOMIC_ADD(&s->be->be_counters.denied_resp, 1);
@@ -481,9 +484,34 @@
}
if (ret) {
- /* we have a matching rule. */
+ /* Always call the action function if defined */
+ if (rule->action_ptr) {
+ switch (rule->action_ptr(rule, sess->fe, sess, NULL, ACT_FLAG_FINAL | ACT_FLAG_FIRST)) {
+ case ACT_RET_YIELD:
+ /* yield is not allowed at this point. If this return code is
+ * used it is a bug, so I prefer to abort the process.
+ */
+ send_log(sess->fe, LOG_WARNING,
+ "Internal error: yield not allowed with tcp-request connection actions.");
+ /* fall through */
+ case ACT_RET_STOP:
+ case ACT_RET_DONE:
+ goto end;
+ case ACT_RET_CONT:
+ break;
+ case ACT_RET_DENY:
+ case ACT_RET_ABRT:
+ case ACT_RET_ERR:
+ case ACT_RET_INV:
+ result = 0;
+ goto end;
+ }
+ continue; /* eval the next rule */
+ }
+
+ /* If not action function defined, check for known actions */
if (rule->action == ACT_ACTION_ALLOW) {
- break;
+ goto end;
}
else if (rule->action == ACT_ACTION_DENY) {
_HA_ATOMIC_ADD(&sess->fe->fe_counters.denied_conn, 1);
@@ -491,7 +519,7 @@
_HA_ATOMIC_ADD(&sess->listener->counters->denied_conn, 1);
result = 0;
- break;
+ goto end;
}
else if (rule->action >= ACT_ACTION_TRK_SC0 && rule->action <= ACT_ACTION_TRK_SCMAX) {
/* Note: only the first valid tracking parameter of each
@@ -512,7 +540,7 @@
if (!(conn->flags & (CO_FL_HANDSHAKE_NOSSL))) {
if (xprt_add_hs(conn) < 0) {
result = 0;
- break;
+ goto end;
}
}
conn->flags |= CO_FL_ACCEPT_PROXY;
@@ -521,38 +549,14 @@
if (!(conn->flags & (CO_FL_HANDSHAKE_NOSSL))) {
if (xprt_add_hs(conn) < 0) {
result = 0;
- break;
+ goto end;
}
}
conn->flags |= CO_FL_ACCEPT_CIP;
}
- else {
- /* Custom keywords. */
- if (!rule->action_ptr)
- break;
- switch (rule->action_ptr(rule, sess->fe, sess, NULL, ACT_FLAG_FINAL | ACT_FLAG_FIRST)) {
- case ACT_RET_YIELD:
- /* yield is not allowed at this point. If this return code is
- * used it is a bug, so I prefer to abort the process.
- */
- send_log(sess->fe, LOG_WARNING,
- "Internal error: yield not allowed with tcp-request connection actions.");
- case ACT_RET_STOP:
- case ACT_RET_DONE:
- break;
- case ACT_RET_CONT:
- continue;
- case ACT_RET_DENY:
- case ACT_RET_ABRT:
- case ACT_RET_ERR:
- case ACT_RET_INV:
- result = 0;
- break;
- }
- break; /* ACT_RET_STOP/DONE */
- }
}
}
+ end:
return result;
}
@@ -582,9 +586,34 @@
}
if (ret) {
- /* we have a matching rule. */
+ /* Always call the action function if defined */
+ if (rule->action_ptr) {
+ switch (rule->action_ptr(rule, sess->fe, sess, NULL, ACT_FLAG_FINAL | ACT_FLAG_FIRST)) {
+ case ACT_RET_YIELD:
+ /* yield is not allowed at this point. If this return code is
+ * used it is a bug, so I prefer to abort the process.
+ */
+ send_log(sess->fe, LOG_WARNING,
+ "Internal error: yield not allowed with tcp-request session actions.");
+ /* fall through */
+ case ACT_RET_STOP:
+ case ACT_RET_DONE:
+ goto end;
+ case ACT_RET_CONT:
+ break;
+ case ACT_RET_DENY:
+ case ACT_RET_ABRT:
+ case ACT_RET_ERR:
+ case ACT_RET_INV:
+ result = 0;
+ goto end;
+ }
+ continue; /* eval the next rule */
+ }
+
+ /* If not action function defined, check for known actions */
if (rule->action == ACT_ACTION_ALLOW) {
- break;
+ goto end;
}
else if (rule->action == ACT_ACTION_DENY) {
_HA_ATOMIC_ADD(&sess->fe->fe_counters.denied_sess, 1);
@@ -592,7 +621,7 @@
_HA_ATOMIC_ADD(&sess->listener->counters->denied_sess, 1);
result = 0;
- break;
+ goto end;
}
else if (rule->action >= ACT_ACTION_TRK_SC0 && rule->action <= ACT_ACTION_TRK_SCMAX) {
/* Note: only the first valid tracking parameter of each
@@ -609,33 +638,9 @@
if (key && (ts = stktable_get_entry(t, key)))
stream_track_stkctr(&sess->stkctr[trk_idx(rule->action)], t, ts);
}
- else {
- /* Custom keywords. */
- if (!rule->action_ptr)
- break;
- switch (rule->action_ptr(rule, sess->fe, sess, NULL, ACT_FLAG_FINAL | ACT_FLAG_FIRST)) {
- case ACT_RET_YIELD:
- /* yield is not allowed at this point. If this return code is
- * used it is a bug, so I prefer to abort the process.
- */
- send_log(sess->fe, LOG_WARNING,
- "Internal error: yield not allowed with tcp-request session actions.");
- case ACT_RET_STOP:
- case ACT_RET_DONE:
- break;
- case ACT_RET_CONT:
- continue;
- case ACT_RET_DENY:
- case ACT_RET_ABRT:
- case ACT_RET_ERR:
- case ACT_RET_INV:
- result = 0;
- break;
- }
- break; /* ACT_RET_STOP/DONE */
- }
}
}
+ end:
return result;
}