MINOR: halog: Add support for extracting captures using -hdr

This patch adds support for extracting captured header fields to halog. A field
can be extracted by passing the `-hdr <block>:<field>` output filter.

Both `<block>` and `<field>` are 1-indexed.

`<block>` refers to the index of the brace-delimited list of headers. If both
request and response headers are captured, then request headers are referenced
by `<block> = 1`, response headers are `2`. If only one direction is captured,
there will only be a single block `1`.

`<field>` refers to a single field within the selected block.

The output will contain one line, possibly empty, per log line processed.
Passing a non-existent `<block>` or `<field>` will result in an empty line.

Example:

    capture request  header a len 50
    capture request  header b len 50
    capture request  header c len 50
    capture response header d len 50
    capture response header e len 50
    capture response header f len 50

`-srv 1:1` will extract request  header `a`
`-srv 1:2` will extract request  header `b`
`-srv 1:3` will extract request  header `c`
`-srv 2:3` will extract response header `f`

This resolves GitHub issue #1146.

(cherry picked from commit 66255f7bbf9dfa18545d96f87d7a0f6fb8684d1c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit a7e8485838e30f921b9d34045f0ebf8672bb0259)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: halog: Add missing newlines in die() messages

This newline is required to correctly print the usage.

(cherry picked from commit a02c7b882f34fbb29cc71bf5ce744b0f42bacbe3)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 75a0a1f88df53ae48874a8173e685d4b77c2fe6f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

CLEANUP: halog: Use consistent indentation in help()

Consistently use 1 Tab per line.

(cherry picked from commit e0992584b9c21018dffad1fef211be6493bf7200)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit aefb6301ee051717442315af46be53f8c8f39e75)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MINOR: halog: Rename -qry to -query

With the query flag moved into the correct help section, there is enough space
for two additional characters.

(cherry picked from commit 385338b30ba7bcd18688b82ad6467014d8b46749)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit a88266698bcf0781cfb96daa74bfe89822c0649e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

DOC: halog: Move the `-qry` parameter into the correct section in help text

This is not an output filter, but instead a modifier. Specifically "only one
may be used at a time" is not true.

see 24b8d693b202b01b649f64ed878d8f9dd1b242e4

(cherry picked from commit a0635f8d1030d52b19bbe89cefabb74151af34f2)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit eb6bf96472ecea058fa4f76430b2886ecbcaccc0)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MINOR: halog: Add -qry parameter allowing to preserve the query string in -uX

Our use-case for this is a dynamic application that performs routing based on
the query string. Without this option all URLs will just point to the central
entrypoint of this location, making the output completely useless.

(cherry picked from commit 24b8d693b202b01b649f64ed878d8f9dd1b242e4)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit b09bdeecdf202f91ade21a92cd6cb62baf914f12)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

DOC: config: Fix alphabetical order of fc_* samples

fc_* samples were not properly ordered. This patch may be backported as far
as 1.8.

(cherry picked from commit 7bd21921d152121297f28651198f6223d7a4a9e2)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 9f074f68ccf8d91e2fc8b7e070b22ff396881ad6)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: sample: fix backend direction flags consecutive to last fix

Commit 7a06ffb85 ("BUG/MEDIUM: sample: Cumulate frontend and backend
sample validity flags") introduced a typo confusing the request and
the response direction when checking for validity of a rule applied
to a backend. This was reported by Coverity in issue #1417.

This needs to be backported where the patch above is backported.

(cherry picked from commit b39e47a52b19e7b62727393f9b8e8bd36330b550)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit ad745105a7dbecdb73b9ced304c65b93f854079b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: sample: Cumulate frontend and backend sample validity flags

When the sample validity flags are computed to check if a sample is used in
a valid scope, the flags depending on the proxy capabilities must be
cumulated. Historically, for a sample on the request, only the frontend
capability was used to set the sample validity flags while for a sample on
the response only the backend was used. But it is a problem for listen or
defaults proxies. For those proxies, all frontend and backend samples should
be valid. However, at many place, only frontend ones are possible.

For instance, it is impossible to set the backend name (be_name) into a
variable from a listen proxy.

This bug exists on all stable versions. Thus this patch should probably be
backported. But with some caution because the code has probably changed
serveral times. Note that nobody has ever noticed this issue. So the need to
backport this patch must be evaluated for each branch.

(cherry picked from commit 7a06ffb854281a08dae8dc2c13669a3a43da1065)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 5f802b319f8b981d57af0e38a45a98bc23516bdc)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: vars: properly set the argument parsing context in the expression

When the expression called in "set-var" uses argments that require late
resolution, the context must be set. At the moment, any unknown argument
is misleadingly reported as "ACL":

    frontend f
        bind :8080
        mode http
        http-request set-var(proc.a) be_conn(foo)

   parsing [b1.cfg:4]: unable to find backend 'foo' referenced in arg 1 \
   of ACL keyword 'be_conn' in proxy 'f'.

Once the context is properly set, it now says the truth:

   parsing [b1.cfg:8]: unable to find backend 'foo' referenced in arg 1 \
   of sample fetch keyword 'be_conn' in http-request expression in proxy 'f'.

This may be backported but is not really important. If so, the preceeding
patches "BUG/MINOR: vars: improve accuracy of the rules used to check
expression validity" and "MINOR: sample: add missing ARGC_ entries" must
be backported as well.

(cherry picked from commit 54b96d99556a27e15ee77b8b498d011cda2771e9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit a59858fe2410c07670b7a2d00f4c0e1aa9a67bf1)
[cf: backported to ease other backports]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MINOR: sample: add missing ARGC_ entries

For a long time we couldn't have arguments in expressions used in
tcp-request, tcp-response etc rules. But now due to the variables
it's possible, and their context in case of failure to resolve an
argument (e.g. backend name not found) is not properly reported
because there is no arg context values in ARGC_* to report them.

Let's add a number of missing ones for tcp-request {connection,
session,content}, tcp-response content, tcp-check, the config
parser (for "set-var" in the global section) and the CLI parser
(for "set-var" on the CLI).

(cherry picked from commit 57467b83565b9c3e112053c92ede66113809d458)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit d9b331c35976a3351fbe1e12099a0fd34da759e3)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: vars: improve accuracy of the rules used to check expression validity

The set-var() expression naturally checks whether expressions are valid
in the context of the rule, but it fails to differentiate frontends from
backends. As such for tcp-content and http-request rules, it will only
accept frontend-compatible sample-fetches, excluding those declared with
SMP_UES_BKEND (a few such as be_id, be_name). For the response it accepts
the backend-compatible expressions only, though it seems that there are
no sample-fetch function that are valid only in the frontend's content,
so that should not cause any problem.

Note that while allowing valid configs to be used, the fix might also
uncover some incorrect configurations where some expressions currently
return nothing (e.g. something depending on frontend declared in a
backend), and which could be rejected, but there does not seem to be
any such keyword. Thus while it should be backported, better not backport
it too far (2.4 and possibly 2.3 only).

(cherry picked from commit 843096d72a4fa2b5b857dde92be9f02258ab2655)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit ba4e2b8769218d0faee865d754e0b83455d95da6)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: stream-int: Block reads if channel cannot receive more data

First of all, we must be careful here because this part was modified and
each time, this introduced a bug. But, in si_update_rx(), we must not
re-enables receives if the channel buffer cannot receive more
data. Otherwise the multiplexer will be wake up for nothing. Because the
stream is woken up when the multiplexer is waiting for more room to move on,
this may lead to a ping-pong loop between the stream and the mux.

Note that for now, it does not fix any known bug. All reported issues in
this area were fixed in another way.

This patch must be backported with a special care. Technically speaking, it
may be backported as far as 2.0.

(cherry picked from commit 69fad00ebf65ad0dedabaf10db43fb3783f7ecae)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit dce066777a3072945c3b52210f503104139c2b0d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: http: Authorization value can have multiple spaces after the scheme

As per RFC7235, there can be multiple spaces in the value of an
Authorization header, between the scheme and the actual authentication
parameters.

This can be backported to all stable versions since basic auth has almost
always been there.

(cherry picked from commit 68c4eae87f2366a9485f5d09250d7ec82d9a1b94)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 2ad2ed46262a19b6012fe0ceb0243fe04aff9b71)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: http-ana: Drain request data waiting the tarpit timeout expiration

When a tarpit action is performed, we must be sure to drain data from the
request channel. Otherwise, the mux on the frontend side may be blocked
because the request channel buffer is full.

This may lead to Two bugs. The first one is a HOL blocking on the H2
multiplexer. A tarpitted stream may block all the others because data are
not drained for the whole tarpit timeout. The second bug is a ping-pong loop
between the multiplexer and the stream. The mux is waiting for more space in
the channel buffer, so it wakes up the stream. And the stream systematically
re-enables receives.

This last part is not pretty clean and it will be addressed with another
fix. But draning request data is a good way to fix both bugs in same time.

This patch must be backported as far as 2.0. The legacy HTTP mode is
probably affected, but I don't know if same bugs may be experienced in this
mode.

(cherry picked from commit b0c87f1c61edbc4e4affd81d2fd02e113528cf03)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit b02d5f024ee379102320e6fe514aa902d1b26a47)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: resolvers: Track api calls with a counter to free resolutions

The kill list introduced in commit f766ec6b5 ("MEDIUM: resolvers: use a kill
list to preserve the list consistency") contains a bug. The deatch_row must
be initialized before calling resolv_process_responses() function. However,
this function is called for the dns code. The death_row is not visible from
the outside. So, it is possible to add a resolution in an uninitialized
death_row, leading to a crash.

But, with the current implementation, it is not possible to handle the
death_row in resolv_process_responses() function because, internally, the
kill list may be freed via a call to resolv_unlink_resolution(). At the end,
we are unable to determine all call chains to guarantee a safe use of the
kill list. It is a shameful observation, but unfortunatly true.

So, to make the fix simple, we track all calls to the public resolvers
api. A counter is incremented when we enter in the resolver code and
decremented when we leave it. This way, we are able to track the recursions
to init and release the kill list only once, at the edge.

Following functions are incrementing/decrementing the recurse counter:

  * resolv_trigger_resolution()
  * resolv_srvrq_expire_task()
  * resolv_link_resolution()
  * resolv_unlink_resolution()
  * resolv_detach_from_resolution_answer_items()
  * resolv_process_responses()
  * process_resolvers()
  * resolvers_finalize_config()
  * resolv_action_do_resolve()

This patch should fix the issue #1404. It must be backported everywhere the
above commit was backported.

(cherry picked from commit 9ed1a0601d6b5ccaf6d410fee4d5d4b49d36652a)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 7a3a5361324d0b40a723f560e977e7370f9471da)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: resolvers: Don't recursively perform requester unlink

When a requester is unlink from a resolution, by reading the code, we can
have this call chain:

_resolv_unlink_resolution(srv->resolv_requester)
  resolv_detach_from_resolution_answer_items(resolution, requester)
    resolv_srvrq_cleanup_srv(srv)
      _resolv_unlink_resolution(srv->resolv_requester)

A loop on the resolution answer items is performed inside
resolv_detach_from_resolution_answer_items(). But by reading the code, it
seems possible to recursively unlink the same requester.

To avoid any loop at this stage, the requester clean up must be performed
before the call to resolv_detach_from_resolution_answer_items(). This way,
the second call to _resolv_unlink_resolution() does nothing and returns
immediately because the requester was already detached from the resolution.

This patch is related to the issue #1404. It must be backported as far as
2.2.

(cherry picked from commit bce6db6c3c208191e8c1962e3ded0dbfc015d052)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit e7e0e4be2cbe7e03b78753b2a09ae903aa9bffa7)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MEDIUM: resolvers: remove the last occurrences of the "safe" argument

This one was used to indicate whether the callee had to follow particularly
safe code path when removing resolutions. Since the code now uses a kill
list, this is not needed anymore.

(cherry picked from commit 6878f80427aa8befc6662d85caddb8affe799b9c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 33360872d1c56a1cc283ba79c736a71bcf7ce464)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MEDIUM: resolvers: use a kill list to preserve the list consistency

When scanning resolution.curr it's possible to try to free some
resolutions which will themselves result in freeing other ones. If
one of these other ones is exactly the next one in the list, the list
walk visits deleted nodes and causes memory corruption, double-frees
and so on. The approach taken using the "safe" argument to some
functions seems to work but it's extremely brittle as it is required
to carefully check all call paths from process_ressolvers() and pass
the argument to 1 there to refrain from deleting entries, so the bug
is very likely to come back after some tiny changes to this code.

A variant was tried, checking at various places that the current task
corresponds to process_resolvers() but this is also quite brittle even
though a bit less.

This patch uses another approach which consists in carefully unlinking
elements from the list and deferring their removal by placing it in a
kill list instead of deleting them synchronously. The real benefit here
is that the complexity only has to be placed where the complications
are.

A thread-local list is fed with elements to be deleted before scanning
the resolutions, and it's flushed at the end by picking the first one
until the list is empty. This way we never dereference the next element
and do not care about its presence or not in the list. One function,
resolv_unlink_resolution(), is exported and used outside, so it had to
be modified to use this list as well. Internal code has to use
_resolv_unlink_resolution() instead.

(cherry picked from commit f766ec6b536725870db2ef48300c371c70c7dda0)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 2075165e20e5aa08382bf290eaa9bf08ad28f579)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

CLEANUP: resolvers: replace all LIST_DELETE with LIST_DEL_INIT

The code as it is uses crossed lists between many elements, and at
many places the code relies on list iterators or emptiness checks,
which does not work with only LIST_DELETE. Further, it is quite
difficult to place debugging code and checks in the current situation,
and gdb is helpless.

This code replaces all LIST_DELETE calls with LIST_DEL_INIT so that
it becomes possible to trust the lists.

(cherry picked from commit aae7320b0dd9a379792d7cbe3304b23bdb045740)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 8c3c523097988005ee69128ace10bcc16c81398b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

CLEANUP: resolvers: simplify resolv_link_resolution() regarding requesters

This function allocates requesters by hand for each and every type. This
is complex and error-prone, and it doesn't even initialize the list part,
leaving dangling pointers that complicate debugging.

This patch introduces a new function resolv_get_requester() that either
returns the current pointer if valid or tries to allocate a new one and
links it to its destination. Then it makes use of it in the function
above to clean it up quite a bit. This allows to remove complicated but
unneeded tests.

(cherry picked from commit 239675e4a955b219e915fa11a1a03c7aacc13ccd)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 4d7ca9149914b1188e117608a96b5608232f0bc5)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

CLEANUP: always initialize the answer_list

Similar to the previous patch, the answer's list was only initialized the
first time it was added to a list, leading to bogus outdated pointer to
appear when debugging code is added around it to watch it. Let's make
sure it's always initialized upon allocation.

(cherry picked from commit 48664c048d17486b80e58459d2d4fa41a36e579e)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit a2373e82f574fdba5fb52ff0ef917b16dd44484a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

CLEANUP: resolvers: do not export resolv_purge_resolution_answer_records()

This code is dangerous enough that we certainly don't want external code
to ever approach it, let's not export unnecessary functions like this one.
It was made static and a comment was added about its purpose.

(cherry picked from commit 2acc160c05c0c8dc28ecce5941451c916d29bc63)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit aab7593db39e1083a77ac792723623bd9c402273)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: mux-h1: Perform a connection shutdown when the h1c is released

When the H1 connection is released, a connection shutdown is now performed.
If it was already performed when the stream was detached, this action has no
effect. But it is mandatory, when an idle H1C is released. Otherwise the
xprt and the socket shutdown is never perfmed. It is especially important
for SSL client connections, because it is the only way to perform a clean
SSL shutdown.

Without this patch, SSL_shutdown is never called, preventing, among other
things, the SSL session caching.

This patch depends on the commit "BUG/MINOR: mux-h1: Save shutdown mode if
the shutdown is delayed". It should be backported as far as 2.0.

(cherry picked from commit e76b4f055d8d6a5514f5c67e37ad7a8858b9eed2)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 37c42549234d4b3caab795af96fa8a9866629e7b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: mux-h1: Save shutdown mode if the shutdown is delayed

The connection shutdown may be delayed if there are pending outgoing
data. The action is performed once data are fully sent. In this case the
mode (dirty/clean) was lost and a clean shutdown was always performed. Now,
the mode is saved to be sure to perform the connection shutdown using the
right mode. To do so, H1C_F_ST_SILENT_SHUT flag is introduced.

This patch should be backported as far as 2.0.

(cherry picked from commit a85c522d42f866d57cd79335a945402b81cb3948)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit db451fb7d790d2675bfeda435b47a560e515b437)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUILD: fix compilation on NetBSD

Use include file <sys/time.h> to fix compilation error with timeval in
some files. This is as reported as 'man 7 system_data_types'. The build
error is reported on NetBSD 9.2.

This should be backported up to 2.2.

(cherry picked from commit 28c5b3c0bc40f96579845f13a9fafa6f63eb10c0)
[wt: only applied to check-t.h]
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 08fc157c389572ae49a1c80d85df242074e4d2bb)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: mux-h2: do not prevent from sending a final GOAWAY frame

Some checks were added by commit 9a3d3fcb5 ("BUG/MAJOR: mux-h2: Don't try
to send data if we know it is no longer possible") to make sure we don't
loop forever trying to send data that cannot leave. But one of the
conditions there is not correct, the one relying on H2_CS_ERROR2. Indeed,
this state indicates that the error code was serialized into the mux
buffer, and since the test is placed before trying to send the data to
the socket, if the connection states only contains a GOAWAY frame, it
may refrain from sending and may close without sending anything. It's
not dramatic, as GOAWAY reports connection errors in situations where
delivery is not even certain, but it's cleaner to make sure the error
is properly sent, and it avoids upsetting h2spec, as seen in github
issue #1422.

Given that the patch above was backported as far as 1.8, this patch will
also have to be backported that far.

Thanks to Ilya for reporting this one.

(cherry picked from commit e6dc7a0129b8b34e7e1ce1f4bda5d4bfee4ab749)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit e90653b0612f53bdada4f59baebf1e2c8796fa41)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MAJOR: buf: fix varint API post- vs pre- increment

A bogus test in b_get_varint(), b_put_varint(), b_peek_varint() shifts
the end of the buffer by one byte. Since the bug is the same in the read
and write functions, the buffer contents remain compatible, which explains
why this bug was not detected earlier. But if the buffer ends on an
aligned address or page, it can result in a one-byte overflow which will
typically cause a crash or an inconsistent behavior.

This API is only used by rings (e.g. for traces and boot messages) and
by DNS responses, so the probability to hit it is extremely low, but a
crash on boot was observed.

This must be backported to 2.2.

(cherry picked from commit dd362b7b24ea24d8f3c882dab1a69cf78af7cc90)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit db5bf3d89cc4290aacb15ec348bb2bcdcaa1c201)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: resolvers: always check a valid item in query_list

The query_list is physically stored in the struct resolution itself,
so we have a list that contains a list to items stored in itself (and
there is a single item). But the list is first initialized in
resolv_validate_dns_response(), while it's scanned in
resolv_process_responses() later after calling the former. First,
this results in crashes as soon as the code is instrumented a little
bit for debugging, as elements from a previous incarnation can appear.

But in addition to this, the presence of an element is checked by
verifying that the return of LIST_NEXT() is not NULL, while it may
never be NULL even for an empty list, resulting in bugs or crashes
if the number of responses does not match the list's contents. This
is easily triggered by testing for the list non-emptiness outside of
the function.

Let's make sure the list is always correct, i.e. it's initialized to
an empty list when the structure is allocated, elements are checked by
first verifying the list is not empty, they are deleted once checked,
and in any case at end so that there are no dangling pointers.

This should be backported, but only as long as the patch fits without
modifications, as adaptations can be risky there given that bugs tend
to hide each other.

(cherry picked from commit 25e010906a016c9d6e8677726e6e55de80010616)
[wt: use resolv_hostname_cmp() in 2.4]
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 51128f86129a1257b59005356253f78411d77e95)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUILD: resolvers: avoid a possible warning on null-deref

Depending on the code that precedes the loop, gcc may emit this warning:

  src/resolvers.c: In function 'resolv_process_responses':
  src/resolvers.c:1009:11: warning: potential null pointer dereference [-Wnull-dereference]
   1009 |  if (query->type != DNS_RTYPE_SRV && flags & DNS_FLAG_TRUNCATED) {
        |      ~~~~~^~~~~~

However after carefully checking, r_res->header.qdcount it exclusively 1
when reaching this place, which forces the for() loop to enter for at
least one iteration, and <query> to be set. Thus there's no code path
leading to a null deref. It's possibly just because the assignment is
too far and the compiler cannot figure that the condition is always OK.
Let's just mark it to please the compiler.

(cherry picked from commit 10c1a8c3bd6a24f51616172a37f1e3ef58f8fa84)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 3a8897f94773849c6dc15218f0f4ca7831a8df85)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MAJOR: resolvers: add other missing references during resolution removal

There is a fundamental design bug in the resolvers code which is that
a list of active resolutions is being walked to try to delete outdated
entries, and that the code responsible for removing them also removes
other elements, including the next one which will be visited by the
list iterator. This randomly causes a use-after-free condition leading
to crashes, infinite loops and various other issues such as random memory
corruption.

A first fix for the memory fix for this was brought by commit 0efc0993e
("BUG/MEDIUM: resolvers: Don't release resolution from a requester
callbacks"). While preparing for more fixes, some code was factored by
commit 11c6c3965 ("MINOR: resolvers: Clean server in a dedicated function
when removing a SRV item"), which inadvertently passed "0" as the "safe"
argument all the time, missing one case of removal protection, instead
of always using "safe". This patch reintroduces the correct argument.

This must be backported with all fixes above.

Cc: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 2a67aa0a51acc2d3469061b8732faf7e597f9c69)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 3223d8574e37b1baaf1e9000244c5b1c07178e0a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MINOR: resolvers: merge address and target into a union "data"

These two fields are exclusive as they depend on the data type.
Let's move them into a union to save some precious bytes. This
reduces the struct resolv_answer_item size from 600 to 576 bytes.

(cherry picked from commit cc8fd4c0404523101f4d1473bcd8589086da022d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 20a023765963c22d7b57ebe41493be848b63669c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: resolvers: use correct storage for the target address

The struct resolv_answer_item contains an address field of type
"sockaddr" which is only 16 bytes long, but which is used to store
either IPv4 or IPv6. Fortunately, the contents only overlap with
the "target" field that follows it and that is large enough to
absorb the extra bytes needed to store AAAA records. But this is
dangerous as just moving fields around could result in memory
corruption.

The fix uses a union and removes the casts that were used to hide
the problem.

Older versions need to be checked and possibly fixed. This needs
to be backported anyway.

(cherry picked from commit b4ca0195a9591614bfcebe9b7fe80c1ce9f94d8d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit c54aabaf7c73e563a4cfdef760e2ae767612c601)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: resolvers: fix truncated TLD consecutive to the API fix

A bug was introduced by commit previous bf9498a31 ("MINOR: resolvers:
fix the resolv_str_to_dn_label() API about trailing zero") as the code
is particularly contrived and hard to test. The output writes the last
char at [i+1] so the trailing zero and return value must be at i+1.

This will have to be backported where the patch above is backported
since it was needed for a fix.

(cherry picked from commit 7b232f132dc9a0a6ee6fa3fabfa6e50196af9240)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 68c7b36f637c5d9a05b32b2868e44ea838f83f78)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MINOR: resolvers: fix the resolv_dn_label_to_str() API about trailing zero

This function suffers from the same API issue as its sibling that does the
opposite direction, it demands that the input string is zero-terminated
*and* that its length *including* the trailing zero is passed on input,
forcing callers to pass length + 1, and itself to use that length - 1
everywhere internally.

This patch addressess this. There is a single caller, which is the
location of the previous bug, so it should probably be backported at
least to keep the code consistent across versions. Note that the
function is called dns_dn_label_to_str() in 2.3 and earlier.

(cherry picked from commit 875ee704dd76b12e5db9ad5c9ae5f2a23b4a4a86)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit ca0170bc83c9c5cd067eaf00283df3a2ae36eb5b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: resolvers: do not reject host names of length 255 in SRV records

An off-by-one issue in buffer size calculation used to limit the output
of resolv_dn_label_to_str() to 254 instead of 255.

This must be backported to 2.0.

(cherry picked from commit 85c15e6bff92c2dae609d76f72c1d231d048dd74)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 2859c162c2e8499e5be77972000aca2cb2e53a67)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: resolver: make sure to always use the correct hostname length

In issue #1411, @jjiang-stripe reports that do-resolve() sometimes seems
to be trying to resolve crap from random memory contents.

The issue is that action_prepare_for_resolution() tries to measure the
input string by itself using strlen(), while resolv_action_do_resolve()
directly passes it a pointer to the sample, omitting the known length.
Thus of course any other header present after the host in memory are
appended to the host value. It could theoretically crash if really
unlucky, with a buffer that does not contain any zero including in the
index at the end, and if the HTX buffer ends on an allocation boundary.
In practice it should be too low a probability to have ever been observed.

This patch modifies the action_prepare_for_resolution() function to take
the string length on with the host name on input and pass that down the
chain. This should be backported to 2.0 along with commit "MINOR:
resolvers: fix the resolv_str_to_dn_label() API about trailing zero".

(cherry picked from commit 947ae125cc3a00c05a5356de177b732fdc55aab5)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit a13a610841988fa9713eae50e3fd91e0fd9038f8)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MINOR: resolvers: fix the resolv_str_to_dn_label() API about trailing zero

This function is bogus at the API level: it demands that the input string
is zero-terminated *and* that its length *including* the trailing zero is
passed on input. While that already looks smelly, the trailing zero is
copied as-is, and is then explicitly replaced with a zero... Not only
all callers have to pass hostname_len+1 everywhere to work around this
absurdity, but this requirement causes a bug in the do-resolve() action
that passes random string lengths on input, and that will be fixed on a
subsequent patch.

Let's fix this API issue for now.

This patch will have to be backported, and in versions 2.3 and older,
the function is in dns.c and is called dns_str_to_dn_label().

(cherry picked from commit bf9498a31beb41078ebf257c54b7acb879bcb98b)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit c1c765f27d2a31f90ffe233d218282aa16d52e7c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: tcpcheck: Properly catch early HTTP parsing errors

When an HTTP response is parsed, early parsing errors are not properly
handled. When this error is reported by the multiplexer, nothing is copied
into the input buffer. The HTX message remains empty but the
HTX_FL_PARSING_ERROR flag is set. In addition CS_FL_EOI is set on the
conn-stream. This last flag must be handled to prevent subscription for
receive events. Otherwise, in the best case, a L7 timeout error is
reported. But a transient loop is also possible if a shutdown is received
because the multiplexer notifies the check of the event while the check
never handles it and waits for more data.

Now, if CS_FL_EOI flag is set on the conn-stream, expect rules are
evaluated. Any error must be handled there.

Thanks to @kazeburo for his valuable report.

This patch should fix the issue #1420. It must be backported at least to
2.4. On 2.3 and 2.2, there is no loop but the wrong error is reported (empty
response instead of invalid one). Thus it may also be backported as far as
2.2.

(cherry picked from commit d16e7dd0e4cf07c9731a1a1c288d003a9f84d1b5)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit bd0174758fed578d685c52271b030b10c475fb2b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: sample: properly verify that variables cast to sample

The various variable-to-sample converters allow to turn a variable to
a sample of type string, sint or binary, but both the string one used
by strcmp() and the binary one used by secure_memcmp() are missing a
pointer check on the ability to the cast, making them crash if a
variable of type addr is used with strcmp(), or if an addr or bool is
used with secure_memcmp().

Let's rely on the new sample_conv_var2smp() function to run the proper
checks.

This will need to be backported to all supported version. It relies on
previous commits:

  CLEANUP: server: always include the storage for SSL settings
  CLEANUP: sample: rename sample_conv_var2smp() to *_sint
  CLEANUP: sample: uninline sample_conv_var2smp_str()
  MINOR: sample: provide a generic var-to-sample conversion function

For backports it's probably easier to check the sample_casts[] pointer
before calling it in sample_conv_strcmp() and sample_conv_secure_memcmp().

(cherry picked from commit 2476ff102faa3217409e8e29acdb077a9a1dcf8a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 8fdaf255c88174222b7c47345f0f0778c34b4305)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MINOR: sample: provide a generic var-to-sample conversion function

We're using variable-to-sample conversion at least 4 times in the code,
two of which are bogus. Let's introduce a generic conversion function
that performs the required checks.

(cherry picked from commit 168e8de1d06adc7aa3e7e2cc2a36935a77c79b9c)
[cf: call to vars_get_by_desc() was updated to remove the third param]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit ea5735057fcf62c74b28fe3f13a0ab96b8014852)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

CLEANUP: sample: uninline sample_conv_var2smp_str()

There's no reason to limit this one to this file, it could be used in
other contexts.

(cherry picked from commit 4034e2cb5895c08bd821d4db6b5c47f90c4cd121)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 05cd2e6d04724d89a8fef29af16b9767d6bcd0ba)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

CLEANUP: sample: rename sample_conv_var2smp() to *_sint

This one only handles integers, contrary to its sibling with the suffix
_str that only handles strings. Let's rename it and uninline it since it
may well be used from outside.

(cherry picked from commit d9be59952975e679f3305e32620ae3572a6a952c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 9cfb71fb345c1c95377c0f2e72aff2a80aa10fc2)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: stream: Keep FLT_END analyzers if a stream detects a channel error

If a channel error (READ_ERRO|READ_TIMEOUT|WRITE_ERROR|WRITE_TIMEOUT) is
detected by the stream, in process_stream(), FLT_END analyers must be
preserved. It is important to be sure to ends filter analysis and be able to
release the stream.

First, filters may release some ressources when FLT_END analyzers are
called. Then, the CF_FL_ANALYZE flag is used to sync end of analysis for the
request and the response. If FLT_END analyzer is ignored on a channel, this
may block the other side and freeze the stream.

This patch must be backported to all stable versions

(cherry picked from commit 813f913444726eafbc06050ce1d1623f5d84bc38)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit c94f727ea1a38656d93097bee551ab1375c8fe6d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: http-ana: Don't eval front after-response rules if stopped on back

http-after-response rules evaluation must be stopped after a "allow". It
means the frontend ruleset must not be evaluated if a "allow" was performed
in the backend ruleset. Internally, the evaluation must be stopped if on
HTTP_RULE_RES_STOP return value. Only the "allow" action is concerned by
this change.

Thanks to this patch, http-response and http-after-response behave in the
same way.

This patch should be backported as far as 2.2.

(cherry picked from commit 597909f4e67866c4f3ecf77f95f2cd4556c0c638)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 085ccd4fe42706230c2a31c162d8664ca6fb16a4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MINOR: initcall: Rename __GLOBL and __GLOBL1.

Rename __GLOBL and __GLOBL1 to __HA_GLOBL and __HA_GLOBL1, as the former are
already defined on FreeBSD.

This should be backported to 2.4, 2.3 and 2.2.

(cherry picked from commit e972c0acde990de30a00f76f7f61470d3325c757)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 1e74052267a3a09a9dba8b2ba6131d4bd1d80923)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: mux_h2: Handle others remaining read0 cases on partial frames

We've found others places where the read0 is ignored because of an
incomplete frame parsing. This time, it happens during parsing of
CONTINUATION frames.

When frames are parsed, incomplete frames are properly handled and
H2_CF_DEM_SHORT_READ flag is set. It is also true for HEADERS
frames. However, for CONTINUATION frames, there is an exception. Besides
parsing the current frame, we try to peek header of the next one to merge
payload of both frames, the current one and the next one. Idea is to create
a sole HEADERS frame before parsing the payload. However, in this case, it
is possible to have an incomplete frame too, not the current one but the
next one. From the demux point of view, the current frame is complete. We
must go to the internal function h2c_decode_headers() to detect an
incomplete frame. And this case was not identified and fixed when
H2_CF_DEM_SHORT_READ flag was introduced in the commit b5f7b5296
("BUG/MEDIUM: mux-h2: Handle remaining read0 cases on partial frames")

This bug was reported in a comment of the issue #1362. The patch must be
backported as far as 2.0.

(cherry picked from commit 485da0b0535cc575ed8d4f3bf407e9101dbb80aa)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 0de3f552a4e19fa28b2beb9dc90f8aa6b9e1fd62)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: stream-int: Defrag HTX message in si_cs_recv() if necessary

The stream interface is now responsible for defragmenting the HTX message of
the input channel if necessary, before calling the mux's .rcv_buf()
function. The defrag is performed if the underlying buffer contains only
input data while the HTX message free space is not contiguous.

The defrag is important here to be sure the mux and the app layer have the
same criteria to decide if a buffer is full or not. Otherwise, the app layer
may wait for more data because the buffer is not full while the mux is
blocked because it needs more space to proceed.

This patch depends on following commits:

  * MINOR: htx: Add an HTX flag to know when a message is fragmented
  * MINOR: htx: Add a function to know if the free space wraps

This patch is related to the issue #1362. It may be backported as far as 2.0
after some observation period (not sure it is required or not).

(cherry picked from commit 2bc364c19105c12dc1dd6b043e81c48b184d4174)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 8d521427742f12b4377c0b01672a278ee974b8ac)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MINOR: htx: Add a function to know if the free space wraps

the htx_space_wraps() function may now be used to know if the free space of
an HTX message wraps. It does the same as b_space_wraps().

(cherry picked from commit 361fbcc14acc4a3bed9161934f2524db8ac2069e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 4a7f6358b0339b2c21f793ae926d274934abbdeb)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MINOR: htx: Add an HTX flag to know when a message is fragmented

HTX_FL_FRAGMENTED flag is now set on an HTX message when it is
fragmented. It happens when an HTX block is removed in the middle of the
message and flagged as unused. HTX_FL_FRAGMENTED flag is removed when all
data are removed from the message or when the message is defragmented.

Note that some optimisations are still possible because the flag can be
avoided in other situations. For instance when the last header of a bodyless
message is removed.

(cherry picked from commit 4697c92c9d7f875b30413a20bfeb56f318c182d1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 95bf88d719fa04bfa89808522971cabcbc6ffd4f)
[cf: Value of HTX_FL_FRAGMENTED flag was changed]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: leastconn: fix rare possibility of divide by zero

An optimization was brought in commit 5064ab6a9 ("OPTIM: lb-leastconn:
do not unlink the server if it did not change") to avoid locking the
server just to discover it did not move. However a mistake was made
because the operation involves a divide with a value that is read
outside of its usual lock, which makes it possible to be zero at the
exact moment we watch it if another thread takes the server down under
the lbprm lock, resulting in a divide by zero.

Therefore we must check that the value is not null there.

This must be backported to 2.4.

(cherry picked from commit 6f97b4ef3383f59ff3218f73fcd21f94335b2c1b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 8031464835df8166666f315515604fe6bc3c5598)
[cf: Above commit was backported to 2.3. Thus this patch should be backported
     too.]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: http-ana: Clear request analyzers when applying redirect rule

A bug was introduced by the commit 2d5650082 ("BUG/MEDIUM: http-ana: Reset
channels analysers when returning an error").

The request analyzers must be cleared when a redirect rule is applied. It is
not a problem if the redirect rule is inside an http-request ruleset because
the analyzer takes care to clear it. However, when it comes from a redirect
ruleset (via the "redirect ..."  directive), because of the above commit,
the request analyzers are no longer cleared. It means some HTTP request
analyzers may be called while the request channel was already flushed. It is
totally unexpected and may lead to crash.

Thanks to Yves Lafon for reporting the problem.

This patch must be backported everywhere the above commit was backported.

(cherry picked from commit d34758849e1f7e070a25022fcfe38e4fc82e22c6)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 9132227728115431dbf379b9006347c2430d0dda)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: filters: Fix a typo when a filter is attached blocking the release

When a filter is attached to a stream, the wrong FLT_END analyzer is added
on the request channel. AN_REQ_FLT_END must be added instead of
AN_RES_FLT_END. Because of this bug, the stream may hang on the filter
release stage.

It seems to be ok for HTTP filters (cache & compression) in HTTP mode. But
when enabled on a TCP proxy, the stream is blocked until the client or the
server timeout expire because data forwarding is blocked. The stream is then
prematurely aborted.

This bug was introduced by commit 26eb5ea35 ("BUG/MINOR: filters: Always set
FLT_END analyser when CF_FLT_ANALYZE flag is set"). The patch must be
backported in all stable versions.

(cherry picked from commit d28b2b2352291991190fd0693fdfd3d7430e6ddb)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 4a9db77783773955bb9d76ac7b2ef1c8c06eed28)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MINOR: tasks: catch TICK_ETERNITY with BUG_ON() in __task_queue()

__task_queue() must absolutely not be called with TICK_ETERNITY or it
will place a never-expiring node upfront in the timers queue, preventing
any timer from expiring until the process is restarted. Code was found
to cause this using "task_schedule(task, now_ms)" which does this one
millisecond every 49.7 days, so let's add a condition against this. It
must never trigger since any process susceptible to trigger it would
already accumulate tasks until it dies.

An extra test was added in wake_expired_tasks() to detect tasks whose
timeout would have been changed after being queued.

An improvement over this could be in the future to use a non-scalar
type (union/struct) for expiration dates so as to avoid the risk of
using them directly like this. But now_ms is already such a valid
time and this specific construct would still not be caught.

This could even be backported to stable versions to help detect other
occurrences if any.

(cherry picked from commit 7a9699916a92a98ca5daaff77e2a25f9bec8817d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit aaed609f9189915ef610cc47d8d1930ebb257b9d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: tcp-rules: Stop content rules eval on read error and end-of-input

For now, tcp-request and tcp-response content rules evaluation is
interrupted before the inspect-delay when the channel's buffer is full, the
RX path is blocked or when a shutdown for reads was received. To sum up, the
evaluation is interrupted when no more input data are expected. However, it
is not exhaustive. It also happens when end of input is reached (CF_EOI flag
set) or when a read error occurred (CF_READ_ERROR flag set).

Note that, AFAIK, it is only a problem on HAProy 2.3 and prior when a H1 to
H2 upgrade is performed. On newer versions, it works as expected because the
stream is not created at this stage.

This patch must be backported as far as 2.0.

(cherry picked from commit cb59e0bc3cbe48eedd88e91b999a471724a6b709)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit fc553c9f0b9cacd4e0ec10bc9ff56879248a9b1c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: tcpcheck: Don't use arg list for default proxies during parsing

During tcp/http check rules parsing, when a sample fetch or a log-format
string is parsed, the proxy's argument list used to track unresolved
argument is no longer passed for default proxies. It means it is no longer
possible to rely on sample fetches depending on the execution context (for
instance 'nbsrv').

It is important to avoid HAProxy crashes because these arguments are
resolved during the configuration validity check. But, default proxies are
not evaluated during this stage. Thus, these arguments remain unresolved.

It will probably be possible to relax this rule. But to ease backports, it
is forbidden for now.

This patch must be backported as far as 2.2. It depends on the commit
"MINOR: arg: Be able to forbid unresolved args when building an argument
list".  It must be adapted for the 2.3 because PR_CAP_DEF capability was
introduced in the 2.4. A solution may be to test The proxy's id agains NULL.

(cherry picked from commit eaba25dd97e95a45ba1fa1e9ce41c951410621c0)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 8551c3430adc063d614cb0bd0cd716fdf5b6bcfb)
[cf: As expected, the test on the proxy was changed. Instead of relying on
PR_CAP_DEF, we test the proxy's ID against NULL. Only default servers have
no id]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MINOR: arg: Be able to forbid unresolved args when building an argument list

In make_arg_list() function, unresolved dependencies are pushed in an
argument list to be resolved later, during the configuration validity
check. It is now possible to forbid such unresolved dependencies by omitting
<al> parameter (setting it to NULL). It is usefull when the parsing context
is not the same than the running context or when the parsing context is lost
after the startup stage. For instance, an argument may be defined in
defaults section during parsing and executed in a frontend/backend section.

(cherry picked from commit 35926a16ac087ac737026206a8420a65a9fce0ca)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 45bb15f6403ece51d1bedb94110d427c1184ea34)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MAJOR: lua: use task_wakeup() to properly run a task once

The Lua tasks registered vi core.register_task() use a dangerous
task_schedule(task, now_ms) to start them, that will most of the
time work by accident, except when the time wraps every 49.7 days,
if now_ms is 0, because it's not valid to queue a task with an
expiration date set to TICK_ETERNITY, as it will fail all wakeup
checks and prevent all subsequent timers from being seen as expired.
The only solution in this case is to restart the process.

Fortunately for the vast majority of users it is extremely unlikely
to ever be met (only one millisecond every 49.7 days is at risk), but
this can be systematic for a process dealing with 1000 req/s, hence
the major tag.

The bug was introduced in 1.6-dev with commit 24f335340 ("MEDIUM: lua:
add coroutine as tasks."), so the fix must be backported to all stable
branches.

(cherry picked from commit e3957f83e070fab55853d4803fcb3aba890091ce)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 790810fe9652cbf26b11a41f43c5592537a7131f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: lua: fix wakeup condition from sleep()

A time comparison was wrong in hlua_sleep_yield(), making the sleep()
code do nothing for periods of 24 days every 49 days. An arithmetic
comparison was performed on now_ms instead of using tick_is_expired().

This bug was added in 1.6-dev by commit 5b8608f1e ("MINOR: lua: core:
add sleep functions") so the fix should be backported to all stable
versions.

(cherry picked from commit 12c02701d304f29ca74f521cfcc5fe1bc6f3e03d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 9cc2e4c9962bf83a0d08629444848f3ddae64473)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

DOC: peers: fix doc "enable" statement on "peers" sections

Checking in code the right keyword is "enabled" and not "enable".

In addition the comment was also completed:

This could appear useless because the "defaults" sections not
yet apply on "peers" sections, but it could be the case in the future.
This statement can currently cancel a previous "disabled" keyword in
the same section.

This patch should be backported in all supported branches (keyword
is present since 1.5)

(cherry picked from commit 620761f934fe24f3943ec2ff346d8ecd0e6a6576)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 7214dcfef69909f516f04f509421c4d915624c51)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: mux-h1/mux-fcgi: Sanitize TE header to only send "trailers"

Only chunk-encoded response payloads are supported by HAProxy. All other
transfer encodings are not supported and will be an issue if the HTTP
compression is enabled. So be sure only "trailers" is send in TE request
headers.

The patch is related to the issue #1301. It must be backported to all stable
versions. Be carefull for 2.0 and lower because the HTTP legacy must also be
fixed.

(cherry picked from commit f56e8465f067c84b820dbedd89e6f44f1e02c179)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit f9bb8d0fbcc945e15e8d1dc8ed792b27f1ca693b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: stream: Stop waiting for more data if SI is blocked on RXBLK_ROOM

If the stream-interface is waiting for more buffer room to store incoming
data, it is important at the stream level to stop to wait for more data to
continue. Thanks to the previous patch ("BUG/MEDIUM: stream-int: Notify
stream that the mux wants more room to xfer data"), the stream is woken up
when this happens. In this patch, we take care to interrupt the
corresponding tcp-content ruleset or to stop waiting for the HTTP message
payload.

To ease detection of the state, si_rx_blocked_room() helper function has
been added. It returns non-zero if the stream interface's Rx path is blocked
because of lack of room in the input buffer.

This patch is part of a series related to the issue #1362. It should be
backported as ar as 2.0, probably with some adaptations. So be careful
during backports.

(cherry picked from commit 7833596ff42d5f5ba2e9f639b9698cec16613058)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 59df812bf1eb75b96f1da1697ce861cafa8b59ff)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: stream-int: Notify stream that the mux wants more room to xfer data

When the mux failed to transfer data to the upper layer because of a lack of
room, it is important to wake the stream up to let it handle this
event. Otherwise, if the stream is waiting for more data, both the stream
and the mux reamin blocked waiting for each other.

When this happens, the mux set the CS_FL_WANT_ROOM flag on the
conn-stream. Thus, in si_cs_recv() we are able to detect this event. Today,
the stream-interface is blocked. But, it is not enough to wake the stream
up. To fix the bug, CF_READ_PARTIAL flag is extended to also handle cases
where a read exception occurred. This flag should idealy be renamed. But for
now, it is good enough. By setting this flag, we are sure the stream will be
woken up.

This patch is part of a series related to the issue #1362. It should be
backported as far as 2.0, probably with some adaptations. So be careful
during backports.

(cherry picked from commit df99408e0d4e59cf54666772a4224a8665743fbd)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 47545caccaf4b429f4fd002062c66d2c2592abcd)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: mux-h1: Adjust conditions to ask more space in the channel buffer

When a message is parsed and copied into the channel buffer, in
h1_process_demux(), more space is requested if some pending data remain
after the parsing while the channel buffer is not empty. To do so,
CS_FL_WANT_ROOM flag is set. It means the H1 parser needs more space in the
channel buffer to continue. In the stream-interface, when this flag is set,
the SI is considered as blocked on the RX path. It is only unblocked when
some data are sent.

However, it is not accurrate because the parsing may be stopped because
there is not enough data to continue. For instance in the middle of a chunk
size. In this case, some data may have been already copied but the parser is
blocked because it must receive more data to continue. If the calling SI is
blocked on RX at this stage when the stream is waiting for the payload
(because http-buffer-request is set for instance), the stream remains stuck
infinitely.

To fix the bug, we must request more space to the app layer only when it is
not possible to copied more data. Actually, this happens when data remain in
the input buffer while the H1 parser is in states MSG_DATA or MSG_TUNNEL, or
when we are unable to copy headers or trailers into a non-empty buffer.

The first condition is quite easy to handle. The second one requires an API
refactoring. h1_parse_msg_hdrs() and h1_parse_msg_tlrs() fnuctions have been
updated. Now it is possible to know when we need more space in the buffer to
copy headers or trailers (-2 is returned). In the H1 mux, a new H1S flag
(H1S_F_RX_CONGESTED) is used to track this state inside h1_process_demux().

This patch is part of a series related to the issue #1362. It should be
backported as far as 2.0, probably with some adaptations. So be careful
during backports.

(cherry picked from commit 46e058dda51cf09ae0a734ce0931be53dcc179a0)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 58f21dae3d82a928ae76a15634c45c8db9bf0aca)
[cf: value of H1S_F_RX_CONGESTED flag was changed]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: http-ana: increment internal_errors counter on response error

A bug was introduced in the commit cff0f739e51 ("MINOR: counters: Review
conditions to increment counters from analysers"). The internal_errors
counter for the target server was incremented twice. The counter for the
session listener needs to be incremented instead.

This must be backported everywhere the commit cff0f739e51 is.

(cherry picked from commit 9a006f9641ceb353c007f8f7e8ae949ace786099)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 84426cd01cd805dc26570312d6c8485f67939b37)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: h1-htx: Fix a typo when request parser is reset

In h1_postparse_req_hdrs(), if we need more space to copy headers, the request
parser is reset. However, because of a typo, it was reset as a response parser
instead of a request one. h1m_init_req() must be called.

This patch must be backported as far as 2.2.

(cherry picked from commit 216d3352b1aac1bff19383b6a6e5476e11e613e9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit c7c20a7692130c42796b1a63e9d72c3e933fe737)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: server: allow 'enable health' only if check configured

Test that checks have been configured on the server before enabling via
the 'enable health' CLI. This mirrors the 'enable agent' command.

Without this, a user can use the command on the server without checks.
This leaves the server in an undefined state. Notably, the stat page
reports the server in check transition.

This condition was left on the following reorg commit.
  2c04eda8b58636ad2ae44e42b1f50f3b5a24a642
  REORG: cli: move "{enable|disable} health" to server.c

This should be backported up to 1.8.

(cherry picked from commit 0f456d502963f3555552d4b3d5f97ed504353d85)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit e636a1106961ea0a0986657fc9943b03eb6f531a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: cli/payload: do not search for args inside payload

The CLI's payload parser is over-complicated and as such contains more
bugs than needed. One of them is that it uses strstr() to find the
ending tag, ignoring spaces before it, while the argument locator
creates a new arg on each space, without checking if the end of the
word appears past the previously found end. This results in "<<" being
considered as the start of a new argument if preceeded by more than
one space, and the payload being damaged with a \0 inserted at the
first space or tab.

Let's make an easily backportable fix for now. This fix makes sure that
the trailing zero from the first line is properly kept after '<<' and
that the end tag is looked for only as an isolated argument and nothing
else. This also gets rid of the unsuitable strstr() call and now makes
sure that strcspn() will not return elements that are found in the
payload.

For the long term the loop must be rewritten to get rid of those
unsuitable strcspn() and strstr() calls which work past each other, and
the cli_parse_request() function should be split into a tokenizer and
an executor that are used from the caller instead of letting the caller
play games with what it finds there.

This should be backported wherever CLI payload is supported, i.e. 2.0+.

(cherry picked from commit f2dda52e784f3eebc693a70b9491a428cbbf188e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit c5c170a0e2b59f1580bfa7578bdc0410a7ccbf0e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUILD: ist: prevent gcc11 maybe-uninitialized warning on istalloc

A new warning is reported by gcc11 when using a pointer to uninitialized
memory block for a function with a const pointer argument. The warning
is triggered for istalloc, used by http_client.c / proxy.c / tcpcheck.c.

This warning is reported because the uninitialized memory block
allocated by malloc should not be passed to a const argument as in ist2.
See https://gcc.gnu.org/onlinedocs/gcc-11.1.0/gcc/Warning-Options.html#index-Wmaybe-uninitialized

This should be backported up to 2.2.

(cherry picked from commit 7a8aff26881c58061d6ca98d3c3e5fc70060c8f4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 3471051fe113619fd3a6165cc301894a085056aa)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

DOC: management: certificate files must be sanitized before injection

A lot of people encounter problems when trying to inject a certificate
file which contains extra informations or empty lines.

This patch adds a paragraph and a sanitizing example.

Must be backported as far as 2.1.

(cherry picked from commit ed8bfadd8d17fc59b8a1f57bb2476cd7df1ce190)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 827347512a198e07f5acf9cb6767a06ef4898036)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: tcpcheck: Improve LDAP response parsing to fix LDAP check

When the LDAP response is parsed, the message length is not properly
decoded. While it works for LDAP servers encoding it on 1 byte, it does not
work for those using a multi-bytes encoding. Among others, Active Directory
servers seems to encode messages or elements length on 4 bytes.

In this patch, we only handle length of BindResponse messages encoded on 1,
2 or 4 bytes. In theory, it may be encoded on any bytes number less than 127
bytes. But it is useless to make this part too complex. It should be ok this
way.

This patch should fix the issue #1390. It should be backported to all stable
versions. While it should be easy to backport it as far as 2.2, the patch
will have to be totally rewritten for lower versions.

(cherry picked from commit 8a0e5f822b84c984681547eb0e9ee00d8a19ce56)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 81011211b71b4263ac096d538ba3823e1b693b08)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MINOR: pools: use mallinfo2() when available instead of mallinfo()

Ilya reported in issue #1391 a build warning on Fedora about mallinfo()
being deprecated in favor of mallinfo2() since glibc-2.33. Let's add
support for it. This should be backported where the following commit is
also backported: 157e39303 ("MINOR: pools: automatically disable
malloc_trim() with external allocators").

(cherry picked from commit c2afb860f297eb82039d821bd25bdd3cf66abe28)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 3bf75694c2ba89c1b705ce94b5543aa30fccf4a7)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MINOR: pools: automatically disable malloc_trim() with external allocators

Pierre Cheynier reported some occasional crashes in malloc_trim() on a
recent glibc when running with jemalloc(). While in theory there should
not be any link between the two, it remains plausible that something
allocated early with one is tentatively freed with the other and that
attempts to trim end up badly. There's no point calling the glibc specific
malloc_trim() with external allocators anyway. However these ones are often
enabled at link time or even at run time with LD_PRELOAD, so we cannot rely
on build options for this.

This patch implements runtime detection for the allocator in use by checking
with mallinfo() that a malloc() call is properly accounted for in glibc's
malloc. It only enables malloc_trim() in this case, and ignores it for
other cases. It's fine to proceed like this because mallinfo() is provided
by a wider range of glibcs than malloc_trim().

This could be backported to 2.4 and 2.3. If so, it will also need previous
patch "CLEANUP: pools: factor all malloc_trim() calls into trim_all_pools()".

(cherry picked from commit 157e393039966fa9a1b4f9a5ca964ec56f28204b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 562bf20a8501f01f56009c959d0e58e590d2ef66)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

CLEANUP: pools: factor all malloc_trim() calls into trim_all_pools()

The code was slightly cleaned up by removing repeated occurrences of ifdefs
and moving that into a single trim_all_pools() function.

(cherry picked from commit ea3323f62c093f2ee5e09b131ab6e3e23d113838)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 33298de642027a51451e40eae7a122f29c18dc09)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: compat: make sure __WORDSIZE is always defined

-Wundef triggered on a MIPS-based musl build on __WORDSIZE that's used
in ultoa_o() and some Lua initialization. The former will fail to convert
integers larger to 1 billion to proper string in this case. Let's make
sure this macro is defined and fall back to values determined from
__SIZEOF_LONG__ otherwise. A cleaner long-term approach would consist
in removing all remaining occurrences of this macro.

This can be backported to all versions.

(cherry picked from commit 4f5485bfad8355b7cf2d62a36897558dfc44bcb6)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 3f41b89598ed8978ee878d35b98a5c0e997c3b05)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

Revert "REGTESTS: mark http_abortonclose as broken"

This reverts commit e3abd92c6c3496bf14bc51b4143fe7c6e72a6acf.

The abortonclose option is fixed again, thanks to the commit 3dae79e59d
("BUG/MEDIUM: stream-int: Don't block SI on a channel policy if EOI is
reached"). The corresponding regtest should work again.

BUG/MEDIUM: stream-int: Don't block SI on a channel policy if EOI is reached

If the end of input is reported by the mux on the conn-stream during a
receive, we leave without evaluating the channel policies. It is especially
important to be able to catch client aborts during server connection
establishment. Indeed, in this case, without this patch, the
stream-interface remains blocked and read events are not forwarded to the
stream. It means it is not possible to detect client aborts.

Thanks to this fix, the abortonclose option should fixed for HAProxy 2.3 and
lower. On 2.4 and 2.5, it seems to work because the stream is created after
the request parsing.

Note that a previous fix of abortonclose option was reverted. This one
should be the right way to fix it. It must carefully be backported as far as
2.0. A observation period on the 2.3 is probably a good idea.

(cherry picked from commit 883d83e83c54c77cd16735716a029670b6317926)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 82219b562b512bda3b71489577a23f0c92ea875f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: systemd: ExecStartPre must use -Ws

This line should disappear in a future version but we should still fix
ExecStartPre with -Ws like we've done in 9def142.

It's a complementary fix that must be backported with 9def142
("BUG/MINOR: systemd: must check the configuration using -Ws").

(cherry picked from commit 6f58c13797dc02a5b0c5be943f6d5396e486bd71)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 0857f96996f669fc17557dfc045c873b4d09b2d3)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: filters: Set right FLT_END analyser depending on channel

A bug was introduced by the commit 26eb5ea35 ("BUG/MINOR: filters: Always
set FLT_END analyser when CF_FLT_ANALYZE flag is set"). Depending on the
channel evaluated, the rigth FLT_END analyser must be set. AN_REQ_FLT_END
for the request channel and AN_RES_FLT_END for the response one.

Ths patch must be backported everywhere the above commit was backported.

(cherry picked from commit 949b6ca961904c7414fc22204461f9882afff4ea)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 2f3ead14a522e1ffb587e8315b0b0a24acab5d56)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: filters: Always set FLT_END analyser when CF_FLT_ANALYZE flag is set

CF_FLT_ANALYZE flags may be set before the FLT_END analyser. Thus if an error is
triggered in the mean time, this may block the stream and prevent it to be
released. It is indeed a problem only for the response channel because the
response analysers may be skipped on early errors.

So, to prevent any issue, depending on the code path, the FLT_END analyser is
systematically set when the CF_FLT_ANALYZE flag is set.

This patch must be backported in all stable branches.

(cherry picked from commit 26eb5ea352c2ba8b25f014222773fb6946004bef)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit feca2a4534092158319d43c65dd23ffd61889a5a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: http-ana: Reset channels analysers when returning an error

When an error is returned to the client, via a call to
http_reply_and_close(), the request channel is flushed and shut down and
HTTP analysis on both direction is finished. So it is safer to centralize
reset of channels analysers at this place. It is especially important when a
filter is attached to the stream when a client abort is detected. Because,
otherwise, the stream remains blocked because request analysers are not
reset.

This bug was hidden for a while. But since the fix 6fcd2d328 ("BUG/MINOR:
stream: Don't release a stream if FLT_END is still registered"), it is
possible to trigger it.

This patch must be backported everywhere the above commit was backported.

(cherry picked from commit 2d56500826d54fca445de2c0170384763b1cfdb1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit db8ba1069af969f0ad344c8c9d1be0a650ca3005)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: stream: Don't release a stream if FLT_END is still registered

When at least one filter is registered on a stream, the FLT_END analyzer is
called on both direction when all other analyzers have finished their
processing. During this step, filters may release any allocated elements if
necessary. So it is important to not skip it.

Unfortunately, if both stream interfaces are closed, it is possible to not
wait the end of this analyzer. It is possible to be in this situation if a
filter must wait and prevents the analyzer completion. To fix the bug, we
now wait FLT_END analyzer is no longer registered on both direction before
releasing the stream.

This patch may be backported as far as 1.7, but AFAIK, no filter is affected
by this bug. So the backport seems to be optional for now. In any case, it
should remain under observation for some weeks first.

(cherry picked from commit 6fcd2d32805d949672babae8ca70806f8b308870)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 223f48e6fa8a3ddb75f41dafa398c5f147637e3e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: lua: Don't yield in channel.append() and channel.set()

Lua functions to set or append data to the input part of a channel must not
yield because new data may be received while the lua script is suspended. So
adding data to the input part in several passes is highly unpredicatble and
may be interleaved with received data.

Note that if necessary, it is still possible to suspend a lua action by
returning act.YIELD. This way the whole action will be reexecuted later
because of I/O events or a timer. Another solution is to call core.yield().

This bug affects all stable versions. So, it may be backported. But it is
probably not necessary because nobody notice it till now.

(cherry picked from commit 3b7bc3e38b7c3e889ca77415cb6b5d91e92cc0c4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: lua: Yield in channel functions only if lua context can yield

When a script is executed, it is not always allowed to yield. Lua sample
fetches and converters cannot yield. For lua actions, it depends on the
context. When called from tcp content ruleset, an action may yield until the
expiration of the inspect-delay timeout. From http rulesets, yield is not
possible.

Thus, when channel functions (dup, get, append, send...) are called, instead
of yielding when it is not allowed and triggering an error, we just give
up. In this case, some functions do nothing (dup, append...), some others
just interrupt the in-progress job (send, forward...). But, because these
functions don't yield anymore when it is not allowed, the script regains the
control and can continue its execution.

This patch depends on "MINOR: lua: Add a flag on lua context to know the
yield capability at run time". Both may be backported in all stable
versions. However, because nobody notice this bug till now, it is probably
not necessary, excepted if someone ask for it.

(cherry picked from commit 055310ff49749ab1f4b3e4bb2ea675fbc597f3f3)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MINOR: lua: Add a flag on lua context to know the yield capability at run time

When a script is executed, a flag is used to allow it to yield. An error is
returned if a lua function yield, explicitly or not. But there is no way to
get this capability in C functions. So there is no way to choose to yield or
not depending on this capability.

To fill this gap, the flag HLUA_NOYIELD is introduced and added on the lua
context if the current script execution is not authorized to yield. Macros
to set, clear and test this flags are also added.

This feature will be usefull to fix some bugs in lua actions execution.

(cherry picked from commit a6e792ff52f895234bb1499e9df5417bf498a8f8)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

[RELEASE] Released version 2.3.14

Released version 2.3.14 with the following main changes :
    - BUG/MEDIUM: h2: match absolute-path not path-absolute for :path
    - BUG/MEDIUM: sock: really fix detection of early connection failures in for 2.3-
    - REGTESTS: abortonclose: after retries, 503 is expected, not close
    - BUG/MINOR: stick-table: fix the sc-set-gpt* parser when using expressions
    - BUG/MEDIUM: base64: check output boundaries within base64{dec,urldec}
    - MINOR: compiler: implement an ONLY_ONCE() macro
    - BUG/MINOR: lua: use strlcpy2() not strncpy() to copy sample keywords
    - BUG/MINOR: ebtree: remove dependency on incorrect macro for bits per long
    - BUG/MINOR threads: Use get_(local|gm)time instead of (local|gm)time
    - BUG/MINOR: tools: Fix loop condition in dump_text()
    - CLEANUP: Add missing include guard to signal.h
    - BUG/MINOR: vars: fix set-var/unset-var exclusivity in the keyword parser
    - DOC: configuration: remove wrong tcp-request examples in tcp-response
    - BUG/MINOR: config: reject configs using HTTP with bufsize >= 256 MB
    - CLEANUP: htx: remove comments about "must be < 256 MB"
    - BUG/MAJOR: htx: fix missing header name length check in htx_add_header/trailer
    - Revert "BUG/MINOR: stream-int: Don't block reads in si_update_rx() if chn may receive"
    - MINOR: action: Use a generic function to check validity of an action rule list
    - REGTESTS: mark http_abortonclose as broken

REGTESTS: mark http_abortonclose as broken

This is a consequence of the revert of the commit below, until a better
fix is found for this issue:

    BUG/MINOR: stream-int: Don't block reads in si_update_rx() if chn may receive

MINOR: action: Use a generic function to check validity of an action rule list

The check_action_rules() function is now used to check the validity of an
action rule list. It is used from check_config_validity() function to check
L5/6/7 rulesets.

(cherry picked from commit 42c6cf950111736327863de5e82036a1d51deb04)
[cf: This patch is in fact a fix because the "tcp-resonse content" ruleset
     was not fully configured. It was ignored during Post-parsing
     stage. This patch should fix a bug reported in #1263 by @ HiggTh. It
     must be backported in all stable versions.]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

Revert "BUG/MINOR: stream-int: Don't block reads in si_update_rx() if chn may receive"

This reverts commit e0dec4b7b258101f6d5faa15234103a45c16f0f8.

At first glance, channel_is_empty() was used on purpose in si_update_rx(),
because of the HTX ("b3e0de46c" MEDIUM: stream-int: Rely only on
SI_FL_WAIT_ROOM to stop data receipt). It is not pretty clear for now why
channel_may_recv() sould not be used here but this change introduce a
possible infinite loop with the stats applet. So, it is safer to revert the
patch, waiting for a better understanding of the probelm.

This means the abortonclose option will be broken again on the 2.3 and lower
versions.

This patch should fix the issue #1360. It must be backported as far as 2.0.

(cherry picked from commit b7308f00cb03e5accd2f6092bf09fc2ac1360ff9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 62fc11cb2da250f6c70ea3a1b3af767d8d0a9b2c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MAJOR: htx: fix missing header name length check in htx_add_header/trailer

Ori Hollander of JFrog Security reported that htx_add_header() and
htx_add_trailer() were missing a length check on the header name. While
this does not allow to overwrite any memory area, it results in bits of
the header name length to slip into the header value length and may
result in forging certain header names on the input. The sad thing here
is that a FIXME comment was present suggesting to add the required length
checks :-(

The injected headers are visible to the HTTP internals and to the config
rules, so haproxy will generally stay synchronized with the server. But
there is one exception which is the content-length header field, because
it is already deduplicated on the input, but before being indexed. As
such, injecting a content-length header after the deduplication stage
may be abused to present a different, shorter one on the other side and
help build a request smuggling attack, or even maybe a response splitting
attack. CVE-2021-40346 was assigned to this problem.

As a mitigation measure, it is sufficient to verify that no more than
one such header is present in any message, which is normally the case
thanks to the duplicate checks:

   http-request  deny if { req.hdr_cnt(content-length) gt 1 }
   http-response deny if { res.hdr_cnt(content-length) gt 1 }

This must be backported to all HTX-enabled versions, hence as far as 2.0.
In 2.3 and earlier, the functions are in src/htx.c instead.

Many thanks to Ori for his work and his responsible report!

(cherry picked from commit 3b69886f7dcc3cfb3d166309018e6cfec9ce2c95)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 1fd2566683f6fb66b180ce9c3d9062ddaa81d6d7)
[wt: code is in src/htx.c in 2.3 and older]
Signed-off-by: Willy Tarreau <w@1wt.eu>

CLEANUP: htx: remove comments about "must be < 256 MB"

Since commit "BUG/MINOR: config: reject configs using HTTP with bufsize
>= 256 MB" we are now sure that it's not possible anymore to have an HTX
block of a size 256 MB or more, even after concatenation thanks to the
tests for len >= htx_free_data_space(). Let's remove these now obsolete
comments.

A BUG_ON() was added in htx_add_blk() to track any such exception if
the conditions would change later, to complete the one that is performed
on the start address that must remain within the buffer.

(cherry picked from commit 3d5f19e04d88e7c8f71cba4ea12e383c91de89f6)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 86cb2cd3c68a0f3072a326def89449e10760423d)
[wt: no change in htx.h, ctx updates in htx.c]
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MINOR: config: reject configs using HTTP with bufsize >= 256 MB

As seen in commit 5ef965606 ("BUG/MINOR: lua: use strlcpy2() not
strncpy() to copy sample keywords"), configs with large values of
tune.bufsize were not practically usable since Lua was introduced,
regardless of the machine's available memory.

In addition, HTX encoding already limits block sizes to 256 MB, thus
it is not technically possible to use that large a buffer size when
HTTP is in use. This is absurdly high anyway, and for example Lua
initialization would take around one minute on a 4 GHz CPU. Better
prevent such a config from starting than having to deal with bug
reports that make no sense.

The check is only enforced if at least one HTX proxy was found, as
there is no techincal reason to block it for configs that are solely
based on raw TCP, and it could still be imagined that some such might
exist with single connections (e.g. a log forwarder that buffers to
cover for the storage I/O latencies).

This should be backported to all HTX-enabled versions (2.0 and above).

(cherry picked from commit 32b51cdf303cb30425000f1db6ecdae5de84ff8d)
[wt: minor ctx adj]
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 95d0810ddec6d778bf8a08f4369bacbc0f19ad6e)
Signed-off-by: Willy Tarreau <w@1wt.eu>

DOC: configuration: remove wrong tcp-request examples in tcp-response

There is a massive abuse of copy-paste in the doc that is visible in
the examples and arguments declaration. Let's at least remove irrelevant
examples for now.

(cherry picked from commit e7267120d5622acaf6fcc5d82f520655cf86e11f)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 9de54ba791d2453709920131738366d796bf5e7e)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MINOR: vars: fix set-var/unset-var exclusivity in the keyword parser

The parser checks first for "set-var" then "unset-var" from the updated
offset instead of testing it only when the other one fails, so it
validates this rule as "unset-var":

    http-request set-varunset-var(proc.a)

This should be backported everywhere relevant, though it's mostly harmless
as it's unlikely that some users are purposely writing this in their conf!

(cherry picked from commit 2819210a8328ff60505e499acf9f8bc323bec840)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit f6bda6191520885fc3af06c3dec8a74e56d46cbd)
Signed-off-by: Willy Tarreau <w@1wt.eu>

CLEANUP: Add missing include guard to signal.h

Found using GitHub's CodeQL scan.

(cherry picked from commit abc6b31ab863be589758b0760b92b5d3a4f903d1)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit af75f82be0c7a822bf23d01fb7bf1b862b8f51e9)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MINOR: tools: Fix loop condition in dump_text()

The condition should first check whether `bsize` is reached, before
dereferencing the offset. Even if this always works fine, due to the
string being null-terminated, this certainly looks odd.

Found using GitHub's CodeQL scan.

This bug traces back to at least 97c2ae13bc0d7961a348102d6719fbcaf34d46d5
(1.7.0+) and this patch should be backported accordingly.

(cherry picked from commit 18795d48a9bb09aedc57e547029828a56322e49d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit cd5521e7ca7472bf69874603c0c5785d4ff1d1e2)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MINOR threads: Use get_(local|gm)time instead of (local|gm)time

Using localtime / gmtime is not thread-safe, whereas the `get_*` wrappers are.

Found using GitHub's CodeQL scan.

The use in sample_conv_ltime() can be traced back to at least
fac9ccfb705702f211f99e67d5f5d5129002086a (first appearing in 1.6-dev3), so all
supported branches with thread support are affected.

(cherry picked from commit 1f269c12dc31bb63db31559cb44c187ab91abb64)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 9da4b1fc83fe083c8f194c61402ba4ffeb5b330c)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MINOR: ebtree: remove dependency on incorrect macro for bits per long

The code used to rely on BITS_PER_LONG to decide on the most efficient
way to perform a 64-bit shift, but this macro is not defined (at best
it's __BITS_PER_LONG) and it's likely that it's been like this since
the early implementation of ebtrees designed on i386. Let's remove the
test on this macro and rely on sizeof(long) instead, it also has the
benefit of letting the compiler validate the two branches.

This can be backported to all versions. Thanks to Ezequiel Garcia for
reporting this one in issue #1369.

(cherry picked from commit cbdc74b4b32af93b4724adcb4b2354bb811693ab)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit d822d9ae2cbc8e713c40c0f7b9e0f2f2225a606d)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MINOR: lua: use strlcpy2() not strncpy() to copy sample keywords

The lua initialization code which creates the Lua mapping of all converters
and sample fetch keywords makes use of strncpy(), and as such can take ages
to start with large values of tune.bufsize because it spends its time zeroing
gigabytes of memory for nothing. A test performed with an extreme value of
16 MB takes roughly 4 seconds, so it's possible that some users with huge
1 MB buffers (e.g. for payload analysis) notice a small startup latency.
However this does not affect config checks since the Lua stack is not yet
started. Let's replace this with strlcpy2().

This should be backported to all supported versions.

(cherry picked from commit 5ef965606b5bacb12769c97f85b2cfd1c4e4ffe7)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit fa92f36beb65b4d0515892c783d40e94d33b131c)
Signed-off-by: Willy Tarreau <w@1wt.eu>

MINOR: compiler: implement an ONLY_ONCE() macro

There are regularly places, especially in config analysis, where we
need to report certain things (warnings or errors) only once, but
where implementing a counter is sufficiently deterrent so that it's
not done.

Let's add a simple ONLY_ONCE() macro that implements a static variable
(char) which is atomically turned on, and returns true if it's set for
the first time. This uses fairly compact code, a single byte of BSS
and is thread-safe. There are probably a number of places in the config
parser where this could be used. It may also be used to implement a
WARN_ON() similar to BUG_ON() but which would only warn once.

(cherry picked from commit 906f7daed16a3b1c4058b77fed44b3c4265e645f)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 6902527b83ed426793b3e89936f27b78c5c21f37)
Signed-off-by: Willy Tarreau <w@1wt.eu>