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>
(cherry picked from commit
151a7c53d893618f6648577ec3c349601ac3c2bf)
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>
(cherry picked from commit
eacc3ffc583db7e393c820ded6e81d6162b96102)
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>
(cherry picked from commit
503d60bac653237d618ecb21c468d65855473484)
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>
(cherry picked from commit
60e98e90233ebceb2d9e199cd100273c65b73e15)
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>
(cherry picked from commit
0c62785f34fd14d612985e0a8b01d57540b136f2)
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>
(cherry picked from commit
8f01f82423f1da7a47f06a51bff54d185572c16a)
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>
(cherry picked from commit
f1dcfdd56cb24260b5c1884941f037a29edc83cb)
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>
(cherry picked from commit
cba2b004accf766a3c6e6aac35852e27cd74cf07)
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>
(cherry picked from commit
48de20ff93df34eef0b152a3701d84fb412079cf)
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>
(cherry picked from commit
47a40f66d72d3be619b2db6c2639f584a4e861b1)
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>
(cherry picked from commit
dfe831816c990b488d1e4316eca7bfbfe4e76bd0)
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>
(cherry picked from commit
db837984a6bba5846f4157cacd675f3685a93afe)
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>
(cherry picked from commit
b66643125323412a64bcfe978edb617cca4c15ce)
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>
(cherry picked from commit
9fe465256cbcaed3fc41c30c6b2b9df93376d639)
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>
(cherry picked from commit
3f6e0d887d30b9a2815d379d5fbf15470854aa65)
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>
(cherry picked from commit
faa6c12a5ceea61b904847a6b481d2efb2ba421b)
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>
(cherry picked from commit
b07ee7114ded568f80fb1de115e11bc355916774)
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>
(cherry picked from commit
b9968b4d131918075c00708fd6ad5b51fc3146c5)
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>
(cherry picked from commit
541d2c9a9b124cb19f4346a8c2fff3ada354380a)
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>
(cherry picked from commit
db39526525ec1124f8742410c573a621311c0ce3)
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>
(cherry picked from commit
38ef948e754e4cab938f219349642c89cf0a79e6)
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>
(cherry picked from commit
479befa356966c12a0a0ead74971750de4ac4499)
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>
(cherry picked from commit
dcb963f9d777af39926e83f20e0a3c65c54f3bc0)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Valentine Krasnobaeva [Mon, 31 Mar 2025 09:47:45 +0000 (11:47 +0200)]
MINOR: compiler: add __nonstring macro
GCC 15 throws the following warning on fixed-size char arrays if they do not
contain terminated NUL:
src/tools.c:2041:25: error: initializer-string for array of 'char' truncates NUL terminator but destination lacks 'nonstring' attribute (17 chars into 16 available) [-Werror=unterminated-string-initialization]
2041 | const char hextab[16] = "
0123456789ABCDEF";
We are using a couple of such definitions for some constants. Converting them
to flexible arrays, like: hextab[] = "
0123456789ABCDEF" may have consequences,
as enlarged arrays won't fit anymore where they were possibly located due to
the memory alignement constraints.
GCC adds 'nonstring' variable attribute for such char arrays, but clang and
other compilers don't have it. Let's wrap 'nonstring' with our
__nonstring macro, which will test if the compiler supports this attribute.
This fixes the issue #2910.
(cherry picked from commit
b3038614696fee43b6eaa826f47f273a8619690d)
[wla: should be backported to every stable branches]
Signed-off-by: William Lallemand <wlallemand@haproxy.com>
(cherry picked from commit
d4234b3c32b79b8dffd70f22c1eb3ba39541acb3)
Signed-off-by: William Lallemand <wlallemand@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>
(cherry picked from commit
051f51fe8417e072bdc949a050e0b5377073adca)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
Aurelien DARRAGON [Wed, 11 Dec 2024 09:42:11 +0000 (10:42 +0100)]
BUG/MINOR: hlua_fcn: restore server pairs iterator pointer consistency
Since 9c91b30 ("MINOR: server: remove prev_deleted server list"), hlua
server pair iterator may use and return invalid (stale) server pointer
if multiple servers were deleted between two iterations.
Indeed, the server refcount mechanism (using srv_take()) is no longer
sufficient as the prev_deleted mitigation was removed.
To ensure server pointer consistency between two yields, the new watcher
mechanism must be used (as it already the case for stats dumping).
Thus in this patch we slightly change the server iteration logic:
hlua_server_list_iterator_context struct now stores the next valid server
pointer, and a watcher is added to ensure this pointer is never stale.
Then in hlua_listable_servers_pairs_iterator(), this next pointer is used
to create the Lua server object, and the next valid pointer is obtained by
leveraging watcher_next().
No backport needed unless 9c91b30 ("MINOR: server: remove prev_deleted
server list") is. Please note that dynamic servers were not supported in
Lua prior to 2.8, so it doesn't make sense to backport this patch further
than 2.8.
(cherry picked from commit
358166ae6a3e98d36b378c7eeab1673cc8b4a4dd)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
d3fd2a825b4520cc76584709123bdc6b781ab93f)
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>
(cherry picked from commit
4c91018f1b50bc121b683b4dd83bdae90ad8f698)
[wla: ssl_clienthello.c didn't exist in <= 3.0, changes were made in
ssl_sock.c instead]
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>
(cherry picked from commit
806f245ea838066ca32a6a2bb1118f90c17f1597)
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>
(cherry picked from commit
a1246d92eeeb50f6b10349fb6848e9cead969c0e)
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>
(cherry picked from commit
6072496521b2feb0ffcae5b72b95adff10291b7b)
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>
(cherry picked from commit
dac40233379db32ba92c8fc0a8265a1fba2ddee7)
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>
(cherry picked from commit
eeb21041a1fab3022d905cfbfe9fad683c52b623)
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>
(cherry picked from commit
67455c2e293fe0af1dfaf12c031de04bff958930)
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>
(cherry picked from commit
3ba302684fcc346ed02c3892335f5ab9e8bceeb8)
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>
(cherry picked from commit
2341a3c06afd1ec3a92d0df5766daf30360374d6)
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>
(cherry picked from commit
1a6a3e5d958e2a42246c221369b96d18715112b1)
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>
(cherry picked from commit
30e59abd95cf22d755791fcd167c6e80d0b2d805)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
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>
(cherry picked from commit
355a3225302d1714792781f2dbee7d2ccfdc2a62)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Amaury Denoyelle [Thu, 23 Jan 2025 09:47:57 +0000 (10:47 +0100)]
MINOR: quic: rename min/max fields for congestion window algo
There was some possible confusion between fields related to congestion
window size min and max limit which cannot be exceeded, and the maximum
value previously reached by the window.
Fix this by adopting a new naming scheme. Enforced limit are now renamed
<limit_max>/<limit_min>, while the previously reached max value is
renamed <cwnd_last_max>.
This should be backported up to 3.1.
(cherry picked from commit
2eb1b0cd96f663b9260ee48921612566417b3b8d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
13c3baf545c93994688c3abd4895e6aede05211a)
[ad: pick to ease next backport]
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>
(cherry picked from commit
52235aed6248ff32832a24530ca8593610c76903)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
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>
(cherry picked from commit
8dfea165a389c44466d58fea9827c51017977d5a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Mon, 2 Jun 2025 15:15:51 +0000 (17:15 +0200)]
[RELEASE] Released version 3.0.11
Released version 3.0.11 with the following main changes :
- BUG/MEDIUM: mux-fcgi: Try to fully fill demux buffer on receive if not empty
- 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
- BUG/MINOR: dns: add tempo between 2 connection attempts for dns servers
- CLEANUP: dns: remove unused dns_stream_server struct member
- 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/MINOR: proxy: only use proxy_inc_fe_cum_sess_ver_ctr() with frontends
- 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/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: sink: detect and warn when using "send-proxy" options with ring servers
- DOC: ring: refer to newer RFC5424
- DOC: config: restore default values for resolvers hold directive
- DOC: config: recommend disabling libc-based resolution with resolvers
- BUG/MINOR: h3: don't insert more than one Host header
- CLEANUP: quic: Useless BIO_METHOD initialization
- MINOR: quic: Add useful error traces about qc_ssl_sess_init() failures
- 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/MINOR: mux-h2: Reset streams with NO_ERROR code if full response was already sent
- BUILD: makefile: enable backtrace by default on musl
- 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/MINOR: limits: compute_ideal_maxconn: don't cap remain if fd_hard_limit=0
- 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>
(cherry picked from commit
240daee3aba09ceb508f5afecf2f7d26f0f06b06)
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>
(cherry picked from commit
dc4d8da0ddc16e146d8df434c4388be71a70e4dd)
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>
(cherry picked from commit
c0d42ff115438f7a2ee83388be87b439d034e79e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Valentine Krasnobaeva [Tue, 18 Mar 2025 15:33:54 +0000 (16:33 +0100)]
BUG/MINOR: limits: compute_ideal_maxconn: don't cap remain if fd_hard_limit=0
'global.fd_hard_limit' stays uninitialized, if haproxy is started with -m
(global.rlimit_memmax). 'remain' is the MAX between soft and hard process fd
limits. It will be always bigger than 'global.fd_hard_limit' (0) in this case.
So, if we reassign 'remain' to the 'global.fd_hard_limit' unconditionally,
calculated then 'maxconn' will be even negative and the DEFAULT_MAXCONN (100)
will be set as the 'ideal_maxconn'.
During the 'global.maxconn' calculations in set_global_maxconn(), if the
provided 'global.rlimit_memmax' is quite big, system will refuse to calculate
based on its 'global.maxconn' and we will do a fallback to the 'ideal_maxconn',
which is 100.
Same problem for the configs with SSL frontends and backends.
This fixes the issue #2899.
This should be backported to v3.1.0.
(cherry picked from commit
060f441199aa97d9735dd553bafd231ca615f723)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
95197ec04e241f33803909c9c6182e832b6ed16e)
[cf: Applied on src/haproxy.c]
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>
(cherry picked from commit
475ce437176e029a1c9143090b2bab0f554b987d)
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>
(cherry picked from commit
1e863408430801b9697737b08d30dc8ae682ca3f)
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>
(cherry picked from commit
32e80070e28489369dbcd5239661410cd28496db)
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>
(cherry picked from commit
90ce6153820b13fdf294f54794ed5a841c7b805c)
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>
(cherry picked from commit
08c48cf43598a489f74d4a128f672545d1577d30)
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>
(cherry picked from commit
ecc8126ef97836044b1bc962fd4b9c91e89c353f)
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>
(cherry picked from commit
476281411fa8542d18e2cdf9160876f7740aaf72)
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>
(cherry picked from commit
a471ebfd4df167bf70b04d623ac13ce39aaf0948)
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>
(cherry picked from commit
21e4940e446143117f7924d12bfc5a5d06b243cb)
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>
(cherry picked from commit
d883c7113fc28d48468c4cc5dc0a27ad3b122497)
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>
(cherry picked from commit
dfde7bd6b0b6f5e87e4ab65059f9a86219c55c95)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Thu, 17 Apr 2025 14:11:14 +0000 (16:11 +0200)]
BUILD: makefile: enable backtrace by default on musl
The reason musl builds was not producing exploitable backtraces was
that the toolchain used appears to automatically omit the frame pointer
at -O2 but leaves it at -O0. This patch just makes sure to always append
-fno-omit-frame-pointer to the BACKTRACE cflags and enables the option
with musl where it now works. This will allow us to finally get
exploitable traces from docker images where core dumps are not always
available.
(cherry picked from commit
f499fa3dcd24b5a17ed97842f5e867bd37739754)
[wt: this should be progressively backported to 3.0 or maybe even 2.8
since a few users have already reported hard-to-debug issues in Docker]
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
d510825598bd5bf28d0c95b8ad5cc2873b4300c8)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Mon, 17 Mar 2025 15:26:35 +0000 (16:26 +0100)]
BUG/MINOR: mux-h2: Reset streams with NO_ERROR code if full response was already sent
On frontend side, when a stream is shut while the response was already fully
sent, it was cancelled by sending a RST_STREAM(CANCEL) frame. However, it is
not accurrate. CANCEL error code must only be used if the response headers
were sent, but not the full response. As stated in the RFC 9113, when the
response was fully sent, to stop the request sending, a RST_STREAM with an
error code of NO_ERROR must be sent.
This patch should solve the issue #1219. It must be backported to all stable
versions.
(cherry picked from commit
e87397bc7d3b386c95d1489d19f29e6d5f1f1482)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
a8af2c733890746cc53ef73553a2f2d699326cd2)
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>
(cherry picked from commit
e31f447c9d6681a4f3f98bfdd7298082adadc7a1)
[cf: If this commit is backported, it is probably good to backport the previous
one ("MEDIUM: hlua: Add function to change the body length of an HTTP
Message")]
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>
(cherry picked from commit
8ac8358e06c8be8260a36d1e76d4078aa50081ff)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
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>
(cherry picked from commit
76636502b94941061a887566d268ffa6f01e61ca)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
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>
(cherry picked from commit
9488feaab3f4aca586eb7f13997a0ea7b6ccf304)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
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>
(cherry picked from commit
fd54c8ce6fb977e4591b73f677caf5743964bb45)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
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>
(cherry picked from commit
e106c8f8f1068fba0923dd3348c6e7cb0bc499ab)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
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>
(cherry picked from commit
67226e170dfa18e377d6ef223ffb292a5d8ddc9e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
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>
(cherry picked from commit
2973c183013bb1a9333903ee25f842044d7dc9c4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
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>
(cherry picked from commit
975a8e8da092d0b26b61514d6973e05281441df9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Mon, 12 May 2025 14:12:13 +0000 (16:12 +0200)]
BUG/MINOR: hlua: Fix Channel:data() and Channel:line() to respect documentation
When the channel API was revisted, the both functions above was added. An
offset can be passed as argument. However, this parameter could be reported
to be out of range if there was not enough input data was received yet. It
is an issue, especially with a tcp rule, because more data could be
received. If an error is reported too early, this prevent the rule to be
reevaluated later. In fact, an error should only be reported if the offset
is part of the output data.
Another issue is about the conditions to report 'nil' instead of an empty
string. 'nil' was reported when no data was found. But it is not aligned
with the documentation. 'nil' must only be returned if no more data cannot
be received and there is no input data at all.
This patch should fix the issue #2716. It should be backported as far as 2.6.
(cherry picked from commit
a5de0e15959a241afc9afb39f1b02a2517894f7b)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
ab8b159b39983e969510bc4b5902984d80f18f62)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Aurelien DARRAGON [Mon, 12 May 2025 09:57:39 +0000 (11:57 +0200)]
BUG/MINOR: threads: fix soft-stop without multithreading support
When thread support is disabled ("USE_THREAD=" or "USE_THREAD=0" when
building), soft-stop doesn't work as haproxy never ends after stopping
the proxies.
This used to work fine in the past but suddenly stopped working with
ef422ced91 ("MEDIUM: thread: make stopping_threads per-group and add
stopping_tgroups") because the "break;" instruction under the stopping
condition is never executed when support for multithreading is disabled.
To fix the issue, let's add an "else" block to run the "break;"
instruction when USE_THREAD is not defined.
It should be backported up to 2.8
(cherry picked from commit
7d057e56af86cdc98c5388986931c247dbafa5b3)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
66657f786a3662f8cb46d6d2adc9e621b1127e83)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Fri, 9 May 2025 17:01:28 +0000 (19:01 +0200)]
BUG/MINOR: ssl/ckch: always free() the previous entry during parsing
The ckch_conf_parse() function is the generic function which parses
crt-store keywords from the crt-store section, and also from a crt-list.
When having multiple time the same keyword, a leak of the previous value
happens. This patch ensure that the previous value is always freed
before overwriting it.
This patch should be backported as far as 3.0.
(cherry picked from commit
311e0aa5c712f38700b7b185c0d5f1aa33c48613)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
adebe6246558ece09c82fbbf3e549cf5e4cdd2ec)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Aurelien DARRAGON [Wed, 7 May 2025 23:01:28 +0000 (01:01 +0200)]
BUG/MINOR: cli: fix too many args detection for commands
d3f928944 ("BUG/MINOR: cli: Issue an error when too many args are passed
for a command") added a new check to prevent the command to run when
too many arguments are provided. In this case an error is reported.
However it turns out this check (despite marked for backports) was
ineffective prior to
20ec1de21 ("MAJOR: cli: Refacor parsing and
execution of pipelined commands") as 'p' pointer was reset to the end of
the buffer before the check was executed.
Now since
20ec1de21, the check works, but we have another issue: we may
read past initialized bytes in the buffer because 'p' pointer is always
incremented in a while loop without checking if we increment it past 'end'
(This was detected using valgrind)
To fix the issue introduced by
20ec1de21, let's only increment 'p' pointer
if p < end.
For 3.2 this is it, now for older versions, since
d3f928944 was marked for
backport, a sligthly different approach is needed:
- conditional p increment must be done in the loop (as in this patch)
- max arg check must moved above "fill unused slots" comment where p is
assigned to the end of the buffer
This patch should be backported with
d3f928944.
(cherry picked from commit
976e0bd32f6d87fccf351edf1b5a18f3e0c5c6ab)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
71f72809886476e5cf3bf77631324e2d6b89071f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Tue, 6 May 2025 09:32:34 +0000 (11:32 +0200)]
BUG/MEDIUM: peers: hold the refcnt until updating ts->seen
In peer_treat_updatemsg(), we call stktable_touch_remote() after
releasing the write lock on the TS, asking it to decrement the
refcnt, then we update ts->seen. Unfortunately this is racy and
causes the issue that Christian reported in issue #2959.
The sequence of events is very hard to trigger manually, but what happens
is the following:
T1. stktable_touch_remote(table, ts, 1);
-> at this point the entry is in the mt_list, and the refcnt is zero.
T2. stktable_trash_oldest() or process_table_expire()
-> these can run, because the refcnt is now zero.
The entry is cleanly deleted and freed.
T1. HA_ATOMIC_STORE(&ts->seen, 1)
-> we dereference freed memory.
A first attempt at a fix was made by keeping the refcnt held during
all the time the entry is in the mt_list, but this is expensive as
such entries cannot be purged, causing lots of skips during
trash_oldest_data(). This managed to trigger watchdogs, and was only
hiding the real cause of the problem.
The correct approach clearly is to maintain the ref_cnt until we
touch ->seen. That's what this patch does. It does not decrement
the refcnt, while calling stktable_touch_remote(), and does it
manually after touching ->seen. With this the problem is gone.
Note that a reproducer involves the following:
- a config with 10 stick-ctr tracking the same table with a
random key between 10M and 100M depending on the machine.
- the expiration should be between 10 and 20s. http_req_cnt
is stored and shared with the peers.
- 4 total processes with such a config on the local machine,
each corresponding to a different peer. 3 of the peers are
bound to half of the cores (all threads) and share the same
threads; the last process is bound to the other half with
its own threads.
- injecting at full load, ~256 conn, on the shared listening
port. After ~2x expiration time to 1 minute the lone process
should segfault in pools code due to a corrupted by_lru list.
This problem already exists in earlier versions but the race looks
narrower. Given how difficult it is to trigger on a given machine
in its current form, it's likely that it only happens once in a
while on stable branches. The fix must be backported wherever the
code is similar, and there's no hope to reproduce it to validate
the backport.
Thanks again to Christian for his amazing help!
(cherry picked from commit
006a3acbde309f11190a003c4ac1c026480444e4)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
a3343abd2cce9e72f2db1cb1e8a80b277a769fd7)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Amaury Denoyelle [Tue, 6 May 2025 16:01:32 +0000 (18:01 +0200)]
BUG/MINOR: quic: reject invalid max_udp_payload size
Add a checks on received max_udp_payload transport parameters. As
defined per RFC 9000, values below 1200 are invalid, and thus the
connection must be closed with TRANSPORT_PARAMETER_ERROR code.
Prior to this patch, an invalid value was silently ignored.
This should be backported up to 2.6. Note that is relies on previous
patch "MINOR: quic: extend return value on TP parsing".
(cherry picked from commit
4bc7aa548adcd9ee424c65cd346e94f8749dce64)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
c806ee0300fd82f6414a172a3c5829afaeb16eac)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Amaury Denoyelle [Tue, 6 May 2025 16:01:09 +0000 (18:01 +0200)]
BUG/MINOR: quic: fix TP reject on invalid max-ack-delay
Checks are implemented on some received transport parameter values,
to reject invalid ones defined per RFC 9000. This is the case for
max_ack_delay parameter.
The check was not properly implemented as it only reject values strictly
greater than the limit set to 2^14. Fix this by rejecting values of 2^14
and above. Also, the proper error code TRANSPORT_PARAMETER_ERROR is now
set.
This should be backported up to 2.6. Note that is relies on previous
patch "MINOR: quic: extend return value on TP parsing".
(cherry picked from commit
ffabfb0fc3ad8774024d152fc31a7711a8a9c382)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
732bba41245e2365eb24bdbb856f5ed44f06d262)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Amaury Denoyelle [Tue, 6 May 2025 16:00:43 +0000 (18:00 +0200)]
BUG/MINOR: quic: use proper error code on invalid received TP value
As per RFC 9000, checks must be implemented to reject invalid values for
received transport parameters. Such values are dependent on the
parameter type.
Checks were already implemented for ack_delay_exponent and
active_connection_id_limit, accordingly with the QUIC specification.
However, the connection was closed with an incorrect error code. Fix
this to ensure that TRANSPORT_PARAMETER_ERROR code is used as expected.
This should be backported up to 2.6. Note that is relies on previous
patch "MINOR: quic: extend return value on TP parsing".
(cherry picked from commit
b60a17aad768369ab7e328949112b50cd78bc987)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
af4048e4e36503557d90e37514a8a3e8e7210c03)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Amaury Denoyelle [Tue, 6 May 2025 15:59:37 +0000 (17:59 +0200)]
BUG/MINOR: quic: reject retry_source_cid TP on server side
Close the connection on error if retry_source_connection_id transport
parameter is received. This is specified by RFC 9000 as this parameter
must not be emitted by a client. Previously, it was silently ignored.
This should be backported up to 2.6. Note that is relies on previous
patch "MINOR: quic: extend return value on TP parsing".
(cherry picked from commit
10f1f1adce032742d60fe14ee780871c4e6a1db1)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
f66a92724a8f8469e98abcaeccf46cc58ea7fb5c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Amaury Denoyelle [Tue, 6 May 2025 15:59:21 +0000 (17:59 +0200)]
BUG/MINOR: quic: use proper error code on invalid server TP
This commit is similar to the previous one. It fixes the error code
reported when dealing with invalid received transport parameters. This
time, it handles reception of original_destination_connection_id,
preferred_address and stateless_reset_token which must not be emitted by
the client.
This should be backported up to 2.6. Note that is relies on previous
patch "MINOR: quic: extend return value on TP parsing".
(cherry picked from commit
a54fdd3d926fabfc438dbaedbd3d08814fb99862)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
b70be5c97c713c12e9e2a2483b7c52a5a849fcd4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Amaury Denoyelle [Tue, 6 May 2025 14:45:23 +0000 (16:45 +0200)]
BUG/MINOR: quic: use proper error code on missing CID in TPs
Handle missing received transport parameter value
initial_source_connection_id / original_destination_connection_id.
Previously, such case would result in an error reported via
quic_transport_params_store(), which triggers a TLS alert converted as
expected as a CONNECTION_CLOSE. The issue is that the error code
reported in the frame was incorrect.
Fix this by returning QUIC_TP_DEC_ERR_INVAL for such conditions. This is
directly handled via quic_transport_params_store() which set the proper
TRANSPORT_PARAMETER_ERROR code for the CONNECTION_CLOSE. However, no
error is reported so the SSL handshake is properly terminated without a
TLS alert. This is enough to ensure that the CONNECTION_CLOSE frame will
be emitted as expected.
This should be backported up to 2.6. Note that is relies on previous
patch "MINOR: quic: extend return value on TP parsing".
(cherry picked from commit
df6bd4909e8dfa64337662a78adc39ca702c1dc7)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
5eb284aa55fa3a7c12060b3834000dd66f88493e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Amaury Denoyelle [Tue, 6 May 2025 16:10:27 +0000 (18:10 +0200)]
MINOR: quic: extend return value during TP parsing
Extend API used for QUIC transport parameter decoding. This is done via
the introduction of a dedicated enum to report the various error
condition detected. No functional change should occur with this patch,
as the only returned code is QUIC_TP_DEC_ERR_TRUNC, which results in the
connection closure via a TLS alert.
This patch will be necessary to properly reject transport parameters
with the proper CONNECTION_CLOSE error code. As such, it should be
backported up to 2.6 with the following series.
(cherry picked from commit
294bf26c06404e35edd4ad3381ccb26e835bd7a1)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
f5a5564f65da6190e1cf4a7c92a1610c2fe03caf)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Aurelien DARRAGON [Fri, 2 May 2025 17:06:44 +0000 (19:06 +0200)]
BUG/MINOR: proxy: only use proxy_inc_fe_cum_sess_ver_ctr() with frontends
proxy_inc_fe_cum_sess_ver_ctr() was implemented in
9969adbc
("MINOR: stats: add by HTTP version cumulated number of sessions and
requests")
As its name suggests, it is meant to be called for frontends, not backends
Also, in
9969adbc, when used under h1_init(), a precaution is taken to
ensure that the function is only called with frontends.
However, this precaution was not applied in h2_init() and qc_init().
Due to this, it remains possible to have proxy_inc_fe_cum_sess_ver_ctr()
being called with a backend proxy as parameter. While it did not cause
known issues so far, it is not expected and could result in bugs in the
future. Better fix this by ensuring the function is only called with
frontends.
It may be backported up to 2.8
(cherry picked from commit
b39825ee45150415d7ed64b7ce785bb946f727bd)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
736bb497acb656e736291f8113650a62bcf96f5f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Wed, 30 Apr 2025 12:32:16 +0000 (14:32 +0200)]
BUG/MINOR: mux-h1: Fix trace message in h1_detroy() to not relay on connection
h1_destroy() may be called to release a H1C after a multiplexer upgrade. In
that case, the connection is no longer attached to the H1C. It must not be
used in the h1 trace message because the connection context is no longer a H1C.
Because of this bug, when a H1>H2 upgrade is performed, a crash may be
experienced if the H1 traces are enabled.
This patch must be backport to all stable versions.
(cherry picked from commit
7dc4e94830ef8fe9b0ffc2901d63b9f3183ed12c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
cad223e19946a6d02261a259c2be5c20096ef2cd)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Wed, 30 Apr 2025 12:16:42 +0000 (14:16 +0200)]
BUG/MINOR: mux-h1: Don't pretend connection was released for TCP>H1>H2 upgrade
When an applicative upgrade of the H1 multiplexer is performed, we must not
pretend the connection was released. Indeed, in that case, a H1 stream is
still their with a stream connector attached on it. It must be detached
first before releasing the H1 connection and the underlying connection. So
it is important to not pretend the connection was already released.
Concretely, in that case h1_process() must return 0 instead of -1. It is
minor error because, AFAIK, it is harmless. But it is not correct. So let's
fix it to avoid futur bugs.
To be clear, this happens when a TCP connection is upgraded to H1 connection
and a H2 preface is detected, leading to a second upgrade from H1 to H2.
This patch may be backport to all stable versions.
(cherry picked from commit
2dc334be61a0a9feaa7b844e122c2c4ce37e1b1a)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
3b774ac747d54f107b25ed1c6e9e16720e78d421)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Aurelien DARRAGON [Tue, 29 Apr 2025 08:22:38 +0000 (10:22 +0200)]
BUG/MINOR: dns: prevent ds accumulation within dss
when dns session callback (dns_session_release()) is called upon error
(ie: when some pending queries were not sent), we try our best to
re-create the applet in order to preserve the pending queries and give
them a chance to be retried. This is done at the end of
dns_session_release().
However, doing so exposes to an issue: if the error preventing queries
from being sent is still encountered over and over the dns session could
stay there indefinitely. Meanwhile, other dns sessions may be created on
the same dns_stream_server periodically. If previous failing dns sessions
don't terminate but we also keep creating new ones, we end up accumulating
failing sessions on a given dns_stream_server, which can eventually cause
ressource shortage.
This issue was found when trying to address ("BUG/MINOR: dns: add tempo
between 2 connection attempts for dns servers")
To fix it, we track the number of failed consecutive sessions for a given
dns server. When we reach the threshold (set to 100), we consider that the
link to the dns server is broken (at least temporarily) and we force
dns_session_new() to fail, so that we stop creating new sessions until one
of the existing one eventually succeeds.
A workaround for this fix consists in setting the "maxconn" parameter on
nameserver directive (under resolvers section) to a reasonnable value so
that no more than "maxconn" sessions may co-exist on the same server at
a given time.
This may be backported to all stable versions.
("CLEANUP: dns: remove unused dns_stream_server struct member") may be
backported to ease the backport.
(cherry picked from commit
5288b39011b2449bfa896f7932c7702b5a85ee77)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
020348974b09cb011159260c08a8b821456029a6)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Aurelien DARRAGON [Tue, 29 Apr 2025 14:48:28 +0000 (16:48 +0200)]
CLEANUP: dns: remove unused dns_stream_server struct member
dns_stream_server "max_slots" is unused, let's get rid of it
(cherry picked from commit
14ebe95a10e7fdc003e369b58463a4744e88fa8e)
[wt: just to help with next backport]
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
73f11d9caafeacbca0b68e660c119cb60fa976fe)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Aurelien DARRAGON [Tue, 29 Apr 2025 18:13:00 +0000 (20:13 +0200)]
BUG/MINOR: dns: add tempo between 2 connection attempts for dns servers
As reported by Lukas Tribus on the mailing list [1], trying to connect to
a nameserver with invalid network settings causes haproxy to retry a new
connection attempt immediately which eventually causes unexpected CPU usage
on the thread responsible for the applet (namely 100% on one CPU will be
observed).
This can be reproduced with the test config below:
resolvers default
nameserver ns1 tcp4@8.8.8.8:53 source 192.168.99.99
listen listen
mode http
bind :8080
server s1 www.google.com resolvers default init-addr none
To fix this the issue, we add a temporisation of one second between a new
connection attempt is retried. We do this in dns_session_create() when we
know that the applet was created in the release callback (when previous
query attempt was unsuccessful), which means initial connection is not
affected.
[1]: https://www.mail-archive.com/haproxy@formilux.org/msg45665.html
This should fix GH #2909 and may be backported to all stable versions.
This patch depends on ("MINOR: applet: add appctx_schedule() macro")
(cherry picked from commit
27236f2218e57d839197e5b6479dc8e67e484e32)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
70653e0831b54a1b2062d377352830cca3f56ea1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Aurelien DARRAGON [Mon, 28 Apr 2025 16:03:36 +0000 (18:03 +0200)]
MINOR: applet: add appctx_schedule() macro
Just like task_schedule() but for applets to wakeup an applet at a
specific time, leverages _task_schedule() internally
(cherry picked from commit
1ced5ef2fdf9a4b4ab0941849be0f4627066331f)
[wt: needed for next dns backport]
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
3fdb657f6b2b02ac6b1b95be0217b06769fc499e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Fri, 25 Apr 2025 16:32:02 +0000 (18:32 +0200)]
BUG/MAJOR: listeners: transfer connection accounting when switching listeners
Since we made it possible for a bind_conf to listen to multiple thread
groups with shards in 2.8 with commit
9d360604bd ("MEDIUM: listener:
rework thread assignment to consider all groups"), the per-listener
connection count was not properly transferred to the target listener
with the connection when switching to another thread group. This results
in one listener possibly reaching high values and another one possibly
reaching negative values. Usually it's not visible, unless a maxconn is
set on the bind_conf, in which case comparisons will quickly put an end
to the willingness to accept new connections.
This problem only happens when thread groups are enabled, and it seems
very hard to trigger it normally, it only impacts sockets having a single
shard, hence currently the CLI (or any conf with "bind ... shards 1"),
where it can be reproduced with a config having a very low "maxconn" on
the stats socket directive (here, 4), and issuing a few tens of
socat <<< "show activity" in parallel, or sending HTTP connections to a
single-shared listener. Very quickly, haproxy stops accepting connections
and eats CPU in the poller which tries to get its connections accepted.
A BUG_ON(l->nbconn<0) after HA_ATOMIC_DEC() in listener_release() also
helps spotting them better.
Many thanks to Christian Ruppert who once again provided a very accurate
report in GH #2951 with the required data permitting this analysis.
This fix must be backported to 2.8.
(cherry picked from commit
f1064c7382e170cd59dd7984808bc436e2b3de7f)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
33ec1e2fb357e46a83be886624cf695259fd1bf6)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Wed, 23 Apr 2025 13:29:00 +0000 (15:29 +0200)]
BUG/MINOR: cli: Issue an error when too many args are passed for a command
When a command is parsed to split it in an array of arguments, by default,
at most 64 arguments are supported. But no warning was emitted when there
were too many arguments. Instead, the arguments above the limit were
silently ignored. It could be an issue for some commands, like "add server",
because there was no way to know some arguments were ignored.
Now an error is issued when too many arguments are passed and the command is
not executed.
This patch should be backported to all stable versions.
(cherry picked from commit
d3f92894471dd7306f857156a5820c53c10f8d88)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
cb0fbd5e1a7cdde5210fea8ef87ede21c431807d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 28 Feb 2025 15:07:00 +0000 (16:07 +0100)]
BUG/MEDIUM: mux-fcgi: Try to fully fill demux buffer on receive if not empty
Don't reserve space for the HTX overhead on receive if the demux buffer is
not empty. Otherwise, the demux buffer may be erroneously reported as full
and this may block records processing. Because of this bug, a ping-pong loop
till timeout between data reception and demux process can be observed.
This bug was introduced by the commit
5f927f603 ("BUG/MEDIUM: mux-fcgi:
Properly handle read0 on partial records"). To fix the issue, if the demux
buffer is not empty when we try to receive more data, all free space in the
buffer can now be used. However, if the demux buffer is empty, we still try
to keep it aligned with the HTX.
This patch must be backported to 3.1.
(cherry picked from commit
0e08252294d5a7389ad42b51b4b931fab2e66f31)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
cc4981548b12e3162fc020be02917f8462f00866)
[cf: This commit must be backported with the commit above]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Tue, 22 Apr 2025 11:53:02 +0000 (13:53 +0200)]
[RELEASE] Released version 3.0.10
Released version 3.0.10 with the following main changes :
- MINOR: log: support "raw" logformat node typecast
- BUG/MINOR: peers: fix expire learned from a peer not converted from ms to ticks
- BUG/MEDIUM: peers: prevent learning expiration too far in futur from unsync node
- BUG/MEDIUM: mux-quic: fix crash on RS/SS emission if already close local
- BUG/MINOR: mux-quic: remove extra BUG_ON() in _qcc_send_stream()
- BUG/MINOR: log: fix gcc warn about truncating NUL terminator while init char arrays
- DOC: config: fix two missing "content" in "tcp-request" examples
- BUILD: compiler: undefine the CONCAT() macro if already defined
- BUG/MINOR: rhttp: fix incorrect dst/dst_port values
- BUG/MINOR: backend: do not overwrite srv dst address on reuse
- BUG/MEDIUM: backend: fix reuse with set-dst/set-dst-port
- BUG/MEDIUM: stream: Fix a possible freeze during a forced shut on a stream
- TESTS: Fix build for filltab25.c
- MINOR: task: add thread safe notification_new and notification_wake variants
- BUG/MINOR: hlua_fcn: fix potential UAF with Queue:pop_wait()
- CLEANUP: log: adjust _lf_cbor_encode_byte() comment
- BUG/MINOR: log: fix CBOR encoding with LOG_VARTEXT_START() + lf_encode_chunk()
- BUG/MEDIUM: sample: fix risk of overflow when replacing multiple regex back-refs
- BUG/MINOR: backend: do not use the source port when hashing clientip
- BUG/MINOR: hlua: fix invalid errmsg use in hlua_init()
- DOC: config: add the missing "profiling.memory" to the global kw index
- BUG/MINOR: http-ana: Properly detect client abort when forwarding the response
- BUG/MEDIUM: http-ana: Report 502 from req analyzer only during rsp forwarding
- BUG/MINOR: sink: add tempo between 2 connection attempts for sft servers (2)
- BUG/MEDIUM: h3: trim whitespaces when parsing headers value
- BUG/MEDIUM: h3: trim whitespaces in header value prior to QPACK encoding
- BUG/MINOR: h3: filter upgrade connection header
- BUG/MINOR: h3: reject invalid :path in request
- BUG/MINOR: h3: reject request URI with invalid characters
- BUG/MEDIUM: hlua: fix hlua_applet_{http,tcp}_fct() yield regression (lost data)
- BUG/MINOR: quic: do not crash on CRYPTO ncbuf alloc failure
- DEBUG: stream: Add debug counters to track some client/server aborts
- BUG/MINOR: stktable: invalid use of stkctr_set_entry() with mixed table types
- BUG/MEDIUM: mux-fcgi: Properly handle read0 on partial records
- DEBUG: fd: add a counter of takeovers of an FD since it was last opened
- MINOR: fd: add a generation number to file descriptors
- MINOR: epoll: permit to mask certain specific events
- DEBUG: epoll: store and compare the FD's generation count with reported event
- MEDIUM: epoll: skip reports of stale file descriptors
- IMPORT: plock: give higher precedence to W than S
- IMPORT: plock: lower the slope of the exponential back-off
- IMPORT: plock: use cpu_relax() for a shorter time in EBO
- BUG/MINOR debug: fix !USE_THREAD_DUMP in ha_thread_dump_fill()
- BUG/MINOR: mux-h2: prevent past scheduling with idle connections
- BUG/MINOR: rhttp: fix reconnect if timeout connect unset
- BUG/MINOR: rhttp: ensure GOAWAY can be emitted after reversal
- MINOR: tools: also protect the library name resolution against concurrent accesses
Willy Tarreau [Fri, 4 Apr 2025 14:56:44 +0000 (16:56 +0200)]
MINOR: tools: also protect the library name resolution against concurrent accesses
This is an extension of
eb41d768f ("MINOR: tools: use only opportunistic
symbols resolution"). It also makes sure we're not calling dladddr() in
parallel to dladdr_and_size(), as a preventive measure against some
potential deadlocks in the inner layers of the libc.
(cherry picked from commit
8d0c6336778fed316a174fd278c78a88f7dd6975)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
86d6b398d481d0d60bed9e509659e6722ce66b15)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Amaury Denoyelle [Thu, 10 Apr 2025 15:41:39 +0000 (17:41 +0200)]
BUG/MINOR: rhttp: ensure GOAWAY can be emitted after reversal
GOAWAY emission should not be emitted before preface. Thus, max_id field
from h2c acting as a server is initialized to -1, which prevents its
emission until preface is received from the peer. If acting as a client,
max_id is initialized to a valid value on the first h2s emission.
This causes an issue with reverse HTTP on the active side. First, it
starts as a client, so the peer does not emit a preface but instead a
simple SETTINGS frame. As role are switched, max_id is initialized much
later when the first h2s response is emitted. Thus, if the connection
must be terminated before any stream transfer, GOAWAY cannot be emitted.
To fix this, ensure max_id is initialized to 0 on h2_conn_reverse() for
active connect side. Thus, a GOAWAY indicating that no stream has been
handled can be generated.
Note that passive connect side is not impacted, as it max_id is
initialized thanks to preface reception.
This should be backported up to 2.9.
(cherry picked from commit
9e6f8ce3282fc5461e30adae3c76cbceb2bc327e)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
05509361e55eda929ff11cf245812c8154728c36)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Amaury Denoyelle [Thu, 10 Apr 2025 16:05:55 +0000 (18:05 +0200)]
BUG/MINOR: rhttp: fix reconnect if timeout connect unset
Active connect on reverse http relies on connect timeout to detect
connection failure. Thus, if this timeout was unset, connection failure
may not be properly detected.
Fix this by fallback on hardcoded value of 1s for connect if timeout is
unset in the configuration. This is considered as a minor bug, as
haproxy advises against running with timeout unset.
This must be backported up to 2.9.
(cherry picked from commit
2b8da5f9ab8639f4213ee21fc5fd5ca12c53cddb)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
cd6e4517fdec392cef375c9ddc75da38a1f07b44)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Amaury Denoyelle [Wed, 9 Apr 2025 12:26:54 +0000 (14:26 +0200)]
BUG/MINOR: mux-h2: prevent past scheduling with idle connections
While reviewing HTTP/2 MUX timeout, it seems there is a possibility that
MUX task is requeued via h2c_update_timeout() with an already expired
date. This can happens with idle connections on two cases :
* first with shut timeout, as timer is not refreshed if already set
* second with http-request and keep-alive timers, which are based on
idle_start
Queuing an already expired task is an undefined behavior. Fix this by
using task_wakeup() instead of task_queue() at the end of
h2c_update_timeout() if such case occurs.
This should be backported up to 2.6.
(cherry picked from commit
3ebdd3ae509236af1f19d80cab98b385a751426b)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
516121499c8edafb67a861bcd1bf41b3cc6f12d2)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Thu, 17 Apr 2025 08:28:37 +0000 (10:28 +0200)]
BUG/MINOR debug: fix !USE_THREAD_DUMP in ha_thread_dump_fill()
The function must make sure to return NULL for foreign threads and
the local buffer for the current thread in this case, otherwise panics
(and sometimes even warnings) will segfault when USE_THREAD_DUMP is
disabled. Let's slightly re-arrange the function to reduce the #if/else
since we have to specifically handle the case of !USE_THREAD_DUMP anyway.
This needs to be backported wherever
b8adef065d ("MEDIUM: debug: on
panic, make the target thread automatically allocate its buf") was
backported (at least 2.8).
(cherry picked from commit
0b56839455c7f45ae58954e1eb3873a3725899dc)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
f668b85ecea96218c24524d91b7d4f1e065fcd74)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Fri, 7 Feb 2025 16:33:49 +0000 (17:33 +0100)]
IMPORT: plock: use cpu_relax() for a shorter time in EBO
Tests have shown that on modern CPUs it's interesting to wait a bit less
in cpu_relax(). Till now we were looping down to 60 iterations and then
switching to just barriers. Increasing the threshold to 90 iterations
left before getting out of the loop improved the average and max time
to grab a write lock by a few percent (e.g. 10% at 1us, 20% at 256ns
or lower). Higher values tend to progressively lose that gain so let's
stick to this one. This was measured on an EPYC 74F3 like previous
measurements that initially led to this value, and the value might
possibly depend on the mask applied to the loop counter.
This is plock commit
74ca0a7307fa6aec3139f27d3b7e534e1bdb748e.
(cherry picked from commit
b957e2f3ef63e1846988bed9d2bbb42213b29fbf)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
71f66d03bd199b326b7231cbe6fe40f60251bb1c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Fri, 7 Feb 2025 16:20:48 +0000 (17:20 +0100)]
IMPORT: plock: lower the slope of the exponential back-off
Along many tests involving both haproxy's scheduler and forwarded
traffic, various exponents and algorithms were attempted for the EBO
and their effects were measured. It was found that a growth in 1.25^N
limited to 128k cycles consistently gives a better latency than 1.5^N
limited to 256k cycles, without degrading general performance. The
measures of the time to grab a write lock on a 48-thread EPYC show
that the number of occurrences of low times was roughly multiplied by
2-3 while the number of occurrences of times above 64us was reduced
by similar factors, to even reach 300 at 64us and limiting the maximum
time by a factor of 4.
The other variants that were experimented with are:
m = ((m + (m >> 1)) + 2) & 0x3ffff; // original
m = ((m + (m >> 1) + (m >> 3)) + 2) & 0x3ffff;
m = ((m + (m >> 1) + (m >> 4)) + 2) & 0x3ffff;
m = ((m + (m >> 1) + (m >> 4)) + 2) & 0x1ffff;
m = ((m + (m >> 1) + (m >> 4)) + 1) & 0x1ffff;
m = ((m + (m >> 2) + (m >> 4)) + 1) & 0x1ffff; // lowest CPU on pl_wr test + good perf
m = ((m + (m >> 2)) + 1) & 0x1ffff; // even lower cpu usage, lowest max
m = ((m + (m >> 1) + (m >> 2)) + 1) & 0x1ffff; // correct but slightly higher maxes
m = ((m + (m >> 1) + (m >> 3)) + 1) & 0x1ffff; // less good than m+m>>2
m = ((m + (m >> 2) + (m >> 3)) + 1) & 0x1ffff; // better but not as good as m+m>>2
m = ((m + (m >> 3) + (m >> 4)) + 1) & 0x1ffff; // less good, lower rates on small coounts.
m = ((m + (m >> 2) + (m >> 3) + (m >> 4)) + 1) & 0x1ffff; // less good as well
m = ((m & 0x7fff) + (m >> 1) + (m >> 4)) + 2;
m = ((m & 0xffff) + (m >> 1) + (m >> 4)) + 2;
This is plock commit
dddd9ee01c522da33c353e2e4d4fd743d8336ec3.
(cherry picked from commit
253fba01a7adf20303ec65cbe256681df588f065)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
890ba0c5b490b9272b9dcd58db876d4f3dcb0aea)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Fri, 7 Feb 2025 15:57:28 +0000 (16:57 +0100)]
IMPORT: plock: give higher precedence to W than S
It was noticed in haproxy that in certain extreme cases, a write lock
subject to EBO may fail for a very long time in front of a large set
of readers constantly trying to upgrade to the S state. The reason is
that among many readers, one will succeed in its upgrade, and this
situation can last for a very long time with many readers upgrading
in turn, while the writer waits longer and longer before trying again.
Here we're taking a reasonable approach which is that the write lock
should have a higher precedence in its attempt to grab the lock. What
is done is that instead of fully rolling back in case of conflict with
a pure S lock, the writer will only release its read part in order to
let the S upgrade to W if needed, and finish its operations. This
guarantees no other seek/read/write can enter. Once the conflict is
resolved, the writer grabs the read part again and waits for readers
to be gone (in practice it could even return without waiting since we
know that any possible wanderers would leave or even not be there at
all, but it avoids a complicated loop code that wouldn't improve the
practical situation but inflate the code).
Thanks to this change, the maximum write lock latency on a 48 threads
AMD with aheavily loaded scheduler went down from 256 to 64 ms, and the
number of occurrences of 32ms or more was divided by 300, while all
occurrences of 1ms or less were multiplied by up to 3 (3 for the 4-16ns
cases).
This is plock commit
b6a28366d156812f59c91346edc2eab6374a5ebd.
(cherry picked from commit
9dd56da73072f88e1b4a81923616b5276013b186)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
101c047e04521318791c7ffb5634a984727bc8ae)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Thu, 30 Jan 2025 15:32:35 +0000 (16:32 +0100)]
MEDIUM: epoll: skip reports of stale file descriptors
Now that we can see that some events are reported for older instances
of a file descriptor, let's skip these ones instead of reporting
dangerous events on them. It might possibly qualify as a bug if it
helps fixing strange issues in certain environments, in which case it
can make sense to backport it along with the following recent patches:
DEBUG: fd: add a counter of takeovers of an FD since it was last opened
MINOR: fd: add a generation number to file descriptors
DEBUG: epoll: store and compare the FD's generation count with reported event
(cherry picked from commit
8235a24782e528b9bf8ca9dd69c0a147556dfcb5)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
cdb4027d54baab39347bf2e1da48aff7d1af5429)
Signed-off-by: Willy Tarreau <w@1wt.eu>