From 6199db2715561bee77bd7c22cc144c6f44050d20 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 --- include/haproxy/stats-t.h | 1 + src/cfgparse.c | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/include/haproxy/stats-t.h b/include/haproxy/stats-t.h index 8d0f0b3..7ecba26 100644 --- a/include/haproxy/stats-t.h +++ b/include/haproxy/stats-t.h @@ -41,6 +41,7 @@ #define STAT_SHOW_FDESC 0x00001000 /* show the field descriptions when possible */ #define STAT_SHMODULES 0x00002000 /* conf: show modules */ #define STAT_HIDE_MAINT 0x00004000 /* hide maint/disabled servers */ +#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 85e36a1..ba50458 100644 --- a/src/cfgparse.c +++ b/src/cfgparse.c @@ -2826,7 +2826,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", @@ -2836,7 +2836,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; @@ -2867,6 +2867,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