From: Christopher Faulet <cfaulet@haproxy.com>
Date: Mon, 25 Apr 2022 12:30:58 +0000 (+0200)
Subject: BUG/MEDIUM: rules: Be able to use captures defined in defaults section
X-Git-Tag: v2.6-dev8~89
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=5796228abad4a0113daaef4be7e42e5234cfa15a;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: rules: Be able to use captures defined in defaults section

Since the 2.5, it is possible to define TCP/HTTP ruleset in defaults
sections. However, rules defining a capture in defaults sections was not
properly handled because they was not shared with the proxies inheriting
from the defaults section. This led to crash when haproxy tried to store a
new capture.

So now, to fix the issue, when a new proxy is created, the list of captures
points to the list of its defaults section. It may be NULL or not. All new
caputres are prepended to this list. It is not a problem to share the same
defaults section between several proxies, because it is not altered and we
take care to not release it when corresponding proxies are freed but only
when defaults proxies are freed. To do so, defaults proxies are now
unreferenced at the end of free_proxy() function instead of the beginning.

This patch should fix the issue #1674. It must be backported to 2.5.
---

diff --git a/reg-tests/http-rules/default_rules.vtc b/reg-tests/http-rules/default_rules.vtc
index f4a8fb9d25..cc726ab08f 100644
--- a/reg-tests/http-rules/default_rules.vtc
+++ b/reg-tests/http-rules/default_rules.vtc
@@ -92,6 +92,30 @@ haproxy h2 -conf {
 
 } -start
 
+haproxy h3 -conf {
+  defaults base-http
+      mode http
+      timeout connect "${HAPROXY_TEST_TIMEOUT-5s}"
+      timeout client  "${HAPROXY_TEST_TIMEOUT-5s}"
+      timeout server  "${HAPROXY_TEST_TIMEOUT-5s}"
+
+      http-request capture hdr(Host)  len 64  # idx 0
+      http-request capture hdr(X-req-1) len 32  # idx 1
+
+  frontend fe1 from base-http
+      bind "fd@${fe1h3}"
+      declare capture request len 32 # idx 2
+
+      http-request capture hdr(X-req-2) id 2
+      http-request return status 200 hdr "X-Capture-1" "%[capture.req.hdr(0)]" hdr "X-Capture-2" "%[capture.req.hdr(1)]" hdr "X-Capture-3" "%[capture.req.hdr(2)]"
+
+  frontend fe2 from base-http
+      bind "fd@${fe2h3}"
+      http-request capture hdr(X-req-2) id 1
+      http-request return status 200 hdr "X-Capture-1" "%[capture.req.hdr(0)]" hdr "X-Capture-2" "%[capture.req.hdr(1)]"
+
+} -start
+
 client c1 -connect ${h1_feh1_sock} {
         txreq -req GET -url /
         rxresp
@@ -109,3 +133,27 @@ client c2 -connect ${h2_feh2_sock} {
         rxresp
         expect resp.status == 200
 } -run
+
+client c3 -connect ${h3_fe1h3_sock} {
+        txreq -req GET -url /  \
+          -hdr "host: v-test"  \
+          -hdr "x-req-1: val1" \
+          -hdr "x-req-2: val2"
+        rxresp
+        expect resp.status == 200
+        expect resp.http.x-capture-1 == "v-test"
+        expect resp.http.x-capture-2 == "val1"
+        expect resp.http.x-capture-3 == "val2"
+} -run
+
+client c4 -connect ${h3_fe2h3_sock} {
+        txreq -req GET -url /  \
+          -hdr "host: v-test"  \
+          -hdr "x-req-1: val1" \
+          -hdr "x-req-2: val2"
+        rxresp
+        expect resp.status == 200
+        expect resp.http.x-capture-1 == "v-test"
+        expect resp.http.x-capture-2 == "val2"
+        expect resp.http.x-capture-3 == "<undef>"
+} -run
diff --git a/src/proxy.c b/src/proxy.c
index 63181fa74a..55f4200dc7 100644
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -148,7 +148,6 @@ void free_proxy(struct proxy *p)
 	if (!p)
 		return;
 
-	proxy_unref_defaults(p);
 	free(p->conf.file);
 	free(p->id);
 	free(p->cookie_name);
@@ -283,6 +282,8 @@ void free_proxy(struct proxy *p)
 
 	h = p->req_cap;
 	while (h) {
+		if (p->defpx && h == p->defpx->req_cap)
+			break;
 		h_next = h->next;
 		free(h->name);
 		pool_destroy(h->pool);
@@ -292,6 +293,8 @@ void free_proxy(struct proxy *p)
 
 	h = p->rsp_cap;
 	while (h) {
+		if (p->defpx && h == p->defpx->rsp_cap)
+			break;
 		h_next = h->next;
 		free(h->name);
 		pool_destroy(h->pool);
@@ -344,6 +347,8 @@ void free_proxy(struct proxy *p)
 
 	HA_RWLOCK_DESTROY(&p->lbprm.lock);
 	HA_RWLOCK_DESTROY(&p->lock);
+
+	proxy_unref_defaults(p);
 	ha_free(&p);
 }
 
@@ -1426,6 +1431,7 @@ void proxy_free_defaults(struct proxy *defproxy)
 {
 	struct acl *acl, *aclb;
 	struct logsrv *log, *logb;
+	struct cap_hdr *h,*h_next;
 
 	ha_free(&defproxy->id);
 	ha_free(&defproxy->conf.file);
@@ -1459,6 +1465,24 @@ void proxy_free_defaults(struct proxy *defproxy)
 	free_act_rules(&defproxy->http_res_rules);
 	free_act_rules(&defproxy->http_after_res_rules);
 
+	h = defproxy->req_cap;
+	while (h) {
+		h_next = h->next;
+		free(h->name);
+		pool_destroy(h->pool);
+		free(h);
+		h = h_next;
+	}
+
+	h = defproxy->rsp_cap;
+	while (h) {
+		h_next = h->next;
+		free(h->name);
+		pool_destroy(h->pool);
+		free(h);
+		h = h_next;
+	}
+
 	if (defproxy->conf.logformat_string != default_http_log_format &&
 	    defproxy->conf.logformat_string != default_tcp_log_format &&
 	    defproxy->conf.logformat_string != clf_http_log_format &&
@@ -1704,6 +1728,12 @@ static int proxy_defproxy_cpy(struct proxy *curproxy, const struct proxy *defpro
 			curproxy->capture_name = strdup(defproxy->capture_name);
 		curproxy->capture_namelen = defproxy->capture_namelen;
 		curproxy->capture_len = defproxy->capture_len;
+
+		curproxy->nb_req_cap = defproxy->nb_req_cap;
+		curproxy->req_cap = defproxy->req_cap;
+
+		curproxy->nb_rsp_cap = defproxy->nb_rsp_cap;
+		curproxy->rsp_cap = defproxy->rsp_cap;
 	}
 
 	if (curproxy->cap & PR_CAP_FE) {