MAJOR: htx: Use htx_set_eom() instead of setting HTX_FL_EOM by hand
The function htx_set_eom() takes care to the HTX_FL_EOM on the HTX message,
by first adding a EOT block if the message is empty. So let's use it
wherever possible.
This commit is tagged as MAJOR because it is an API change.
In the commit 32b51cdf3 ("BUG/MINOR: config: reject configs using HTTP with
bufsize >= 256 MB"), the maximum possible size for buffers is limited to 256
MB. So, on HTX side, it means a bloc cannot have a relative address higher
than this value. So on the 32-bits integer used to store the relative block
address, only 28 bits can be used, and 4 bits remains unused.
So these 4 bits were reused to store flags on block themselves. In this
patch, the support for these flags is introduced and the HTX API was adapted
to deal with it.
MEDIUM: htx: Insert headers and trailers before corresponding end-of block
htx_add_header() and htx_add_trailer() were updated to automatically insert
the block at the right place, just before the corresponding end-of block.
Indeed, it is invalid, unexepected and probably unsupported to insert a
header after the EOH block without moving it. So it is easier to handle it
in HTX function directly.
MEDIUM: htx/tree-wide: Remove tests on HTX_BLK_UNUSED when possible
Thanks to previous changes, it is possible to remove all tests on
HTX_BLK_UNUSED outside of the HTX API. Indeed, because htx_get_*_blk() and
htx_remove_blk() functions are now skipping automatically unused blocks, we
are sure to never get such blocks from outside.
MEDIUM: htx: Skip UNUSED blocs when getting previous,next and first blocks
htx_get_prev_blk(), htx_get_next_blk() and htx_get_first_blk() functions
were modified to automatically skip UNUSED blocks. So now, a valid block or
NULL is returned. In addition, htx_remove_blk() was adapted accordingly to
also skip UNUSED blocks.
Thanks to this change, the loops on HTX messages could be simplified by
removing test on UNUSED blocks, while it remains harmless.
BUG/MINOR: mux-h2: Reject trailers received on a tunneled stream
For a tunneled stream, the end of the HTTP message is reported just after
the headers. It is an "early" EOM: from the HTTP point of view, the message
is finished and only tunneled data may be exchanged then.
However, a HEADERS frame received after the initial one on such a stream
was still handled as a trailers block. The trailers were thus pushed in an
HTTP message already ended, leading to an invalid HTX message where blocks
are inserted after the end of the message. The stream is then aborted when
the outgoing multiplexer handles the unexpected block. So a client is able
to kill its own tunnel by sending a bogus HEADERS frame.
Such a frame is now rejected with a stream error (PROTOCOL_ERROR). The test
is performed in h2c_dec_hdrs(), at the only place where a HEADERS frame is
identified as a trailers block. This way, both directions are covered. The
aborted tunnel attempts are also covered because the early EOM was reported
in that case too.
This patch should be backported to all stable versions. Note that
h2c_report_glitch() only exists since 3.0. The corresponding line must be
removed for older versions.
MINOR: htx: Add macros for the header/trailer name and value max lengths
The maximum length of the name and the value of a header or a trailer were
hardcoded in the different places where they are tested. Let's define
HTX_HDR_NAME_MAX_LEN and HTX_HDR_VALUE_MAX_LEN to avoid these magic values
and document why these limits exist.
BUG/MAJOR: htx: Check the header/trailer length limits when one is updated
The name and the value lengths of a header or a trailer are respectively
encoded on 8 and 20 bits in the block descriptor. So the name must not
exceed 255 bytes and the value must not exceed 1048575 bytes. This is
properly tested when a header or a trailer is added, since commit 3b69886f7
("BUG/MAJOR: htx: fix missing header name length check in
htx_add_header/trailer"). But no test was performed when one is updated.
htx_replace_blk_value(), used to update a part of a header or a trailer
value, may grow it above the limit. And htx_replace_header(), used to
replace a full header, does not check its arguments at all. In both cases,
the length silently overflows on the block type, changing it. For instance,
a HDR block becomes an EOH block while its value length wraps to a small
value.
Only the value length limit is reachable from the configuration, via a
"replace-header" or a "replace-value" rule, and only if "tune.bufsize" was
set above 1MB, because the block must first fit in the buffer. In that case,
a client sending a large enough header is enough to hit the issue.
So, perform the same tests than those performed when a header or a trailer
is added, instead of corrupting the message. In addition, a BUG_ON() was
added in htx_change_blk_value_len() and htx_set_blk_value_len() to catch any
other caller missing the test.
Reported by: Adam Crawford
This patch must be backported to all stable versions.
BUG/MINOR: tools: Don't try to anonymize a NULL string
hash_anon() function is used to anonymize a string. It is called at several
places (via HA_ANON_* macros), especially from debugging functions. It is
hard to be sure the string to hash is always defined. So let's return
immediately if it is NULL.
Among other things, this fix avoids to get a crash when the server name
created by a LUA CO-SOCKET is anonymized. Indeed, this server is created
with no name.
This patch should fix the issue #3491. It must be backported as far as 2.8.
BUG/MEDIUM: htx: Fix the position returned by htx_defrag()
In htx_defrag(), the <new> counter was used to compute the new position of
the start-line and of the referenced block, if any. But this counter was
incremented for each block of the source message while consecutive DATA
blocks may be merged when they are copied. In this case, no new block is
created in the temporary message, so <new> was desynchronized from the real
position.
As a consequence, htx_defrag() could return a block located after the tail
of the message, thus pointing on the payload part instead of a block
descriptor. The caller then used it to copy data, at a pseudo-random offset,
corrupting the message.
htx_replace_blk_value() is the only concerned caller because it is the only
one referencing a block that may be a DATA block. htx_replace_header() and
htx_replace_stline() reference a header or a start-line, always located
before any DATA block. So, to trigger the bug, an expansion requiring a
defragmentation must be performed on a DATA block preceded by at least two
mergeable DATA blocks.
The positions are now computed from the block really created or updated. The
<new> counter is thus no longer necessary and was removed.
The flaw exists since the HTX was introduced. However the only known way to
expand a DATA block this way is the decompression filter, added in 3.5-dev.
So no released version should be affected. This patch may be backported to
all stable versions as a precaution.
BUG/MINOR: ssl/ocsp: Report an error when an empty OCSP response is received
When the response of the OCSP update request has no payload, no OCSP
response is loaded but a successful update is reported anyway. The number of
successes is incremented, the last update date is refreshed and the entry is
reinserted in the update tree with the regular delay, as if everything was
fine.
The "Content-Type" header of the response is already checked to be
"application/ocsp-response". So an empty payload is an error. It is now
reported as an OCSP response check failure.
This patch depends on:
* "MINOR: ssl/ocsp: Remove the useless res_payload callback of the http-client".
MINOR: ssl/ocsp: Remove the useless res_payload callback of the http-client
The OCSP update task does not process the response payload on the fly. The
whole payload is accumulated in the http-client response buffer and only
retrieved when the response is fully received, on the res_end callback. So
there is no reason to define a res_payload callback. It was only used to set
the HC_OCSP_RES_BODY flag, to know if a payload was received or not. Testing
the content of the response buffer is enough to do so.
The res_payload callback and the HC_OCSP_RES_BODY flag are thus removed.
This way, the OCSP update task now works the same way than the ACME one.
BUG/MEDIUM: http-client/cli: Notify the http-client when the response is consumed
In HTX mode, the "httpclient" CLI command does not use httpclient_res_xfer()
to consume the response. It dumps the HTX message stored in the http-client
response buffer and resets it by hand.
Thus, since commit b24260ec9 ("BUG/MEDIUM: http-client: Only consume input
buffer when hc one is empty"), the applet stops to consume data and is never
woken up again. The command is stuck just after the response headers, till
the server timeout expiration.
httpclient_res_consumed() is now called when the response buffer is reset.
This patch depends on :
* MINOR: http-client: Add a function to notify some data were consumed
It must be backported as far as 3.3 with commit b24260ec9.
MINOR: http-client: Add a function to notify some data were consumed
Since commit b24260ec9 ("BUG/MEDIUM: http-client: Only consume input buffer
when hc one is empty"), the http-client applet stops to consume data as soon
as its response buffer is not empty. Only httpclient_res_xfer() is able to
unblock it. However, some callers are directly accessing the httpclient
response buffer. These ones have no way to notify the applet it can fill
the response buffer again.
httpclient_res_consumed() is added to do so and is now called from
httpclient_res_xfer(). In addition, the applet is no longer only woken up
when the response buffer is emptied, but as soon as some room was released.
Because the response buffer is no longer necessarily released when data are
consumed, the I/O handler must now rely on b_data() instead of b_is_null()
to decide to stop the reception. Otherwise, an emptied but still allocated
buffer would block the applet for good.
This patch is required to fix a bug and must be backported as far as 3.3
with the next commit.
BUG/MEDIUM: acme: Set HTTPCLIENT_O_RES_ACCUM option on the http-client
The ACME task does not consume the response payload on the fly. It waits for
the end of the response to parse the JSON object stored in the http-client
response buffer.
Thus, since commit b24260ec9 ("BUG/MEDIUM: http-client: Only consume input
buffer when hc one is empty"), the http-client applet stops to consume data
as soon as the first part of the payload is stored in its response buffer,
waiting for the caller to consume it. But it never happens. So, when the
response is not received in one time, the ACME task is stuck till the server
timeout expiration.
The HTTPCLIENT_O_RES_ACCUM option is now set to instruct the http-client to
continue to fill its response buffer.
This patch depends on :
* BUG/MEDIUM: http-client: Don't use the httpclient context if it was released
* MINOR: http-client: Add an option to not stop the reception of the response
It must be backported as far as 3.3 with commit b24260ec9.
BUG/MEDIUM: ssl/ocsp: Set HTTPCLIENT_O_RES_ACCUM option on the http-client
The OCSP update task does not consume the response payload on the fly. It
waits for the end of the response to check and load the OCSP response stored
in the http-client response buffer.
Thus, since commit b24260ec9 ("BUG/MEDIUM: http-client: Only consume input
buffer when hc one is empty"), the http-client applet stops to consume data
as soon as the first part of the payload is stored in its response buffer,
waiting for the caller to consume it. But it never happens. So, when the
OCSP response is not received in one time, the update is stuck till the
server timeout expiration and always fails.
The HTTPCLIENT_O_RES_ACCUM option is now set to instruct the http-client to
continue to fill its response buffer.
This patch depends on :
* BUG/MEDIUM: http-client: Don't use the httpclient context if it was released
* MINOR: http-client: Add an option to not stop the reception of the response
It should fix issue #3498. It must be backported as far as 3.3 with commit b24260ec9.
MINOR: http-client: Add an option to not stop the reception of the response
Since commit b24260ec9 ("BUG/MEDIUM: http-client: Only consume input buffer
when hc one is empty"), the http-client applet stops to consume data as soon
as its response buffer is not empty. The applet is only unblocked when the
caller consumes these data by calling httpclient_res_xfer().
However, some callers are not consuming the response payload on the
fly. They are waiting for the end of the response to retrieve the whole
payload from the httpclient response buffer. For these callers,
httpclient_res_xfer() is never called. So the applet remains blocked and the
response is never fully received. The transfer is stuck till the server
timeout expiration.
To fix this, the HTTPCLIENT_O_RES_ACCUM option is added. It must be set by
the callers accumulating the whole response payload in the httpclient
response buffer. In that case, the applet must not stop the reception. Of
course, it means the whole payload must fit in the response buffer for these
callers. But it was already the case.
This patch depends on:
* "BUG/MEDIUM: http-client: Don't use the httpclient context if it was released".
It is required to fix a bug and must be backported as far as 3.3 with the
next commits.
BUG/MEDIUM: http-client: Don't use the httpclient context if it was released
The I/O handler of the http-client applet may be called after the applet
release. In this case, the httpclient context is no longer available and the
I/O handler must not use it. It is properly handled at the beginning of the
function. However, the code added at the end, on the "out" label, to stop
the data reception is evaluated in all cases, including when the httpclient
context is NULL. It leads to a NULL-pointer dereference.
So, now, <hc> is tested first.
This patch must be backported as far as 3.3 with commit b24260ec9
("BUG/MEDIUM: http-client: Only consume input buffer when hc one is empty").
BUG/MEDIUM: task: Do not destroy a task that is not ours in task_destroy
In task_destroy(), if the task is not exclusively ours, then just use
task_kill(). If it is a shared task, then there is a race condition if
another thread tries to schedule it at the same time, which could result
in the task being scheduled after been freed.
Released version 3.5-dev7 with the following main changes :
- BUG/MINOR: quic: leak of the NEW_TOKEN frame on token generation failure
- MINOR: quic: implement the AEAD integrity limit
- DOC: config: clarify tune tune.bufsize doc regarding various limits
- BUG/MEDIUM: hlua: Never release a cosocket applet from the GC
- BUG/MINOR: mux-h1: Return an error on trailers if last chunk is unfinished
- BUG/MINOR: hlua: Always return nil if there is no data to dup for HTTP messages
- BUG/MEDIUM: htx: Don't reset flags when source must be preserved during transfer
- BUG/MINOR: flt-http-comp: Rely on htx_set_eom() to restore EOM flag
- MINOR: httpclient: Don't manually set HTX_FL_EOM flag after calling htx_xfer()
- MINOR: htx: Remove htx_xfer_blks function
- CLEANUP: htx: Remove ref to EOM block on HTX_FL_EOM flag comment
- DOC: htx: Fix comment of htx_expect_more() function
- REORG: htx: Move functions to deal with first block after ones about next block
- MINOR: mux-h1: loop on HTX blocks using _blk() functions to look for WS key
- BUG/MINOR: stream/debug: harden the stream dump function regarding signals
- MINOR: stconn: prefer to replace endpoints rather than detaching them
- BUILD: fd: unshare is not available on NetBSD
- MINOR: ring: add "-r" to "show events" to dump in raw mode (undoes "-s")
- MINOR: ring: permit to combine options on "show events"
- MINOR: ring/cli: improve the "show events" parser regarding options
- MINOR: ring/cli: also support "show events -h" to display the help
- MINOR: http: define new HTTP status codes for parsers
- MINOR: http-htx: add a new generic trailers parser
- REGTESTS: http-messaging: add tests for the H2 and H3 trailers
- MEDIUM: mux-h2: rely on the generic trailers parser
- MINOR: mux-h2: report more accurate traces and glitches on trailers errors
- CLEANUP: h2: get rid of the now unused h2_make_htx_trailers()
- MEDIUM: h3: rely on the generic trailers parser
- BUG/MINOR: h3: reject truncated frames with mandatory payload
- BUG/MEDIUM: h3: do not block FIN on empty DATA frame
- BUG/MINOR: h3: handle empty HEADERS frame as specified
- BUG/MINOR: h3: handle unknown frame type on request stream as specified
- BUG/MINOR: h3: only increment frame type counter on new header
- BUILD: fd: Make sure to include sched.h
- BUG/MEDIUM: http-ana: apply tunnel timeout with data filters
- SCRIPTS: run-regtests: Increase internal buffer size used for logs
- REGTESTS: specify explicit chroot in confs using unix-stream servers
- MAJOR: startup: use chroot auto by default when available
- BUG/MINOR: ssl: don't pass the address of a size_t as an int* to OpenSSL
- BUG/MEDIUM: map: do not overwrite the fetch context in pat_match_regm()
- REGTESTS: map: check map_regm inside an ACL over an iterating fetch
- BUG/MINOR: cli: Make sure two instances of _getsocks don't run
- BUG/MEDIUM: listener: Don't drop suspend/resume deferred requests
- MEDIUM: twork: Introduce a new cross-thread/thread-group deferred work API
- MEDIUM: listener: make the rx_agent use the generic twork API
- MINOR: fd: add fd_tables_are_split() to test for split FD tables
- BUG/MEDIUM cli: dump other thread groups' FDs from their own threads
- BUG/MINOR: stream: resolve foreign groups' addresses for "show sess"
- REGTESTS: http-messaging: add tests for the H1 trailers
- MINOR: http-htx: pass extra flags to the HTTP trailers parser
- MINOR: http-htx: condition uppercase acceptance in trailer names to HTTP_PF_UPCASE_OK
- MEDIUM: h1: switch to the generic trailers parser
- REGTESTS: update trailers tests to cover H1's refusal of forbidden names
- BUG/MEDIUM: http_act: fix normalize-uri normalizer selection with a condition
- REGTESTS: uri_normalizer: test a normalizer used with a condition
- BUG/MEDIUM: cli: Don't parse next command if input buffer is empty
- BUG/MEDIUM: cli: Don't read a command or payload line into a full buffer
- BUG/MINOR: cli: Reject payload with no command
- BUG/MINOR: pattern: do not match a string with an embedded \0 on its prefix
- REGTESTS: http-rules: check "-m str" on a sample with an embedded \0
- BUILD: ssl: ASN1_STRING_length() is deprecated since OpenSSL 4.1
- IMPORT: uslz: reject codes from an empty distance table
- MINOR: htx: don't reserve a block when there is no free space left
- MEDIUM: jwt: Reject "none" alg in jwt_verify by default
- BUG/MINOR: ssl: Fix leak of ca_list in cafile_entry
- BUG/MINOR: sample: reject a \0 byte in url_dec, json_query and jwt_*_query
- DOC: configuration: clarify the validity ranges of sample types
- BUG/MEDIUM: ssl: don't set a 0 timeout on an expired TLS session
- BUG/MEDIUM: htx: Reserve a block descriptor for zero-sized blocks
- CLEANUP: stream: Remove an excess newline in the stream dump
- BUG/MEDIUM: tcp-rules: Don't restart tcp-request content evaluation on yield
- BUG/MINOR: proxy: pass PR_FL_DEF_EXPLICIT_MODE through cascaded defaults
- MAJOR: filters: Revert the filter-sequence directive
- MINOR: promex: properly use alt_name and val instead of hard-coded values
- MINOR: mworker: assign a unique ID to the master process
- MINOR: config: add a "worker-id" global keyword to identify the worker
- MINOR: sample: add the "master_id" and "worker_id" sample fetches
- MINOR: stats: report the master and worker IDs in "show info"
- MINOR: promex: expose the master and worker IDs as labels
- MINOR: extcheck: pass the master and worker IDs to external checks
- MINOR: stats: report the process IDs on the HTML page with "stats show-node"
- REGTESTS: mcli: check the master and worker unique identifiers
- DOC: config: fix indent issue introduced with worker id
- MINOR: sample: add the "be_sort" converter
- REGTESTS: converter: add a test for the "be_sort" converter
REGTESTS: converter: add a test for the "be_sort" converter
Check the 4 supported element sizes, the unsigned ordering (an element
made of 0xff bytes must be sorted last), a single-element input and an
input whose length is not a multiple of the element size, for which the
conversion must fail.
This converter sorts in ascending numerical order the big-endian unsigned
integers of <elem_size> bytes stored in a binary sample. <elem_size> must
be 1, 2, 4 or 8, and the input length must be a multiple of <elem_size>,
otherwise the conversion fails.
Since the elements are unsigned and stored in network byte order, comparing
their bytes starting from the most significant one gives the numerical
order, so a simple memcmp() over the element width is enough and no byte
swapping is needed.
It is mainly intended to normalize a list of binary values whose order may
vary from one client to another before hashing it, for instance the cipher
list or the extension list of a TLS client hello:
DOC: config: fix indent issue introduced with worker id
A paragraph break and poor indent was left over by recent commit 3639aac31f ("MINOR: config: add a "worker-id" global keyword to
identify the worker"), let's fix it. No backport needed.
REGTESTS: mcli: check the master and worker unique identifiers
The test forces the worker's ID from the configuration so that it can be
matched exactly, both from the "worker_id" sample fetch function, from
$HAPROXY_WORKER_ID and from "show info", while the master's one, which is
generated, is matched against the UUID version 7 format, in the master's
environment, in "show info" and through the "master_id" sample fetch
function.
A second instance runs without a master and lets its worker ID be
generated, which permits to check that "master_id" then finds nothing and
that the generated identifier matches its exported copy.
Both instances also verify what "stats show-node" reports on the HTML
page, with and without a master.
MINOR: stats: report the process IDs on the HTML page with "stats show-node"
This adds the worker-id and (optionally) master-id on the output next to
the node when "stats show-node" is present, since these generally go
together. No escaping is needed since guid_is_valid_fmt() only allows
alphanumerical values and '.', ':', '-' and '_'.
MINOR: extcheck: pass the master and worker IDs to external checks
By default, i.e. unless "external-check preserve-env" is used, the
environment of an external check is not inherited but entirely rebuilt
from the curated list in extcheck_envs[], so the two new identifiers
would not be visible from a check script. Let's add them there, next to
PATH, as a new process-scoped group before the proxy and server ones.
It's also cleaner because we pass the real directives, not the env
variables which could have been modified.
MINOR: promex: expose the master and worker IDs as labels
This adds support for emitting the master id when defined (when master
is running) and the worker id. Note that we add a check for the value
being a non-empty string, which remains valid for the few other field
that is always defined (node), so no impact on existing fields.
MINOR: sample: add the "master_id" and "worker_id" sample fetches
These two new sample fetch functions return the unique identifiers of the
master and of the current worker process, so that they can be placed in
logs, in response headers, or be used in any other expression.
The "master_id" one fails when there is no master, i.e. when not running
in master-worker mode, so that a log or a header will simply not report
anything there instead of an inaccurate value.
Note that these functions rely on internal storage, not environment
variables, so they are unaffected by "setenv" directives.
MINOR: config: add a "worker-id" global keyword to identify the worker
The worker process now also has a unique identifier. Contrary to the
master's one, it may be replaced using the new "worker-id" keyword in
the global section. Like for the master, a time-ordered UUID (version 7)
is generated at boot (allowing to compare older vs newer process).
The identifier is immediately exported as HAPROXY_WORKER_ID so that it
may be referenced from the worker's configuration and be inherited by
the processes we may fork (e.g. external checks). It may even be
referenced by the worker-id directive itself to combine a static part
with the dynamically generated one.
MINOR: mworker: assign a unique ID to the master process
The master process now has a unique identifier, either generated at boot
as a time-ordered UUID (version 7), or inherited from the
HAPROXY_MASTER_ID environment variable when a valid one is already set
there (inherited from a previous master instance). An invalid value is
simply replaced by a generated one, since there is generally nothing the
user can do about it. The identifier is exported to the environment so
that it is both preserved by the next master upon reload (re-exec) and
visible from the worker, where it may be referenced as
"$HAPROXY_MASTER_ID" from the configuration.
The format is validated using guid_is_valid_fmt() so that it is exactly
the same as the one of the proxies' and servers' "guid": at most 127
characters among letters, digits, '.', ':', '-' and '_'.
Note that both mworker_reexec() and the worker restore their initial
environment before going further, so the variable has to be set again
after that point, hence the two extra setenv() calls, just like for a
few other such variables.
MINOR: promex: properly use alt_name and val instead of hard-coded values
Some string-based info from the "show info" output are returned as gauges
in promex, and the way it's done consists in forcing their label's name
and value to the desired name and values, then faking a value of 1 on the
gauge. Since 3.2 with commit b03e05cd36 ("MINOR: stats: add alt name info
to stat_cols_info where relevant"), this label's name appears in the stats
info's alt_name field. Thus there's no need to hard-code it again in the
promex code. Let's first do this for the "node" label.
MAJOR: filters: Revert the filter-sequence directive
The filter-sequence directive was introduced to be able to define a
different evaluation order of filters for the request and the response. The
first usage should be to mix the compression and the decompression, when a
specific order must be defined.
However, there were several bugs. And working on fixing those bugs, it
appeared that the feature was underdesigned. To fix the bugs, the syntax of
the directive must be changed. And even this way, it will be difficult to
use it properly.
In fact, we really underestimate the changes to perform. So we decided to
revert the directive for now, especially to remove it from the 3.4. It is
not really mandatory on this version. Because of all current bugs, it was
probably not used yet on production. On 3.5, the feature will be fully and
deeply redesigned because it is mandatory.
This patch reverts the following commits:
* efe1099c3 ("BUG/MEDIUM: filters: check the filter name before comparing it")
* 3eebd5f67 ("REGTESTS: add a test for "filter-sequence" directive")
* 8d28c0e37 ("MEDIUM: filters: add "filter-sequence" directive")
BUG/MINOR: proxy: pass PR_FL_DEF_EXPLICIT_MODE through cascaded defaults
Basha Mougamadou reported that in the following setup:
defaults http
mode http
timeout client 10s
timeout server 10s
defaults fast-http from http
timeout server 1s
Trying to add a backend using "from fast-http" would be rejected because
the "fast-http" section has no explicit "mode" directive. This is indeed
true, but not correct in that it is inherited from the "http" section
where it is explicit, so there it must be considered as explicit as well.
The reason is that the flag PR_FL_DEF_EXPLICIT_MODE is not inherited from
defaults sections in proxy_defproxy_cpy(). Let's just add it to the set
of such inherited flags to fix the problem.
BUG/MEDIUM: tcp-rules: Don't restart tcp-request content evaluation on yield
The commit 9f1e9ee0e ("DEBUG: stream: Display the currently running rule in
stream dump") introduced a regression. The currently evaluated "tcp-request
content" rule was reset on resume while it should not. If it yielded again,
the rules evaluation was restarted to the first rule.
The "tcp-response content" rule evaluation was properly modified by the
commit above. The same must be performed gor the "tcp-request content" rule
evaluation.
This patch should fix the issue #3496. It must be backported as far as 3.2.
CLEANUP: stream: Remove an excess newline in the stream dump
When the function dumping a stream was update by the commit c044fdbf4
("BUG/MINOR: stream/debug: harden the stream dump function regarding
signals"), an extra newline character was forgotten. Let's remove it.
This patch must be backported with the commit above.
BUG/MEDIUM: htx: Reserve a block descriptor for zero-sized blocks
htx_reserve_nxblk() only refused to reserve a new block when
<blksz> was greater than the free space available for the payload, as
reported by htx_free_data_space(). However this function returns 0 in two
different cases: when there is exactly enough room left for a block
descriptor and no payload, but also when there is not even enough room for
a block descriptor. Both cases cannot be distinguished when a zero-sized
block is added, because the test is then always false.
In that case, the function goes on and hands out a block descriptor that
does not fit in the blocks table. Concretely, with <head> set to 0 and less
than sizeof(struct htx_blk) bytes left in the message, the defragmentation
has nothing to reclaim and the descriptor is placed inside the payload
part, silently overwriting the end of the last block. In addition, the tail
is incremented, so htx_used_space() becomes greater than the message size
and htx_free_space() wraps around to a huge value. None of the sanity
checks at the end of the function is able to detect it.
The free space is now explicitly tested against the size of a block
descriptor. This way, a zero-sized block is refused when there is no room
left for its descriptor. It is fixed in htx_reserve_nxblk() to also cover
htx_add_data_atonce(), which has the same flaw in its own test, and
htx_add_header()/htx_add_trailer(), which compute a null size when both the
name and the value are empty.
The only known caller able to request a zero-sized block is the
decompression filter, when there is no room left to drain the decompressed
data. It was introduced in 3.5-dev, so no released version should be
affected. However the flaw exists in the HTX API since its introduction in
1.9, so this patch may be backported as hardening to all stable versions.
BUG/MEDIUM: ssl: don't set a 0 timeout on an expired TLS session
Commit 9f1d590999 ("BUG/MEDIUM: ssl: enforce tune.ssl.lifetime across
TLS1.3 session renewals") clamps a resumed session's timeout to what is
left of its original lifetime. Once that deadline is reached there is
nothing left, and the session was given a timeout of 0.
OpenSSL encodes the session timeout as an optional ASN.1 field which it
omits when the value is zero, and the decoder then substitutes its own
default (3 seconds) instead. construct_stateless_ticket() first measures
the ticket, then re-encodes it, so the two lengths no longer match and
an internal consistency check fails. The resumed handshake is aborted
with an SSL_AD_INTERNAL_ERROR alert (80) instead of gracefully falling
back to a full handshake:
Only OpenSSL 3.1 and older are affected, newer versions don't compare
session ages in whole seconds anymore.
Fix this by never handing out a zero timeout: when nothing is left of
the original lifetime, clear the session's id context instead. The
session then no longer matches any SSL_CTX, so OpenSSL drops it and
performs a full handshake. This also makes the expired session really
disappear from the stateful cache, which a 0 timeout failed to do since
it was read back as the 3 seconds default.
This fixes issue #3499.
This must be backported as far as 2.8, where 9f1d590999 was backported.
DOC: configuration: clarify the validity ranges of sample types
Sample types are not super clear about what they contain nor how they
cast. It's trivial for developers because they deal with that all the
day but the user-facing doc is much less obvious about that. Let's
add a descriptive list of each types in 7.3. This could be backported.
BUG/MINOR: sample: reject a \0 byte in url_dec, json_query and jwt_*_query
A string sample cannot contain a \0 byte by design: this is what
distinguishes it from a binary sample, and every consumer relies on it.
Yet url_dec decoded "%00" to a \0, json_query decoded "\u0000" to one,
and jwt_header_query/jwt_payload_query could return one from the base64
decoded JSON. Such a sample was then matched by "-m str" on its prefix
only, so "/public%00/admin" matched "/public" and an allow-list could be
bypassed.
Commit 442f583f4 ("BUG/MINOR: pattern: do not match a string with an
embedded \0 on its prefix") worked around it in pat_match_str(), which
is the wrong place since such a string must not exist at all.
This commit reverts it and fixes the producers instead: url_decode() now
fails on "%00" like it does on an invalid sequence, and json_query and
jwt_*_query fail on a decoded \0. This is also cheaper, as the string
is checked once where it is produced instead of on every call to
pat_match_str().
This must be backported to all stable branches, in place of 442f583f4
where it was not yet backported.
MEDIUM: jwt: Reject "none" alg in jwt_verify by default
In RFC8725 section 3.2, they specify that "JWT libraries SHOULD NOT
consume JWTs using "none" unless explicitly requested by the caller".
This patch disables the support for the "none" algorithm by default in
jwt_verify converter and adds a global option that explicitely allows
it.
MINOR: htx: don't reserve a block when there is no free space left
htx_reserve_max_data() computed the available room up front but tested
the empty-message case before testing that room:
int32_t len = htx_free_data_space(htx);
if (htx->head == -1)
goto rsv_new_block;
if (!len)
return (struct htx_ret){.ret = 0, .blk = NULL};
So a message which is both empty and has no room at all went to
rsv_new_block with len == 0. That matters because htx_reserve_nxblk()
only rejects a reservation when "blksz > htx_free_data_space(htx)", so
a zero-sized one passes even when the free space is zero, and the
"Empty message" path then evaluates htx_get_blk(htx, 0), that is
htx->blocks + (htx->size - sizeof(struct htx_blk)). For any size below
sizeof(struct htx_blk) this subtraction underflows the uint32_t and
yields a pointer roughly 4 GB away, which is immediately written to.
The case that is trivial to reach in theory is the shared empty HTX
(htx_empty, size 0) that htxbuf() and htx_from_buf() return for an
unallocated buffer. No caller can do it today: the only caller,
h1_parse_full_contig_chunks(), returns earlier when the maximum is not
larger than sizeof(struct htx_blk), and the H1 mux allocates the
destination buffer before parsing into it. So this is not a fix for a
reachable bug, just one less trap for whoever adds the next caller.
Swapping the two tests is transparent for every other case: when the
message is empty but its buffer is allocated, the free space is
necessarily non-zero, so the early return cannot trigger and we still
reach rsv_new_block as before.
IMPORT: uslz: reject codes from an empty distance table
gen_huffman_table() may be asked to accept a table with no symbol at
all (allow_no_symbols, the distance table). It used to return success
without initializing the table, but that storage aliases the persistent
header-detection buffer (hdr_detect), so gethuff() could walk header
bytes out of bounds. Make the two root entries terminal with an
out-of-range symbol so any code decoded against an empty table is
rejected.
BUILD: ssl: ASN1_STRING_length() is deprecated since OpenSSL 4.1
OpenSSL 4.1.0-alpha1 deprecates ASN1_STRING_length() in favor of
ASN1_STRING_get_length(), which only differs by returning a size_t
instead of an int. This breaks the build when warnings are fatal:
src/ssl_sock.c:4655:9: error: 'ASN1_STRING_length' is deprecated: Since OpenSSL 4.1; use ASN1_STRING_get_length() [-Werror=deprecated-declarations]
Let's simply remap the old name on the new function when built with
OpenSSL 4.1 and above, all the call places being compatible with both
return types.
This could be backported to all versions where we want to support
OpenSSL 4.1.
REGTESTS: http-rules: check "-m str" on a sample with an embedded \0
Check that a case-sensitive exact match does not match a longer string
carrying an embedded \0, from url_dec or json_query, that the
case-insensitive match still works, and that "%00" in the raw path can
be matched as text.
BUG/MINOR: pattern: do not match a string with an embedded \0 on its prefix
pat_match_str() looks the sample up in the pattern tree with
ebst_lookup(), which reads it as a C string and stops at the first \0.
A sample "/public\0/admin" with <data> = 14 was thus matched as
"/public" by the default case-sensitive "-m str" exact match, while the
case-insensitive match, which uses the list, compares the length and
rejected it.
Such samples are produced by converters that decode client data, e.g.
url_dec ("%00") or json_query ("\u0000"), so an allow-list written as
"http-request deny unless { path,url_dec -m str /public }" could be
bypassed with "/public%00/admin". Maps (map, map_str) use the same
matcher and were affected too.
Patterns are loaded as C strings and never contain a \0, so such a
sample cannot be an exact match. Check it once at the top of
pat_match_str() and return no match.
Note that a deny-list such as "-m str /admin" used to reject
"/admin%00zz" by accident and now lets it through, since it is not an
exact match. The has_ctl() converter can be used to reject any decoded
\0.
This is the same issue as the one fixed for the ssl_*_dn() fetches in
commit 449133cba ("BUG/MINOR: ssl: reject an embedded \0 in the
ssl_*_dn(entry) fetches"), fixed here in the matcher so that all
producers are covered.
No error is reported during command line parsing when a payload pattern is
found with no command. It is not really a big deal because when the command
is processed, because there is no command, nothing is executed. It is
harmless.
But, accordingly to the doc, the payload is the last argument of a
command. So a command is required. Thus, an error is now reported when a
payload is received with no command.
This patch could be backported to 3.4. Older versions could benefit of this
patch. If we decide to backport it farther, the patch will have to be
adapted.
BUG/MEDIUM: cli: Don't read a command or payload line into a full buffer
Commit e3c175871 ("BUG/MEDIUM: cli: Don't parse next command if input buffer
is empty") fixed a crash triggered when a command line exactly filling the
buffer was finished by a payload pattern, by no longer trying to parse a
next command when the input buffer was empty.
However this only addressed the case where no more data was available. When
a command line exactly fills the buffer and ends with a payload pattern, the
payload buffer is created with a size of 0 (no room left). If payload data
is then received on a subsequent read, the loop was not interrupted and
b_getline() was called with a length of 'b_room(buf) - 1'. Because b_room()
returns a size_t, an empty buffer made this length wrap around to SIZE_MAX,
and the line was copied past the end of the trash chunk, leading to a buffer
overflow.
To fix the issue, no attempt to read a command or a payload line is
performed when there is no room left in the destination buffer. In that
case, we fall back to the existing path that reallocates a bigger (dynamic)
payload buffer, which is guaranteed to be given a non-zero size, or reports
an error for the command line. This way b_getdelim() and b_getline() are
never called on a full buffer and the size_t underflow can no longer happen.
On older versions, 1-byte OOB write is also possible, but with a lower
severity.
This patch must be backported to 3.4, along with commit e3c175871. And it
should be adapted and backported to all other stable versions.
BUG/MEDIUM: cli: Don't parse next command if input buffer is empty
In the loop of the command line parser, we always tried to get the next
command, even when the input buffer was empty. However it was an issue if
the size of command line was exactly the buffer size and it was finished by
a payload pattern. In that case, an attempt to allocate a dynamic buffer for
the payload was performed with a size of 0, which led to a crash.
To fix the issue, the loop is now interrupted with there is no more data to
parse in the input buffer. This way, when an dynamic buffer must be
(re)allocated, we are sure its size will never be 0. To be sure it is true,
a BUG_ON() was added on the new dynamic buffer size.
To mitigate the issue, "tune.cli.max-payload-size" parameter can be set to
0.
Youngkwang Lee [Wed, 9 Sep 2026 08:31:04 +0000 (17:31 +0900)]
REGTESTS: uri_normalizer: test a normalizer used with a condition
The existing tests always use the normalizers unconditionally, so the
selection of the normalizer when a condition directly follows a
normalizer whose sub-option is omitted was not covered.
The request uses both a percent sequence and duplicate slashes so that
the expected result differs depending on which normalizer is picked:
percent-decode-unreserved leaves the duplicate slashes alone, while
path-merge-slashes, which was wrongly selected before the previous
commit, would merge them and leave the percent sequence encoded.
Youngkwang Lee [Wed, 9 Sep 2026 08:31:03 +0000 (17:31 +0900)]
BUG/MEDIUM: http_act: fix normalize-uri normalizer selection with a condition
The normalizers that accept an optional sub-option ("strict" for
percent-to-uppercase and percent-decode-unreserved, "full" for
path-strip-dotdot) only set rule->action in the branch handling the
sub-option and in the branch handling the end of the arguments. When
the sub-option is omitted and an "if" or "unless" keyword directly
follows the normalizer name, none of these branches is taken and
rule->action is left untouched.
As a result the rule ends up with action 0, which is
ACT_NORMALIZE_URI_PATH_MERGE_SLASHES, and a configuration such as:
http-request normalize-uri percent-decode-unreserved if { ... }
silently merges duplicate slashes instead of decoding percent
sequences. This is easily missed because the rule still does
something, and for a URI without duplicate slashes the request is
left unchanged, which looks like the rule was simply skipped.
Set the default action right after the normalizer name is matched and
let the sub-option overwrite it. The check rejecting unknown arguments
is kept, guarded by a test on the argument being non-empty.
This should fix GitHub issue #3493.
This may be backported to all stable versions where the affected
normalizers exist. The three of them were introduced in 2.4-dev by
commits 560e1a635 ("MINOR: uri_normalizer: Add support for supressing
leading `../` for dotdot normalizer"), a40719337 ("MINOR:
uri_normalizer: Add a `percent-upper` normalizer") and 2e4a18e04
("MINOR: uri_normalizer: Add a `percent-decode-unreserved`
normalizer").
Signed-off-by: Youngkwang Lee <ykglory0216@gmail.com>
Let's now use http_trailers_to_htx() instead of htx_add_all_trailers()
to parse HTTP/1 trailers. Upper case is permitted and thats the only
difference with H2/H3 for now.
The parser is stricter than the previous function, so the following
names are now rejected as it is for other versions:
MINOR: http-htx: condition uppercase acceptance in trailer names to HTTP_PF_UPCASE_OK
The trailers parser currently rejects uppercase chars in the name.
Let's add a new flag to indicate that they are OK. This will be used
by HTTP/1. However this is not sufficient, we now need to use isteqi()
instead of isteq() to compare header names.
Neither H2 nor H3 need to pass that new flag, hence why the change is
minimal.
MINOR: http-htx: pass extra flags to the HTTP trailers parser
Let's introduce HTTP parser flags (HTTP_PF_*) and rely on them for the
trailers parser. This one will need them to decide to enable upper case
characters or not in header names in order to reuse it for H1 parsing.
For now we only pass the direction. The current parser doesn't use it
but the goal is to generalize the mechanism to other parsers in the
future.
REGTESTS: http-messaging: add tests for the H1 trailers
The Transfer-Encoding test is extended to cover the HTTP/1 trailers,
both on the request and on the response path.
Valid trailers are checked to be forwarded in both directions. vtest is
not able to parse HTTP/1 trailers (it stops on the CRLF following the
last chunk), so they are read using "recv" and only the number of bytes
emitted by haproxy is verified. This is sufficient to prove that the
spaces surrounding the values were trimmed, since it changes the length,
and a second trailer-less message is exchanged over the same connection
to make sure nothing was emitted after the trailers.
The trailers rejected by the H1 parser are checked to result in a 400 on
the request path and in a 502 on the response path: invalid token in the
field name, missing colon, pseudo-header, empty field name and a CR not
followed by an LF in the value. Note that each message must be emitted
at once, otherwise haproxy has already forwarded the beginning of the
message when the invalid trailers are parsed and can only abort the
stream instead of responding with an error.
BUG/MINOR: stream: resolve foreign groups' addresses for "show sess"
A stream in another group can't have its addresses resolved from the CLI
thread when using per-thread group file descriptor tables, as
getsockname/getpeername need the owner group's FD table. Ask the owning thread
to resolve a batch and cache them in the connections, picked up when the dump
resumes.
BUG/MEDIUM cli: dump other thread groups' FDs from their own threads
"show fd" data (getsockname/getpeername) is only valid in the FD's owner
group. With split FD tables, dump each foreign group's FDs from one of
its own threads via twork, one buffer at a time, forwarded to the client.
Olivier Houchard [Thu, 20 Aug 2026 16:34:45 +0000 (18:34 +0200)]
MEDIUM: twork: Introduce a new cross-thread/thread-group deferred work API
Introduce twork, a way to execute code on another thread, or another
thread group. The main motivation for that was to be able to obtain
information about file descriptor belonging to other thread groups, when
running with per-thread-group file descriptor tables.
The functions are :
int twork_call_on(struct twork *tw, int tid,
union twork_arg (*fn)(union twork_arg),
union twork_arg arg,
void (*done)(union twork_arg arg, union twork_arg ret));
Runs function fn on thread tid, providing arg as an argument, and
calling done from the caller thread when the work is complete.
int twork_call_tgroup(struct twork *tw, uint tgid,
union twork_arg (*fn)(union twork_arg),
union twork_arg arg,
void (*done)(union twork_arg arg, union twork_arg ret));
Same thing, but running on thread group tgid instead.
"done" is optional, if there is no need to be notified that the work is
complete.
"tw" is optional, if none is provided, one will be allocated
dynamically. Note that you can only cancel a twork if you're using one
that you provided by yourself.
int twork_cancel(struct twork *tw);
Cancels the tw, assuming it is not already running. It will return 1 on
success, and 0 on failure, in which case the work will be completed, and
the "done" function will be called.
BUG/MEDIUM: listener: Don't drop suspend/resume deferred requests
When using per-thread group file descriptors, we have to send messages
to other thread groups to tell them to suspend or resume listeners. Add
a current status for each thread group, instead of looking for the
global listener state, to know the current state for each thread group,
otherwise a rapid succession of suspend/resume could lead to an
inconsistant state.
BUG/MINOR: cli: Make sure two instances of _getsocks don't run
It should never happen, but two instances of _getsocks could run at the
same time, and cause a deadlock, so make sure that never happens by only
allowing one at the same time.
REGTESTS: map: check map_regm inside an ACL over an iterating fetch
This test covers the bug fixed by this commit:
7646c657a BUG/MEDIUM: map: do not overwrite the fetch context in pat_match_regm()
It uses map_regm in an ACL over req.hdr() with no occurrence. The first
value does not match, so the ACL runs a second iteration of the fetch,
which crashed the process before the fix. A request with two header
values checks that the fetch cursor survives the first iteration.
Thank you to @Michael-JRead for having produced the first version of this reg
test.
BUG/MEDIUM: map: do not overwrite the fetch context in pat_match_regm()
pat_match_regm() stored the address of the thread-local <pmatch> array
into <smp->ctx.a[0]> so that sample_conv_map() could read the capture
offsets back for exp_replace(). But this slot belongs to the sample
fetch, which uses it to keep an iteration cursor.
When map_regm is used inside an ACL over an iterating fetch (for example
req.hdr() with no occurrence), acl_exec_cond() re-runs the fetch to get
the next occurrence without clearing <smp->ctx>. On the second pass the
fetch reads back the <pmatch> pointer as its own cursor and dereferences
regex offsets as a pointer, which crashes the process. A single client
request is enough to trigger it.
Do not alias the slot: sample_conv_map() reads the thread-local <pmatch>
directly, since nothing runs another regex between pattern_exec_match()
filling it and this read. This is how http_replace_hdrs() and the
replace-path action already use <pmatch>.
Thank you to @Michael-JRead for having reported this issue.
This bug was introduced in 1.7 by commit 8feaa661b ("MINOR: map: Add
regex matching replacement").
BUG/MINOR: ssl: don't pass the address of a size_t as an int* to OpenSSL
aes_process() passes &out->data, which is a size_t, to the EVP update and
final routines through an (int*) cast. OpenSSL writes an int there, so on
a 64-bit big-endian platform (s390x, ppc64) the resulting length lands in
the high half of the size_t, and the buffer's data count becomes the real
length shifted left by 32 bits. The subsequent write at out->area +
out->data then targets a wild address.
Concretely, on such builds the "aes_gcm_dec" / "aes_cbc_dec" converters
and the JWE decryption paths corrupt memory as soon as they process any
input, so any client able to submit a token to them can crash the worker.
Little-endian builds, which are the vast majority, happen to work by
chance since the count starts at zero and only the low half is written.
Let's use a proper local int for the OpenSSL output length and assign it
back to the buffer's data count.
The shared helper comes from commit f0e64de75 ("MINOR: ssl: Factorize AES
GCM data processing") in 3.4 but the same cast existed in the AES
converters before that. It must be backported to all stable versions.
MAJOR: startup: use chroot auto by default when available
3.4 implemented "chroot auto", "chroot /", unprivileged chroot and a
warning for super-user when chroot is not used. We were not doing it
for unprivileged users because this would have caused confusion with
configs triggering warnings only on certain OSes or environments.
Now's time to switch to automatic chroot attempt on startup whenever
deemed possible.
For this, we reuse the previous rules to decide to emit a warning when
a chroot is possible but not done, and instead of complaining that no
chroot is done, we just attempt it before emitting the warning only in
case of failure.
This is marked major because it may break some old rare configs that
were emitting the warning in 3.4 due to the lack of chroot, when the
user expected to access for example a UNIX socket at run time, or
using an external check. The fix remains easy (and is explained in
the warning), just use "chroot /".
The change aims at increasing the default security level for users by
default (i.e. with the least config settings), but it's not critical,
so feedback will determine if we keep it as-is, if we adjust it, or if
we revert it.
REGTESTS: specify explicit chroot in confs using unix-stream servers
For convenience reasons, a small tens of tests make use of unix sockets
on server lines, and as such cannot be chrooted. Given that non-chrooted
startups are frowned upon and even emit a warning for privileged users,
let's explicitly use the recommended "chroot /" for them.
Manu Nicolas [Thu, 3 Sep 2026 14:10:54 +0000 (14:10 +0000)]
BUG/MEDIUM: http-ana: apply tunnel timeout with data filters
The tunnel timeout is installed by process_stream() only once all
analysers are gone. HTTP data filters keep their transfer and end
analysers attached after a protocol upgrade. A stream using, for
example, set-bandwidth-limit therefore remains on its client and server
timeouts even though both HTTP messages are in tunnel mode. A short
client timeout can close a WebSocket before its heartbeat.
Apply the per-stream tunnel timeout when the second HTTP message enters
tunnel mode. Data-filter analysers remain active and continue to process
tunneled data.
Commit 5e9662ddd63a730235deec27489c7e8897fea21d made it so that instead
of checking for the existence of CLONE_FILES, we'd define
HA_HAVE_UNSHARE if file unsharing is available. To determine that, it
checks for the existence of CLONE_FILES, however without including
sched.h, it will never be defined, and file unsharing will never be
used.
Fix that by properly including sched.h.
BUG/MINOR: h3: only increment frame type counter on new header
For every new HTTP/3 received frame, a counter related to the frame type
is incremented. This operation is performed directly in h3_rcv_buf()
parsing loop.
Prior to this patch, increment would be called too many times when
dealing with large frames. Fix this by ensuring that the increment is
only reported once per frame. This is now performed directly after
HTTP/3 frame header parsing.
BUG/MINOR: h3: handle unknown frame type on request stream as specified
HTTP/3 specification allows the ability to define extra frame types. An
implementation must silently ignored unknown frame types.
Prior to this patch, haproxy stack completely discards an unknown frame.
However, on a request stream, if this acts as the last stream frame, it
would lead to a missing HTX EOM before the stream is closed on timeout.
Now, an HTX EOM is properly reported to the upper layer, which ensures
the request can successfully be achieved. A reset is emitted though if
no headers were previously received as in this case the HTTP request is
incomplete.
BUG/MINOR: h3: handle empty HEADERS frame as specified
This patch ensures empty HEADERS frame are treated as expected by
rfc9114.
In some conditions, such frame is invalid and the stream is closed with
FRAME_UNEXPECTED h3 error. This is the case if this is the first frame
interpreted as header as it lacks mandatory pseudo-headers. This is also
the case if trailers were already received, but this case is already
handled by a previous patch.
The only condition in which an empty HEADERS frame may be valid is when
it is interpreted as trailers. QUIC FIN may be signalled in the same
occasion, thus h3_trailers_to_htx() has been extended to add a call to
htx_set_eom() to report the stream closure to the upper layer.
This is considered minor as it's doubtful that any H3 client will use an
empty HEADERS frame.
BUG/MEDIUM: h3: do not block FIN on empty DATA frame
A HTTP/3 DATA frame may be empty as its payload is entirely optional.
However, prior to this patch, H3 stream parsing was interrupted in this
case, unless other frames follow. The main problem is that it would lead
to a missing EOM if QUIC FIN is reported with an empty DATA frame,
causing the stream to freeze until timeout.
The first step of this patch is to first remove the early return for
empty HTTP/3 frame payload. This has the effect to allow calling the
different frame handlers with an empty payload. The current patch is
dedicated to HTTP/3 DATA support; other patches will follow to ensure
the safety of the other frame types handlers.
The second part of this patch is to adjust h3_data_to_htx() which is the
HTTP/3 data frame handler. It is now safe to call it with an empty
frame, with or without FIN. It will default to a noop operation, unless
FIN is set, in which case htx_set_eom() is used to report the closure of
the stream read channel.
BUG/MINOR: h3: reject truncated frames with mandatory payload
Most of HTTP/3 control frames contain a fix set of fields. If a frame is
truncated, an implementation must close the connection with
H3_FRAME_ERROR code.
This patch adds some basic checks on CANCEL_PUSH, PUSH_PROMISE and
MAX_PUSH_ID : if the length is null, H3_FRAME_ERROR code is used.
This bug can be considered minor as push is not supported by haproxy.
They were completely ignored before backend support as only a server is
expected to emit them.
h3_trailers_to_htx() performed exactly the same controls as the generic
http_trailers_to_htx() parser, but was relying on traces and glitches
for each failure case that was codified on the latter parser.
Let's just drop the parsing loop for a direct call to the function and
a check on the return codes to decide what trace/glitch to emit.
It's worth noting that H3_ERR_MESSAGE_ERROR is not set when failing to
add a header, not sure whether this is a design decision or an overlook.
In any case this behavior was preserved.
Note that pseudo-headers are detected on their syntax here, while h2
used to detect them on their assigned number, so this remains a tiny
difference between the two parsers at this point.
MEDIUM: mux-h2: rely on the generic trailers parser
http_trailers_to_htx() was made from h2_make_htx_trailers(), so it's
expected that we can get rid of the latter in favor of the former to
aim towards a generic parser.
For now we don't exploit the extended return codes, we only check for
HTTP_PRS_SUCCESS to detect parsing failures.
REGTESTS: http-messaging: add tests for the H2 and H3 trailers
Two new tests cover the trailers parsers of the H2 and the H3 muxes, so
that any change to these parsers can be checked against the current
behaviour.
h2_trailers.vtc checks that valid trailers are forwarded with their
leading and trailing spaces removed, on the request as well as on the
response path, and that all the trailers rejected by the parser cause
the stream to be reset: literal and indexed pseudo-headers, upper case
letters and non-token characters in the names, NUL/CR/LF in the values,
and the fields affecting the message framing or the routing.
Since vtest doesn't speak H3, h3_trailers.vtc stacks two HTTP layers in
the same haproxy: the H2 requests emitted by vtest are converted to H3
and sent over a local qmux connection to a second frontend which
converts them back to H2 for the vtest servers. Only valid trailers are
checked there because the invalid ones cannot reach the H3 parser: HTX
lower cases the trailer names and the H3 mux drops the trailers which
are forbidden on the wire.
A few notes about the tricks used in these tests:
- a trailers frame is emitted using "txreq -noadd", which sends a
HEADERS frame with ES+EH set and no pseudo-header at all ;
- trailers vtest cannot encode (a NUL in a value) or decode (an empty
value as the last field) are crafted by hand with sendhex ;
- haproxy doesn't ack the settings of an H2 server before it has
something else to send, so the servers must not wait for that ack ;
- the window update emitted after the request headers is consumed
before sending invalid trailers, so that the reset is necessarily
the next frame to be received ;
- "option http-buffer-request" makes sure the invalid trailers are
detected while the stream is still open and that no server is
involved.
MINOR: http-htx: add a new generic trailers parser
http_trailers_to_htx() merges h2_make_htx_trailers() and
h3_trailers_to_htx() into a unified function that takes a fields list,
performs controls and adds them to HTX. It returns http_parser_status
codes so that the caller knows what happens. For now the function is
not ready to handle HTTP/1 because it unconditionally rejects upper
case letters in field names.
The parser uses two modes to detect pseudo-headers:
- syntax-based (it starts with a colon)
- number-based (an index number replaces the string)
This is because the former is used by h3, the latter by h2.
MINOR: http: define new HTTP status codes for parsers
The new http_parser_status enum contains codes for HTTP header field
parsers (headers and trailers). This will be used by generic parsers
to return accurate error information on what exactly was invalid.
MINOR: ring/cli: improve the "show events" parser regarding options
The "show events" parser remains counter-intuitive in that the options
are historically placed after the ring name, making it behave differently
from commonly used tools and other CLI commands.
Let's relax the parser to take any -XXX as a set of option and the only
word not starting with - as the sink name. That way it becomes possible
to parse options before or after the sink name. The doc was updated to
reflect this and present available options as an enumeration rather than
an unreadable wall of text.
MINOR: ring: permit to combine options on "show events"
The "show events" command supports a handful of options starting with
a "-", but only a few combinations were manually checked for (nw and
wn), other ones may not be combined.
This patch changes the parser to permit any combination, continuing to
process them in declaration order, so that -nsw or -rnw work. The doc
was updated.
This also updates the help message which would previously not list all
options by lack of room.
MINOR: ring: add "-r" to "show events" to dump in raw mode (undoes "-s")
In anticipation for a possible future change of the default output which
might eventually become sanitized by default, let's add "-r" which does
the opposite of "-s", and turns the output back to raw. This can be used
by tools trying to be future-proof that do not want to depend on the
default setting.
Define HA_HAVE_UNSHARE which tests CLONE_FILES with an exception on
NetBSD which has the CLONE_FILES define for clone() but doesn't provide
unshare().
src/thread.c: In function ‘start_extra_tgroup_threads’:
src/thread.c:244:21: error: implicit declaration of function ‘unshare’ [-Werror=implicit-function-declaration]
244 | if (unshare(CLONE_FILES) != 0) {
| ^~~~~~~
MINOR: stconn: prefer to replace endpoints rather than detaching them
sc_detach_endp() is used to detach an endpoint, after which we either
assign a new one or completely release the object. Thus, it is guaranteed
that an stconn may not be found with no endpoint. However this is true in
the regular code flow, not at the signal level. And running "show threads"
during a retry, or triggering watchdog warnings on it can witness such a
stream executing instructions at the end of sc_detach_endp() before the
new endp is assigned, causing segfaults during dumps as described in
github issue #3490.
The new sedesc is allocated before releasing the old one, so we have
everything to perform a replacement. For this reason, sc_detach_endp()
was now replaced with sc_replace_endp(). It takes an optional new
endpoint to assign. When detaching, we just continue to pass NULL.
But when switching, we pass the newly allocated one and an external
observer may never see a NULL in the variable.
Note that we don't use atomics here (not really needed, and not enabled
without threads anyway). No failure was observed with millions of thread
dumps and millions of dumped streams. But if it were to still happen on
some specific archs, it would be fine to switch to an atomic store (it
was tested and works as well).
This may have to be backported to a few stable versions if it helps
improve the overall reliability during dumps, but let's wait a bit
first.
BUG/MINOR: stream/debug: harden the stream dump function regarding signals
As seen in GH issue #3490, it is sometimes possible to see a stream in
an intermediate state during a dump (due to the dump being called by a
signal handler). The code was already designed in a defensive way for
this, but not sufficiently.
Indeed, while a stream my never be seen in a stable state with NULL in
any of its sedesc, there exist a short period during sc_reset_endp()
where the backend sedesc is NULL for a few nanoseconds, between the
NULL and the new value, and if a dump signal happens in the middle,
the __strm_dump_to_buffer() function will crash.
This patch modifies __strm_dump_to_buffer() so that the sc, sd and conns
are taken for each side when entering the function and used as tests every
time they're about to be dereferenced. This way certain fields will be
omitted whenever such situations happen.
This new version now resists heavy connection retries combined with
1ms warnings and two parallel "show threads" loops..
This should be backported to stable releases. It is likely that it will
not easily apply to all versions, so the changes might more easily be
re-done; the principle consists in conditioning the dereference of
sc->sedesc for each side (which also happens when reading flags).
MINOR: mux-h1: loop on HTX blocks using _blk() functions to look for WS key
In h1_search_websocket_key() function, instead of looping on the block
position, we now use functions returning a blocks. This commit is mandatory
for next changes.
REORG: htx: Move functions to deal with first block after ones about next block
functions htx_get_first_* were moved after htx_get_next_*() ones. There is no
change here but it is mandatory for the next commit. htx_get_first_blk() will
rely on htx_get_next().
MINOR: httpclient: Don't manually set HTX_FL_EOM flag after calling htx_xfer()
The htx_xfer() function already takes care to copy HTX_FL_EOM flag when it
is required. So there is no reason to still do it in the HTTP client I/O
handler function.
BUG/MINOR: flt-http-comp: Rely on htx_set_eom() to restore EOM flag
A theoritical bug was introduced by the commit 30890fd90 ("BUG/MEDIUM:
flt-http-comp: Block HTX_FL_EOM flag till the end of compression"). On the
normal case, the HTX_FL_EOM flag was restored without checking if the HTX
message was empty or not. It could be an issue. Instead, htx_set_eom()
function must be used.
This patch must be backported with the commit above.
BUG/MEDIUM: htx: Don't reset flags when source must be preserved during transfer
When htx_xfer() is called with HTX_XFER_KEEP_SRC_BLKS flag, to preserve
blocks from the source HTX message, the source flags must not be reset at
the end of the transfer.
This bug can be triggered with L7-retries are configured with small
buffers. In that case, the HTX_FL_EOM flag is removed from the request
fowarded to the backend multiplexer.
BUG/MINOR: hlua: Always return nil if there is no data to dup for HTTP messages
In _hlua_http_msg_dup(), we must always return nil if no data was
duplicated. The fact the end of the message was reached or not should not
interfere. So let's remove the extra condition on HTX_FL_EOM flag and always
return nil if no data was duplicated.
This patch could be backported to all supported versions.
projects / haproxy.git / log