From ce3ca6b309cd035fe9f41dbd87e897a5351ed4f2 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Fri, 12 Feb 2021 11:49:25 +0100 Subject: [PATCH] BUG/MINOR: stats: revert the change on ST_CONVDONE In 2.1, commit ee4f5f83d ("MINOR: stats: get rid of the ST_CONVDONE flag") introduced a subtle bug. By testing curproxy against defproxy in check_config_validity(), it tried to eliminate the need for a flag to indicate that stats authentication rules were already compiled, but by doing so it left the issue opened for the case where a new defaults section appears after the two proxies sharing the first one: defaults mode http stats auth foo:bar listen l1 bind :8080 listen l2 bind :8181 defaults # just to break above This config results in: [ALERT] 042/113725 (3121) : proxy 'f2': stats 'auth'/'realm' and 'http-request' can't be used at the same time. [ALERT] 042/113725 (3121) : Fatal errors found in configuration. Removing the last defaults remains OK. It turns out that the cleanups that followed that patch render it useless, so the best fix is to revert the change (with the up-to-date flags instead). The flag was marked as belonging to the config. It's not exact but it's the closest to the reality, as it's not there to configure the behavior but ti mention that the config parser did its job. This could be backported as far as 2.1, but in practice it looks like nobody ever hit it. (cherry picked from commit 5bbc676608f654ae76c7a4cc5852a443bfe8bd41) Signed-off-by: Christopher Faulet (cherry picked from commit 6199db2715561bee77bd7c22cc144c6f44050d20) Signed-off-by: Christopher Faulet (cherry picked from commit ef2c61682b6f8bf383f5c65a89a0e13d15f71eb3) Signed-off-by: Christopher Faulet --- include/types/stats.h | 1 + src/cfgparse.c | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/include/types/stats.h b/include/types/stats.h index 9a26a3a..38ed54e 100644 --- a/include/types/stats.h +++ b/include/types/stats.h @@ -35,6 +35,7 @@ #define STAT_SHDESC 0x00000400 /* conf: show description */ #define STAT_SHLGNDS 0x00000800 /* conf: show legends */ #define STAT_SHOW_FDESC 0x00001000 /* show the field descriptions when possible */ +#define STAT_CONVDONE 0x00008000 /* conf: rules conversion done */ #define STAT_BOUND 0x00800000 /* bound statistics to selected proxies/types/services */ #define STAT_STARTED 0x01000000 /* some output has occurred */ diff --git a/src/cfgparse.c b/src/cfgparse.c index a4526b3..c38c06a 100644 --- a/src/cfgparse.c +++ b/src/cfgparse.c @@ -2913,7 +2913,7 @@ int check_config_validity() } } - if (curproxy->uri_auth && curproxy->uri_auth != defproxy.uri_auth && + if (curproxy->uri_auth && !(curproxy->uri_auth->flags & STAT_CONVDONE) && !LIST_ISEMPTY(&curproxy->uri_auth->http_req_rules) && (curproxy->uri_auth->userlist || curproxy->uri_auth->auth_realm )) { ha_alert("%s '%s': stats 'auth'/'realm' and 'http-request' can't be used at the same time.\n", @@ -2923,7 +2923,7 @@ int check_config_validity() } if (curproxy->uri_auth && curproxy->uri_auth->userlist && - (curproxy->uri_auth != defproxy.uri_auth || + (!(curproxy->uri_auth->flags & STAT_CONVDONE) || LIST_ISEMPTY(&curproxy->uri_auth->http_req_rules))) { const char *uri_auth_compat_req[10]; struct act_rule *rule; @@ -2954,6 +2954,7 @@ int check_config_validity() free(curproxy->uri_auth->auth_realm); curproxy->uri_auth->auth_realm = NULL; } + curproxy->uri_auth->flags |= STAT_CONVDONE; } out_uri_auth_compat: -- 1.7.10.4