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.]
This commit is contained in:
Aurelien DARRAGON 2022-11-18 09:17:29 +01:00 committed by Christopher Faulet
parent ce7928d19c
commit 5ad2b64262
2 changed files with 14 additions and 3 deletions

View File

@ -5213,8 +5213,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;
}

View File

@ -437,8 +437,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);