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 8f0961e6411cc15d4daed7566410420738c242dc..5b4ede7c96d310cacc85d9b36977032b1015012b 100644 (file)
--- a/include/types/action.h
+++ b/include/types/action.h
@@ -59,6 +59,11 @@ enum act_flag {
        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 @@ enum act_name {
        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 9d513495285f409c2ca2c759faa0ce58d149b88e..7f14c4b8aff00789bca7b57b42fc5c412e9667a8 100644 (file)
--- a/src/http_ana.c
+++ b/src/http_ana.c
@@ -2973,6 +2973,43 @@ static enum rule_result http_req_get_intercept_rule(struct proxy *px, struct lis
                        }
                }
 
+               /* 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 @@ static enum rule_result http_req_get_intercept_rule(struct proxy *px, struct lis
                                }
                                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 @@ static enum rule_result http_req_get_intercept_rule(struct proxy *px, struct lis
                                }
                                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 @@ static enum rule_result http_res_get_intercept_rule(struct proxy *px, struct lis
 
                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 @@ resume_execution:
                                }
                                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 6027e1250c721fc63f45ec1c93427c6e1e4de2a8..988c8e9b4dd871348a07cc0b86a2e7cc1b0de676 100644 (file)
--- a/src/tcp_rules.c
+++ b/src/tcp_rules.c
@@ -153,9 +153,36 @@ int tcp_inspect_request(struct stream *s, struct channel *req, int an_bit)
                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 @@ resume_execution:
                                       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 @@ int tcp_inspect_response(struct stream *s, struct channel *rep, int an_bit)
                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 @@ resume_execution:
                                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 @@ resume_execution:
        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 @@ int tcp_exec_l4_rules(struct session *sess)
                }
 
                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 @@ int tcp_exec_l4_rules(struct session *sess)
                                        _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 @@ int tcp_exec_l4_rules(struct session *sess)
                                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 @@ int tcp_exec_l4_rules(struct session *sess)
                                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 @@ int tcp_exec_l5_rules(struct session *sess)
                }
 
                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 @@ int tcp_exec_l5_rules(struct session *sess)
                                        _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 @@ int tcp_exec_l5_rules(struct session *sess)
                                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;
 }