CLEANUP: http: rename the misleading http_check_access_rule

Several bugs were introduced recently due to a misunderstanding of how
this function works and what it was supposed to do. Since it's supposed
to only return the pointer to a rule which aborts further processing of
the request, let's rename it to avoid further issues.

The function was also slightly cleaned up without any functional change.

diff --git a/src/proto_http.c b/src/proto_http.c
index be2644244697a39f2aba1b0a446a36aadaaac184..6470efd459b9f900234e2f0fc72d252eb0730bbf 100644 (file)
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -3063,61 +3063,69 @@ int http_handle_stats(struct session *s, struct channel *req)
 }
 
 /* Executes the http-request rules <rules> for session <s>, proxy <px> and
- * transaction <txn>. Returns NULL if it executed all rules, or a pointer to
- * the last rule if it had to stop before the end (auth, deny). It may set
- * the TX_CLDENY on txn->flags if it encounters a deny rule.
+ * transaction <txn>. Returns the first rule that prevents further processing
+ * of the request (auth, deny, ...) or NULL if it executed all rules or stopped
+ * on an allow. It may set the TX_CLDENY on txn->flags if it encounters a deny
+ * rule.
  */
 static struct http_req_rule *
-http_check_access_rule(struct proxy *px, struct list *rules, struct session *s, struct http_txn *txn)
+http_req_get_intercept_rule(struct proxy *px, struct list *rules, struct session *s, struct http_txn *txn)
 {
        struct http_req_rule *rule;
        struct hdr_ctx ctx;
 
        list_for_each_entry(rule, rules, list) {
-               int ret = 1;
-
                if (rule->action >= HTTP_REQ_ACT_MAX)
                        continue;
 
-               /* check condition, but only if attached */
+               /* check optional condition */
                if (rule->cond) {
+                       int ret;
+
                        ret = acl_exec_cond(rule->cond, px, s, txn, SMP_OPT_DIR_REQ|SMP_OPT_FINAL);
                        ret = acl_pass(ret);
 
                        if (rule->cond->pol == ACL_COND_UNLESS)
                                ret = !ret;
+
+                       if (!ret) /* condition not matched */
+                               continue;
                }
 
-               if (ret) {
-                       switch (rule->action) {
-                       case HTTP_REQ_ACT_ALLOW:
-                               return NULL;
-                       case HTTP_REQ_ACT_DENY:
-                               txn->flags |= TX_CLDENY;
-                               return rule;
-                       case HTTP_REQ_ACT_AUTH:
-                               return rule;
-                       case HTTP_REQ_ACT_SET_HDR:
-                               ctx.idx = 0;
-                               /* remove all occurrences of the header */
-                               while (http_find_header2(rule->arg.hdr_add.name, rule->arg.hdr_add.name_len,
-                                                        txn->req.chn->buf->p, &txn->hdr_idx, &ctx)) {
-                                       http_remove_header2(&txn->req, &txn->hdr_idx, &ctx);
-                               }
-                               /* now fall through to header addition */
-
-                       case HTTP_REQ_ACT_ADD_HDR:
-                               chunk_printf(&trash, "%s: ", rule->arg.hdr_add.name);
-                               memcpy(trash.str, rule->arg.hdr_add.name, rule->arg.hdr_add.name_len);
-                               trash.len = rule->arg.hdr_add.name_len;
-                               trash.str[trash.len++] = ':';
-                               trash.str[trash.len++] = ' ';
-                               trash.len += build_logline(s, trash.str + trash.len, trash.size - trash.len, &rule->arg.hdr_add.fmt);
-                               http_header_add_tail2(&txn->req, &txn->hdr_idx, trash.str, trash.len);
-                               break;
+
+               switch (rule->action) {
+               case HTTP_REQ_ACT_ALLOW:
+                       return NULL; /* "allow" rules are OK */
+
+               case HTTP_REQ_ACT_DENY:
+                       txn->flags |= TX_CLDENY;
+                       return rule;
+
+               case HTTP_REQ_ACT_AUTH:
+                       return rule;
+
+               case HTTP_REQ_ACT_SET_HDR:
+                       ctx.idx = 0;
+                       /* remove all occurrences of the header */
+                       while (http_find_header2(rule->arg.hdr_add.name, rule->arg.hdr_add.name_len,
+                                                txn->req.chn->buf->p, &txn->hdr_idx, &ctx)) {
+                               http_remove_header2(&txn->req, &txn->hdr_idx, &ctx);
                        }
+                       /* now fall through to header addition */
+
+               case HTTP_REQ_ACT_ADD_HDR:
+                       chunk_printf(&trash, "%s: ", rule->arg.hdr_add.name);
+                       memcpy(trash.str, rule->arg.hdr_add.name, rule->arg.hdr_add.name_len);
+                       trash.len = rule->arg.hdr_add.name_len;
+                       trash.str[trash.len++] = ':';
+                       trash.str[trash.len++] = ' ';
+                       trash.len += build_logline(s, trash.str + trash.len, trash.size - trash.len, &rule->arg.hdr_add.fmt);
+                       http_header_add_tail2(&txn->req, &txn->hdr_idx, trash.str, trash.len);
+                       break;
                }
        }
+
+       /* we reached the end of the rules, nothing to report */
        return NULL;
 }
 
@@ -3178,13 +3186,13 @@ int http_process_req_common(struct session *s, struct channel *req, int an_bit,
        session_inc_be_http_req_ctr(s);
 
        /* evaluate http-request rules */
-       http_req_last_rule = http_check_access_rule(px, &px->http_req_rules, s, txn);
+       http_req_last_rule = http_req_get_intercept_rule(px, &px->http_req_rules, s, txn);
 
        /* evaluate stats http-request rules only if http-request is OK */
        if (!http_req_last_rule) {
                do_stats = stats_check_uri(s->rep->prod, txn, px);
                if (do_stats)
-                       http_req_last_rule = http_check_access_rule(px, &px->uri_auth->http_req_rules, s, txn);
+                       http_req_last_rule = http_req_get_intercept_rule(px, &px->uri_auth->http_req_rules, s, txn);
        }
        else
                do_stats = 0;