From 3ff528d876a5565218b1f9f8ef9f3926111b9239 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Thu, 27 Dec 2012 10:46:37 +0100 Subject: 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. --- src/proto_http.c | 88 ++++++++++++++++++++++++++++++-------------------------- 1 file changed, 48 insertions(+), 40 deletions(-) diff --git a/src/proto_http.c b/src/proto_http.c index 3b5a478..276923e 100644 --- a/src/proto_http.c +++ b/src/proto_http.c @@ -3062,61 +3062,69 @@ int http_handle_stats(struct session *s, struct channel *req) } /* Executes the http-request rules for session , proxy and - * transaction . 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 . 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_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; } @@ -3132,7 +3140,7 @@ int http_process_req_common(struct session *s, struct channel *req, int an_bit, struct http_txn *txn = &s->txn; struct http_msg *msg = &txn->req; struct acl_cond *cond; - struct http_req_rule *http_req_last_rule = NULL; + struct http_req_rule *intercept_rule = NULL; struct redirect_rule *rule; struct cond_wordlist *wl; int do_stats; @@ -3177,13 +3185,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); + intercept_rule = http_req_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) { + if (!intercept_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); + intercept_rule = http_req_intercept_rule(px, &px->uri_auth->http_req_rules, s, txn); } else do_stats = 0; @@ -3293,8 +3301,8 @@ int http_process_req_common(struct session *s, struct channel *req, int an_bit, /* we can be blocked here because the request needs to be authenticated, * either to pass or to access stats. */ - if (http_req_last_rule && http_req_last_rule->action == HTTP_REQ_ACT_AUTH) { - char *realm = http_req_last_rule->arg.auth.realm; + if (intercept_rule && intercept_rule->action == HTTP_REQ_ACT_AUTH) { + char *realm = intercept_rule->arg.auth.realm; if (!realm) realm = do_stats?STATS_DEFAULT_REALM:px->id; -- 1.7.12.4.dirty