From: Aurelien DARRAGON Date: Wed, 6 Sep 2023 13:02:05 +0000 (+0200) Subject: MEDIUM: httpclient/logs: rely on per-proxy post-check instead of global one X-Git-Tag: v2.9-dev5~23 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=7a12e2d369a46ce020a7d20c55dd8c1c7b99c0bc;p=thirdparty%2Fhaproxy.git MEDIUM: httpclient/logs: rely on per-proxy post-check instead of global one httpclient used to register a global post-check function to iterate over all known proxies and post-initialize httpclient related ones (mainly for logs initialization). But we currently have an issue: post_sink_resolve() function which is also registered using REGISTER_POST_CHECK() macro conflicts with httpclient_postcheck() function. This is because post_sink_resolve() relies on proxy->logsrvs to be correctly initialized already, and httpclient_postcheck() may create and insert new logsrvs entries to existing proxies when executed. So depending on which function runs first, we could run into trouble. Hopefully, to this day, everything works "by accident" due to http_client.c file being loaded before sink.c file when compiling source code. But as soon as we would move one of the two functions to other files, or if we rename files or make changes to the Makefile build recipe, we could break this at any time. To prevent post_sink_resolve() from randomly failing in the future, we now make httpclient postcheck rely on per-proxy post-checks by slightly modifying httpclient_postcheck() function so that it can be registered using REGISTER_POST_PROXY_CHECK() macro. As per-proxy post-check functions are executed right after config parsing for each known proxy (vs global post-check which are executed a bit later in the init process), we can be certain that functions registered using global post-check macro, ie: post_sink_resolve(), will always be executed after httpclient postcheck, effectively resolving the ordering conflict. This should normally not cause visible behavior changes, and while it could be considered as a bug, it's probably not worth backporting it since the only way to trigger the issue is through code refactors, unless we want to backport it to ease code maintenance of course, in which case it should easily apply for >= 2.7. --- diff --git a/src/http_client.c b/src/http_client.c index 21fdb80b2d..8346f02f6d 100644 --- a/src/http_client.c +++ b/src/http_client.c @@ -1350,11 +1350,11 @@ static int httpclient_precheck() return 0; } -static int httpclient_postcheck() +/* Initialize the logs for every proxy dedicated to the httpclient */ +static int httpclient_postcheck_proxy(struct proxy *curproxy) { int err_code = ERR_NONE; struct logsrv *logsrv; - struct proxy *curproxy = NULL; char *errmsg = NULL; #ifdef USE_OPENSSL struct server *srv = NULL; @@ -1364,61 +1364,57 @@ static int httpclient_postcheck() if (global.mode & MODE_MWORKER_WAIT) return ERR_NONE; - /* Initialize the logs for every proxy dedicated to the httpclient */ - for (curproxy = proxies_list; curproxy; curproxy = curproxy->next) { + if (!(curproxy->cap & PR_CAP_HTTPCLIENT)) + return ERR_NONE; /* nothing to do */ - if (!(curproxy->cap & PR_CAP_HTTPCLIENT)) - continue; - - /* copy logs from "global" log list */ - list_for_each_entry(logsrv, &global.logsrvs, list) { - struct logsrv *node = malloc(sizeof(*node)); + /* copy logs from "global" log list */ + list_for_each_entry(logsrv, &global.logsrvs, list) { + struct logsrv *node = malloc(sizeof(*node)); - if (!node) { - memprintf(&errmsg, "out of memory."); - err_code |= ERR_ALERT | ERR_FATAL; - goto err; - } - - memcpy(node, logsrv, sizeof(*node)); - LIST_INIT(&node->list); - LIST_APPEND(&curproxy->logsrvs, &node->list); - node->ring_name = logsrv->ring_name ? strdup(logsrv->ring_name) : NULL; - node->conf.file = logsrv->conf.file ? strdup(logsrv->conf.file) : NULL; + if (!node) { + memprintf(&errmsg, "out of memory."); + err_code |= ERR_ALERT | ERR_FATAL; + goto err; } - if (curproxy->conf.logformat_string) { - curproxy->conf.args.ctx = ARGC_LOG; - if (!parse_logformat_string(curproxy->conf.logformat_string, curproxy, &curproxy->logformat, - LOG_OPT_MANDATORY|LOG_OPT_MERGE_SPACES, - SMP_VAL_FE_LOG_END, &errmsg)) { - memprintf(&errmsg, "failed to parse log-format : %s.", errmsg); - err_code |= ERR_ALERT | ERR_FATAL; - goto err; - } - curproxy->conf.args.file = NULL; - curproxy->conf.args.line = 0; + + memcpy(node, logsrv, sizeof(*node)); + LIST_INIT(&node->list); + LIST_APPEND(&curproxy->logsrvs, &node->list); + node->ring_name = logsrv->ring_name ? strdup(logsrv->ring_name) : NULL; + node->conf.file = logsrv->conf.file ? strdup(logsrv->conf.file) : NULL; + } + if (curproxy->conf.logformat_string) { + curproxy->conf.args.ctx = ARGC_LOG; + if (!parse_logformat_string(curproxy->conf.logformat_string, curproxy, &curproxy->logformat, + LOG_OPT_MANDATORY|LOG_OPT_MERGE_SPACES, + SMP_VAL_FE_LOG_END, &errmsg)) { + memprintf(&errmsg, "failed to parse log-format : %s.", errmsg); + err_code |= ERR_ALERT | ERR_FATAL; + goto err; } + curproxy->conf.args.file = NULL; + curproxy->conf.args.line = 0; + } #ifdef USE_OPENSSL - /* initialize the SNI for the SSL servers */ + /* initialize the SNI for the SSL servers */ - for (srv = curproxy->srv; srv != NULL; srv = srv->next) { - if (srv->xprt == xprt_get(XPRT_SSL)) { - srv_ssl = srv; - } + for (srv = curproxy->srv; srv != NULL; srv = srv->next) { + if (srv->xprt == xprt_get(XPRT_SSL)) { + srv_ssl = srv; } - if (srv_ssl && !srv_ssl->sni_expr) { - /* init the SNI expression */ - /* always use the host header as SNI, without the port */ - srv_ssl->sni_expr = strdup("req.hdr(host),field(1,:)"); - err_code |= server_parse_sni_expr(srv_ssl, curproxy, &errmsg); - if (err_code & ERR_CODE) { - memprintf(&errmsg, "failed to configure sni: %s.", errmsg); - goto err; - } + } + if (srv_ssl && !srv_ssl->sni_expr) { + /* init the SNI expression */ + /* always use the host header as SNI, without the port */ + srv_ssl->sni_expr = strdup("req.hdr(host),field(1,:)"); + err_code |= server_parse_sni_expr(srv_ssl, curproxy, &errmsg); + if (err_code & ERR_CODE) { + memprintf(&errmsg, "failed to configure sni: %s.", errmsg); + goto err; } -#endif } +#endif err: if (err_code & ERR_CODE) { @@ -1432,7 +1428,7 @@ err: /* initialize the proxy and servers for the HTTP client */ REGISTER_PRE_CHECK(httpclient_precheck); -REGISTER_POST_CHECK(httpclient_postcheck); +REGISTER_POST_PROXY_CHECK(httpclient_postcheck_proxy); static int httpclient_parse_global_resolvers(char **args, int section_type, struct proxy *curpx, const struct proxy *defpx, const char *file, int line,