From: Willy Tarreau Date: Sun, 27 Jul 2008 20:02:32 +0000 (+0200) Subject: [MEDIUM] acl: when possible, report the name and requirements of ACLs in warnings X-Git-Tag: v1.3.16-rc1~221 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=dd64f8d394330f04ccdf6081586c84feb5ff27ff;p=thirdparty%2Fhaproxy.git [MEDIUM] acl: when possible, report the name and requirements of ACLs in warnings When an ACL is referenced at a wrong place (eg: response during request, layer7 during layer4), try to indicate precisely the name and requirements of this ACL. Only the first faulty ACL is returned. A small change consisting in iterating that way may improve reports : cap = ACL_USE_any_unexpected while ((acl=cond_find_require(cond, cap))) { warning() cap &= ~acl->requires; } This will report the first ACL of each unsupported type. But doing so will mangle the error reporting a lot, so we need to rework error reports first. --- diff --git a/include/proto/acl.h b/include/proto/acl.h index ae64a5053f..fffce48156 100644 --- a/include/proto/acl.h +++ b/include/proto/acl.h @@ -87,6 +87,11 @@ struct acl_cond *parse_acl_cond(const char **args, struct list *known_acl, int p */ int acl_exec_cond(struct acl_cond *cond, struct proxy *px, struct session *l4, void *l7, int dir); +/* Reports a pointer to the first ACL used in condition which requires + * at least one of the USE_FLAGS in . Returns NULL if none matches. + */ +struct acl *cond_find_require(struct acl_cond *cond, unsigned int require); + /* Return a pointer to the ACL within the list starting at , or * NULL if not found. */ diff --git a/src/acl.c b/src/acl.c index 30c30de6c1..ed41e91ed0 100644 --- a/src/acl.c +++ b/src/acl.c @@ -1096,6 +1096,30 @@ int acl_exec_cond(struct acl_cond *cond, struct proxy *px, struct session *l4, v } +/* Reports a pointer to the first ACL used in condition which requires + * at least one of the USE_FLAGS in . Returns NULL if none matches. + * The construct is almost the same as for acl_exec_cond() since we're walking + * down the ACL tree as well. It is important that the tree is really walked + * through and never cached, because that way, this function can be used as a + * late check. + */ +struct acl *cond_find_require(struct acl_cond *cond, unsigned int require) +{ + struct acl_term_suite *suite; + struct acl_term *term; + struct acl *acl; + + list_for_each_entry(suite, &cond->suites, list) { + list_for_each_entry(term, &suite->terms, list) { + acl = term->acl; + if (acl->requires & require) + return acl; + } + } + return NULL; +} + + /************************************************************************/ /* All supported keywords must be declared here. */ /************************************************************************/ diff --git a/src/cfgparse.c b/src/cfgparse.c index 1f1755ff9f..09f63a40c7 100644 --- a/src/cfgparse.c +++ b/src/cfgparse.c @@ -1244,8 +1244,13 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int inv) cond->line = linenum; if (cond->requires & ACL_USE_RTR_ANY) { - Warning("parsing [%s:%d] : switching rule involves some response-only criteria which will be ignored.\n", - file, linenum); + struct acl *acl; + const char *name; + + acl = cond_find_require(cond, ACL_USE_RTR_ANY); + name = acl ? acl->name : "(unknown)"; + Warning("parsing [%s:%d] : acl '%s' involves some response-only criteria which will be ignored.\n", + file, linenum, name); } rule = (struct switching_rule *)calloc(1, sizeof(*rule)); diff --git a/src/proto_tcp.c b/src/proto_tcp.c index 3d93a34b10..cc96033c8d 100644 --- a/src/proto_tcp.c +++ b/src/proto_tcp.c @@ -377,6 +377,7 @@ static int tcp_parse_tcp_req(char **args, int section_type, struct proxy *curpx, if (!strcmp(args[1], "content")) { int action; + int warn = 0; int pol = ACL_COND_NONE; struct acl_cond *cond; struct tcp_rule *rule; @@ -410,17 +411,32 @@ static int tcp_parse_tcp_req(char **args, int section_type, struct proxy *curpx, if (pol != ACL_COND_NONE && (cond = parse_acl_cond((const char **)args+4, &curpx->acl, pol)) == NULL) { retlen = snprintf(err, errlen, - "Error detected in %s '%s' while parsing '%s' condition", + "error detected in %s '%s' while parsing '%s' condition", proxy_type_str(curpx), curpx->id, args[3]); return -1; } + // FIXME: how to set this ? + // cond->line = linenum; + if (cond->requires & (ACL_USE_RTR_ANY | ACL_USE_L7_ANY)) { + struct acl *acl; + const char *name; + + acl = cond_find_require(cond, ACL_USE_RTR_ANY|ACL_USE_L7_ANY); + name = acl ? acl->name : "(unknown)"; + + retlen = snprintf(err, errlen, + "acl '%s' involves some %s criteria which will be ignored.", + name, + (acl->requires & ACL_USE_RTR_ANY) ? "response-only" : "layer 7"); + warn++; + } rule = (struct tcp_rule *)calloc(1, sizeof(*rule)); rule->cond = cond; rule->action = action; LIST_INIT(&rule->list); LIST_ADDQ(&curpx->tcp_req.inspect_rules, &rule->list); - return 0; + return warn; } snprintf(err, errlen, "unknown argument '%s' after '%s' in %s '%s'",