Handle reception of a MAX_STREAMS frame for unidirectional stream usage
when using QMux. This simply consists in using qcc_recv_max_streams() as
with QUIC protocol.
Ensure reception of STOP_SENDING via QMux protocol is properly handled.
This simply consists in using qcc_recv_stop_sending() which will update
the associated QCS if found.
The check on the OCSP response expire time is based on the "Next Update"
field of the response, converted by my_timegm function that returns a
time_t (signed long). It is then stored in the 'expire' field of the
certificate_ocsp structure which is typed as a signed long.
When loading an OCSP response, if the "Next Update" time is too far in
the future and we are running on a 32 bits machine, we might end up with
negative times ireturned by my_timegm, which make the comparison with
the current date fail and raises the "OCSP single response: no longer
valid." error message.
This problem typically happens in the ocsp_auto_update.vtc regtest since
the loaded OCSP response have a "Next Update" field in 2050.
This patch simply changes the type of the expire field to an unsigned
long since the 'my_timegm' function does not return '-1' in case of
error, contrary to the standard 'timegm' one.
Ths patch can be backported to all stable branches.
Use xprt_add_l6hs() at the end of connect_server() if selected MUX layer
relies on a temporary handshake prior to its initialization. This
functions is noop is SSL layer is active.
This change is necessary to support clear QMux on the backend side.
Recently defined <init_xprt> from mux_proto_list is used to render the
code as generic as possible.
Activates xprt_qmux layer if necessary via session_accept_fd(). This is
necessary to be able to support QMux in clear. This operation is noop if
SSL is active, as in this case xprt_qmux will be activated after the SSL
handshake completion.
To ensure MUX init is delayed when running with clear QMux, mask
CO_FL_WAIT_XPRT_L6 is added to test if the embryonic task must be
started instead.
Define a new connection flag mask CO_FL_WAIT_XPRT_L6. This will be used
to indicate that a XPRT layer is running on top of layer 6. For now,
only xprt_qmux implements this method of operation.
Extend get_alpn() for xprt_qmux layer. If lower layer does not implement
ALPN negotiation, return a statically default protocol value. Currently
this is set to "h3".
This change is required to support QMux in clear without SSL. In the
future, it could be useful to configure the default protocol, for
example by extending the syntax for the "proto" keyword.
When QMux protocol is used, xprt_qmux layer is setup after SSL handshake
completion but prior to the MUX initialization. Once transport
parameters exchange is successful, the layer is removed and the MUX is
started.
The layer setup operation was performed directly on ssl_sock_io_cb().
Simplify the code by extracting it in a dedicated function
xprt_add_l6hs(). The function is generic so the requested XPRT layer
must be passed as argument.
The code is mostly identical. One difference is that a check is
performed to ensure no SSL handshake is pending. If this is the case,
the function is a noop. This will become useful to support QMux
transparently both in clear or on top of SSL.
Another minor addition is that CO_FL_XPRT_READY flag is automatically
resetted by xprt_add_l6hs(). This allows the code to use
conn_xprt_start() standard function after XPRT init.
A recent patch has introduced <init_xprt> mux_proto_list member. This
allows to activate QMux on SSL handshake completion without explicit
"proto qmux" setting.
Thanks to this change, on SSL handshake completion it is not necessary
anymore to check for CO_FL_QMUX_* flags.
The various tcp_option_* converters rely on tcp_fullhdr_find_opt() to
find the option. However, the same bug as fixed in commit dbf471f99a
("BUG/MAJOR: net_helper: ip.fp infinite loop on malformed tcp options")
was also present there, by which an option of length 0 could be looped
over indefinitely. In practice this does not happen since such options
are not valid, but if passed encoded in an HTTP header for example, it
could possibly be passed.
While fixing it, let's check for length >1 in all 3 locations insteead
of only non-zero, since there's no point processing a malformed option
that wouldn't even be properly skipped.
This fix doesn't need to be backported, unless the ip.fp series is.
Thanks to @Vincent55 for reporting this issue.
When threads are disabled, "static __decl_spinlock(foo);" ends up as
"static;", causing a build warning when threads are disabled. We don't
need it to be static so let's drop "static" here. No backport is needed,
this is 3.4-only.
Released version 3.4-dev13 with the following main changes :
- BUG/MINOR: backend: correct parameter value validation in get_server_ph_post()
- BUG/MINOR: config/dns: properly fail on duplicate nameserver name detection
- BUG/MEDIUM: dns: fix long loops in additional records parse on name failure
- BUG/MEDIUM: resolvers: fix name compression pointer validation in resolv_read_name()
- BUG/MEDIUM: dns: fix memory leak of sockaddr in dns_session_init() error path
- CLEANUP: proxy: fix tiny mistakes in parse error messages
- CLEANUP: dns: fix misleading error messages in dns_stream_init()
- BUG/MINOR: server: better handling of OOM in srv_set_fqdn()
- BUG/MINOR: servers: use proper source of pool_conn_name in srv_settings_cpy()
- BUG/MEDIUM: server/cli: unlock server lock on failure in cli_parse_set_server
- BUG/MINOR: resolvers: fix dangling list pointer in resolvers_new() error paths
- BUG/MINOR: dns: fix dangling dgram pointer on dns_dgram_init() failure path
- BUG/MINOR: proxy: use proxy_drop() in parse_new_proxy() error path
- CLEANUP: resolvers: properly initialize the sample in resolv_action_do_resolve()
- BUG/MINOR: resolvers: report the expression error in the do-resolve() action parser
- BUG/MINOR: resolvers: fix leaked dgram and dns_ring struct in parse_resolve_conf()
- BUG/MINOR: resolvers: fix leaked fields on cfg_parse_resolvers() error paths
- BUG/MINOR: resolvers: fix missing task_idle destruction in resolvers_destroy()
- CLEANUP: proxy: fix duplicate declaration of cli_find_frontend in proxy.h
- CLEANUP: address a few typos and copy-paste errors in httpclient and dns
- DOC: internal: add a few rules about internal core principles
- BUG/MINOR: session/trace: use distinct flags for SESS_EV_END and _ERR
- CLEANUP: stick-table: uniformize the different action_inc_gpc*()
- REGTESTS: do not run quic/tls13_ssl_crt-list_filters in quic openssl compat mode
- REGTESTS: quic/issuers_chain_path: do not forget to enable QUIC compat mode
- BUG/MINOR: sock: store the connection error status
- BUG/MINOR: check: properly report errno in chk_report_conn_err()
- CLEANUP: tcpcheck: mention that we're a bit far for a sync errno
- BUG/MINOR: jwt: fix possible memory leak in convert_ecdsa_sig() error path
- CLEANUP: jwe: fix theoretical overflow in AAD length calculation
- DOC: config: further clarify that resolvers "default" exists
- MINOR: proxy: remove the experimental status on dynamic backends
- BUG/MEDIUM: limits: properly account for global.maxpipes in compute_ideal_maxconn()
- BUG/MINOR: jws: fix OpenSSL 3.0 version check from > to >=
- BUG/MINOR: jws: Add missing return value check (EVP_PKEY_get_bn_param)
- BUG/MINOR: server: Properly handle init-state value during haproxy startup
- BUG/MINOR: httpclient-cli: Destroy http-client context if failing to start it
- BUG/MEDIUM: h1: Skip all h2c values from Upgrade headers during parsing
- BUG/MINOR: h1: Don't mask websocket protocol if multiple protocols used
- MINOR: haterm: Don't init haterm master pipe if not used
- CLEANUP: haterm: Remove "(too old kernel)" from warning message during init
- BUG/MINOR: httpclient-cli: fix uninit variable in error label
- MINOR: mux: Rename the "token" from mux_proto_list to mux_proto
- MEDIUM: connections: Use both mux_proto and alpn to pick a mux
- MINOR: connection: define conn_select_mux_fe()
- MINOR: connection: define conn_select_mux_be()
- MINOR: connection/mux_quic: add MUX <init_xprt> field for QMux handshake
- MINOR: proxy/server: reject TCP ALPN h3 without experimental
- MEDIUM: ssl: allow h3/QMux negotiation without explicit proto
- BUG/MINOR: server: accept server IDs above 2^31 and clarify error message
- BUG/MINOR: backend: fix balance hash calculation when using hash-type none
- MINOR: server: support hash-key id32 for a cleaner distribution
- MINOR: backend: support hash-key guid for a stabler distribution
- MINOR: startup: support unprivileged chroot if possible
- MEDIUM: startup: add automatic chroot feature
- MINOR: h2: explain committed_extra_streams dec on h2_init() error
- OPTIM: h2: do not update committed streams if elasticity disabled
- MINOR: mux_quic: implement basic committed_extra_streams accounting
- MINOR: quic: use stream elasticity value for initial advertisement
- MINOR: mux_quic: define ms_bidi_rel QCC member
- MAJOR: mux_quic: support stream elasticity during connection lifetime
- BUG/MEDIUM: servers: Store the connection hash with the parameter cache
- BUG/MINOR: prevent conn leak in case of xprt_qmux init failure
- BUILD: traces: set a few __maybe_unused on vars used only for traces
- BUILD: traces: add USE_TRACE allowing to disable traces
- MINOR: startup: do not execute chroot() when "/"
- MEDIUM: startup: warn when chroot is not set for root
- BUG/MEDIUM: servers: Don't forget to set srv_hash when needed
- DOC: fix typo on QUIC stream.max-concurrent reference
- BUG/MINOR: mux_quic: do not exceed stream.max-concurrent on backend side
- BUG/MINOR: htx: Fix value of HTX_XFER_HDRS_ONLY flag
- MEDIUM: htx: Improve htx_xfer API to not count HTX meta-data
- BUG/MEDIUM: applet: Fix transfer of HTX data to the applet
- BUG/MEDIUM: htx: Alloc a chunk of right size in htx_replace_blk_value()
- MEDIUM: stick-tables: Avoid freeing elements while holding a lock
- MINOR: intops: add a multiply overflow detection for ulong and size_t
- CLEANUP: tree-wide: use array_size_or_fail() in array size for allocations
- DOC: update supported gcc and openssl versions in INSTALL
Instead of relying on malloc(n*size), we now pass array_size_or_fail(n,m)
so that it becomes possible to detect overflow. This is particularly
interesting for global settings that might be set large enough to cause
overflows on 32-bit systems for example, resulting in small values that
then cause trouble. Now the overflow will be detected at allocation time.
Around 25 locations were updated.
Sometimes we'd like to know if some products overflow, so let's add a
pair of functions for this, for ulong and for size_t. For recent enough
compilers (gcc >= 5, clang >= 3.4) we just use __builtin_mul_overflow()
otherwise we rely on a division and a comparison before performing the
operation.
A third function, array_size_or_fail() computes the size of an array
of m elements of n bytes each, and returns the total size if it fits
in a size_t, otherwise ~0 if it does not so that passing this to
malloc() or any other variant would fail by trying to exhaust the
entire memory space.
In stksess_trash_oldest(), and process_tables_expire(), avoid freeing
elements while holding two locks, as it could be very costly.
Instead, build a linked list of elements to be free'd, and do so once we
no longer hold any lock.
This may help with github issue #3380, and may be backported to 3.3.
Since support for large buffers was added, we must be careful when chunks
are allocated. Indeed, depending on the context a large chunks may be
required if data are copied from a large buffer.
In htx_replace_blk_value() function, when a defragmentation is necessary,
the data to be replaced are copied to a chunk before the
defragmentation. However, I forgot to get large chunk when necessary by
calling alloc_trash_chunk_sz() instead of alloc_trash_chunk(). Because of
this issue, it is possible to copy data to a too small chunk, leading to a
crash.
So let's fix the issue.
Thanks to Vincent55 for finding and reporting this.
No backport needed.
appctx_htx_snd_buf() function is relying on htx_xfer() function to transfer
HTX blocks when a swap of buffers is not possible. However, it was not
properly using this function.
Indeed, originally htx_xfer() was designed to transfer blocks with a limit,
the <count> parameter, which included the blocks payload and the
meta-data. It was aligned with all calls, except for the transfer of HTX
data to the applet, in appctx_htx_snd_buf() function. In that case, the
<count> parameter is the amount of data forwarded by the stream to the
applet. So meta-data are not included.
Thanks to the previous commit ("MEDIUM: htx: Improve htx_xfer API to not count
HTX meta-data"), it is now possible to instruct htx_xfer() function that
<count> parameter does not include the meta-data.
Because of this bug, crashes can be experienced when transferring HTX data
to an applet. At first glance, lua HTTP applets and the http client are
concerned.
Stable versions from 3.3 to 3.0 are also affected. But this patch cannot be
backported as is because htx_xfer() function does not exist on these
versions.
Thaks to Yon Harlicaj for finding and reporting this.
(https://x.com/nvmb3r - https://www.linkedin.com/in/eljon-harlicaj/)
This patch add the ability to the htx_xfer() function to transfer data
without acounting the meta-data. By default, the <count> variable includes
the meta-data. But by setting the flag HTX_XFER_NO_METADATA, It is possible
to transfer HTX blocks without count meta-data. In that case, <count> will
not contain the blocks meta-data and the return value will not include them.
HTX_XFER_* flags must be declared as a bitfield. However, value of
HTX_XFER_HDRS_ONLY was set of 0x03 while it should be 0x04. So let's fix it.
This patch must be backported where the htx_xfer() function was backported
(5ead611cc "MEDIUM: htx: Add htx_xfer function to replace htx_xfer_blks").
Fix usage of stream.max-concurrent QUIC setting on the backend side.
Contrary to frontend connections, this limit must be enforced by QUIC
MUX directly. This is necessary as the peer may allow a larger number of
concurrent streams via its flow control.
First, QUIC TP initial max bidi streams value is now set to 0. This is
fine as only the HTTP/3 client is expected to open bidirectional
streams.
The most important changes is performed in qcm_avail_streams(). The
value first depends on the peer flow control. Now, it is further reduced
if necessary to not exceed the configured BE stream.max-concurrent.
Note that this new behavior may further increases current limitation on
QUIC BE reuse when a QCS instance is kept while its upper stream layer
is detached. In this case there is a risk that the connection is not
reinserted in the correct server pool, as an idle or avail one.
This is a breaking change as BE stream.max-concurrent keyword setting
meaning is changed in effect. However, this does not necessitate extra
warnings as the previous usage was in effect useless. Furthermore, QUIC
on the backend side is still considered as experimental.
This can be backported up to 3.3.
Commit 8aa854ab26a7daa613a17548f1fe1d0adb8cf61b made it so we'd store
the hash corresponding to the server parameters, so that we could detect
if we're still talking to the same server, and not use those parameters
if not.
However, when updating those parameters, we forgot to store the new
hash, which would result in the new parameters never be used, and
breakling 0RTT.
Fix that by properly update the hash when needed.
This should be backported when 8aa854ab26a7daa613a17548f1fe1d0adb8cf61b
is backported.
We're still regularly seeing insecure configs where chroot is missing.
Now that we have "chroot auto", there's no excuse for not knowing where
to chroot, so let's detect that we're starting as root, detect that the
process is allowed to chroot (i.e. no capability issue, or some hardened
containers), and if no chroot is set, let's emit a warning explaining how
to silence it, i.e. either "chroot auto" or "chroot /".
Most likely we'll start using "chroot auto" by default in 3.5 if no
usability issue is reported.
We'll recommend to use "chroot /" to explicitly disable chroot, however
there might be configurations where it would cause problems to just issue
the syscall (typically some hardened containers), so let's make sure that
"chroot /" is a nop in this case.
This reduces the total code size by 6-10% and speeds up the build a
bit. It can be further reduced by disabling the trace decoding code
inside certain subsystems like muxes. But at least like this it will
help users on small systems to reduce the footprint when not needed
by explicitly passing USE_TRACE=0 (they remain enabled by default).
In case of XPRT_QMUX init failure on the frontend side, the connection
must immediately be released. This is not the case on the backend side
as a stream can supervize the connection lifetime.
This patch performs the connection free via conn_complete_session(). As
conn is flagged with CO_FL_ERROR, this will automatically fail and
invoke session_kill_embryonic(), which ensures the session and its
connection are both freed as wanted in this case.
No need to backport.
When we store the negociated server parameters, such as the ALPN, also
store the calculated hash with the connection. If it is different, as
can happen because the IP address is different because set-dst was used,
we certainly do not want to reuse the information in the cache,
otherwise we could end up using the wrong ALPN and mux.
That means we already have to calculate the hash in connect_server()
now, while before we would not do it for Websockets, if we could not do
connection reuse, as that's all the hash was used for.
This should fix Github issue #3386
This should be backported as far as 3.2.
qcc_release_remote_stream() is called each time a remote stream is
closed. Flow control accounting is updated and when necessary, a
MAX_STREAMS_BIDI frame is prepared to allow the peer to initiate new
streams.
This patch extends stream elasticity features with the QUIC bidirection
stream flow control mechanism. The announced value can now be possibly
reduced depending on conn_calc_max_streams().
The first step is to decrement closed streams from the global committed
extra streams total. This must be performed conn_calc_max_streams() to
ensure the calculation will be valid.
Then, there is two cases depending on conn_calc_max_streams() result. If
the value is less than the peer still remaining stream window, nothing
more is performed. If the opposite case, flow control must be increased
and a MAX_STREAMS_BIDI frame is prepared, with the value adjusted to not
exceed the stream elasticity limit. Global extra streams total is then
finally incremented.
This calcul also ensures that when all streams are closed, global extra
streams accounting operations are decremented by 1, as a connection
always has access to one stream which is excluded from the global total.
Note that if stream elasticity is not active, flow control increases
principle is unchanged and remains statically performed.
This patch is labelled as major as it complexifies bidirectional stream
flow control mechanisme. This is a sensitive operation as there is a
risk of connection freeze if flow control updates are inadvertently
skipped.
Add a new QCC member <ms_bidi_rel>. This represents the number of
concurrent streams advertised similarly to ms_bidi, but as a relative
value.
This patch does not introduce any functional change. For now,
<ms_bidi_rel> will be equal to <ms_bidi_init>. However, with the
implementation of stream elasticity and dynamic adjustment for
concurrent max-streams-bidi, the former will be required to keep the
last advertised value.
When stream elasticity is active, the maximum number of concurrent bidi
streams advertised via transport parameters is now reduced depending on
the connection load. This is implemented via conn_calc_max_streams()
which returns the value to use.
This is not applied on listeners with enabled 0-RTT. Indeed, for such
connections, clients are expected to reuse the previously seen transport
parameters. The server on the other hand must not decrease several
values on the newly advertised params, in particular for the maximum
number of concurrent bidi streams. The simplest way to prevent 0-RTT
failure is to not mix stream elasticity with it.
Note that the 0-RTT limitation is only applied for the initial value :
during the connection lifetime, stream elasticity can still be used by
the MUX to dynamically reduce the stream window. This will be
implemented in a future patch.
Account QUIC frontend connections into committed_extra_streams when
stream elasticity setting is active. This is performed in QCC init and
release functions.
This patch has no impact on QUIC subsystem for now. Connections will
still allow a static number of concurrent streams based on
tune.quic.fe.stream.max-concurrent. However, this has a direct
repercussion on H2 subsystem, as a higher count of QUIC connections will
reduce the concurrent streams allowed there.
When streams-elasticity is enabled in the configuration, H2 mux is
responsible to update the global committed_extra_streams value.
Adjust these operations to ensure they are skipped if streams-elasticity
is disabled, which is the current default. This prevents unnecessary
atomic operations in this case.
No need to backport unless streams-elasticity feature is picked in older
releases.
h2_init() is now responsible to increment committed_extra_streams for
new frontend connections, in relation to the newly implemented
stream-elasticity feature. In case of an early error, a mirroring
decrement is executed on fail_stream label.
However, for now this error label can only be selected via BE conns. In
fact, it's not yet possible for h2_init() to fail after the extra
streams increment.
However, the decrement operation is kept to prevent any omissions in
case of future evolutions of h2_init() error path. To prevent reporting
of a possible dead code, add an extra comment which summarizes the
situation.
It is now possible to use "chroot auto" in the configuration. This lets
haproxy create an anonymous (cleaned up after the process terminates)
and read-only directory for chroot. This directory is created in /tmp;
we might want to support creating it in a different directory in the
future, either by respecting $TMPDIR or by allowing an optional
directory after the "auto" keyword.
When server fleets are constantly updated, using a stable distribution
across a bunch of load balancers can be convenient. The addr and port
already provide a bit of this but for situations were addresses might
differ between sites or change dynamically this does not work. The guid
is perfect for this because by definition it's supposed to designate a
single server and be unique. So when two servers anywhere have the same,
the tool that provisionned them promises that they are the same server.
So here we introduce "hash-key guid" which performs a 32-bit hash on
the GUID value. When no guid is provided, a fallback is performed on
ID, as is done for other keys.
The "id" hash-key scales the ID by a factor of 16 that tries to leave
room between the nodes on the 32-bit space to permit smooth weight
variations (e.g. during slowstart). However this does not deal well
with overlaps between server IDs. For example, assigning IDs that are
only multiples of 256 million to 16 servers yields traffic only on
one since in practice they all have the same 28 lower bits.
The new "id32" hash key bridges this gap by using the full 32-bit ID
of the server as the key. On the other hand, the user must be careful
not to switch the hash function to "none" when using incremental IDs
because in this case they might be very poorly distributed. But this
can be convenient for automated provisionning systems which assign
IDs themselves, as the full 32 bits are used now.
The "hash-type xxx none" is broken for keys that are not in type string
because the sample fetch call casts them to SMP_T_BIN, that tends to
preserve the original format (integers, IP addresses etc), but the
gen_hash() function in case of BE_LB_HFCN_NONE expects to read a string
representing a number, that it parses to retrieve the value, and just
fails on many binary types. For example, the following just always
returns key 0:
balance hash rand()
hash type consistent none
An ugly workaround is to make sure the expression returns a string, for
example this:
balance hash rand(),concat()
hash type consistent none
In order to fix most cases here, we force the conversion to type string
when using BE_LB_HFCN_NONE, but a better approach would require a larger
rework and split gen_hash() or change it to accept an integer as well,
so that the caller could cast to SMP_T_INT for BE_LB_HFCN_NONE and pass
the resulting number already parsed with the least information loss. In
this case even IPv4 addresses would be preserved.
The current approach at least addresses the initially envisioned use
cases, and the limitations have been added to the doc. This can be
backported to 3.0 though it's not really important.
Due to the check of the stored value instead of the parsed one, it was not
permitted to use server IDs above 2^31 while they are perfectly possible.
Let's refine the parsing and also update the error message to indicate the
range. The doc was also refined to reflect the relation with hash-key.
This may be backported though it wouldn't have any effect on working
configs.
Implements automatic selection of QMux MUX if "h3" ALPN has been
negotiated on top of TCP/SSL.
The first part of this change is to define "alpn" member of
mux_proto_list. This is necessary so that conn_get_best_mux_entry() can
select it when "h3" has been chosen. As a side-effect, this also
automatically sets a default ALPN to "h3" for bind lines with "proto
qmux".
The most important change is to adapt the SSL layer. On handshake
completion, the eligible MUX is retrieved via conn_select_mux_fe/be()
functions. If xprt_qmux is required by it, MUX init is delayed and QMux
handshake is started first.
This last change is necessary as connection flags CO_FL_QMUX_RECV/SEND
are only set if "proto qmux" is explicitely set. In case xprt_qmux is
activated via pure ALPN negotiation, these flags are also set on
xprt_qmux_init(). This is mandatory to ensure emission/reception of QMux
transport parameters will be performed as expected.
Add a postparsing check on TCP ALPN bind and server setting. An error is
reported if the token "h3" is present and expose-experimental-directives
is not globally activated. This ensures that QMux protocol won't be
selected if experimental features are not explicitely requested.
The check is not performed though if "proto qmux" is explicitely
defined, as this setting already checks for experimental support.
Currently, it's not possible to activate QMux without any explicit
"proto qmux" config. However, this will be implemented in a next patch,
so this check will become necessary.
The first part of this patch defines a new mux_proto_list field named
<xprt_init>. This allows to define an extra XPRT layer which should be
activated first prior to the MUX creation both on frontend and backend
sides.
This is immediately used for QMux mux_proto_list to require XPRT_QMUX
handshake. With this change, activation of QMux connection flags in
session_accept_fd() and connect_server() are adjusted to take into
account <init_xprt> field. This approach is much more evolutive than
relying on the previous MUX name.
Change in connect_server() will also be necessary to support QMux
activation on a TCP server with h3 ALPN without explicit "proto qmux".
This guarantees that MUX initialization is delayed after QMux handshake.
This patch is similar to the previous one but this time for backend
connections. The MUX selection code is directly extracted from
conn_install_mux_chk() and conn_install_mux_be().
Define a new function conn_select_mux_fe().
The objective is to have a preliminary function to determine the MUX
which will be used without initializing it. This will be useful for MUX
which relies on a specific XPRT handshake prior to its startup, which is
the case for QMux protocol.
The code of conn_select_mux_fe() is identical to the beginning of
conn_install_mux_fe() with a similar MUX selection logic. However,
connection MUX initialization is not performed in this case. In a future
patch, both functions should be merged together to reduce code
duplication.
In conn_get_best_mux() and conn_get_best_mux_entry(), the mux name was
provided sometimes based on the "proto" directive, sometimes based on
the ALPN, but in any case, it was compared again the mux_proto_list
mux_proto field. This is not correct, as ALPN can be different from the
internal mux_proto. So enhance those functions so that they wll accept
an ALPN as well. If a mux_proto is provided, that will be used, if not,
and if an ALPN is provided, then that will be used, and compared against
the ALPN provided by the mux, if any.
In struct mux_proto_list, rename the "token" field to "mux_proto". That
field should only be used to match the name provided in the "proto"
directive, and it will be soon.
This should be a no-op.
The following patch fixes a leak in case of httpclient_start() failure
in the httpclient_cli code by adding httpclient_destroy() call on error
path.
c53256adbc
BUG/MINOR: httpclient-cli: Destroy http-client context if failing to start it
However, error label may be selected prior to httpclient allocation if
CLI arguments are incorrect. This can cause a crash due to a deferencing
of an uninitialized variable. This has been detected via a compilation
error :
src/httpclient_cli.c: In function 'hc_cli_parse':
src/httpclient_cli.c:162:2: error: 'hc' may be used uninitialized in this function [-Werror=maybe-uninitialized]
162 | httpclient_destroy(hc);
| ^~~~~~~~~~~~~~~~~~~~~~
This must be backported along the above patch, which is scheduled up to
the 2.6 release.
