Until now, if the call to 'ssl_sock_load_ocsp' raised an error, because
of a missing issuer certificate for instance, we would send a fatal
error code and the init would fail.
With this patch we only consider this as a non-critical error and we
will send a warning instead. In such a case the OCSP response will not
be loaded and stapling or auto update features will not work for the
certificate.
This keyword, when alone, was already deprecated in 3.3 and marked
for removal in 3.5. Now using it results in an error message inviting
to start haproxy with -W or -Ws instead.
In addition, "master-worker exit-on-failure", which used to automatically
enable the master-worker mode, now doesn't set it and complains if used
without. -W/-Ws. A special case was made for config checks though (-c).
Also worth noting that "daemon" applies to master as well as max-reloads,
and will soon need to be adjusted.
for url-dec converter, a trailing null byte is added at the end of the input
sample because it is requested by url_decode() function. However, when the
buffer was full, the last byte was crushed by the trailing null byte. In
that case, the last character was lost and not decoded.
Now, the converter just fails by returning 0.
This patch could be backported to all supported versions but it is a very
minor issue.
These new samples can be used to get, resepectively, the backend value for
the maximum connection retries and the current value applied to the stream.
They could be used to know if a "set-retries" action was performed.
A dedicated reg-test was also added.
It is possible to overwrite the configured max-retries value with the
"set-retries" action. However there is an issue with the listeners. When the
backend is set, this value is reset with the backend value. However, when
the backend is unchanged, the custom value must be preserved.
To fix the issue, when the stream is created, we set the max-retries to the
listener value. It remains 0 for pure-frontend. And the backend value is
used only if it is not the same proxy.
The documentation of set-retries action was improved.
This patch should be backported as far as 3.2.
Timeouts may be overwritten via the action "set-timeout". Client timeout is
supported on frontends, connect/queue/tunnel/server timeouts are supported
on backends and tarpit timeout is supported on both.
However, there is an issue with the listener. In that case, the action is
always executed in the frontend context and the backend-side timeouts are
then reset, when the backend is selected. If it is another backend, it is
logical. But most of time, the backend is unchanged. And in that case, these
timeouts must be preserved.
Documentation of set-timeout action was improved.
This patch should be backported to all supported versions. Be carefull, all
timeouts are not supported in 3.3 and lower (3.3->3.0: server/tunnel/client
- 2.8->2.6: server/tunnel)
When an HTX header or an HTX start-line is added or updated, a test is now
performed to be sure the whole headers size, including the start-line never
exceeds the regular buffer size. It is mandatory because it is now possible
to use larger buffers. However, it remains impossible to send HEADERS frame
in H2 and QUIC larger than a regular buffer. So we must be sure the HTTP
analysis will never produce too big headers because they will be rejected
later, at the forwarding stage. The purpose of large buffers is to be able
to store large payload, larger than regular buffers. Headers must remain
quite small.
This commit depends on the following one:
MINOR: htx: Add a field to save the headers data size
Both should be backported to 3.4 to avoid any trouble with large buffers. It
is not strictly speaking a bug, but it will avoid hazardous behavior
depending on the payload size.
The size of headers present in an HTX message are now counted. A dedicated
field was added in the HTX structure to do so. This patch is quite simple
but it will be mandatory to be able to perform some tests on headers. One of
them is to be sure headers never exceed the regular buffer size, even when a
large buffer is used.
All calls to get_trash_chunk_sz() and alloc_trash_chunk_sz() were reviewed
to be sure we never retrieve a large chunk when it is not expected. Some
calls were not upgrade, but several calls now rely on get_best_trash_chunk()
and alloc_best_trash_chunk() functions.
The idea of the fix is to get a large buffer only when the original buffer
is already a large buffer and the expected size of data exceeds the size of
a regular buffer. This should prevent unexpected memory usage.
This commit relies on the previous one:
MINOR: chunk: Add function to get a large/regular chunk depending on a buffer
Both must be backported to 3.4.
get_best_trash_chunk() function was added to be able to get a large or a
regular chunk depending on a given size but never larget than a given
buffer. It will be usefull in a futur fix, to prevent unexpected large chunk
usage.
alloc_best_trash_chunk() function is similar but instead of returning one of
the static chunks, it allocate it from the corresponding pool, large or
regular.
For a chunk size exactly equal to bufsize/2, the "max_size" variable could
underflow if a separator is provided. Indeed, "max_size" is first set to
(trash->size - 2 * chunk_size), so to 0. And on the first iteration, the
separator length is removed, making it to underflow.
On 3.3 and lower it is especially an issue with samples larger than
bufsize/2 because data could be written ouside of the buffer. On 3.4 and
3.5, it is not an issue because we fail to retrieve a chunk.
This should be backported to all supported versions.
The proxy used to parse healthcheck sections was not inserted in the proxies
list. So its initialization was not properly finalized. Among other things,
it was an issue for some arguments, like regular expressions.
This proxy is now inserted in the proxies list.
This patch should fix the issue #3440. It must be backported to 3.4.
When encountering an unexpected escape sequence, treat it as a literal
character instead of skipping it.
This is a deliberate choice for two reasons:
- to avoid a desynchronization between the h->data counter and the
buffer content, which would otherwise leave uninitialized memory
("garbage") in the destination buffer;
- to ensure that an invalid configuration string triggers a parsing
error immediately (fail-fast), rather than resulting in a silently
malformed configuration.
Thank to @dirkmueller for having reported this issue.
No need to backport. huf arrived with this current version.
When a stream is freed, hld_strm_task() was unconditionally re-queuing
the user task, even when no streams were left or when no update was
needed. This caused spurious task wakeups, which could incorrectly inflate
connection and request rate counters to come for the -R option implementation.
Only queue the user task if there are remaining streams to process, and
properly update the expiration time.
No need to backport. haload arrived with this current version.
Several QUIC related keywords were removed in 3.4. The legacy options
were marked as deprecated and scheduled for removal in 3.5. This patch
applies this removal for the upcoming 3.5.
Commit 061754b249 attempted to make it
possible to close file descriptors belonging to other thread groups by
using thread isolation.
The problem is, closing other thread groups' fds usually happens when we
destroy a listener, in which case we hold the listener lock. If any
other thread tries to get that lock while we're waiting for the thread
isolation, then they will deadlock.
This can happen with any type of listener, but it is easier to reproduce
with a suspend/resume loop with ABNS sockets, as a suspend translates to
a close here.
To fix that, instead of using thread isolation, do something similar to
what's done when the fd belongs to our thread group. Increase the tgid
ref counter, so that we're sure nobody will close the fd while we're
dealing with it, then set the FD_MUST_CLOSE bit and set thread_mask to
0.
At this point, if no thread was running on that fd, no one will and we
can safely close it. So just call _fd_delete_orphan() if the
running_mask is 0 and if the FD_MUST_CLOSE bit is still there, otherwise
we can safely assume another thread will take care of it.
This should be backported up to 2.8.
For frontend connections, quic_conn layer is able to reject any new
streams opened after MUX closure. This is necessary as the peer may not
have been notified yet of the closure.
This operation is unnecessary on backend side. This is due to the fact
that only HTTP protocols are currently supported on top of QUIC, with
requests initiated by the client. For requests started before the MUX
closure, either they are already completed or closed early with a
STOP_SENDING emitted during stream shut.
Prior to this patch, spurrious RESET_STREAM could have been emitted on
backend connections after MUX closure as quic_conn stream_max_bidi was
not correctly set. Now reject is only performed for frontend connections
so this should not occured anymore.
This should be backported up to 3.3.
Prior to this patch, shut stream callback only handles write channel
closure. In case of an early closure, a RESET_STREAM would be emitted.
On the frontend side in most cases this is sufficient as read channel is
already closed, as HTTP/3 GET requests has been fully received. However,
this may not be the case for POST requests. Also, on the backend side,
haproxy acts a client. In this case, a stream early closure will
typically happen before receiving the full response. Nothing will be
emitted (RESET_STREAM is unnecessary as write channel is already
closed), thus the server peer will continue to emit.
To fix this situation, the current patch implement read channel closure
on shut if SE_SHR_RESET is set. Callback lclose from app_ops is called
with a new dedicated mode for read channel closure, which will result in
a STOP_SENDING frame generated by H3 and hq transcoders. This will
instruct the peer to stop emission.
This should be backported up to 3.3. Note that this depends on the
following patch :
dde3ee06c30f20091443bdafdda0e0294f7ac26b
MINOR: mux_quic: use separate error code for STOP_SENDING
Prior to this patch, a single error code was registrable at the QCS
level. This code was used both for RESET_STREAM and STOP_SENDING
emission. It was specified via qcc_reset_stream().
This patch extends the API so that now a dedicated error code is
implemented for STOP_SENDING as well. This may be necessary as both
frames can be sent in different context, with a diverging error code.
This patch is required to implement STOP_SENDING emission during shut
callback when read channel is closed.
When stream detach callback is called, the default behavior is to free
the associated QCS instance. However, QCS may be preserved in so-called
detached state if there is remaining data to sent.
This condition is checked via qcs_is_close_local() which ensures that
either FIN or a RESET_STREAM was emitted. However, this does not take
into account a scheduled STOP_SENDING emission, which can happen in case
of request abort for example.
Adjusts qcm_strm_detach() to also take into account STOP_SENDING
emission before freeing or keeping a detached QCS instance. As a
complement, QCS have to be purged after STOP_SENDING emission when
reaching completion.
On frontend side, this bug is probably only visible in case of HTTP/3
POST. When dealing with GET, FIN is most of the time received earlier,
which render STOP_SENDING unnecessary. This issue however has a bigger
impact on the backend side. In case of stream abort, for example on
timeout, the server may be left unnotified and will continue to emit
STREAM data despite QCS closure on haproxy client side.
Note that this fix also has a side effect on backend connection reuse.
Indeed it may increase the rate of QCS in detached state. This may
prevent an idle connection to be reinserted in the server pool, without
any possibility to reinsert it later. In the end this causes a lower
reuse rate. This is an issue which must be addressed in a dedicated
patch. For now, add a COUNT_IF_HOT() to report when such situation
occurs.
This should be backported to all stable releases, after a period of
observation. COUNT_IF_HOT() is unnecessary on 3.2 and below.
The various trace sources always have the same pattern:
- trace events
- trace source
- trace decoding function
Dropping these when USE_TRACE=0 definitely makes sense. There are two
modes of definition here:
- those designed after mux_h2 which interleave #define and the entry
definition in the event. These ones cannot be removed without a
significant code move to split the #define and usage apart. Instead
here we mark the struct __maybe_unused, so that the compiler will
just not implement it.
- those designed like stream.c where defines are separated. Here we
can simply enclose the events definition inside the USE_TRACE guard
For most of these the static declaration of the trace function was moved
after the events so that the #if defined(USE_TRACE) could be placed between
the two. Nothing else was changed.
This saves another 51 kB of object code when USE_TRACE=0.
It requires essentially a few ifdefs and to add a dummy definition of
h3_trace_header() to completely disable traces in H3. This reduces the
object code by 35 kB.
The function takes a buffer in argument which is the target buffer. The
first calls properly use it but the subsequent ones, probably due to
reused/moved code, directly write into &trace_buf, thus ignoring the
buf argument. Fortunately all call places pass &trace_buf for buf, so
it currently has no impact but could possibly change.
No backport is needed, but it doesn't hurt to backport it if it helps.
By just moving a few definitions, creating two dummy inline functions and
a few ifdefs, we can get rid of the entire trace generation code in the
H2 mux and save ~96 kB. This is what this patch does. Even the trace_h2
struct is removed in this case.
When disabling traces, "conn" isn't used between ctx assignment and its
first usage, and as usual, gcc wrongly believes that a null check in a
shared function implies the checked argument may be NULL where it's used,
leading to this warning:
src/ssl_sock.c: In function 'ssl_sock_handshake.constprop':
src/ssl_sock.c:6049:7: warning: null pointer dereference [-Wnull-dereference]
Assigning ctx after the conn_ctrl_ready() check is sufficient to shut it
up, so let's do this. It should also result in slightly better code.
The TRACE macro allows to leave empty args and automatically turns them
into zeroes. However it also limits how we can remap the macro, because
functions do not accept this for example. There are very few places all
over the code where ',,' exists in TRACE calls, so let's explicitly add
the 0 there. It could even make some editors' syntax highlighting happier.
In quic_transport_params_store(), we call qc_early_transport_params_cpy()
if edata_accepted is set, which copies one by one all tx_params into the
locally allocated etps struct, and later after updates we call
qc_early_transport_params_validate() to check if they changed. It turns
out that when USE_TRACE is disabled, gcc 4 to 13 are confused and believe
that one or several of the fields compared in the later function might be
used uninitialized. A careful code inspection proves that this is not the
case. Setting them to zero in the _cpy() function makes the warning
disappear, it's really an issue related to variable propagation it seems,
which can explain why it doesn't happen with traces (code is a bit more
complex). Gcc-13 only emits a warning about a single field, and gcc-14
completely solved it. Playing with consts, __maybe_unused etc has no
effect.
One thing works however, it is to mark the _validate() function noinline.
In this case it is implemented normally and the compiler doesn't put its
nose into the propagation path and doesn't complain.
Such comments are always scary because one may seriously wonder whether
the compiler emits valid code when it says this...
It should be backported to 3.4 which experiences the same warning with
USE_TRACE=0.
A QUIC stream may be aborted to ignore future data read. This also
prepares a STOP_SENDING frame to instruct the peer to close its write
channel.
This capability is exposed via qcc_abort_stream_read() which should be
guarded against multiple invokation for a single stream. This was
checked via QC_SF_TO_STOP_SENDING flag. However, this flag is resetted
once STOP_SENDING frame is emitted. Thus in theory it could be possible
to emit several STOP_SENDING for a single stream.
This patch improves this by using QC_SF_READ_ABORTED flag check. This
flag is set during qcc_abort_stream_read() and never removed even after
STOP_SENDING frame is emitted.
This bug was never encountered in a real situation. However, this patch
is necessary to definitely guarantee that it cannot occur.
This should be backported up to 2.8.
On success, h3_rcv_buf() returns the number of parsed STREAM bytes which
are removed by the caller afterwards. A success value is mandatory so
that the underlying QUIC packet is acknowledged.
When H3 parser detects an error during HEADERS or DATA parsing, the
stream or the connetcion is flagged for closure. If there is remaining
frames, they are simply ignored and h3_rcv_buf() returns the remaining
input buffer size.
However, this value is wrong in case one or several frames were already
parsed before the invalid frame in the same h3_rcv_buf() invokation.
This instructs caller to only remove a subset of the data and parsing is
restarted on a random boundary. Most of the times this generates again a
new final yet invalid error, possibly overwriting a stream error with a
full connection closure.
This patch fixes the return value in case of an error during HEADERS or
DATA parsing by ensuring that total variable is always incremented
instead of being directly assigned.
This must be backported up to 2.8.
http_cookie_merge() function is responsible to add a cookie header and merge
all values from a list. However, it was performed by appending values by
hand, using the pointer to the header value and changing the block
length. This was totally by-passing the HTX API.
For now, there is now bug because the function is called by h2 and quic
muxes when a HTTP message is parsed. And the cookie header is the last one
inserted. The HTX message is never fragmented and data from other blocs
cannot be overwritten. But, it could be an issue if it is called in another
context, from the HTTP analysis for instance.
To fix the issue, the function now relies on htx_replace_blk_value()
function to add the value separator first and then a cookie value.
This patch must be backported as far as 2.6.
In qcs_http_rcv_buf() function, when the buffers cannot be swapped and
htx_xfer() function is called, the way the EOM flag is handled is buggy. The
htx_xfer() function is responsible to tranfer HTX flags from the QCS message
to the CS one. And when it is performed, HTX flags of the original message
are reset.
So, the following test on the EOM flag when the QCS message is empty is
never true. Because of this bug, QC_SF_EOI_SUSPENDED flag is never tested on
this code path and <fin> variable is not set to 1 as expected.
To fix the issue, we must test the EOM flag on the CS message.
This patch must be backported to 3.4.
Recent fix of some HTX muxes to drain remaining data when the stream is in
closed state revealed a bug, mainly due to a corner case of the HTX API.
It is possible to have an empty HTX message with a parsing/internal
error. In that case, the underlying buffer remains full. It is mandatory to
prevent any buffer release and be sure the error will be handeled.
On the other end, at several places, when data must be transfer from an HTX
message to another one, we try to swap underlying buffers instead of
performing a bloc-per-bloc copy. To do so, we rely on b_xfer() function. One
condition is that the destination message must be empty. And here is the
issue. The HTX message can be empty but the buffer can also be full because
an HTX error was triggered earlier and not handled yet. In that case,
attempting to call b_xfer() leads to a crash because the destination buffer
is full. It is not expected to call b_xfer() if there is not enough space in
the destination buffer.
So, it appears the HTX API should be improved/fixed but first of all, the
bug must be fixed. Especially because stable versions are also affected. The
htx_is_empty_noerr() function was added to know if a HTX message is empty
and no error was reported on it. And this function is now used, instead of
htx_is_empty(), to know if we can safely swap the underlying buffers or not.
the FCGI, H2 and QUIC multiplexers are concerned. The HTTP client and the
applet API were also fixed while it seems harder to trigger the bug at these
places.
The fix must be backported to all supported versions.
ASCII characters with a value smaller than '0' were not properly
detected as invalid characters, leading to incorrect behavior. The
strl2irc() and strl2llrc() functions are not impacted because this
situation is detected by their overflow checks.
Fixes Github issue #3357.
The default global configuration tuning settings (tune.memory.hot-size,
expose-experimental-directives, and tune.pipesize) were lost when the
-G option was used.
The bug was caused by an incorrect scope: these default settings were
nested inside the block that generates the default global header, which
is skipped/overwritten when -G is provided. Fix this by closing the
conditional block early.
This patch depends on this commit:
"01f4e33ea MINOR: hbuf: new lightweight hbuf API"
Must be backported to 3.4.
app_log() and send_log() build the message with vsnprintf(), which stops
at the first NUL byte and therefore cannot emit an arbitrary binary
payload.
Add two variants that pass a pre-built <msg> of <len> bytes straight to
__send_log() without formatting it, so embedded NUL bytes are preserved:
* app_log_raw() : takes an explicit list of loggers and a tag
* send_log_raw() : derives both from a proxy
The send path still strips trailing LF / NUL bytes (kept for the legacy
text logs), so the message must be self-terminating by its own encoding
and must not rely on a meaningful trailing '\n' or NUL.
This patch imports the implementation of haload, a lightweight,
multi-threaded traffic generator designed to benchmark HTTP infrastructures
under heavy loads. Built onto HAProxy's highly scalable
architecture, it natively supports HTTP/1, HTTP/2, and HTTP/3 (QUIC).
It uses the previously exposed initialization functions, the no-listener mode,
the lightweight hbuf API, and the specialized hldstream object types to
dynamically derive and generate its configuration in memory from basic
command-line inputs. By leveraging HAProxy's internal HTX
(Internal HTTP Native Representation) format, haload abstractly manipulates
HTTP elements independently of the wire protocol. This
abstraction allows it to generate unified requests and process responses
seamlessly across HTTP/1.1, HTTP/2, or HTTP/3 without duplicating the payload
handling logic for each version.
- Makefile:
Introduce the 'haload' compilation target and define HALOAD_OBJS.
- src/haload.c, include/haproxy/haload.h:
Add user and stream task scheduling handlers, HTX-driven traffic orchestration
mechanisms, and terminal benchmarking statistical summary rendering.
- src/haload_init.c:
Implement program arguments parsing, fileless HAProxy memory configuration
generation, and target URL allocations.
- src/stconn.c:
Wire up sc_attach_mux() to properly allocate the specific tasklet
context when dealing with a haload stream.
- doc/haload.txt:
Add detailed documentation covering compilation, flags, and usage examples.
Export _srv_parse_kw() and srv_postinit() so they can be called from
haload (to come), which needs to configure servers using HAProxy's configuration
parser keywords.
This patch exports sc_new() by removing its static storage class and
adding its prototype to include/haproxy/stconn.h.
This is required to allow external modules, such as the upcoming haload
benchmarking tool, to allocate and initialize new stream connectors
from a stream endpoint descriptor (sedesc).
haload is the successor to the h1load HTTP benchmarking tool.
This patch adds haload stream definitions as arguments for the TRACE API.
These will be used by the upcoming haload module, which will handle
hldstream struct objects instead of regular stream structs.
Introduce the new <no_listener_mode> global variable to define a new operating mode
for haproxy. This variable can be set to 1 to allow haproxy to start without
any listeners. Without such a setting, haproxy refuses to start without listener.
During the initialization cycle, setting this variable to 1 ensures that the
lack of configured listeners is no longer treated as a fatal error. This allows
programs based on haproxy source code to initialize the stack and use its
features even without a frontend. This will be the case for haload.
Add a new lightweight hbuf API to buffer formatted strings, similar to the
existing buffer API (struct buffer), extracting the code which already does this
in haterm_init.c. This is used by haterm to build its configuration in memory
(fileless mode). And this will be used by haload to do the same thing.
Update haterm to use this new API.
Note: hstream_str_buf_append() has been renamed to hbuf_str_append().
port_range was never freed. That used not to be a problem, but now that
we can dynamically add and remove servers, it becomes one, as that leads
to a memory leak each time a server with a "source" directive is destroyed.
However, just adding a free() is not enough. We have to add a refcount,
because the server is not the only one with a reference to it. We may
also have one in fdinfo, so that we know which port to release when we
finally close the fd.
So add a refcount, and make sure to call port_range_release() when a
server is destroyed.
This should be backported up to 3.0.
Add the ability to rename a HAProxy server at runtime via the CLI:
set server <backend>/<server> name <newname>
This is useful in slot-based dynamic scaling setups where servers are
pre-allocated with generic names (e.g. srv001, srv002) but the operator
wants the names to reflect the current workload (e.g. pod name or
IP:port) for observability and server-state-file consistency.
The implementation:
- validates the new name: non-empty, passes invalid_char() check
(allows [A-Za-z0-9_:.-]), and fits in the event data name field
- requires the server to be administratively in maintenance mode
(same precondition as 'del server')
- rejects the rename if the server has SRV_F_NAME_REFD set (use-server
target, track target, sample-fetch ARGT_SRV referent) - keeps the
running state consistent with the configuration text
- re-indexes the server in the name tree under thread_isolate(),
mirroring the locking pattern used by 'add server' / 'del server'
- publishes a new EVENT_HDL_SUB_SERVER_NAME event with the old and
new names so downstream consumers (logs, observability backends)
can track the rename
- frees the old name immediately under thread isolation: srv_name
sample consumers (ACLs, log formats, ...) act on the fetched pointer
within the current task and do not retain it across wake-ups, so
no extra deferred-free machinery is needed
There is no opt-in directive: like 'add server' and 'del server', the
operation is gated by the server's properties rather than by a
per-backend toggle. This avoids the runtime-surprise failure mode
where an operator discovers at the CLI that renaming is forbidden by
a missing 'option server-rename' rather than by an actual structural
reference.
This feature was discussed in:
https://github.com/haproxy/haproxy/issues/952
