From 0cf811a5f941261176b67046dbc542d0479ff4a7 Mon Sep 17 00:00:00 2001 From: Tim Duesterhus Date: Wed, 5 Feb 2020 21:00:50 +0100 Subject: [PATCH] MINOR: acl: Warn when an ACL is named 'or' Consider a configuration like this: > acl t always_true > acl or always_false > > http-response set-header Foo Bar if t or t The 'or' within the condition will be treated as a logical disjunction and the header will be set, despite the ACL 'or' being falsy. This patch makes it an error to declare such an ACL that will never work. This patch may be backported to stable releases, turning the error into a warning only (the code was written in a way to make this trivial). It should not break anything and might improve the users' lifes. --- src/cfgparse-listen.c | 8 ++++++++ src/fcgi-app.c | 11 ++++++++++- src/flt_spoe.c | 7 +++++++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/cfgparse-listen.c b/src/cfgparse-listen.c index 70627c30a..77a5d5b8f 100644 --- a/src/cfgparse-listen.c +++ b/src/cfgparse-listen.c @@ -807,6 +807,14 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int kwm) goto out; } + if (strcasecmp(args[1], "or") == 0) { + ha_warning("parsing [%s:%d] : acl name '%s' will never match. 'or' is used to express a " + "logical disjunction within a condition.\n", + file, linenum, args[1]); + err_code |= ERR_ALERT | ERR_FATAL; + goto out; + } + if (parse_acl((const char **)args + 1, &curproxy->acl, &errmsg, &curproxy->conf.args, file, linenum) == NULL) { ha_alert("parsing [%s:%d] : error detected while parsing ACL '%s' : %s.\n", file, linenum, args[1], errmsg); diff --git a/src/fcgi-app.c b/src/fcgi-app.c index f7108c376..7b28e3a9c 100644 --- a/src/fcgi-app.c +++ b/src/fcgi-app.c @@ -885,11 +885,20 @@ static int cfg_parse_fcgi_app(const char *file, int linenum, char **args, int kw ha_alert("parsing [%s:%d] : character '%c' is not permitted in acl name '%s'.\n", file, linenum, *err, args[1]); err_code |= ERR_ALERT | ERR_FATAL; + goto out; } - else if (parse_acl((const char **)args+1, &curapp->acls, &errmsg, &curapp->conf.args, file, linenum) == NULL) { + if (strcasecmp(args[1], "or") == 0) { + ha_warning("parsing [%s:%d] : acl name '%s' will never match. 'or' is used to express a " + "logical disjunction within a condition.\n", + file, linenum, args[1]); + err_code |= ERR_ALERT | ERR_FATAL; + goto out; + } + if (parse_acl((const char **)args+1, &curapp->acls, &errmsg, &curapp->conf.args, file, linenum) == NULL) { ha_alert("parsing [%s:%d] : error detected while parsing ACL '%s' : %s.\n", file, linenum, args[1], errmsg); err_code |= ERR_ALERT | ERR_FATAL; + goto out; } } else if (!strcmp(args[0], "set-param")) { diff --git a/src/flt_spoe.c b/src/flt_spoe.c index e3328cc01..06c70d24f 100644 --- a/src/flt_spoe.c +++ b/src/flt_spoe.c @@ -3991,6 +3991,13 @@ cfg_parse_spoe_message(const char *file, int linenum, char **args, int kwm) err_code |= ERR_ALERT | ERR_FATAL; goto out; } + if (strcasecmp(args[1], "or") == 0) { + ha_warning("parsing [%s:%d] : acl name '%s' will never match. 'or' is used to express a " + "logical disjunction within a condition.\n", + file, linenum, args[1]); + err_code |= ERR_ALERT | ERR_FATAL; + goto out; + } if (parse_acl((const char **)args + 1, &curmsg->acls, &errmsg, &curproxy->conf.args, file, linenum) == NULL) { ha_alert("parsing [%s:%d] : error detected while parsing ACL '%s' : %s.\n", file, linenum, args[1], errmsg);