89 lines
3.4 KiB
Diff
89 lines
3.4 KiB
Diff
From 61882cc68c8336016f158f74c0944e1b047c6f5f Mon Sep 17 00:00:00 2001
|
|
From: Aurelien DARRAGON <adarragon@haproxy.com>
|
|
Date: Fri, 18 Nov 2022 09:17:29 +0100
|
|
Subject: [PATCH] BUG/MINOR: http_ana/txn: don't re-initialize txn and req var
|
|
lists
|
|
|
|
In http_create_txn(): vars_init_head() was performed on both s->vars_txn
|
|
and s->var_reqres lists.
|
|
|
|
But this is wrong, these two lists are already initialized upon stream
|
|
creation in stream_new().
|
|
Moreover, between stream_new() and http_create_txn(), some variable may
|
|
be defined (e.g.: by the frontend), resulting in lists not being empty.
|
|
|
|
Because of this "extra" list initialization, already defined variables
|
|
can be lost.
|
|
This causes txn dependant code not being able to access previously defined
|
|
variables as well as memory leak because http_destroy_txn() relies on these
|
|
lists to perform the purge.
|
|
|
|
This proved to be the case when a frontend sets variables and lua sample
|
|
fetch is used in backend section as described in GH #1935.
|
|
Many thanks to Darragh O'Toole for his detailed report.
|
|
|
|
Removing extra var_init_head (x2) in http_create_txn() to fix the issue.
|
|
Adding somme comments in the code in an attempt to prevent future misuses
|
|
of s->var_reqres, and s->var_txn lists.
|
|
|
|
It should be backported in every stable version.
|
|
(This is an old bug that seems to exist since 1.6-dev6)
|
|
|
|
[cf: On 2.0 and 1.8, for the legacy HTTP code, vars_init() are used during
|
|
the TXN cleanup, when the stream is reused. So, these calls must be
|
|
moved from http_init_txn() to http_reset_txn() and not removed.]
|
|
|
|
(cherry picked from commit 5ad2b642625b89cdf4f5fd26a598fc480abdc806)
|
|
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
|
|
|
|
Conflict: NA
|
|
Reference: https://git.haproxy.org/?p=haproxy-2.6.git;a=commit;h=61882cc68c8336016f158f74c0944e1b047c6f5f
|
|
---
|
|
src/http_ana.c | 6 ++++--
|
|
src/stream.c | 11 ++++++++++-
|
|
2 files changed, 14 insertions(+), 3 deletions(-)
|
|
|
|
diff --git a/src/http_ana.c b/src/http_ana.c
|
|
index 2b2cfdc56..379b480a8 100644
|
|
--- a/src/http_ana.c
|
|
+++ b/src/http_ana.c
|
|
@@ -5224,8 +5224,10 @@ struct http_txn *http_create_txn(struct stream *s)
|
|
|
|
txn->auth.method = HTTP_AUTH_UNKNOWN;
|
|
|
|
- vars_init_head(&s->vars_txn, SCOPE_TXN);
|
|
- vars_init_head(&s->vars_reqres, SCOPE_REQ);
|
|
+ /* here we don't want to re-initialize s->vars_txn and s->vars_reqres
|
|
+ * variable lists, because they were already initialized upon stream
|
|
+ * creation in stream_new(), and thus may already contain some variables
|
|
+ */
|
|
|
|
return txn;
|
|
}
|
|
diff --git a/src/stream.c b/src/stream.c
|
|
index c04dd565c..7eaf03995 100644
|
|
--- a/src/stream.c
|
|
+++ b/src/stream.c
|
|
@@ -435,8 +435,17 @@ struct stream *stream_new(struct session *sess, struct stconn *sc, struct buffer
|
|
s->req_cap = NULL;
|
|
s->res_cap = NULL;
|
|
|
|
- /* Initialise all the variables contexts even if not used.
|
|
+ /* Initialize all the variables contexts even if not used.
|
|
* This permits to prune these contexts without errors.
|
|
+ *
|
|
+ * We need to make sure that those lists are not re-initialized
|
|
+ * by stream-dependant underlying code because we could lose
|
|
+ * track of already defined variables, leading to data inconsistency
|
|
+ * and memory leaks...
|
|
+ *
|
|
+ * For reference: we had a very old bug caused by vars_txn and
|
|
+ * vars_reqres being accidentally re-initialized in http_create_txn()
|
|
+ * (https://github.com/haproxy/haproxy/issues/1935)
|
|
*/
|
|
vars_init_head(&s->vars_txn, SCOPE_TXN);
|
|
vars_init_head(&s->vars_reqres, SCOPE_REQ);
|
|
--
|
|
2.33.0
|
|
|