Amaury Denoyelle [Fri, 11 Jul 2025 12:25:30 +0000 (14:25 +0200)]
BUG/MEDIUM: h3: handle interim response properly on FE side
On frontend side, HTTP/3 layer is responsible to transcode an HTX
response message into HTTP/3 HEADERS frame. This operations is handled
via h3_resp_headers_send().
Prior to this patch, if HTX EOM was encountered in the HTX message after
response transcoding, <fin> was reported to the QMUX layer. This will in
turn cause FIN stream bit to be set when the response is emitted.
However, this is not correct as a single HTX response can be constitued
of several interim message, each delimited by EOM block.
Most of the time, this bug will cause the client to close the connection
as it is invalid to receive an interim response with FIN bit set.
Fixes this by now properly differentiate interim and final response.
During interim response transcoding, the new flag H3_SF_SENT_INTERIM
will be set, which will prevent <fin> to be reported. Thus, <fin> will
only be notified for the final response.
This must be backported up to 2.6. Note that it relies on the previous
patch which also must be taken.
(cherry picked from commit
e7b3a69c594ab14cf25e9b8d038635aef4b8229f)
[ad: fix context due to comments on H3_SF flags]
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
(cherry picked from commit
7bf10050d9a98a2e01aebde00b99ebcb60003a4b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Amaury Denoyelle [Thu, 10 Jul 2025 13:27:23 +0000 (15:27 +0200)]
MINOR: qmux: change API for snd_buf FIN transmission
Previous patches have fixes interim response encoding via
h3_resp_headers_send(). However, it is still necessary to adjust h3
layer state-machine so that several successive HTTP responses are
accepted for a single stream.
Prior to this, QMUX was responsible to decree that the final HTX message
was encoded so that FIN stream can be emitted. However, with interim
response, MUX is in fact unable to properly determine this. As such,
this is the responsibility of the application protocol layer. To reflect
this, app_ops snd_buf callback is modified so that a new output argument
<fin> is added to it.
Note that for now this commit does not bring any functional change.
However, it will be necessary for the following patch. As such, it
should be backported prior to it to every versions as necessary.
(cherry picked from commit
f349df44b4e21d8bf9b575a0aa869056a2ebaa58)
[ad: fix context due to mising comment on h3_snd_buf]
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
(cherry picked from commit
96896b408d0fb0b45e5ebf59cc74a5a5c2b03dc9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Amaury Denoyelle [Tue, 15 Jul 2025 16:15:36 +0000 (18:15 +0200)]
BUG/MINOR: h3: ensure that invalid status code are not encoded (FE side)
On frontend side, H3 layer transcodes HTX status code into HTTP/3
HEADERS frame. This is done by calling qpack_encode_int_status().
Prior to this patch, the latter function was also responsible to reject
an invalid value, which guarantee that only valid codes are encoded
(between 100 and 999 values). However, this is not practical as it is
impossible to differentiate between an invalid code error and a buffer
room exhaustation.
Changes this so that now HTTP/3 layer first ensures that HTX code is
valid. The stream is closed with H3_INTERNAL_ERROR if invalid value is
present. Thus, qpack_encode_int_status() will only report an error due
to buffer room exhaustion. If a small buffer is used, a standard buffer
will be reallocated which should be sufficient to encode the response.
The impact of this bug is minimal. Its main benefit is code clarity,
while also removing an unnecessary realloc when confronting with an
invalid HTTP code.
This should be backported at least up to 3.1. Prior to it, smallbuf
mechanism isn't present, hence the impact of this patch is less
important. However, it may still be backported to older versions, which
should facilitate picking patches for HTTP 1xx interim response support.
(cherry picked from commit
d8b34459b52027d0621ebb0ac146fa277ab8e2ba)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
(cherry picked from commit
eb73c1d08783c49581876c9e95bad0b5a24fd6fc)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Amaury Denoyelle [Tue, 15 Jul 2025 08:59:07 +0000 (10:59 +0200)]
BUG/MINOR: h3: properly realloc buffer after interim response encoding
Previous commit fixes encoding of several following HTTP response
message when interim status codes are first reported. However,
h3_resp_headers_send() still was unable to interrupt encoding if output
buffer room was not sufficient. This case may be likely because small
buffers are used for headers encoding.
This commit fixes this situation. If output buffer is not empty prior to
response encoding, this means that a previous interim response message
was already encoded before. In this case, and if remaining space is not
sufficient, use buffer release mechanism : this allows to restart
response encoding by using a newer buffer. This process has already been
used for DATA and trailers encoding.
This must be backported up to 2.6. However, note that buffer release
mechanism is not present for version 2.8 and lower. In this case, qcs
flag QC_SF_BLK_MROOM should be enough as a replacement.
(cherry picked from commit
d59bdfb8ec5b5af284672af8a89bf0d1662d8efd)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
(cherry picked from commit
aa92894f3ca042653f6bb568997bd7b548e2be55)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Amaury Denoyelle [Tue, 15 Jul 2025 08:58:06 +0000 (10:58 +0200)]
BUG/MEDIUM: h3: do not overwrite interim with final response
An HTTP response may contain several interim response message prior (1xx
status) to a final response message (all other status codes). This may
cause issues with h3_resp_headers_send() called for response encoding
which assumes that it is only call one time per stream, most notably
during output buffer handling.
This commit fixes output buffer handling when h3_resp_headers_send() is
called multiple times due to an interim response. Prior to it, interim
response was overwritten with newer response message. Most of the time,
this resulted in error for the client due to QPACK decoding failure.
This is now fixed so that each response is encoded one after the other.
Note that if encoding of several responses is bigger than output buffer,
an error is reported. This can definitely occurs as small buffer are
used during header encoding. This situation will be improved by the next
patch.
This must be backported up to 2.6.
(cherry picked from commit
1290fb731d9bb84777d811b69cf71106b89f92cf)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
(cherry picked from commit
888fe7cc77106cc620c0c98777ac5345a2e6eeef)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Wed, 9 Jul 2025 13:20:41 +0000 (15:20 +0200)]
BUG/MEDIUM: http-client: Notify applet has more data to deliver until the EOM
When we leave the I/O handler with an unfinished request, we must report the
applet has more data to deliver. Otherwise, when the channel request buffer
is emptied, the http-client applet is not always woken up to forward the
remaining request data.
This issue was probably revealed by commit "BUG/MEDIUM: http-client: Don't
wake http-client applet if nothing was xferred". It is only an issue with
large POSTs, when the payload is streamed.
This patch must be backported as far as 2.6 with the commit above. But on
older versions, the applet API may differ. So be careful.
(cherry picked from commit
0b97bf36fad6fbced8077ac416a1399c05806c7d)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
(cherry picked from commit
dd3ba78fc13a99e79052496d1851cff65c24f73c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Tue, 8 Jul 2025 06:45:10 +0000 (08:45 +0200)]
BUG/MEDIUM: http-client: Drain the request if an early response is received
When a large request is sent, it is possible to have a response before the
end of the request. It is valid from HTTP perspective but it is an issue
with the current design of the http-client. Indded, the request and the
response are handled sequentially. So the response will be blocked, waiting
for the end of the request. Most of time, it is not an issue, except when
the request transfer is blocked. In that case, the applet is blocked.
With the current API, it is not possible to handle early response and
continue the request transfer. So, this case cannot be handle. In that case,
it seems reasonnable to drain the request if a response is received. This
way, the request transfer, from the caller point of view, is never blocked
and the response can be properly processed.
To do so, the action flag HTTPCLIENT_FA_DRAIN_REQ is added to the
http-client. When it is set, the request payload is just dropped. In that
case, we take care to not report the end of input to properly report the
request was truncated, especially in logs.
It is only an issue with large POSTs, when the payload is streamed.
This patch must be backported as far as 2.6.
(cherry picked from commit
25b0625d5c27a209c38dcac6a81c58495e5360af)
[ad: context adjustement due to missing HTTPCLIENT_O_RES_HTX in 3.2]
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
(cherry picked from commit
eadb777452f8470ddaeb0ffdb59c130ce1621823)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Tue, 8 Jul 2025 06:57:16 +0000 (08:57 +0200)]
BUG/MINOR: http-client: Reject any 101-switching-protocols response
Protocol updages are not supported by the http-client. So report an error is
a 101-switching-protocols response is received. Of course, it is unexpected
because the API is not designed to support upgrades. But it is better to
properly handle this case.
This patch could be backported as far as 2.6. It depends on the commit
"BUG/MINOR: http-client: Ignore 1XX interim responses in non-HTX mode".
(cherry picked from commit
8ba754108d70d25d796e3dc166155503aab180b2)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
(cherry picked from commit
0bfaef56d630a355d85fcdf2b733e479fee8595b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Tue, 8 Jul 2025 06:38:31 +0000 (08:38 +0200)]
BUG/MINOR: http-client: Ignore 1XX interim responses in non-HTX mode
When the response is re-formatted in raw message, the 1XX interim responses
must be skipped. Otherwise, information of the first interim response will
be saved (status line and headers) and those from the final response will be
dropped.
Note that for now, in HTX-mode, the interim messages are removed.
This patch must be backported as far as 2.6.
(cherry picked from commit
9d10be33aebb08058c37d7dcd15874e42be74a29)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
(cherry picked from commit
8abebc8d061c29dcdf511efba2321cf2a6df00d6)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Tue, 8 Jul 2025 06:17:49 +0000 (08:17 +0200)]
BUG/MEDIUM: http-client: Ask for more room when request data cannot be xferred
When the request payload cannot be xferred to the channel because its buffer
is full, we must request for more room by calling sc_need_room(). It is
important to be sure the httpclient applet will not be woken up in loop to
push more data while it is not possible.
It is only an issue with large POSTs, when the payload is streamed.
This patch must be backported as far as 2.6. Note that on 2.6,
sc_need_room() only takes one argument.
(cherry picked from commit
e4a0d40c62594b4504fd7443158731fef54e040a)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
(cherry picked from commit
5377ea587fba88b56e5249008be153cb529f066b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Tue, 8 Jul 2025 06:04:01 +0000 (08:04 +0200)]
BUG/MEDIUM: http-client: Properly inc input data when HTX blocks are xferred
When HTX blocks from the requests are transferred into the channel buffer,
the return value of htx_xfer_blks() function must not be used to increment
the channel input value because meta data are counted here while they are
not part of input data. Because of this bug, it is possible to forward more
data than these present in the channel buffer.
Instead, we look at the input data before and after the transfer and the
difference is added.
It is only an issue with large POSTs, when the payload is streamed.
This patch must be backported as far as 2.6.
(cherry picked from commit
d9ca8f6b71cd17bae0718f0b1e9da919fc00264d)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
(cherry picked from commit
208ef1d7aebc11133e8c64966be475a521da5409)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Tue, 8 Jul 2025 05:46:26 +0000 (07:46 +0200)]
BUG/MEDIUM: http-client: Don't wake http-client applet if nothing was xferred
When data are transferred to or from the htt-pclient, the applet is
systematically woken up, even when no data are transferred. This could lead
to needlessly wakeups. When called from a lua script, if data are blocked
for a while, this leads to a wakeup ping-pong loop where the http-client
applet is woken up by the lua script which wakes back the script.
To fix the issue, in httpclient_req_xfer() and httpclient_res_xfer()
functions, we now take care to not wake the http-client applet up when no
data are transferred.
This patch must be backported as far as 2.6.
(cherry picked from commit
fffdac42df4848420028f89b14c2a90c12dbf9ab)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
(cherry picked from commit
b35b0aaf82d625c8f78f734fb5b4c0deebd1697a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Wed, 9 Jul 2025 13:52:33 +0000 (15:52 +0200)]
BUG/MINOR: listener: really assign distinct IDs to shards
A fix was made in 3.0 for the case where sharded listeners were using
a same ID with commit
0db8b6034d ("BUG/MINOR: listener: always assign
distinct IDs to shards"). However, the fix is incorrect. By checking the
ID of temporary node instead of the kept one in bind_complete_thread_setup()
it ends up never inserting the used nodes at this point, thus not reserving
them. The side effect is that assigning too close IDs to subsequent
listeners results in the same ID still being assigned twice since not
reserved. Example:
global
nbthread 20
frontend foo
bind :8000 shards by-thread id 10
bind :8010 shards by-thread id 20
The first one will start a series from 10 to 29 and the second one a
series from 20 to 39. But 20 not being inserted when creating the shards,
it will remain available for the post-parsing phase that assigns all
unassigned IDs by filling holes, and two listeners will have ID 20.
By checking the correct node, the problem disappears. The patch above
was marked for backporting to 2.6, so this fix should be backported that
far as well.
(cherry picked from commit
dd49f1ee6230eaf608f3afa880ef0a6d2b84456e)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
(cherry picked from commit
c12e3b66be1eec8e5ccf6ca25871df77864c2b79)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Mon, 11 Aug 2025 15:24:11 +0000 (17:24 +0200)]
MEDIUM: ssl/cli: relax crt insertion in crt-list of type directory
In previous versions of haproxy, insertions of certificates in a
crt-list from the CLI would require to have the path of the directory,
in the path of the certificate. This would help avoiding that the
certificate wasn't loaded upon a reload because it is not at the right
place.
However, since version 3.0 and crt-store, the name stored in the tree
could be an alias and not a path, so that does not make sense anymore.
Even though path would be right, the check is not right anymore in this
case.
The tool or user inserting the certificate must now check itself that
the certificate was placed at the right spot on the filesystem.
Reported in issue #3053.
Could be backported as far as haproxy 3.0.
(cherry picked from commit
55d561042c81f8853364a62b4d5eaba2555c2673)
Signed-off-by: William Lallemand <wlallemand@haproxy.com>
(cherry picked from commit
780f1e8a3301b49d74d336c2dbf3400a2d8d1cfb)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Mon, 11 Aug 2025 14:19:17 +0000 (16:19 +0200)]
DOC: management: clarify usage of -V with -c
In ticket #3065 an user complained that no success message is printed
anymore when using -c. The message does not appear by default since
version 2.9. This patch clarify the documentation.
Must be backported as far as 2.8.
(cherry picked from commit
f4f93c56c17165e7098dc98cecf49e656e57afff)
Signed-off-by: William Lallemand <wlallemand@haproxy.com>
(cherry picked from commit
d650878491600e281bf1ea15b378237d28e50b69)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Aurelien DARRAGON [Mon, 28 Jul 2025 18:14:53 +0000 (20:14 +0200)]
BUG/MINOR: hlua: take default-path into account with lua-load-per-thread
As discussed in GH #3051, default-path is not taken into account when
loading files using lua-load-per-thread. In fact, the initial
hlua_load_state() (performed on first thread which parses the config)
is successful, but other threads run hlua_load_state() later based
on config hints which were saved by the first thread, and those config
hints only contain the file path provided on the lua-load-per-thread
config line, not the absolute one. Indeed, `default-path` directive
changes the current working directory only for the thread parsing the
configuration.
To fix the issue, when storing config hints under hlua_load_per_thread()
we now make sure to save the absolute file path for `lua-load-per-thread'
argument.
Thanks to GH user @zhanhb for having reported the issue
It may be backported to all stable versions.
(cherry picked from commit
2ffe515d97a80318f1f3a84f8e18015500b8ef01)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
ac1be87c8359abad1c1a55f5ea6822d4362706db)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Aurelien DARRAGON [Fri, 25 Jul 2025 14:03:21 +0000 (16:03 +0200)]
BUG/MEDIUM: logs: fix sess_build_logline_orig() recursion with options
Since
ccc43412 ("OPTIM: log: use thread local lf_buildctx to stop pushing
it on the stack"), recursively calling sess_build_logline_orig(), which
may for instance happen when leveraging %ID (or unique-id fetch) for the
first time, would lead to undefined behavior because the parent
sess_build_logline_orig() build context was shared between recursive calls
(only one build ctx per thread to avoid pushing it on the stack for each
call)
In short, the parent build ctx would be altered by the recursive calls,
which is obviously not expected and could result in log formatting errors.
To fix the issue but still avoid polluting the stack with large lf_buildctx
struct, let's move the static 256 bytes build buffer out of the buildctx
so that the buildctx is now stored in the stack again (each function
invokation has its own dedicated build ctx). On the other hand, it's
acceptable to have only 1 256 bytes build buffer per thread because the
build buffer is not involved in recursives calls (unlike the build ctx)
Thanks to Willy and Vincent Gramer for spotting the bug and providing
useful repro.
It should be backported in 3.0 with
ccc43412.
(cherry picked from commit
31adfb6c15df075967bffe909509cb49583dd0d1)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
ce6822eade1b4a1677c621b3d1f67422679f2285)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Mon, 21 Jul 2025 09:30:41 +0000 (11:30 +0200)]
BUG/MEDIUM: dns: Reset reconnect tempo when connection is finally established
The issue was introduced by commit
27236f221 ("BUG/MINOR: dns: add tempo
between 2 connection attempts for dns servers"). In this patch, to delay the
reconnection, a timer is used on the appctx when it is created. This
postpones the appctx initialization. However, once initialized, the
expiration time of the underlying task is not reset. So, it is always
considered as expired and the appctx is woken up in loop.
The fix is quite simple. In dns_session_init(), the expiration time of the
appctx's task is alwaus set to TICK_ETERNITY.
This patch must be backported everywhere the commit above was backported. So
as far as 2.8 for now but possibly to all stable versions.
(cherry picked from commit
765f14e0e34054cd805b9974f531145d3e796967)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
4f820213ea0fbcbb13ecb26567d1f9ee5b8aa116)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 18 Jul 2025 14:09:20 +0000 (16:09 +0200)]
BUG/MEDIUM: hlua: Report to SC when output data are blocked on a lua socket
It is a fix similar to the previous one ("BUG/MEDIUM: hlua: Report to SC
when data were consumed on a lua socket"), but for the write side. The
writer must notify the cosocket it needs more space in the request buffer to
produce more data by calling sc_need_room(). Otherwise, there is nothing to
prevent to wake the cosocket applet up again and again.
This patch must be backported as far as 2.8, and maybe to 2.6 too.
(cherry picked from commit
7e96ff6b84b7d7e0805a9a4e87a0a85b6b976df8)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
839755b1d401928df89b4fd8b146a20a17e28a3d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 18 Jul 2025 14:07:16 +0000 (16:07 +0200)]
BUG/MEDIUM: hlua: Report to SC when data were consumed on a lua socket
The lua cosocket are quite strange. There is an applet used to handle the
connection and writer and readers subscribed on it to write or read
data. Writers and readers are tasks woken up by the cosocket applet when
data can be consumed or produced, depending on the channels buffers
state. Then the cosocket applet is woken up by writers and readers when read
or write events were performed.
It means the cosocket applet has only few information on what was produced
or consumed. It is the writers and readers responsibility to notify any
blocking. Among other things, the readers must take care to notify the
stream on top of the cosocket applet that some data was consumed. Otherwise,
it may remain blocked, waiting for a write event (a write event from the
stream point of view is a read event from the cosocket point of view).
Thie patch must be backported as far as 2.8, and maybe to 2.6 too.
(cherry picked from commit
21e45a61d134786f42b046666aacced7a6ce3cc0)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
4e09e4d9d024c2e71027e7df43ed7db6c99a714f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 18 Jul 2025 09:14:26 +0000 (11:14 +0200)]
BUG/MINOR: hlua: Skip headers when a receive is performed on an HTTP applet
When an HTTP applet tries to retrieve data, the request headers are still in
the buffer. But, instead of being silently removed, their size is removed
from the amount of data retrieved. When the request payload is fully
retrieved, it is not an issue. But it is a problem when a length is
specified. The data are shorten from the headers size.
So now, we take care to silently remove headers.
This patch must be backported to all stable versions.
(cherry picked from commit
5b5ecf848dbe7467afb67ea70951b4e56f39dab5)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
8daaeb14e8e89c936a82a4ddff9c54dc1c93b92b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
David Carlier [Wed, 2 Jul 2025 13:11:30 +0000 (14:11 +0100)]
DOC: deviceatlas build clarifications
Update accordingly the related documentation, removing/clarifying confusing
parts as it was more complicated than it needed to be.
(cherry picked from commit
e7c59a7a8435fa58377400aa88c241bfa111409e)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
2d6d7f335091a36f278f31a24ad2661ab66d9090)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
David Carlier [Wed, 2 Jul 2025 13:00:07 +0000 (14:00 +0100)]
BUILD/MEDIUM: deviceatlas: fix when installed in custom locations.
We are reusing DEVICEATLAS_INC/DEVICEATLAS_LIB when the DeviceAtlas
library had been compiled and installed with cmake and make install targets.
Works fine except when ldconfig is unaware of the path, thus adding
cflags/ldflags into the mix.
Ideally, to be backported down to the lowest stable branch.
(cherry picked from commit
0e8e20a83f4dd789443b9176cd93573bcfa75c20)
Signed-off-by: William Lallemand <wlallemand@haproxy.com>
(cherry picked from commit
770ad55db942bc69d436939bfdf1a7072d5572f9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Wed, 2 Jul 2025 08:54:03 +0000 (10:54 +0200)]
BUILD: dev/phash: remove the accidentally committed a.out file
Commit
41f28b3c53 ("DEV: phash: Update 414 and 431 status codes to phash")
accidentally committed a.out, resulting in build/checkout issues when
locally rebuilt. Let's drop it.
This should be backported to 3.1.
(cherry picked from commit
e34a0a50ae1a1da031fb9ef82c1941f6ece0d24f)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
4ca1814f87d4804361aaf35b2f567c6ba3ce207e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Fri, 24 Jan 2025 16:53:04 +0000 (17:53 +0100)]
BUG/MINOR: httpclient: wrongly named httpproxy flag
The HC_F_HTTPPROXY flag was wrongly named and does not use the correct
value, indeed this flag was meant to be used for the httpclient API, not
the httpclient CLI.
This patch fixes the problem by introducing HTTPCLIENT_FO_HTTPPROXY
which has must be set in hc->flags.
Also add a member 'options' in the httpclient structure, because the
member flags is reinitialized when starting.
Must be backported as far as 3.0.
(cherry picked from commit
519abefb57da1ae21fc557213cae8b21cdaa2797)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
008045a03eeca63712546e030a4549671d00ffc1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Remi Tricot-Le Breton [Mon, 30 Jun 2025 14:56:23 +0000 (16:56 +0200)]
DOC: Fix 'jwt_verify' converter doc
Contrary to what the doc says, the jwt_verify converter only works with
a public key and not a full certificate for certificate based protocols
(everything but HMAC).
This patch should be backported up to 2.8.
(cherry picked from commit
5c3d0a554b3db024aef62826b67821ca6a1383ee)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
a1845360477633ee572d2ef071dba0fd12512223)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Remi Tricot-Le Breton [Mon, 30 Jun 2025 14:56:22 +0000 (16:56 +0200)]
BUG/MINOR: jwt: Copy input and parameters in dedicated buffers in jwt_verify converter
When resolving variable values the temporary trash chunks are used so
when calling the 'jwt_verify' converter with two variable parameters
like in the following line, the input would be overwritten by the value
of the second parameter :
var(txn.bearer),jwt_verify(txn.jwt_alg,txn.cert)
Copying the values into dedicated alloc'ed buffers prevents any new call
to get_trash_chunk from erasing the data we need in the converter.
This patch can be backported up to 2.8.
(cherry picked from commit
3465f88f8ab9c3f163d73938765f741c2b7e6a67)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
33eb0792afff79fa9cd596e87e4796f6d518d2c7)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Mon, 30 Jun 2025 14:23:39 +0000 (16:23 +0200)]
BUG/MEDIUM: mux-h2: Properly handle connection error during preface sending
On backend side, an error at connection level during the preface sending was
not properly handled and could lead to a spinning loop on process_stream()
when the h2 stream on client side was blocked, for instance because of h2
flow control.
It appeared that no transition was perfromed from the PREFACE state to an
ERROR state on the H2 connection when an error occurred on the underlying
connection. In that case, the H2 connection was woken up in loop to try to
receive data, waking up the upper stream at the same time.
To fix the issue, an H2C error must be reported. Most state transitions are
handled by the demux function. So it is the right place to do so. First, in
PREFACE state and on server side, if an error occurred on the TCP
connection, an error is now reported on the H2 connection. REFUSED_STREAM
error code is used in that case. In addition, in that case, we also take
care to properly handle the connection shutdown.
This patch should fix the issue #3020. It must be backported to all stable
versions.
(cherry picked from commit
5ba0a2d5270f2ba52a3022578e52fb5709bff3cb)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
0232f26d8862229c079188a5e0141904d7db3dc1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Tue, 24 Jun 2025 06:26:14 +0000 (08:26 +0200)]
BUG/MEDIUM: hlua: Forbid any L6/L7 sample fetche functions from lua services
It was already forbidden to use HTTP sample fetch functions from lua
services. An error is triggered if it happens. However, the error must be
extended to any L6/L7 sample fetch functions.
Indeed, a lua service is an applet. It totally unexepected for an applet to
access to input data in a channel's buffer. These data have not been
analyzed yet and are still subject to any change. An applet, lua or not,
must never access to "not forwarded" data. Only output data are
available. For now, if a lua applet relies on any L6/L7 sampel fetch
functions, the behavior is undefined and not consistent.
So to fix the issue, hlua flag HLUA_F_MAY_USE_HTTP is renamed to
HLUA_F_MAY_USE_CHANNELS_DATA. This flag is used to prevent any lua applet to
use L6/L7 sample fetch functions.
This patch could be backported to all stable versions.
(cherry picked from commit
a2a142bf40c76114cf85dfe1f48d7b14ec70ad5f)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
236a6faff2a8abd7221b13fac1746488d335868e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Mon, 30 Jun 2025 14:20:29 +0000 (16:20 +0200)]
MINOR: ssl: check TLS1.3 ciphersuites again in clienthello with recent AWS-LC
Patch
ed9b8fec49 ("BUG/MEDIUM: ssl: AWS-LC + TLSv1.3 won't do ECDSA in
RSA+ECDSA configuration") partly fixed a cipher selection problem with
AWS-LC. However this was not checking anymore if the ciphersuites was
available in haproxy which is still a problem.
The problem was fixed in AWS-LC 1.46.0 with this PR
https://github.com/aws/aws-lc/pull/2092.
This patch allows to filter again the TLS13 ciphersuites with recent
versions of AWS-LC. However, since there are no macros to check the
AWS-LC version, it is enabled at the next AWS-LC API version change
following the fix in AWS-LC v1.50.0.
This could be backported where
ed9b8fec49 was backported.
(cherry picked from commit
7fc8ab0397f01c58a31f203b4d97c79cff3ae244)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
860f5f0536a100c383c75779ff7a6ffbc434019c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Valentine Krasnobaeva [Thu, 26 Jun 2025 14:49:12 +0000 (16:49 +0200)]
BUG/MINOR: tools: use my_unsetenv instead of unsetenv
Let's use our own implementation of unsetenv() instead of the one, which is
provided in libc. Implementation from libc may vary in dependency of UNIX
distro. Implemenation from libc.so.1 ported on Illumos (see the link below) has
caused an eternal loop in the clean_env(), where we invoke unsetenv().
(https://github.com/illumos/illumos-gate/blob/master/usr/src/lib/libc/port/gen/getenv.c#L411C1-L456C1)
This is reported at GitHUB #3018 and the reporter has proposed the patch, which
we really appreciate! But looking at his fix and to the implementations of
unsetenv() in FreeBSD libc and in Linux glibc 2.31, it seems, that the algorithm
of clean_env() will perform better with our my_unsetenv() implementation.
This should be backported in versions 3.1 and 3.2.
(cherry picked from commit
a9afc10ae8741ac34bc049856de46d2891b7be27)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
09ba9ac74bd85059233e553452ce12b3ba724649)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Thu, 26 Jun 2025 16:01:02 +0000 (18:01 +0200)]
SCRIPTS: drop the HTML generation from announce-release
It has not been used over the last 5 years or so and systematically
requires manual removal. Let's just stop producing it. Also take
this opportunity to add the missing link to /discussions.
(cherry picked from commit
27baa3f9ff7f50f3751aa541daaa22a4ca577b5a)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
eae27fc288eb2d33fea53d60cbc5362d3eaffc01)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Frederic Lecaille [Wed, 25 Jun 2025 08:15:50 +0000 (10:15 +0200)]
MINOR: quic: Useless TX buffer size reduction in closing state
There is no need to limit the size of the TX buffer to QUIC_MIN_CC_PKTSIZE bytes
when the connection is in closing state. There is already a test which limits the
number of bytes to be used from this TX buffer after this useless test removed.
It limits this number of bytes to the size of the TX buffer itself:
if (end > (unsigned char *)b_wrap(buf))
end = (unsigned char *)b_wrap(buf);
This is exactly what is needed when the connection is in closing state. Indeed,
the size of the TX buffers are limited to reduce the memory usage. The connection
only needs to send short datagrams with at most 2 packets with a CONNECTION_CLOSE*
frames. They are built only one time and backed up into small TX buffer allocated
from a dedicated pool.
The size of this TX buffer is QUIC_MAX_CC_BUFSIZE which depends on QUIC_MIN_CC_PKTSIZE:
#define QUIC_MIN_CC_PKTSIZE 128
#define QUIC_MAX_CC_BUFSIZE (2 * (QUIC_MIN_CC_PKTSIZE + QUIC_DGRAM_HEADLEN))
This size is smaller than an MTU.
This patch should be backported as far as 2.9 to ease further backports to come.
(cherry picked from commit
c898b29e6486ec17442555b2df4929c77684298f)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
1699cc9df385cbb3abe23547553fee2b47da490f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Frederic Lecaille [Mon, 23 Jun 2025 14:52:09 +0000 (16:52 +0200)]
BUG/MINOR: quic: wrong QUIC_FT_CONNECTION_CLOSE(0x1c) frame encoding
This is an old bug which was there since this commit:
MINOR: quic: Avoid zeroing frame structures
It seems QUIC_FT_CONNECTION_CLOSE was confused with QUIC_FT_CONNECTION_CLOSE_APP
which does not include a "frame type" field. This field was not initialized
(so with a random value) which prevent the packet to be built because the
packet builder supposes the packet with such frames are very short.
Must be backported as far as 2.6.
(cherry picked from commit
1e6d8f199c9943eca13dec48e8676377356c81d0)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
1f73ddeb5c49ad6e742efb7deca305798dc43896)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Wed, 25 Jun 2025 12:41:45 +0000 (14:41 +0200)]
DOC: configuration: add details on prefer-client-ciphers
prefer-client-ciphers does not work exactly the same way when used with
a dual algorithm stack (ECDSA + RSA). This patch details its behavior.
This patch must be backported in every maintained version.
Problem was discovered in #2988.
(cherry picked from commit
370a8cea4a2680cf27d5be61163bada27d541347)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
5000d32a2488a47cf817bebcf023312510d0cddc)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Mon, 23 Jun 2025 05:50:01 +0000 (07:50 +0200)]
BUG/MINOR: log: Be able to use %ID alias at anytime of the stream's evaluation
In a log-format string, using "%[unique-id]" or "%ID" should be equivalent.
However, for the first one, the unique ID is generated when the sample fetch
function is called. For the alias, it is not true. It that case, the
stream's unique ID is generated when the log message is emitted. Otherwise,
by default, the unique id is automatically generated at the end of the HTTP
request analysis.
So, if the alias "%ID" is use in a log-format string anywhere before the end
of the request analysis, the evaluation failed and the ID is considered as
empty. It is not consistent and in contradiction with the "%ID"
documentation.
To fix the issue, instead of evaluating the unique ID when the log message
is emitted, it is now performed on demand when "%ID" format is evaluated.
This patch should fix the issue #3016. It should be backported to all stable
versions. It relies on the following commit:
* BUG/MINOR: stream: Avoid recursive evaluation for unique-id based on itself
(cherry picked from commit
20a82027ceb7a46ce2a0cbe05e40c63d132601f7)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
609d9447e2ecfe6bc4854aa2c3ee9154a97b2710)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Mon, 23 Jun 2025 05:33:06 +0000 (07:33 +0200)]
BUG/MINOR: stream: Avoid recursive evaluation for unique-id based on itself
There is nothing that prevent a "unique-id-format" to reference itself,
using '%ID' or '%[unique-id]'. If the sample fetch function is used, it
leads to an infinite loop, calling recursively the function responsible to
generate the unique ID.
One solution is to detect it during the configuration parsing to trigger an
error. With this patch, we just inhibit recursive calls by considering the
unique-id as empty during its evaluation. So "id-%[unique-id]" lf string
will be evaluated as "id-".
This patch must be backported to all stable versions.
(cherry picked from commit
fb7b5c8a53cb4f19a223abd20660d47162aa8708)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
dfbb00ac1410af65d2befe3e94afb6cfc137b220)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Amaury Denoyelle [Wed, 18 Jun 2025 13:12:31 +0000 (15:12 +0200)]
BUG/MINOR: mux-quic/h3: properly handle too low peer fctl initial stream
Previously, no check on peer flow-control was implemented prior to open
a local QUIC stream. This was a small problem for frontend
implementation, as in this case haproxy as a server never opens
bidirectional streams.
On frontend, the only stream opened by haproxy in this case is for
HTTP/3 control unidirectional data. If the peer uses an initial value
for max uni streams set to 0, it would violate its flow control, and the
peer will probably close the connection. Note however that RFC 9114
mandates that each peer defines minimal initial value so that at least
the control stream can be created.
This commit improves the situation of too low initial max uni streams
value. Now, on HTTP/3 layer initialization, haproxy preemptively checks
flow control limit on streams via a new function
qcc_fctl_avail_streams(). If credit is already expired due to a too
small initial value, haproxy preemptively closes the connection using
H3_ERR_GENERAL_PROTOCOL_ERROR. This behavior is better as haproxy is now
the initiator of the connection closure.
This should be backported up to 2.8.
(cherry picked from commit
805a070ab920d14b22a6b7beac3b0648e684b2d2)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
47a4954c3974b85fc1b30ea723bf1d809e66e630)
[cf: context adjustment]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Valentine Krasnobaeva [Tue, 17 Jun 2025 13:33:12 +0000 (13:33 +0000)]
DOC: config: prefer-last-server: add notes for non-deterministic algorithms
Add some notes which load-balancing algorithm can be considered as
deterministic or non-deterministic and add some examples for each type.
This was asked via mailing list to clarify the usage of
prefer-last-server option.
This can be backported to all stable versions.
(cherry picked from commit
cdb2f8d780a27778cc23669693a9910e07135e8a)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
d6ca14b764039ed1c5d21d2fa83706996fd0a717)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Aurelien DARRAGON [Fri, 1 Aug 2025 13:33:56 +0000 (15:33 +0200)]
BUG/MEDIUM: hlua_fcn: ensure systematic watcher cleanup for server list iterator
In 358166a ("BUG/MINOR: hlua_fcn: restore server pairs iterator pointer
consistency"), I wrongly assumed that because the iterator was a temporary
object, no specific cleanup was needed for the watcher.
In fact watcher_detach() is not only relevant for the watcher itself, but
especially for its parent list to remove the current watcher from it.
As iterators are temporary objects, failing to remove their watchers from
the server watcher list causes the server watcher list to be corrupted.
On a normal iteration sequence, the last watcher_next() receives NULL
as target so it successfully detaches the last watcher from the list.
However the corner case here is with interrupted iterators: users are
free to break away from the iteration loop when a specific condition is
met for instance from the lua script, when this happens
hlua_listable_servers_pairs_iterator() doesn't get a chance to detach the
last iterator.
Also, Lua doesn't tell us that the loop was interrupted,
so to fix the issue we rely on the garbage collector to force a last
detach right before the object is freed. To achieve that, watcher_detach()
was slightly modified so that it becomes possible to call it without
knowing if the watcher is already detached or not, if watcher_detach() is
called on a detached watcher, the function does nothing. This way it saves
the caller from having to track the watcher state and makes the API a
little more convenient to use. This way we now systematically call
watcher_detach() for server iterators right before they are garbage
collected.
This was first reported in GH #3055. It can be observed when the server
list is browsed one than more time when it was already browsed from Lua
for a given proxy and the iteration was interrupted before the end. As the
watcher list is corrupted, the common symptom is watcher_attach() or
watcher_next() not ending due to the internal mt_list call looping
forever.
Thanks to GH users @sabretus and @sabretus for their precious help.
It should be backported everywhere 358166a was.
(cherry picked from commit
aeff2a3b2a87ae5572e73944eb56e732bb98295f)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
(cherry picked from commit
97bb595f25b56bc6d8b68d75a6251a53fa86c4e1)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
William Lallemand [Thu, 12 Jun 2025 14:50:08 +0000 (16:50 +0200)]
BUG/MEDIUM: ssl/clienthello: ECDSA with ssl-max-ver TLSv1.2 and no ECDSA ciphers
Patch
23093c72 ("BUG/MINOR: ssl: suboptimal certificate selection with TLSv1.3
and dual ECDSA/RSA") introduced a problem when prioritizing the ECDSA
with TLSv1.3.
Indeed, when a client with TLSv1.3 capabilities announce a list of
ECDSA sigalgs, a list of TLSv1.3 ciphersuites compatible with ECDSA,
but only RSA ciphers for TLSv1.2, and haproxy is configured to a
ssl-max-ver TLSv1.2, then haproxy would use the ECDSA keypair, but the
client wouldn't be able to process it because TLSv1.2 was negociated.
HAProxy would be configured like that:
ssl-default-bind-options ssl-max-ver TLSv1.2
And a client could be used this way:
openssl s_client -connect localhost:8443 -cipher ECDHE-ECDSA-AES128-GCM-SHA256 \
-ciphersuites TLS_AES_256_GCM_SHA384:TLS_CHACHA20_POLY1305_SHA256:TLS_AES_128_GCM_SHA256
This patch fixes the issue by checking if TLSv1.3 was configured before
allowing ECDSA is an TLSv1.3 ciphersuite is in the list.
This could be backported where
23093c72 ("BUG/MINOR: ssl: suboptimal
certificate selection with TLSv1.3 and dual ECDSA/RSA") was backported.
However this is quite sensible and we should wait a bit before the
backport.
This should fix issue #2988
(cherry picked from commit
4a298c6c5c64ecbbc8df1351df4b410216f95828)
Signed-off-by: William Lallemand <wlallemand@haproxy.com>
(cherry picked from commit
b552780290616a66ed9eb4247250c7239d159a90)
Signed-off-by: William Lallemand <wlallemand@haproxy.com>
Christopher Faulet [Mon, 16 Jun 2025 14:33:04 +0000 (16:33 +0200)]
BUG/MEDIUM: check: Set SOCKERR by default when a connection error is reported
When a connection error is reported, we try to collect as much information
as possible on the connection status and the server status is adjusted
accordingly. However, the function does nothing if there is no connection
error and if the healthcheck is not expired yet. It is a problem when an
internal error occurred. It may happen at many places and it is hard to be
sure an error is reported on the connection. And in fact, it is already a
problem when the multiplexer allocation fails. In that case, the healthcheck
is not interrupted as it should be. Concretely, it could only happen when a
connection is established.
It is hard to predict the effects of this bug. It may be unimportant. But it
could probably lead to a crash. To avoid any issue, a SOCKERR status is now
set by default when a connection error is reported. There is no reason to
report a connection error for nothing. So a healthcheck failure must be
reported. There is no "internal error" status. So a socket error is
reported.
This patch must be backport to all stable versions.
(cherry picked from commit
54d74259e9860c7017903221f27525be89983a75)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
18542f3a8b2c42598ca1e5061507c8393bc3ce2d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Mon, 16 Jun 2025 14:29:07 +0000 (16:29 +0200)]
MINOR: cli: handle EOS/ERROR first
It is not especially a bug fixed. But APPCTX_FL_EOS and APPCTX_FL_ERROR
flags must be handled first. These flags are set by the applet itself and
should mark the end of all processing. So there is not reason to get the
output buffer in first place.
This patch could be backported as far as 3.0.
(cherry picked from commit
fb7665552668df368bbc85ef56beca11c356f53a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
9a874dc54c3a6eb061f19d0a65e882d17d4b1a3e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Mon, 16 Jun 2025 13:48:04 +0000 (15:48 +0200)]
BUG/MEDIUM: cli: Don't consume data if outbuf is full or not available
The output buffer must be available to process a command, at least to be
able to emit error messages. When this buffer is full or cannot be
allocated, we must wait. In that case, we must take care to notify the SE
will not consume input data. It is important to avoid wakeup in loop,
especially when the client aborts.
When the output buffer is available again and no longer full, and the CLI
applet is waiting for a command line, it must notify it will consume input
data.
This patch must be backported as far as 3.0.
(cherry picked from commit
396f0252bf3a400c9dbfdaf730e994bb44f198dd)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
de668f5eb86f0370357042badc07a90e5d565576)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 13 Jun 2025 09:19:50 +0000 (11:19 +0200)]
BUG/MINOR: http-ana: Properly handle keep-query redirect option if no QS
The keep-query redirect option must do nothing is there is no query-string.
However, there is a bug. When there is no QS, an error is returned, leading
to return a 500-internal-error to the client.
To fix the bug, instead of returning 0 when there is no QS, we just skip the
QS processing.
This patch should fix the issue #3005. It must be backported as far as 3.1.
(cherry picked from commit
edb8f2bb60d11cc92895e6cf1cb05e5b6cdc51d7)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
c7fc5c4741e70322cb97bb1d913ff6db86ab0ab2)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Amaury Denoyelle [Wed, 11 Jun 2025 16:26:10 +0000 (18:26 +0200)]
BUG/MINOR: config/server: reject QUIC addresses
QUIC is not implemented on the backend side. To prevent any issue, it is
better to reject any server configured which uses it. This is done via
_srv_parse_init() which is used both for static and dynamic servers.
This should be backported up to all stable versions.
(cherry picked from commit
1ecf2e9babe9d2505cebfc9e0f64454be66c2905)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
4b2ae7c9c644b5b27af732dc15df3d2fc7558693)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Tue, 10 Jun 2025 17:03:44 +0000 (19:03 +0200)]
BUG/MINIR: h1: Fix doc of 'accept-unsafe-...-request' about URI parsing
The description of tests performed on the URI in H1 when
'accept-unsafe-violations-in-http-request' option is wrong. It states that
only characters below 32 and 127 are blocked when this option is set,
suggesting that otherwise, when it is not set, all invalid characters in the
URI, according to the RFC3986, are blocked.
But in fact, it is not true. By default all character below 32 and above 127
are blocked. And when 'accept-unsafe-violations-in-http-request' option is
set, characters above 127 (excluded) are accepted. But characters in
(33..126) are never checked, independently of this option.
This patch should fix the issue #2906. It should be backported as far as
3.0. For older versions, the docuementation could also be clarified because
this part is not really clear.
Note the request URI validation is still under discution because invalid
characters in (33.126) are never checked and some users request a stricter
parsing.
(cherry picked from commit
b2f64af3413e623aff2f2413cbbef6ac27f44f21)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
3b01e537edd2b50c449ede6cb312caa45786ca35)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Olivier Houchard [Tue, 10 Jun 2025 12:39:22 +0000 (12:39 +0000)]
BUG/MEDIUM: fd: Use the provided tgid in fd_insert() to get tgroup_info
In fd_insert(), use the provided tgid to ghet the thread group info,
instead of using the one of the current thread, as we may call
fd_insert() from a thread of another thread group, that will happen at
least when binding the listeners. Otherwise we'd end up accessing the
thread mask containing enabled thread of the wrong thread group, which
can lead to crashes if we're binding on threads not present in the
thread group.
This should fix Github issue #2991.
This should be backported up to 2.8.
(cherry picked from commit
6993981cd6e81448cd6a21ca32f21f2b548aa1b3)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
12989a221bdf829a36f73d25ac947e95af7af59a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Frederic Lecaille [Wed, 4 Jun 2025 09:49:14 +0000 (11:49 +0200)]
BUG/MINOR: quic: Missing SSL session object freeing
qc_alloc_ssl_sock_ctx() allocates an SSL_CTX object for each connection. It also
allocates an SSL object. When this function failed, it freed only the SSL_CTX object.
The correct way to free both of them is to call qc_free_ssl_sock_ctx().
Must be backported as far as 2.6.
(cherry picked from commit
6b746330692380f7e88fc757b7124b5a9c88ebd8)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
aaef9e0000fb2fc6011d9a95e6e7f5241863db8f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Wed, 4 Jun 2025 06:48:48 +0000 (08:48 +0200)]
BUG/MINOR: mux-spop: Fix null-pointer deref on SPOP stream allocation failure
When we try to allocate a new SPOP stream, if an error is encountered,
spop_strm_destroy() is called to released the eventually allocated
stream. But, it must only be called if a stream was allocated. If the
reported error is an SPOP stream allocation failure, we must just leave to
avoid null-pointer dereference.
This patch should fix point 1 of the issue #2993. It must be backported as
far as 3.1.
(cherry picked from commit
8c4bb8cab37f72c451bc7685eaf58cb1c2f5fae2)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
f4f45adb3f98570d817c6e63b662001ca95292d3)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Tue, 3 Jun 2025 12:50:38 +0000 (14:50 +0200)]
BUG/MEDIUM: check: Requeue healthchecks on I/O events to handle check timeout
When a healthchecks is processed, once the first wakeup passed to start the
check, and as long as the expiration timer is not reached, only I/O events
are able to wake it up. It is an issue when there is a check timeout
defined. Especially if the connect timeout is high and the check timeout is
low. In that case, the healthcheck's task is never requeue to handle any
timeout update. When the connection is established, the check timeout is set
to replace the connect timeout. It is thus possible to report a success
while a timeout should be reported.
So, now, when an I/O event is handled, the healthcheck is requeue, except if
an success or an abort is reported.
Thanks to Thierry Fournier for report and the reproducer.
This patch must be backported to all stable versions.
(cherry picked from commit
7c788f0984623f727a71ae4aee9917ddeac1b59d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
73b733e2e0bf6d6e9914c3b58a632f2b4dcb2d8d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Mon, 2 Jun 2025 07:18:08 +0000 (09:18 +0200)]
DOC: config: Fix a typo in 2.7 (Name format for maps and ACLs)
"identified" was used instead of "identifier". May be backported as far as
3.0
(cherry picked from commit
8e8cdf114b1961b008fd108dbe6b6c4aa0ce2b68)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
81e566388907ca03c56ee30ad745ddd93ce04a97)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Fri, 30 May 2025 15:13:21 +0000 (17:13 +0200)]
BUILD: tools: properly define ha_dump_backtrace() to avoid a build warning
In resolve_sym_name() we declare a few symbols that we want to be able
to resolve. ha_dump_backtrace() was declared with a struct buffer instead
of a pointer to such a struct, which has no effect since we only want to
get the function's pointer, but produces a build warning with LTO, so
let's fix it.
This can be backported to 3.0.
(cherry picked from commit
b88164d9c0eb1540c9b787478162c254ac947e8d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
6330c20026e79f54a6ead77d6c9573945a074744)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Fri, 16 May 2025 12:58:52 +0000 (14:58 +0200)]
BUG/MEDIUM: h1/h2/h3: reject forbidden chars in the Host header field
In continuation with
9a05c1f574 ("BUG/MEDIUM: h2/h3: reject some
forbidden chars in :authority before reassembly") and the discussion
in issue #2941, @DemiMarie rightfully suggested that Host should also
be sanitized, because it is sometimes used in concatenation, such as
this:
http-request set-url https://%[req.hdr(host)]%[pathq]
which was proposed as a workaround for h2 upstream servers that require
:authority here:
https://www.mail-archive.com/haproxy@formilux.org/msg43261.html
The current patch then adds the same check for forbidden chars in the
Host header, using the same function as for the patch above, since in
both cases we validate the host:port part of the authority. This way
we won't reconstruct ambiguous URIs by concatenating Host and path.
Just like the patch above, this can be backported afer a period of
observation.
(cherry picked from commit
df00164fdd98d15e832daad34fb23249083bfb9c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Mon, 12 May 2025 15:45:33 +0000 (17:45 +0200)]
BUG/MEDIUM: h2/h3: reject some forbidden chars in :authority before reassembly
As discussed here:
https://github.com/httpwg/http2-spec/pull/936
https://github.com/haproxy/haproxy/issues/2941
It's important to take care of some special characters in the :authority
pseudo header before reassembling a complete URI, because after assembly
it's too late (e.g. the '/'). This patch does this, both for h2 and h3.
The impact on H2 was measured in the worst case at 0.3% of the request
rate, while the impact on H3 is around 1%, but H3 was about 1% faster
than H2 before and is now on par.
It may be backported after a period of observation, and in this case it
relies on this previous commit:
MINOR: http: add a function to validate characters of :authority
Thanks to @DemiMarie for reviving this topic in issue #2941 and bringing
new potential interesting cases.
(cherry picked from commit
9a05c1f57490ba3adb378ad8e6e26830425514e7)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Mon, 12 May 2025 15:39:08 +0000 (17:39 +0200)]
MINOR: http: add a function to validate characters of :authority
As discussed here:
https://github.com/httpwg/http2-spec/pull/936
https://github.com/haproxy/haproxy/issues/2941
It's important to take care of some special characters in the :authority
pseudo header before reassembling a complete URI, because after assembly
it's too late (e.g. the '/').
This patch adds a specific function which was checks all such characters
and their ranges on an ist, and benefits from modern compilers
optimizations that arrange the comparisons into an evaluation tree for
faster match. That's the version that gave the most consistent performance
across various compilers, though some hand-crafted versions using bitmaps
stored in register could be slightly faster but super sensitive to code
ordering, suggesting that the results might vary with future compilers.
This one takes on average 1.2ns per character at 3 GHz (3.6 cycles per
char on avg). The resulting impact on H2 request processing time (small
requests) was measured around 0.3%, from 6.60 to 6.618us per request,
which is a bit high but remains acceptable given that the test only
focused on req rate.
The code was made usable both for H2 and H3.
(cherry picked from commit
ebab479cdf34255cd6162d2e843645f88b95327f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Mon, 2 Jun 2025 13:58:52 +0000 (15:58 +0200)]
[RELEASE] Released version 3.1.8
Released version 3.1.8 with the following main changes :
- BUG/MINOR: quic: do not crash on CRYPTO ncbuf alloc failure
- BUG/MINOR: cli: Issue an error when too many args are passed for a command
- BUG/MAJOR: listeners: transfer connection accounting when switching listeners
- MINOR: applet: add appctx_schedule() macro
- CLEANUP: dns: remove unused dns_stream_server struct member
- BUG/MINOR: dns: add tempo between 2 connection attempts for dns servers
- BUG/MINOR: dns: prevent ds accumulation within dss
- BUG/MINOR: mux-h1: Don't pretend connection was released for TCP>H1>H2 upgrade
- BUG/MINOR: mux-h1: Fix trace message in h1_detroy() to not relay on connection
- BUG/MEDIUM: connections: Report connection closing in conn_create_mux()
- BUG/MINOR: proxy: only use proxy_inc_fe_cum_sess_ver_ctr() with frontends
- MINOR: quic: rename min/max fields for congestion window algo
- MINOR: quic: extend return value during TP parsing
- BUG/MINOR: quic: use proper error code on missing CID in TPs
- BUG/MINOR: quic: use proper error code on invalid server TP
- BUG/MINOR: quic: reject retry_source_cid TP on server side
- BUG/MINOR: quic: use proper error code on invalid received TP value
- BUG/MINOR: quic: fix TP reject on invalid max-ack-delay
- BUG/MINOR: quic: reject invalid max_udp_payload size
- BUG/MEDIUM: peers: hold the refcnt until updating ts->seen
- BUG/MINOR: cli: fix too many args detection for commands
- BUG/MEDIUM: quic: free stream_desc on all data acked
- BUG/MINOR: ssl/ckch: always free() the previous entry during parsing
- BUG/MINOR: threads: fix soft-stop without multithreading support
- BUG/MINOR: hlua: Fix Channel:data() and Channel:line() to respect documentation
- BUG/MINOR: spoe: Don't report error on applet release if filter is in DONE state
- BUG/MEDIUM: mux-spop: Respect the negociated max-frame-size value to send frames
- BUG/MEDIUM: mux-spop: Wait end of handshake to declare a spop connection ready
- BUG/MINOR: mux-spop: Use the right bitwise operator in spop_ctl()
- BUG/MINOR: mux-spop: Don't report error for stream if ACK was already received
- BUG/MINOR: mux-spop: Make the demux stream ID a signed integer
- BUG/MINOR: mux-spop: Don't open new streams for SPOP connection on error
- MINOR: mux-spop: Don't set SPOP connection state to FRAME_H after ACK parsing
- BUG/MEDIUM: mux-spop: Remove frame parsing states from the SPOP connection state
- BUG/MEDIUM: mux-spop: Properly handle CLOSING state
- BUG/MEDIUM: spop-conn: Report short read for partial frames payload
- BUG/MEDIUM: mux-spop; Don't report a read error if there are pending data
- DEBUG: mux-spop: Review some trace messages to adjust the message or the level
- BUG/MINOR: sink: detect and warn when using "send-proxy" options with ring servers
- BUG/MEDIUM: peers: also limit the number of incoming updates
- DOC: ring: refer to newer RFC5424
- DOC: config: restore default values for resolvers hold directive
- DOC: config: recommend disabling libc-based resolution with resolvers
- MEDIUM: stick-tables: Limit the number of old entries we remove
- BUG/MINOR: mux-quic: do not decode if conn in error
- MINOR: quic: refactor BBR API
- BUG/MINOR: quic: ensure cwnd limits are always enforced
- BUG/MEDIUM: mux-spop: Properly detect truncated frames on demux to report error
- DOC: configuration: explicit multi-choice on bind shards option
- BUG/MINOR: h3: don't insert more than one Host header
- BUILD: debug: mark ha_crash_now() as attribute(noreturn)
- CLEANUP: quic: Useless BIO_METHOD initialization
- MINOR: quic: Add useful error traces about qc_ssl_sess_init() failures
- BUG/MEDIUM: wdt: always ignore the first watchdog wakeup
- MEDIUM: hlua: Add function to change the body length of an HTTP Message
- BUG/MEDIUM: stconn: Disable 0-copy forwarding for filters altering the payload
- BUG/MEDIUM: server: fix crash after duplicate GUID insertion
- BUG/MEDIUM: server: fix potential null-deref after previous fix
- BUG/MAJOR: cache: Crash because of wrong cache entry deleted
- DOC: configuration: fix the example in crt-store
- BUG/MINOR: h3: Set HTX flags corresponding to the scheme found in the request
- BUG/MEDIUM: hlua: Properly detect shudowns for TCP applets based on the new API
- BUG/MEDIUM: hlua: Fix getline() for TCP applets to work with applet's buffers
- REGTESTS: Make the script testing conditional set-var compatible with Vtest2
- REGTESTS: Explicitly allow failing shell commands in some scripts
- CI: vtest: Rely on VTest2 to run regression tests
- CI: vtest: Fix the build script to properly work on MaOS
- BUG/MEDIUM: httpclient: Throw an error if an lua httpclient instance is reused
- DOC: hlua: Add a note to warn user about httpclient object reuse
- DOC: hlua: fix a few typos in HTTPMessage.set_body_len() documentation
Willy Tarreau [Tue, 27 May 2025 17:31:12 +0000 (19:31 +0200)]
DOC: hlua: fix a few typos in HTTPMessage.set_body_len() documentation
A few typos were noticed while gathering info for the 3.2 announce
messages, this fixes them, and will probably constitute the last
commit of this release. There's no need to backport it unless commit
94055a5e7 ("MEDIUM: hlua: Add function to change the body length of
an HTTP Message") is backported.
(cherry picked from commit
21ce685fcdc2931cf6f5aafbf1f7cb68c9c7a1a1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Tue, 27 May 2025 16:48:21 +0000 (18:48 +0200)]
DOC: hlua: Add a note to warn user about httpclient object reuse
It is not supported to reuse an lua httpclient instance to process several
requests. A new object must be created for each request. Thanks to the
previous patch ("BUG/MEDIUM: httpclient: Throw an error if an lua httpclient
instance is reused"), an error is now reported if this happens. But it is
not obvious for users. So the lua-api docuementation was updated accordingly.
This patch is related to issue #2986. It should be backported with the
commit above.
(cherry picked from commit
cb7a2444d149de8e0ce89d7b4a450e5c36822412)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Tue, 27 May 2025 16:35:30 +0000 (18:35 +0200)]
BUG/MEDIUM: httpclient: Throw an error if an lua httpclient instance is reused
It is not expected/supported to reuse an httpclient instance to process
several requests. A new instance must be created for each request. However,
in lua, there is nothing to prevent a user to create an httpclient object
and use it in a loop to process requests.
That's unfortunate because this will apparently work, the requests will be
sent and a response will be received and processed. However internally some
ressources will be allocated and never released. When the next response is
processed, the ressources allocated for the previous one are definitively
lost.
In this patch we take care to check that the httpclient object was never
used when a request is sent from a lua script by checking
HTTPCLIENT_FS_STARTED flags. This flag is set when a httpclient applet is
spawned to process a request and never removed after that. In lua, the
httpclient applet is created when the request is sent. So, it is the right
place to do this test.
This patch should fix the issue #2986. It should be backported as far as
2.6.
(cherry picked from commit
50fca6f0b7ebee94b5ebe402e37af9d7d945fe64)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Tue, 27 May 2025 12:48:48 +0000 (14:48 +0200)]
CI: vtest: Fix the build script to properly work on MaOS
"config.h" header file is new in VTest2 and includes must be adapted to be
able to build VTest on MacOS. Let's add "-I." to make it work.
(cherry picked from commit
508e074a320cab94997b1e3a2a6a3c6c2d0501ac)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Tue, 27 May 2025 12:32:34 +0000 (14:32 +0200)]
CI: vtest: Rely on VTest2 to run regression tests
VTest2 (https://github.com/vtest/VTest2) was released and is a remplacement
for VTest. VTest was archived. So let's use the new version now.
If this commit is backported, the 2 following commits must also be
backported:
*
2808e3577 ("REGTESTS: Explicitly allow failing shell commands in some scripts")
*
82c291124 ("REGTESTS: Make the script testing conditional set-var compatible with Vtest2")
(cherry picked from commit
6a18d28ba20439a94ec38e6670e1b7b2a864bde8)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Mon, 26 May 2025 05:57:21 +0000 (07:57 +0200)]
REGTESTS: Explicitly allow failing shell commands in some scripts
Vtest2, that should replaced Vtest in few months, will reject any failing
commands in shell blocks. However, some scripts are executing some commands,
expecting an error to be able to parse the error output. So, now use "set
+e" in those scripts to explicitly state failing commads are expected.
It is just used for non-final commands. At the end, the shell block must
still report a success.
(cherry picked from commit
2808e3577fb72b693198c3f4e8f9284cb69675b3)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Mon, 26 May 2025 05:49:50 +0000 (07:49 +0200)]
REGTESTS: Make the script testing conditional set-var compatible with Vtest2
VTest2 will replaced VTest in few months. There is not so much change
expected. One of them is that a User-Agent header is added by default in all
requests, except if an custom one is already set or if "-nouseragent" option
is used. To still be compatible with VTest, it is not possible to use the
option to avoid the header addition. So, a custom user-agent is added in the
last test of "sample_fetches/cond_set_var.vtc" to be sure it will pass with
Vtest and Vtest2. It is mandatory because the request length is tested.
(cherry picked from commit
82c291124882d75625122e7f81b965fa2853e720)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Tue, 27 May 2025 05:46:44 +0000 (07:46 +0200)]
BUG/MEDIUM: hlua: Fix getline() for TCP applets to work with applet's buffers
The commit
e5e36ce09 ("BUG/MEDIUM: hlua/cli: Fix lua CLI commands to work
with applet's buffers") fixed the TCP applets API to work with applets using
its own buffers. Howver the getline() function was not updated. It could be
an issue for anyone registering a CLI commands reading lines.
This patch should be backported as far as 3.0.
(cherry picked from commit
c0ecef71d7ceaf404bc18241bdb878a4e72033b4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Mon, 26 May 2025 16:24:53 +0000 (18:24 +0200)]
BUG/MEDIUM: hlua: Properly detect shudowns for TCP applets based on the new API
The internal function responsible to receive data for TCP applets with
internal buffers is buggy. Indeed, for these applets, the buffer API is used
to get data. So there is no tests on the SE to properly detect connection
shutdowns. So, it must be performed by hand after the call to b_getblk_nc().
This patch must be backported as far as 3.0.
(cherry picked from commit
c64781c2c8b307fba7499fc70102d4246e850240)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Mon, 26 May 2025 09:28:04 +0000 (11:28 +0200)]
BUG/MINOR: h3: Set HTX flags corresponding to the scheme found in the request
When a ":scheme" pseudo-header is found in a h3 request, the
HTX_SL_F_HAS_SCHM flag must be set on the HTX message. And if the scheme is
'http' or 'https', the corresponding HTX flag must also be set. So,
respectively, HTX_SL_F_SCHM_HTTP or HTX_SL_F_SCHM_HTTPS.
It is mainly used to send the right ":scheme" pseudo-header value to H2
server on backend side.
This patch could be backported as far as 2.6.
(cherry picked from commit
da9792cca85366e3da4d716c0eb4a6a8bb9c1e62)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Sun, 25 May 2025 14:52:00 +0000 (16:52 +0200)]
DOC: configuration: fix the example in crt-store
Fix a bad example in the crt-store section. site1 does not use the "web"
crt-store but the global one.
Must be backported as far as 3.0 however the section was 3.12 in
previous version.
(cherry picked from commit
d607940915b097cdbb85910a0fac50ffafe075e1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Remi Tricot-Le Breton [Fri, 23 May 2025 17:33:58 +0000 (19:33 +0200)]
BUG/MAJOR: cache: Crash because of wrong cache entry deleted
When "vary" is enabled, we can have multiple entries for a given primary
key in the cache tree. There is a limit to how many secondary entries
can be inserted for a given key. When we try to insert a new secondary
entry, if the limit is already reached, we can try to find expired
entries with the same primary key, and if the limit is still reached we
want to abort the current insertion and to remove the node that was just
inserted.
In commit "a29b073: MEDIUM: cache: Add refcount on cache_entry" though,
a regression was introduced. Instead of removing the entry just inserted
as the comments suggested, we removed the second to last entry and
returned NULL. We then reset the eb.key of the cache_entry in the caller
because we assumed that the entry was already removed from the tree.
This means that some entries with an empty key were wrongly kept in the
tree and the last secondary entry, which keeps the number of secondary
entries of a given key was removed.
This ended up causing some crashes later on when we tried to iterate
over the elements of this given key. The crash could occur in multiple
places, either when trying to retrieve an entry or to add some new ones.
This crash was raised in GitHub issue #2950.
The fix should be backported up to 3.0.
(cherry picked from commit
90441e9bfe89c8df5d319f794f2cc0ee6e3797e7)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Thu, 22 May 2025 16:09:12 +0000 (18:09 +0200)]
BUG/MEDIUM: server: fix potential null-deref after previous fix
A valid build warning was reported in the CI with latest commit
b40ce97ecc
("BUG/MEDIUM: server: fix crash after duplicate GUID insertion"). Indeed,
if the first test in the function fails, we branch to the err label
with guid==NULL and will crash there. Let's just test guid before
dereferencing it for freeing.
This needs to be backported to 3.0 as well since the commit above was
meant to go there.
(cherry picked from commit
28c7a22790a587c6a3ee1652188ad6786d59b687)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Amaury Denoyelle [Thu, 22 May 2025 15:48:58 +0000 (17:48 +0200)]
BUG/MEDIUM: server: fix crash after duplicate GUID insertion
On "add server", if a GUID is defined, guid_insert() is used to add the
entry into the global GUID tree. If a similar entry already exists, GUID
insertion fails and the server creation is eventually aborted.
A crash could occur in this case because of an invalid memory access via
guid_remove(). The latter is caused via free_server() as the server
insertion is rejected. The invalid occurs on GUID key.
The issue occurs because of guid_insert(). The function properly
deallocates the GUID key on duplicate insertion, but it failed to reset
<guid.node.key> to NULL. This caused the invalid memory access on
guid_remove(). To fix this, ensure that key member is properly resetted
on guid_insert() error path.
This must be backported up to 3.0.
(cherry picked from commit
b40ce97ecca0e43c45f6a15e3a7184436d5cb467)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 16 May 2025 12:19:52 +0000 (14:19 +0200)]
BUG/MEDIUM: stconn: Disable 0-copy forwarding for filters altering the payload
It is especially a problem with Lua filters, but it is important to disable
the 0-copy forwarding if a filter alters the payload, or at least to be able
to disable it. While the filter is registered on the data filtering, it is
not an issue (and it is the common case) because, there is now way to
fast-forward data at all. But it may be an issue if a filter decides to
alter the payload and to unregister from data filtering. In that case, the
0-copy forwarding can be re-enabled in a hardly precdictable state.
To fix the issue, a SC flags was added to do so. The HTTP compression filter
set it and lua filters too if the body length is changed (via
HTTPMessage.set_body_len()).
Note that it is an issue because of a bad design about the HTX. Many info
about the message are stored in the HTX structure itself. It must be
refactored to move several info to the stream-endpoint descriptor. This
should ease modifications at the stream level, from filter or a TCP/HTTP
rules.
This should be backported as far as 3.0. If necessary, it may be backported
on lower versions, as far as 2.6. In that case, it must be reviewed and
adapted.
(cherry picked from commit
f45a632bad6fbcdadeba41038103a5738f8d709d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 16 May 2025 12:10:31 +0000 (14:10 +0200)]
MEDIUM: hlua: Add function to change the body length of an HTTP Message
There was no function for a lua filter to change the body length of an HTTP
Message. But it is mandatory to be able to alter the message payload. It is
not possible update to directly update the message headers because the
internal state of the message must also be updated accordingly.
It is the purpose of HTTPMessage.set_body_len() function. The new body
length myst be passed as argument. If it is an integer, the right
"Content-Length" header is set. If the "chunked" string is used, it forces
the message to be chunked-encoded and in that case the "Transfer-Encoding"
header.
This patch should fix the issue #2837. It could be backported as far as 2.6.
(cherry picked from commit
94055a5e73e9feed8c0aed8cae751e1896be90bd)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Tue, 20 May 2025 13:52:44 +0000 (15:52 +0200)]
BUG/MEDIUM: wdt: always ignore the first watchdog wakeup
With commit
a06c215f08 ("MEDIUM: wdt: always make the faulty thread
report its own warnings"), when the TH_FL_STUCK flag was flipped on,
we'd then go to the panic code instead of giving a second chance like
before the commit. This can trigger rare cases that only happen with
moderate loads like was addressed by commit
24ce001771 ("BUG/MEDIUM:
wdt: fix the stuck detection for warnings"). This is in fact due to
the loss of the common "goto update_and_leave" that used to serve
both the warning code and the flag setting for probation, and it's
apparently what hit Christian in issue #2980.
Let's make sure we exit naturally when turning the bit on for the
first time. Let's also update the confusing comment at the end of
the check that was left over by latest change.
Since the first commit was backported to 3.1, this commit should be
backported there as well.
(cherry picked from commit
0a8bfb5b900f017c17bf6460669a18cf693b5927)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Frederic Lecaille [Thu, 15 May 2025 08:18:09 +0000 (10:18 +0200)]
MINOR: quic: Add useful error traces about qc_ssl_sess_init() failures
There were no traces to diagnose qc_ssl_sess_init() failures from QUIC traces.
This patch add calls to TRACE_DEVEL() into qc_ssl_sess_init() and its caller
(qc_alloc_ssl_sock_ctx()). This was useful at least to diagnose SSL context
initialization failures when porting QUIC to the new OpenSSL 3.5 QUIC API.
Should be easily backported as far as 2.6.
(cherry picked from commit
894595b711835dba9a7432fa364ea57806b2bb0c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Frederic Lecaille [Tue, 13 May 2025 14:15:51 +0000 (16:15 +0200)]
CLEANUP: quic: Useless BIO_METHOD initialization
This code is there from QUIC implementation start. It was supposed to
initialize <ha_quic_meth> as a BIO_METHOD static object. But this
BIO_METHOD is not used at all!
Should be backported as far as 2.6 to help integrate the next patches to come.
(cherry picked from commit
a2822b17769408ce8fd17a4db4d58e5a4c0a1454)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Fri, 16 May 2025 14:12:12 +0000 (16:12 +0200)]
BUILD: debug: mark ha_crash_now() as attribute(noreturn)
Building on MIPS64 with clang16 incorrectly reports some uninitialized
value warnings in stats-proxy.c due to some calls to ABORT_NOW() where
the compiler didn't know the code wouldn't return. Let's properly mark
the function as noreturn, and take this opportunity for also marking it
unused to avoid possible warnings depending on the build options (if
ABORT_NOW is not used). No backport needed though it will not harm.
(cherry picked from commit
0a91c6dcaebd41ead51f3654ca96d564ccda32af)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Fri, 16 May 2025 12:51:13 +0000 (14:51 +0200)]
BUG/MINOR: h3: don't insert more than one Host header
Let's make sure we drop extraneous Host headers after having compared
them. That also works when :authority was already present. This way,
like for h1 and h2, we only keep one copy of it, while still making
sure that Host matches :authority. This way, if a request has both
:authority and Host, only one Host header will be produced (from
:authority). Note that due to the different organization of the code
and wording along the evolving RFCs, here we also check that all
duplicates are identical, while h2 ignores them as per RFC7540, but
this will be re-unified later.
This should be backported to stable versions, at least 2.8, though
thanks to the existing checks the impact is probably nul.
(cherry picked from commit
b84762b3e0c0e7708ddc98ae6b721ed10dc1be30)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Basha Mougamadou [Wed, 14 May 2025 15:50:47 +0000 (15:50 +0000)]
DOC: configuration: explicit multi-choice on bind shards option
From the documentation, this wasn't clear enough that shards should
be followed by one of the options number / by-thread / by-group.
Align it with existing options in documentation so that it becomes
more explicit.
(cherry picked from commit
824bb93e18043876a0fa30eeb3de50ea0ccb4ea5)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Wed, 14 May 2025 07:22:45 +0000 (09:22 +0200)]
BUG/MEDIUM: mux-spop: Properly detect truncated frames on demux to report error
There was no test in the demux part to detect truncated frames and to report
an error at the connection level. The SPOP streams were properly switch to
half-closed state. But waiting the associated SPOE applets were woken up and
released, the SPOP connection could be woken up several times for nothing. I
never triggered the watchdog in that case, but it is not excluded.
Now, at the end of the demux function, if a specific test was added to
detect truncated frames to report an error and close the connection.
This patch must be backported to 3.1.
(cherry picked from commit
16314bb93c2c2c47c648da880ab55bf2ac01d9d4)
[wt: update ctx]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Amaury Denoyelle [Mon, 20 Jan 2025 15:24:21 +0000 (16:24 +0100)]
BUG/MINOR: quic: ensure cwnd limits are always enforced
Congestion window is limit by a minimal and maximum values which can
never be exceeded. Min value is hardcoded to 2 datagrams as recommended
by the specification. Max value is specified via haproxy configuration.
These values must be respected each time the congestion window size is
adjusted. However, in some rare occasions, limit were not always
enforced. Fix this by implementing wrappers to set or increment the
congestion window. These functions ensure limits are always applied
after the operation.
Additionnally, wrappers also ensure that if window reached a new maximum
value, it is saved in <cwnd_last_max> field.
This should be backported up to 2.6, after a brief period of
observation.
(cherry picked from commit
7bad88c35c7547d52ac170ed9f89f29cccd6c46c)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Amaury Denoyelle [Mon, 28 Apr 2025 07:22:37 +0000 (09:22 +0200)]
MINOR: quic: refactor BBR API
Write minor adjustments to QUIC BBR functions. The objective is to
centralize every modification of path cwnd field.
No functional change. This patch will be useful to simplify
implementation of global QUIC Tx memory usage limitation.
(cherry picked from commit
c01d4552881301093d982efbfb4734ba5d4c65b6)
[ad: pick to simplify backport of next commit]
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Amaury Denoyelle [Wed, 23 Apr 2025 15:06:22 +0000 (17:06 +0200)]
BUG/MINOR: mux-quic: do not decode if conn in error
Add an early return to qcc_decode_qcs() if QCC instance is flagged on
error and connection is scheduled for immediate closure.
The main objective is to ensure to not trigger BUG_ON() from
qcc_set_error() : if a stream decoding has set the connection error, do
not try to process decoding on other streams as they may also encounter
an error. Thus, the connection is closed asap with the first encountered
error case.
This should be backported up to 2.6, after a period of observation.
(cherry picked from commit
6c5030f703e29bfd8deeace111bcedc6835c7065)
[ad: context adjustement, due to multiple Rx bufs not available in 3.1]
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Olivier Houchard [Fri, 2 May 2025 12:05:37 +0000 (12:05 +0000)]
MEDIUM: stick-tables: Limit the number of old entries we remove
Limit the number of old entries we remove in one call of
stktable_trash_oldest(), as we do so while holding the heavily contended
update write lock, so we'd rather not hold it for too long.
This helps getting stick tables perform better under heavy load.
(cherry picked from commit
d2d4c3eb6566145d30eb38dc96b2b79d3f1db8fc)
[wt: backported since situation encountered in 3.1 as well without this
patch. It also contains the definition for STKTABLE_MAX_UPDATES_AT_ONCE
from the previous commit]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Fri, 9 May 2025 08:30:30 +0000 (10:30 +0200)]
DOC: config: recommend disabling libc-based resolution with resolvers
Using both libc and haproxy resolvers can lead to hard to diagnose issues
when their bevahiour diverges; recommend using only one type of resolver.
Should be backported to stable versions.
Link: https://www.mail-archive.com/haproxy@formilux.org/msg45663.html
Co-authored-by: Lukas Tribus <lukas@ltri.eu>
(cherry picked from commit
4e20fab7ac85c0018e77a52657f773982f8c1875)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Aurelien DARRAGON [Wed, 30 Apr 2025 14:56:00 +0000 (16:56 +0200)]
DOC: config: restore default values for resolvers hold directive
Default values for hold directive (resolver context) used to be documented
but this was lost when the keyword description was reworked in 24b319b
("Default value is 10s for "valid", 0s for "obsolete" and 30s for
others.")
Restoring the part that describes the default value.
It may be backported to all stable versions with 24b319b
(cherry picked from commit
4bceca83fc14269ef8f2528294bc3cdb3fcc6c99)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Lukas Tribus [Mon, 28 Apr 2025 12:07:31 +0000 (12:07 +0000)]
DOC: ring: refer to newer RFC5424
In the ring configuration example we refer to RFC3164 - the original BSD
syslog protocol without support for structured data (SDATA).
Let's refer to RFC5424 instead so SDATA is by default forwarded if
someone copy & pastes from the documentation:
https://discourse.haproxy.org/t/structured-data-lost-when-forwarding-logs-voa-syslog-forwarding-feature/11741/5
Should be backported to 2.6.
(cherry picked from commit
5f9ce99c799eaa8f58518201b58c32d970b10cdf)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Thu, 15 May 2025 13:41:50 +0000 (15:41 +0200)]
BUG/MEDIUM: peers: also limit the number of incoming updates
There's a configurable limit to the number of messages sent to a
peer (tune.peers.max-updates-at-once), but this one is not applied to
the receive side. While it can usually be OK with default settings,
setups involving a large tune.bufsize (1MB and above) regularly
experience high latencies and even watchdogs during reloads because
the full learning process sends a lot of data that manages to fill
the entire buffer, and due to the compactness of the protocol, 1MB
of buffer can contain more than 100k updates, meaning taking locks
etc during this time, which is not workable.
Let's make sure the receiving side also respects the max-updates-at-once
setting. For this it counts incoming updates, and refrains from
continuing once the limit is reached. It's a bit tricky to do because
after receiving updates we still have to send ours (and possibly some
ACKs) so we cannot just leave the loop.
This issue was reported on 3.1 but it should progressively be backported
to all versions having the max-updates-at-once option available.
(cherry picked from commit
b26f3fe54e0ca5d54cc4fbcd771bfefbe728991c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Aurelien DARRAGON [Thu, 15 May 2025 13:54:13 +0000 (15:54 +0200)]
BUG/MINOR: sink: detect and warn when using "send-proxy" options with ring servers
using "send-proxy" or "send-proxy-v2" option on a ring server is not
relevant nor supported. Worse, on 2.4 it causes haproxy process to
crash as reported in GH #2965.
Let's be more explicit about the fact that this keyword is not supported
under "ring" context by ignoring the option and emitting a warning message
to inform the user about that.
Ideally, we should do the same for peers and log servers. The proper way
would be to check servers options during postparsing but we currently lack
proper cross-type server postparsing hooks. This will come later and thus
will give us a chance to perform the compatibilty checks for server
options depending on proxy type. But for now let's simply fix the "ring"
case since it is the only one that's known to cause a crash.
It may be backported to all stable versions.
(cherry picked from commit
098a5e5c0b9e6821c03945295dd1350e5d834eb3)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Wed, 14 May 2025 07:39:03 +0000 (09:39 +0200)]
DEBUG: mux-spop: Review some trace messages to adjust the message or the level
Some trace messages were not really accurrate, reporting a CLOSED connection
while only an error was reported on it. In addition, an TRACE_ERROR() was
used to report a short read on HELLO/DISCONNECT frames header. But it is not
an error. a TRACE_DEVEL() should be used instead.
This patch could be backported to 3.1 to ease future backports.
(cherry picked from commit
e2ae8a74e87d32730e3edba26626518fd33e3a58)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Wed, 14 May 2025 07:33:46 +0000 (09:33 +0200)]
BUG/MEDIUM: mux-spop; Don't report a read error if there are pending data
When an read error is detected, no error must be reported on the SPOP
connection is there are still some data to parse. It is important to be sure
to process all data before reporting the error and be sure to not truncate
received frames. However, we must also take care to handle short read case
to not wait data that will never be received.
This patch must be backported to 3.1.
(cherry picked from commit
6e46f0bf934360186aa878d75f8bf81eaae0c141)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Wed, 14 May 2025 07:20:08 +0000 (09:20 +0200)]
BUG/MEDIUM: spop-conn: Report short read for partial frames payload
When a frame was not fully received, a short read must be reported on the
SPOP connection to help the demux to handle truncated frames. This was
performed for frames truncated on the header part but not on the payload
part. It is now properly detected.
This patch must be backported to 3.1.
(cherry picked from commit
71feb49a9f0ef1142970fca98ca8eebd516928a3)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Tue, 13 May 2025 17:04:01 +0000 (19:04 +0200)]
BUG/MEDIUM: mux-spop: Properly handle CLOSING state
The CLOSING state was not handled at all by the SPOP multiplexer while it is
mandatory when a DISCONNECT frame was sent and the mux should wait for the
DISCONNECT frame in reply from the agent. Thanks to this patch, it should be
fixed.
In addition, if an error occurres during the AGENT HELLO frame parsing, the
SPOP connection is no longer switched to CLOSED state and remains in ERROR
state instead. It is important to be able to send the DISCONNECT frame to
the agent instead of closing the TCP connection immediately.
This patch depends on following commits:
* BUG/MEDIUM: mux-spop: Remove frame parsing states from the SPOP connection state
* MINOR: mux-spop: Don't set SPOP connection state to FRAME_H after ACK parsing
* BUG/MINOR: mux-spop: Don't open new streams for SPOP connection on error
* BUG/MINOR: mux-spop: Make the demux stream ID a signed integer
All the series must be backported to 3.1.
(cherry picked from commit
ddc5f8d92e3613ec7060b0ee8bdb220866182e86)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Tue, 13 May 2025 16:55:32 +0000 (18:55 +0200)]
BUG/MEDIUM: mux-spop: Remove frame parsing states from the SPOP connection state
SPOP_CS_FRAME_H and SPOP_CS_FRAME_P states, that were used to handle frame
parsing, were removed. The demux process now relies on the demux stream ID
to know if it is waiting for the frame header or the frame
payload. Concretly, when the demux stream ID is not set (dsi == -1), the
demuxer is waiting for the next frame header. Otherwise (dsi >= 0), it is
waiting for the frame payload. It is especially important to be able to
properly handle DISCONNECT frames sent by the agents.
SPOP_CS_RUNNING state is introduced to know the hello handshake was finished
and the SPOP connection is able to open SPOP streams and exchange NOTIFY/ACK
frames with the agents.
It depends on the following fixes:
* MINOR: mux-spop: Don't set SPOP connection state to FRAME_H after ACK parsing
* BUG/MINOR: mux-spop: Make the demux stream ID a signed integer
This change will be mandatory for the next fix. It must be backported to 3.1
with the commits above.
(cherry picked from commit
a3940614c24049d2c1ee9f686e0579e3a2e49b33)
[wt: adj ctx WRT tevt which is absent from 3.1
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Tue, 13 May 2025 16:41:09 +0000 (18:41 +0200)]
MINOR: mux-spop: Don't set SPOP connection state to FRAME_H after ACK parsing
After the ACK frame was parsed, it is useless to set the SPOP connection
state to SPOP_CS_FRAME_H state because this will be automatically handled by
the demux function. If it is not an issue, but this will simplify changes
for the next commit.
(cherry picked from commit
6b0f7de4e34c7da1ec7ae82e8e8a298823e50d72)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Tue, 13 May 2025 16:35:29 +0000 (18:35 +0200)]
BUG/MINOR: mux-spop: Don't open new streams for SPOP connection on error
Till now, only SPOP connections fully closed or those with a TCP connection on
error were concerned. But available streams could be reported for SPOP
connections in error or closing state. But in these states, no NOTIFY frames
will be sent and no ACK frames will be parsed. So, no new SPOP streams should be
opened.
This patch should be backported to 3.1.
(cherry picked from commit
197eaaadfd636441ca6cc5d3e35c52197537bbd6)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Tue, 13 May 2025 16:26:25 +0000 (18:26 +0200)]
BUG/MINOR: mux-spop: Make the demux stream ID a signed integer
The demux stream ID of a SPOP connection, used when received frames are
parsed, must be a signed integer because it is set to -1 when the SPOP
connection is initialized. It will be important for the next fix.
This patch must be backported to 3.1.
(cherry picked from commit
cbc10b896e647d5c62d9b021f3e2dd391d14dc4f)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Tue, 13 May 2025 16:05:26 +0000 (18:05 +0200)]
BUG/MINOR: mux-spop: Don't report error for stream if ACK was already received
When a SPOP connection was closed or was in error, an error was
systematically reported on all its SPOP streams. However, SPOP streams that
already received their ACK frame must be excluded. Otherwise if an agent
sends a ACK and close immediately, the ACK will be ignored because the SPOP
stream will handle the error first.
This patch must be backported to 3.1.
(cherry picked from commit
6d68beace5435f24abc4292f22021e4465b4dcee)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Wed, 30 Apr 2025 13:58:53 +0000 (15:58 +0200)]
BUG/MINOR: mux-spop: Use the right bitwise operator in spop_ctl()
Becaues of a typo, '||' was used instead of '|' to test the SPOP conneciton
flags and decide if the mux is ready or not. The regression was introduced
in the commit
fd7ebf117 ("BUG/MEDIUM: mux-spop: Wait end of handshake to
declare a spop connection ready").
This patch must be backported to 3.1 with the commit above.
(cherry picked from commit
2ccfebcebff254c95b6a70e987f3d960b38499c1)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Mon, 28 Apr 2025 06:01:40 +0000 (08:01 +0200)]
BUG/MEDIUM: mux-spop: Wait end of handshake to declare a spop connection ready
A SPOP connection must not be considered as ready while the hello handshake
is not finished with success. In addition, no error or shutdown must have
been reported for the underlying connection. Otherwise a freshly openned
spop connexion may be reused while it is in fact dead, leading to a
connection retry.
This patch must be backported to 3.1.
(cherry picked from commit
fd7ebf117bdefdcfbd1c30a079585867eb22a57d)
Signed-off-by: Willy Tarreau <w@1wt.eu>