Willy Tarreau [Fri, 21 Feb 2020 16:40:25 +0000 (17:40 +0100)]
BUILD: fix recent build failure on unaligned archs
Last commit 2c315ee ("BUG/MEDIUM: ebtree: don't set attribute packed
without unaligned access support") accidently enclosed the semicolon
in the ifdef so it will fail when HA_UNALIGNED is not set.
Willy Tarreau [Fri, 21 Feb 2020 15:31:22 +0000 (16:31 +0100)]
CLEANUP: http/h1: rely on HA_UNALIGNED_LE instead of checking for CPU families
Now that we have flags indicating the CPU's capabilities, better use
them instead of missing some updates for new CPU families (ARMv8 was
missing there).
Willy Tarreau [Fri, 21 Feb 2020 14:47:36 +0000 (15:47 +0100)]
BUG/MEDIUM: ebtree: don't set attribute packed without unaligned access support
An alignment issue on Sparc64 with ebtrees was reported in early 2017
here https://www.mail-archive.com/haproxy@formilux.org/msg25937.html and
a similar one was finally reported in issue #512.
The problem has its roots in the fact that 64-bit keys will end up being
unaligned on such archs which do not support unaligned accesses. But on
most platforms supporting unaligned accesses, dealing with smaller nodes
results in better performance.
One of the possible problems caused by attribute packed there is that it
promotes a structure both to be unaligned and unpadded, which may come
with fun if some fields of the struct itself are accessed and such a node
is placed at an unaligned location. It's not a problem for regular unaligned
accesses but may become one for atomic operations such as the CAS on leaf_p
that's used in struct task. In practice we know that this struct is properly
aligned and is a very edge case so this patch adds comments there to remind
to be careful about it.
This patch depends on previous patch "MINOR: compiler: move CPU capabilities
definition from config.h and complete them" and could be backported to all
stable branches. It fixes issues #512 and #9.
Willy Tarreau [Fri, 21 Feb 2020 14:40:58 +0000 (15:40 +0100)]
MINOR: compiler: move CPU capabilities definition from config.h and complete them
These ones are irrelevant to the config but rather to the platform, and
as such are better placed in compiler.h.
Here we take the opportunity for declaring a few extra capabilities:
- HA_UNALIGNED : CPU supports unaligned accesses
- HA_UNALIGNED_LE : CPU supports unaligned accesses in little endian
- HA_UNALIGNED_FAST : CPU supports fast unaligned accesses
- HA_UNALIGNED_ATOMIC : CPU supports unaligned accesses in atomics
This will help remove a number of #ifdefs with arch-specific statements.
Willy Tarreau [Fri, 21 Feb 2020 12:45:58 +0000 (13:45 +0100)]
BUG/MEDIUM: shctx: make sure to keep all blocks aligned
The blocksize and the extra field are not necessarily aligned on a
machine word. This can result in crashing an align-sensitive machine
when initializing the shctx area. Let's round both sizes up to a pointer
size to make this safe everywhere.
This fixes issue #512. This should be backported as far as 1.8.
Jerome Magnin [Fri, 21 Feb 2020 09:49:12 +0000 (10:49 +0100)]
CLEANUP: sample: use iststop instead of a for loop
In sample_fetch_path we can use iststop() instead of a for loop
to find the '?' and return the correct length. This requires commit
"MINOR: ist: add an iststop() function".
Jerome Magnin [Fri, 21 Feb 2020 09:37:48 +0000 (10:37 +0100)]
BUG/MINOR: http: http-request replace-path duplicates the query string
In http_action_replace_uri() we call http_get_path() in the case of
a replace-path rule. http_get_path() will return an ist pointing to
the start of the path, but uri.ptr + uri.len points to the end of the
uri. As as result, we are matching against a string containing the
query, which we append to the "path" later, effectively duplicating
the query string.
This patch uses the iststop() function introduced in "MINOR: ist: add
an iststop() function" to find the '?' character and update the ist
length when needed.
This fixes issue #510.
The bug was introduced by commit 262c3f1a ("MINOR: http: add a new
"replace-path" action"), which was backported to 2.1 and 2.0.
Jerome Magnin [Fri, 21 Feb 2020 09:33:12 +0000 (10:33 +0100)]
MINOR: ist: add an iststop() function
Add a function that finds a character in an ist and returns an
updated ist with the length of the portion of the original string
that doesn't contain the char.
Willy Tarreau [Thu, 20 Feb 2020 10:11:50 +0000 (11:11 +0100)]
MINOR: mux-h1: pass CO_RFL_READ_ONCE to the lower layers when relevant
When we're in H1_MSG_RQBEFORE or H1_MSG_RPBEFORE, we know that the
first message is highly likely the only one and that it's pointless
to try to perform a second recvfrom() to complete a first partial
read. This is similar to what used to be done in the older I/O methods
with the CF_READ_DONTWAIT flag on the channel. So let's pass
CO_RFL_READ_ONCE to the transport layer during rcv_buf() in this case.
By doing so, in a test involving keep-alive connections with a non-null
client think time, we remove 20% of the recvfrom() calls, all of which
used to systematically fail. More precisely, we observe a drop from 5.0
recvfrom() per request with 60% failure to 4.0 per request with 50%
failure.
Willy Tarreau [Thu, 20 Feb 2020 10:04:40 +0000 (11:04 +0100)]
MINOR: connection: introduce a new receive flag: CO_RFL_READ_ONCE
This flag is currently supported by raw_sock to perform a single recv()
attempt and avoid subscribing. Typically on the request and response
paths with keep-alive, with short messages we know that it's very likely
that the first message is enough.
Willy Tarreau [Fri, 21 Feb 2020 08:58:29 +0000 (09:58 +0100)]
CLEANUP: connection: remove the definitions of conn_xprt_{stop,want}_{send,recv}
This marks the end of the transition from the connection polling states
introduced in 1.5-dev12 and the subscriptions in that arrived in 1.9.
The socket layer can now safely use its FD while all upper layers rely
exclusively on subscriptions. These old functions were removed. Some may
deserve some renaming to improved clarty though. The single call to
conn_xprt_stop_both() was dropped in favor of conn_cond_update_polling()
which already does the same.
Willy Tarreau [Fri, 21 Feb 2020 09:34:19 +0000 (10:34 +0100)]
MINOR: connection: remove the last calls to conn_xprt_{want,stop}_*
The last few calls to conn_xprt_{want,stop}_{recv,send} in the central
connection code were replaced with their strictly exact equivalent fd_*,
adding the call to conn_ctrl_ready() when it was missing.
Willy Tarreau [Fri, 21 Feb 2020 09:24:51 +0000 (10:24 +0100)]
MINOR: tcp/uxst/sockpair: use fd_want_send() instead of conn_xprt_want_send()
Just like previous commit, we don't need to pass through the connection
layer anymore to enable polling during a connect(), we know the FD, so
let's simply call fd_want_send().
Willy Tarreau [Fri, 21 Feb 2020 09:21:46 +0000 (10:21 +0100)]
MINOR: raw_sock: directly call fd_stop_send() and not conn_xprt_stop_send()
Now that we know that the connection layer is transparent for polling
changes, we have no reason for hiding behind conn_xprt_stop_send() and
can safely call fd_stop_send() on the FD once the buffer is empty.
Willy Tarreau [Fri, 21 Feb 2020 07:46:19 +0000 (08:46 +0100)]
MEDIUM: connection: remove the intermediary polling state from the connection
Historically we used to require that the connections held the desired
polling states for the data layer and the socket layer. Then with muxes
these were more or less merged into the transport layer, and now it
happens that with all transport layers having their own state, the
"transport layer state" as we have it in the connection (XPRT_RD_ENA,
XPRT_WR_ENA) is only an exact copy of the undelying file descriptor
state, but with a delay. All of this is causing some difficulties at
many places in the code because there are still some locations which
use the conn_want_* API to remain clean and only rely on connection,
and count on a later collection call to conn_cond_update_polling(),
while others need an immediate action and directly use the FD updates.
Since our updates are now much cheaper, most of them being only an
atomic test-and-set operation, and since our I/O callbacks are deferred,
there's no benefit anymore in trying to "cache" the transient state
change in the connection flags hoping to cancel them before they
become an FD event. Better make such calls transparent indirections
to the FD layer instead and get rid of the deferred operations which
needlessly complicate the logic inside.
This removes flags CO_FL_XPRT_{RD,WR}_ENA and CO_FL_WILL_UPDATE.
A number of functions related to polling updates were either greatly
simplified or removed.
Two places were using CO_FL_XPRT_WR_ENA as a hint to know if more data
were expected to be sent after a PROXY protocol or SOCKSv4 header. These
ones were simply replaced with a check on the subscription which is
where we ought to get the autoritative information from.
Now the __conn_xprt_want_* and their conn_xprt_want_* counterparts
are the same. conn_stop_polling() and conn_xprt_stop_both() are the
same as well. conn_cond_update_polling() only causes errors to stop
polling. It also becomes way more obvious that muxes should not at
all employ conn_xprt_{want|stop}_{recv,send}(), and that the call
to __conn_xprt_stop_recv() in case a mux failed to allocate a buffer
is inappropriate, it ought to unsubscribe from reads instead. All of
this definitely requires a serious cleanup.
Willy Tarreau [Thu, 20 Feb 2020 10:23:43 +0000 (11:23 +0100)]
CLEANUP: epoll: place the struct epoll_event in the stack
Historically we used to have a global epoll_event for various
manipulations involving epoll_ctl() and when threads were added,
this was turned to a thread_local, which is needlessly expensive
since it's just a temporary variable. Let's move it to a local
variable wherever it's called instead.
Willy Tarreau [Fri, 21 Feb 2020 09:13:03 +0000 (10:13 +0100)]
MINOR: checks: do not call conn_xprt_stop_send() anymore
While trying to address issue #253, Commit 5909380c ("BUG/MINOR: checks:
stop polling for write when we have nothing left to send") made sure that
we stop polling for writes when the buffer is empty. This was actually
more a workaround than a bug fix because by doing so we may be stopping
polling for an intermediary transport layer without acting on the check
itself in case there's SSL or send-proxy in the chain for example, thus
the approach is wrong. In practice due to the small size of check
requests, this will not have any impact. At best, we ought to unsubscribe
for sending, but that's already the case when we arrive in this function.
But given that the root cause of the issue was addressed later in commits cc705a6b, c5940392 and ccf3f6d1, we can now safely revert this change.
It was confirmed on the faulty config that this change doesn't have any
effect anymore on the test.
Willy Tarreau [Fri, 21 Feb 2020 08:44:39 +0000 (09:44 +0100)]
BUG/MINOR: mux: do not call conn_xprt_stop_recv() on buffer shortage
In H1/H2/FCGI, the *_get_buf() functions try to disable receipt of data
when there's no buffer available. But they do so at the lowest possible
level, which is unrelated to the upper transport layers which may still
be trying to feed data based on subscription. The correct approach here
would theorically be to only disable subscription, though when we get
there, the subscription will already have been dropped, so we can safely
just remove that call.
It's unlikely that this could have had any practical impact, as the upper
xprt layer would call this callback which would fail an not resubscribe.
Having the lowest layer disabled would just be temporary since when
re-enabling reading, a subscribe at the end of data would re-enable it.
Backport should not harm but seems useless at this point.
BUG/MAJOR: http-ana: Always abort the request when a tarpit is triggered
If an client error is reported on the request channel (CF_READ_ERROR) while a
session is tarpitted, no error is returned to the client. Concretly,
http_reply_and_close() function is not called. This function is reponsible to
forward the error to the client. But not only. It is also responsible to abort
the request. Because this function is not called when a read error is reported
on the request channel, and because the tarpit analyzer is the last one, there
is nothing preventing a connection attempt on a server while it is totally
unexpected.
So, a useless connexion on a backend server may be performed because of this
bug. If an HTTP load-balancing algorithm is used on the backend side, it leads
to a crash of HAProxy because the request was already erased.
If you have tarpit rules and if you use an HTTP load-balancing algorithm on your
backends, you must apply this patch. Otherwise a simple TCP reset on a tarpitted
connexion will most likely crash your HAProxy. A safe workaround is to use a
silent-drop rule or a deny rule instead of a tarpit.
This bug also affect the legacy code. It is in fact an very old hidden bug. But
the refactoring of process_stream() in the 1.9 makes it visible. And,
unfortunately, with the HTX, it is easier to hit it because many processing has
been moved in lower layers, in the muxes.
It must be backported as far as 1.9. For the 2.0 and the 1.9, the legacy HTTP
code must also be patched the same way. For older versions, it may be backported
but the bug seems to not impact them.
Thanks to Olivier D <webmaster@ajeux.com> to have reported the bug and provided
all the infos to analyze it.
Tim Duesterhus [Wed, 19 Feb 2020 10:41:13 +0000 (11:41 +0100)]
BUG/MINOR: ssl: Stop passing dynamic strings as format arguments
gcc complains rightfully:
src/ssl_sock.c: In function ‘ssl_load_global_issuers_from_path’:
src/ssl_sock.c:9860:4: warning: format not a string literal and no format arguments [-Wformat-security]
ha_warning(warn);
^
MINOR: http-ana: Match on the path if the monitor-uri starts by a /
if the monitor-uri starts by a slash ('/'), the matching is performed against
the request's path instead of the request's uri. It is a workaround to let the
HTTP/2 requests match the monitor-uri. Indeed, in HTTP/2, clients are encouraged
to send absolute URIs only.
This patch is not tagged as a bug, because the previous behavior matched exactly
what the doc describes. But it may surprise that HTTP/2 requests don't match the
monitor-uri.
This patch may be backported to 2.1 because URIs of HTTP/2 are stored using the
absolute-form starting this version. For previous versions, this patch will only
helps explicitely absolute HTTP/1 requests (and only the HTX part because on the
legacy HTTP, all the URI is matched).
BUG/MINOR: http-ana: Matching on monitor-uri should be case-sensitive
The monitor-uri should be case-sensitive. In reality, the scheme and the host
part are case-insensitives and only the path is case-sensive. But concretely,
since the start, the matching on the monitor-uri is case-sensitive. And it is
probably the expected behavior of almost all users.
This patch must be backported as far as 1.9. For HAProxy 2.0 and 1.9, it must be
applied on src/proto_htx.c.
Willy Tarreau [Tue, 18 Feb 2020 13:42:33 +0000 (14:42 +0100)]
REGTESTS: use "command -v" instead of "which"
Ilya reported that the "which" utility is not that much portable and is
absent from Fedora. "type -p" is not portable either, and the correct
solution appears to be "command -v", so let's use this for now, we can
change it again in the future in case of problems.
Emmanuel Hocdet [Fri, 4 Jan 2019 10:08:20 +0000 (11:08 +0100)]
MINOR: ssl: add "issuers-chain-path" directive.
Certificates loaded with "crt" and "crt-list" commonly share the same
intermediate certificate in PEM file. "issuers-chain-path" is a global
directive to share intermediate chain certificates in a directory. If
certificates chain is not included in certificate PEM file, haproxy
will complete chain if issuer match the first certificate of the chain
stored via "issuers-chain-path" directive. Such chains will be shared
in memory.
Willy Tarreau [Tue, 18 Feb 2020 13:27:44 +0000 (14:27 +0100)]
BUG/MINOR: sample: exit regsub() in case of trash allocation error
As reported in issue #507, since commiy 07e1e3c93e ("MINOR: sample:
regsub now supports backreferences") we must not proceed in regsub()
if we fali to allocate a trash (which in practice never happens). No
backport needed.
BUG/MINOR: stream: Don't incr frontend cum_req counter when stream is closed
This counter is already incremented when a new request is received (or if an
error occurred waiting it). So it must not be incremented when the stream is
terminated, at the end of process_strem(). This bug was introduced by the commit cff0f739e ("MINOR: counters: Review conditions to increment counters from
analysers").
BUG/MINOR: http-htx: Don't return error if authority is updated without changes
When an Host header is updated, the autority part, if any, is also updated to
keep the both syncrhonized. But, when the update is performed while there is no
change, a failure is reported while, in reality, no update is necessary. This
bug was introduced by the commit d7b7a1ce5 ("MEDIUM: http-htx: Keep the Host
header and the request start-line synchronized").
This commit was pushed in the 2.1. But on this version, the bug is hidden
because rewrite errors are silently ignored. And because it happens when there
is no change, if the rewrite fails, noone notices it. But since the 2.2, rewrite
errors are now fatals by default. So when the bug is hit, a 500 error is
returned to the client. Without this fix, a workaround is to disable the strict
rewriting mode (see the "strict-mode" HTTP rule).
The following HTTP rule is a good way to reproduce the bug if a request with an
authority is received. In HTT2, it is pretty common.
acl host_header_exists req.hdr(host) -m found
http-request set-header host %[req.hdr(host)] if host_header_exists
This patch must be backported to 2.1 and everywhere the commit d7b7a1ce5 is
backported. It should fix the issue #494.
BUG/MINOR: filters: Count HTTP headers as filtered data but don't forward them
In flt_analyze_http_headers() HTTP analyzer, we must not forward systematically
the headers. We must only count them as filtered data (ie. increment the offset
of the right size). It is the http_payload callback responsibility to decide to
forward headers or not by forwarding at least 1 byte of payload. And there is
always at least 1 byte of payload to forward, the EOM block.
This patch depends on following commits:
* MINOR: filters: Forward data only if the last filter forwards something
* MINOR: http-htx: Add a function to retrieve the headers size of an HTX message
This patch must be backported with commits above as far as 1.9. In HAProxy 2.0
and 1.9, the patch must be adapted because of the legacy HTTP code.
MINOR: filters: Forward data only if the last filter forwards something
In flt_tcp_payload() and flt_http_payload(), if the last filter does not
forwarding anything, nothing is forwarded, not even the already filtered
data. For now, this patch is useless because the last filter is always sync with
the stream's offset. But it will be mandatory for a bugfix.
MINOR: http-htx: Add a function to retrieve the headers size of an HTX message
http_get_hdrs_size() function may now be used to get the bytes held by headers
in an HTX message. It only works if the headers were not already
forwarded. Metadata are not counted here.
Willy Tarreau [Mon, 17 Feb 2020 05:34:11 +0000 (06:34 +0100)]
BUG/MINOR: tools: also accept '+' as a valid character in an identifier
The function is_idchar() was added by commit 36f586b ("MINOR: tools:
add is_idchar() to tell if a char may belong to an identifier") to
ease matching of sample fetch/converter names. But it lacked support
for the '+' character used in "base32+src" and "url32+src". A quick
way to figure the list of supported sample fetch+converter names is
to issue the following command:
Jerome Magnin [Sun, 16 Feb 2020 18:20:19 +0000 (19:20 +0100)]
MINOR: sample: regsub now supports backreferences
Now that the configuration parser is more flexible with samples,
converters and their arguments, we can leverage this to enable
support for backreferences in regsub.
Willy Tarreau [Sun, 16 Feb 2020 09:46:37 +0000 (10:46 +0100)]
BUG/MINOR: arg: fix again incorrect argument length check
Recent commit 807aef8a14 ("BUG/MINOR: arg: report an error if an argument
is larger than bufsize") aimed at fixing the argument length check but
relied on the fact that the end of string was not reached, except that
it forgot to consider the delimiters (comma and parenthesis) which are
valid conditions to break out of the loop. This used to break simple
expressions like "hdr(xff,1)". Thanks to Jérôme for reporting this.
Willy Tarreau [Sat, 15 Feb 2020 14:24:28 +0000 (15:24 +0100)]
SCRIPTS: announce-release: use mutt -H instead of -i to include the draft
Commit 0f5ce6014a ("SCRIPTS: announce-release: place the send command
in the mail's header") broke the announce-release script: by not having
to edit the message at all anymore, mutt does nothing when sending, but
it still does if the message is edited (which was the case before). With
some testing, it appears that mutt -H does work when there's no change,
so let's use this instead. This should be backported till 1.7.
Willy Tarreau [Sat, 15 Feb 2020 13:54:28 +0000 (14:54 +0100)]
BUG/MINOR: arg: report an error if an argument is larger than bufsize
Commit ef21facd99 ("MEDIUM: arg: make make_arg_list() support quotes
in arguments") removed the chunk_strncpy() to fill the trash buffer
as the input is being parsed, and accidently dropped the jump to the
error path in case the argument is too large, which is now fixed.
No backport is needed, this is for 2.2. This addresses issue #502.
Willy Tarreau [Fri, 14 Feb 2020 16:33:06 +0000 (17:33 +0100)]
MEDIUM: log-format: make the LF parser aware of sample expressions' end
For a very long time it used to be impossible to pass a closing square
bracket as a valid character in argument to a sample fetch function or
to a converter because the LF parser used to stop on the first such
character found and to pass what was between the first '[' and the first
']' to sample_parse_expr().
This patch addresses this by passing the whole string to sample_parse_expr()
which is the only one authoritative to indicate the first character that
does not belong to the expression. The LF parser then verifies it matches
a ']' or fails. As a result it is finally possible to write rules such as
the following, which is totally valid an unambigous :
Willy Tarreau [Fri, 14 Feb 2020 15:50:14 +0000 (16:50 +0100)]
MINOR: sample: make sample_parse_expr() able to return an end pointer
When an end pointer is passed, instead of complaining that a comma is
missing after a keyword, sample_parse_expr() will silently return the
pointer to the current location into this return pointer so that the
caller can continue its parsing. This will be used by more complex
expressions which embed sample expressions, and may even permit to
embed sample expressions into arguments of other expressions.
Willy Tarreau [Fri, 14 Feb 2020 12:37:20 +0000 (13:37 +0100)]
MEDIUM: arg: make make_arg_list() support quotes in arguments
Now it becomes possible to reuse the quotes within arguments, allowing
the parser to distinguish a ',' or ')' that is part of the value from
one which delimits the argument. In addition, ',' and ')' may be escaped
using a backslash. However, it is also important to keep in mind that
just like in shell, quotes are first resolved by the word tokenizer, so
in order to pass quotes that are visible to the argument parser, a second
level is needed, either using backslash escaping, or by using an alternate
type.
For example, it's possible to write this to append a comma:
Note that due to the wide use of '\' in front of parenthesis in regex,
the backslash character will purposely *not* escape parenthesis, so that
'\)' placed in quotes is passed verbatim to a regex engine.
Willy Tarreau [Fri, 14 Feb 2020 10:34:35 +0000 (11:34 +0100)]
MEDIUM: arg: copy parsed arguments into the trash instead of allocating them
For each and every argument parsed by make_arg_list(), there was an
strndup() call, just so that we have a trailing zero for most functions,
and this temporary buffer is released afterwards except for strings where
it is kept.
Proceeding like this is not convenient because 1) it performs a huge
malloc/free dance, and 2) it forces to decide upfront where the argument
ends, which is what prevents commas and right parenthesis from being used.
This patch makes the function copy the temporary argument into the trash
instead, so that we avoid the malloc/free dance for most all non-string
args (e.g. integers, addresses, time, size etc), and that we can later
produce the contents on the fly while parsing the input. It adds a length
check to make sure that the argument is not longer than the buffer size,
which should obviously never be the case but who knows what people put in
their configuration.
Willy Tarreau [Fri, 14 Feb 2020 07:40:37 +0000 (08:40 +0100)]
MEDIUM: arg: make make_arg_list() stop after its own arguments
The main problem we're having with argument parsing is that at the
moment the caller looks for the first character looking like an end
of arguments (')') and calls make_arg_list() on the sub-string inside
the parenthesis.
Let's first change the way it works so that make_arg_list() also
consumes the parenthesis and returns the pointer to the first char not
consumed. This will later permit to refine each argument parsing.
Willy Tarreau [Fri, 14 Feb 2020 17:27:10 +0000 (18:27 +0100)]
MINOR: sample/acl: use is_idchar() to locate the fetch/conv name
Instead of scanning a string looking for an end of line, ')' or ',',
let's only accept characters which are actually valid identifier
characters. This will let the parser know that in %[src], only "src"
is the sample fetch name, not "src]". This was done both for samples
and ACLs since they are the same here.
Willy Tarreau [Fri, 14 Feb 2020 10:31:41 +0000 (11:31 +0100)]
MINOR: chunk: implement chunk_strncpy() to copy partial strings
This does like chunk_strcpy() except that the maximum string length may
be limited by the caller. A trailing zero is always appended. This is
particularly handy to extract portions of strings to put into the trash
for use with libc functions requiring a nul-terminated string.
MINOR: mux-fcgi: Make the capture of the path-info optional in pathinfo regex
Now, only one capture is mandatory in the path-info regex, the one matching the
script-name. The path-info capture is optional. Of couse, it must be defined to
fill the PATH_INFO parameter. But it is not mandatory. This way, it is possible
to get the script-name part from the path, excluding the path-info.
This patch is small enough to be backported to 2.1.
BUG/MINOR: mux-fcgi: Forbid special characters when matching PATH_INFO param
If a regex to match the PATH_INFO parameter is configured, it systematically
fails if a newline or a null character is present in the URL-decoded path. So,
from the moment there is at least a "%0a" or a "%00" in the request path, we
always fail to get the PATH_INFO parameter and all the decoded path is used for
the SCRIPT_NAME parameter.
It is probably not the expected behavior. Because, most of time, these
characters are not expected at all in a path, an error is now triggered when one
of these characters is found in the URL-decoded path before trying to execute
the path_info regex. However, this test is not performed if there is no regex
configured.
Note that in reality, the newline character is only a problem when HAProxy is
complied with pcre or pcre2 library and conversely, the null character is only a
problem for the libc's regex library. But both are always excluded to avoid any
inconsistency depending on compile options.
An alternative, not implemented yet, is to replace these characters by another
one. If someone complains about this behavior, it will be re-evaluated.
This patch must be backported to all versions supporting the FastCGI
applications, so to 2.1 for now.
Olivier Houchard [Fri, 14 Feb 2020 12:23:45 +0000 (13:23 +0100)]
BUG/MEDIUM: muxes: Use the right argument when calling the destroy method.
When calling the mux "destroy" method, the argument should be the mux
context, not the connection. In a few instances in the mux code, the
connection was used (mainly when the session wouldn't handle the idle
connection, and the server pool was fool), and that could lead to random
segfaults.
Willy Tarreau [Wed, 12 Feb 2020 17:21:11 +0000 (18:21 +0100)]
SCRIPTS: make announce-release executable again
I managed to mess up with the file's permission while using a temporary
one during last release, and to backport the non-exec version everywhere.
This can be backported as far as 1.7 now.
MINOR: build: add aix72-gcc build TARGET and power{8,9} CPUs
As haproxy wont build on AIX 7.2 using the old "aix52" TARGET a new
TARGET was introduced which adds two special CFLAGS to prevent the
loading of AIXs xmem.h and var.h. This is done by defining the
corresponding include-guards _H_XMEM and _H_VAR. Without excluding
those headers-files the build fails because of redefinition errors:
1)
CC src/mux_fcgi.o
In file included from /usr/include/sys/uio.h:90,
from /opt/freeware/lib/gcc/powerpc-ibm-aix7.1.0.0/8.3.0/include-fixed/sys/socket.h:104,
from include/common/compat.h:32,
from include/common/cfgparse.h:25,
from src/mux_fcgi.c:13:
src/mux_fcgi.c:204:13: error: expected ':', ',', ';', '}' or '__attribute__' before '.' token
struct ist rem_addr;
^~~~~~~~
2)
CC src/cfgparse-listen.o
In file included from include/types/arg.h:31,
from include/types/acl.h:29,
from include/types/proxy.h:41,
from include/proto/log.h:34,
from include/common/cfgparse.h:30,
from src/mux_h2.c:13:
include/types/vars.h:30:8: error: redefinition of 'struct var'
struct var {
^~~
Futhermore, to enable multithreading via USE_THREAD, the atomic
library was added to the LDFLAGS. Finally, two new CPUs were added
to simplify the usage of power8 and power9 optimizations.
This TARGET was only tested on GCC 8.3 and may or may not work on
IBM's native C-compiler (XLC).
Willy Tarreau [Wed, 12 Feb 2020 09:15:34 +0000 (10:15 +0100)]
BUG/MINOR: listener: enforce all_threads_mask on bind_thread on init
When intializing a listener, let's make sure the bind_thread mask is
always limited to all_threads_mask when inserting the FD. This will
avoid seeing listening FDs with bits corresponding to threads that are
not active (e.g. when using "bind ... process 1/even"). The side effect
is very limited, all that was identified is that atomic operations are
used in fd_update_events() when not necessary. It's more a matter of
long-term correctness in practice.
This fix might be backported as far as 1.8 (then proto_sockpair must
be dropped).
Willy Tarreau [Wed, 12 Feb 2020 09:01:29 +0000 (10:01 +0100)]
BUG/MEDIUM: listener: only consider running threads when resuming listeners
In bug #495 we found that it is possible to resume a listener on an
inexistent thread. This happens when a bind's thread_mask contains bits
out of the active threads mask, such as when using "1/odd" or "1/even".
The thread_mask was used as-is to pick a thread number to re-enable the
listener, and given that the highest number is used, 1/odd or 1/even can
produce quite high thread numbers and crash the process by queuing some
entries into non-existent lists.
This bug is an incomplete fix of commit 413e926ba ("BUG/MAJOR: listener:
fix thread safety in resume_listener()") though it will only trigger if
some bind lines are explicitly bound to thread numbers higher than the
thread count. The fix must be backported to all branches having the fix
above (as far as 1.8, though the code is different there, see the commit
message in 1.8 for changes).
There are a few other places where bind_thread is used without
enforcing all_thread_mask, namely when doing fd_insert() while creating
listeners. It seems harmless but would probably deserve another fix.
Willy Tarreau [Tue, 11 Feb 2020 09:17:52 +0000 (10:17 +0100)]
CLEANUP: mini-clist: simplify nested do { while(1) {} } while (0)
While looking for other occurrences of do { continue; } while (0) I
found these few leftovers in mini-clist where an outer loop was made
around "do { } while (0)" then another loop was placed inside just to
handle the continue. Let's clean this up by just removing the outer
one. Most of the patch is only the inner part of the loop that is
reindented. It was verified that the resulting code is the same.
Willy Tarreau [Tue, 11 Feb 2020 09:08:05 +0000 (10:08 +0100)]
BUG/MINOR: connection: correctly retry I/O on signals
Issue #490 reports that there are a few bogus constructs of the famous
"do { if (cond) continue; } while (0)" in the connection code, that are
used to retry on I/O failures caused by receipt of a signal. Let's turn
them into the more correct "while (1) { if (cond) continue; break }"
instead. This may or may not be backported, it shouldn't have any
visible effect.
Ilya Shipitsin [Tue, 11 Feb 2020 08:16:02 +0000 (13:16 +0500)]
BUILD: cirrus-ci: switch to "snap" images to unify openssl naming
"snap" images are updated frequently, while regular images are updated quarterly.
so, mixing "snap" and regular images lead to package naming mismatch, which will occur every
quarter. we cannot use 11.3 release image, it is broken, so we have to use 11.3 "snap" image.
Thus let us use all "snap" images. 13-snap is first introduced with this commit.
Willy Tarreau [Tue, 11 Feb 2020 05:43:37 +0000 (06:43 +0100)]
BUG/MINOR: unix: better catch situations where the unix socket path length is close to the limit
We do have some checks for the UNIX socket path length to validate the
full pathname of a unix socket but the pathname extension is only taken
into account when using a bind_prefix. The second check only matches
against MAXPATHLEN. So this means that path names between 98 and 108
might successfully parse but fail to bind. Let's adjust the check in
the address parser and refine the error checking at the bind() step.
Willy Tarreau [Tue, 11 Feb 2020 03:38:56 +0000 (04:38 +0100)]
BUG/MAJOR: mux-h2: don't wake streams after connection was destroyed
In commit 477902b ("MEDIUM: connections: Get ride of the xprt_done
callback.") we added an inconditional call to h2_wake_some_streams()
in h2_wake(), though we must not do it if the connection is destroyed
or we end up with a use-after-free. In this case it's already done in
h2_process() before destroying the connection anyway.
Let's just add this test for now. A cleaner approach might consist in
doing it in the h2_process() function itself when a connection status
change is detected.
BUG/MEDIUM: tcp-rules: Fix track-sc* actions for L4/L5 TCP rules
A bug was introduced during TCP rules refactoring by the commit ac98d81f4
("MINOR: http-rule/tcp-rules: Make track-sc* custom actions"). There is no
stream when L4/L5 TCP rules are evaluated. For these rulesets, In track-sc*
actions, we must take care to rely on the session instead of the stream.
Because of this bug, any evaluation of L4/L5 TCP rules using a track-sc* action
leads to a crash of HAProxy.
No backport needed, except if the above commit is backported.
The code which is supposed to apply the bind_conf configuration on the
SSL_CTX was not called correctly. Indeed it was called with the previous
SSL_CTX so the new ones were left with default settings. For example the
ciphers were not changed.
BUG/MINOR: http-act: Set stream error flag before returning an error
In action_http_set_status(), when a rewrite error occurred, the stream error
flag must be set before returning the error.
No need to backport this patch except if commit 333bf8c33 ("MINOR: http-rules:
Set SF_ERR_PRXCOND termination flag when a header rewrite fails") is
backported. This bug was reported in issue #491.
Willy Tarreau [Fri, 7 Feb 2020 07:26:49 +0000 (08:26 +0100)]
SCRIPTS: backport: fix the master branch detection
The condition was inverted. When the branch was the master, it was
harmless because it caused an extra "checkout master", but when it
was not the master, the commit could be applied to the wrong branch
and it could even possibly not match the name to stop on.
Willy Tarreau [Fri, 7 Feb 2020 07:10:06 +0000 (08:10 +0100)]
SCRIPTS: announce-release: place the send command in the mail's header
I'm fed up with having to scroll my terminals trying to look for the
mail send command printed 30 minutes before the release, let's have
it copied into the e-mail template itself, and replace the old headers
that used to be duplicated there and that are not needed anymore.
Willy Tarreau [Fri, 7 Feb 2020 03:12:19 +0000 (04:12 +0100)]
[RELEASE] Released version 2.2-dev2
Released version 2.2-dev2 with the following main changes :
- BUILD: CI: temporarily mark openssl-1.0.2 as allowed failure
- MEDIUM: cli: Allow multiple filter entries for "show table"
- BUG/MEDIUM: netscaler: Don't forget to allocate storage for conn->src/dst.
- BUG/MINOR: ssl: ssl_sock_load_pem_into_ckch is not consistent
- BUILD: stick-table: fix build errors introduced by last stick-table change
- BUG/MINOR: cli: Missing arg offset for filter data values.
- MEDIUM: streams: Always create a conn_stream in connect_server().
- MEDIUM: connections: Get ride of the xprt_done callback.
- CLEANUP: changelog: remove the duplicate entry for 2.2-dev1
- BUILD: CI: move cygwin builds to Github Actions
- MINOR: cli: Report location of errors or any extra data for "show table"
- BUG/MINOR: ssl/cli: free the previous ckch content once a PEM is loaded
- CLEANUP: backend: remove useless test for inexistent connection
- CLEANUP: backend: shut another false null-deref in back_handle_st_con()
- CLEANUP: stats: shut up a wrong null-deref warning from gcc 9.2
- BUG/MINOR: ssl: increment issuer refcount if in chain
- BUG/MINOR: ssl: memory leak w/ the ocsp_issuer
- BUG/MINOR: ssl: typo in previous patch
- BUG/MEDIUM: connections: Set CO_FL_CONNECTED in conn_complete_session().
- BUG/MINOR: ssl/cli: ocsp_issuer must be set w/ "set ssl cert"
- MEDIUM: connection: remove CO_FL_CONNECTED and only rely on CO_FL_WAIT_*
- BUG/MEDIUM: 0rtt: Only consider the SSL handshake.
- MINOR: stream-int: always report received shutdowns
- MINOR: connection: remove CO_FL_SSL_WAIT_HS from CO_FL_HANDSHAKE
- MEDIUM: connection: use CO_FL_WAIT_XPRT more consistently than L4/L6/HANDSHAKE
- MINOR: connection: remove checks for CO_FL_HANDSHAKE before I/O
- MINOR: connection: do not check for CO_FL_SOCK_RD_SH too early
- MINOR: connection: don't check for CO_FL_SOCK_WR_SH too early in handshakes
- MINOR: raw-sock: always check for CO_FL_SOCK_WR_SH before sending
- MINOR: connection: remove some unneeded checks for CO_FL_SOCK_WR_SH
- BUG/MINOR: stktable: report the current proxy name in error messages
- BUG/MEDIUM: mux-h2: make sure we don't emit TE headers with anything but "trailers"
- MINOR: lua: Add hlua_prepend_path function
- MINOR: lua: Add lua-prepend-path configuration option
- MINOR: lua: Add HLUA_PREPEND_C?PATH build option
- BUILD: cfgparse: silence a bogus gcc warning on 32-bit machines
- BUG/MINOR: http-ana: Increment the backend counters on the backend
- BUG/MINOR: stream: Be sure to have a listener to increment its counters
- BUG/MEDIUM: streams: Move the conn_stream allocation outside #IF USE_OPENSSL.
- REGTESTS: make the set_ssl_cert test require version 2.2
- BUG/MINOR: ssl: Possible memleak when allowing the 0RTT data buffer.
- MINOR: ssl: Remove dead code.
- BUG/MEDIUM: ssl: Don't forget to free ctx->ssl on failure.
- BUG/MEDIUM: stream: Don't install the mux in back_handle_st_con().
- MEDIUM: streams: Don't close the connection in back_handle_st_con().
- MEDIUM: streams: Don't close the connection in back_handle_st_rdy().
- BUILD: CI: disable slow regtests on Travis
- BUG/MINOR: tcpchecks: fix the connect() flags regarding delayed ack
- BUG/MINOR: http-rules: Always init log-format expr for common HTTP actions
- BUG/MINOR: connection: fix ip6 dst_port copy in make_proxy_line_v2
- BUG/MINOR: dns: allow 63 char in hostname
- MINOR: proxy: clarify number of connections log when stopping
- DOC: word converter ignores delimiters at the start or end of input string
- MEDIUM: raw-sock: remove obsolete calls to fd_{cant,cond,done}_{send,recv}
- BUG/MINOR: ssl/cli: fix unused variable with openssl < 1.0.2
- MEDIUM: pipe/thread: reduce the locking overhead
- MEDIUM: pipe/thread: maintain a per-thread local cache of recently used pipes
- BUG/MEDIUM: pipe/thread: fix atomicity of pipe counters
- MINOR: tasks: move the list walking code to its own function
- MEDIUM: tasks: implement 3 different tasklet classes with their own queues
- MEDIUM: tasks: automatically requeue into the bulk queue an already running tasklet
- OPTIM: task: refine task classes default CPU bandwidth ratios
- BUG/MEDIUM: connections: Don't forget to unlock when killing a connection.
- MINOR: task: permanently flag tasklets waking themselves up
- MINOR: task: make sched->current also reflect tasklets
- MINOR: task: detect self-wakeups on tl==sched->current instead of TASK_RUNNING
- OPTIM: task: readjust CPU bandwidth distribution since last update
- MINOR: task: don't set TASK_RUNNING on tasklets
- BUG/MEDIUM: memory_pool: Update the seq number in pool_flush().
- MINOR: memory: Only init the pool spinlock once.
- BUG/MEDIUM: memory: Add a rwlock before freeing memory.
- BUG/MAJOR: memory: Don't forget to unlock the rwlock if the pool is empty.
- MINOR: ssl: ssl-load-extra-files configure loading of files
- SCRIPTS: add a new "backport" script to simplify long series of backports
- BUG/MINOR: ssl: we may only ignore the first 64 errors
- SCRIPTS: use /usr/bin/env bash instead of /bin/bash for scripts
- BUG/MINOR: ssl: clear the SSL errors on DH loading failure
- CLEANUP: hpack: remove a redundant test in the decoder
- CLEANUP: peers: Remove unused static function `free_dcache`
- CLEANUP: peers: Remove unused static function `free_dcache_tx`
- CONTRIB: debug: add missing flags SF_HTX and SF_MUX
- CONTRIB: debug: add the possibility to decode the value as certain types only
- CONTRIB: debug: support reporting multiple values at once
- BUG/MINOR: http-act: Use the good message to test strict rewritting mode
- MINOR: global: Set default tune.maxrewrite value during global structure init
- MINOR: http-rules: Set SF_ERR_PRXCOND termination flag when a header rewrite fails
- MINOR: http-htx: Emit a warning if an error file runs over the buffer's reserve
- MINOR: htx: Add a function to append an HTX message to another one
- MINOR: htx/channel: Add a function to copy an HTX message in a channel's buffer
- BUG/MINOR: http-ana: Don't overwrite outgoing data when an error is reported
- MINOR: dns: Dynamically allocate dns options to reduce the act_rule size
- MINOR: dns: Add function to release memory allocated for a do-resolve rule
- BUG/MINOR: http-ana: Reset HTX first index when HAPRoxy sends a response
- BUG/MINOR: http-ana: Set HTX_FL_PROXY_RESP flag if a server perform a redirect
- MINOR: http-rules: Add a flag on redirect rules to know the rule direction
- MINOR: http-rules: Handle the rule direction when a redirect is evaluated
- MINOR: http-ana: Rely on http_reply_and_close() to handle server error
- MINOR: http-ana: Add a function for forward internal responses
- MINOR: http-ana/http-rules: Use dedicated function to forward internal responses
- MEDIUM: http: Add a ruleset evaluated on all responses just before forwarding
- MEDIUM: http-rules: Add the return action to HTTP rules
- MEDIUM: http-rules: Support extra headers for HTTP return actions
- CLEANUP: lua: Remove consistency check for sample fetches and actions
- BUG/MINOR: http-ana: Increment failed_resp counters on invalid response
- MINOR: lua: Get the action return code on the stack when an action finishes
- MINOR: lua: Create the global 'act' object to register all action return codes
- MINOR: lua: Add act:wake_time() function to set a timeout when an action yields
- MEDIUM: lua: Add ability for actions to intercept HTTP messages
- REGTESTS: Add reg tests for the HTTP return action
- REGTESTS: Add a reg test for http-after-response rulesets
- BUILD: lua: silence a warning on systems where longjmp is not marked as noreturn
- MINOR: acl: Warn when an ACL is named 'or'
- CONTRIB: debug: also support reading values from stdin
- SCRIPTS: backport: use short revs and resolve the initial commit
- BUG/MINOR: acl: Fix type of log message when an acl is named 'or'
Tim Duesterhus [Thu, 6 Feb 2020 21:04:03 +0000 (22:04 +0100)]
BUG/MINOR: acl: Fix type of log message when an acl is named 'or'
The patch adding this check initially only issued a warning, instead of
being fatal. It was changed before committing. However when making this
change the type of the log message was not changed from `ha_warning` to
`ha-alert`. This patch makes this forgotten adjustment.
Willy Tarreau [Thu, 6 Feb 2020 17:38:19 +0000 (18:38 +0100)]
SCRIPTS: backport: use short revs and resolve the initial commit
I find myself often getting trapped into calling "backport 2.0 HEAD" which
doesn't work because "HEAD" is passed as the argument to cherry-pick in
other repos. Let's resolve it first. And also let's shorten the commit IDs
to make the error messages more readable and to ease copy-paste.
Willy Tarreau [Thu, 6 Feb 2020 17:17:50 +0000 (18:17 +0100)]
CONTRIB: debug: also support reading values from stdin
This is convenient when processing large dumps, it allows to copy-paste
values to inspect from one window to another, or to directly transfer
a "show fd"/"show stream" output through sed. In order to do this, simply
pass "-" alone instead of the value and they will all be read one line at
a time from stdin. For example, in order to quickly print the different
set of connection flags from "show fd", this is sufficient:
sed -ne 's/^.* cflg=\([^ ]*\).*/\1/p' | contrib/debug/flags conn -
Tim Duesterhus [Wed, 5 Feb 2020 20:00:50 +0000 (21:00 +0100)]
MINOR: acl: Warn when an ACL is named 'or'
Consider a configuration like this:
> acl t always_true
> acl or always_false
>
> http-response set-header Foo Bar if t or t
The 'or' within the condition will be treated as a logical disjunction
and the header will be set, despite the ACL 'or' being falsy.
This patch makes it an error to declare such an ACL that will never
work. This patch may be backported to stable releases, turning the
error into a warning only (the code was written in a way to make this
trivial). It should not break anything and might improve the users'
lifes.
Willy Tarreau [Thu, 6 Feb 2020 14:55:41 +0000 (15:55 +0100)]
BUILD: lua: silence a warning on systems where longjmp is not marked as noreturn
If the longjmp() call is not flagged as "noreturn", for example, because the
operating system doesn't target a gcc-compatible compiler, we may get this
warning when building Lua :
src/hlua.c: In function 'hlua_panic_ljmp':
src/hlua.c:128:1: warning: no return statement in function returning non-void [-Wreturn-type]
static int hlua_panic_ljmp(lua_State *L) { longjmp(safe_ljmp_env, 1); }
^~~~~~
The function's prototype cannot be changed because it must be compatible
with Lua's callbacks. Let's simply enclose the call inside WILL_LJMP()
which we created exactly to signal a call to longjmp(). It lets the compiler
know we won't get back into the function and that the return statement is
not needed.
REGTESTS: Add a reg test for http-after-response rulesets
A reg test has been added to ensure the evaluation of http-after-responses rules
is functionnal for all kind of responses (server, applet and internal
responses).
REGTESTS: Add reg tests for the HTTP return action
2 reg tests have been added to ensure the HTTP return action is functionnal. A
reg test is about returning error files. The other one is about returning
default responses and responses based on string or file payloads.
MEDIUM: lua: Add ability for actions to intercept HTTP messages
It is now possible to intercept HTTP messages from a lua action and reply to
clients. To do so, a reply object must be provided to the function
txn:done(). It may contain a status code with a reason, a header list and a
body. By default, if an empty reply object is used, an empty 200 response is
returned. If no reply is passed when txn:done() is called, the previous
behaviour is respected, the transaction is terminated and nothing is returned to
the client. The same is done for TCP streams. When txn:done() is called, the
action is terminated with the code ACT_RET_DONE on success and ACT_RET_ERR on
error, interrupting the message analysis.
The reply object may be created for the lua, by hand. Or txn:reply() may be
called. If so, this object provides some methods to fill it:
* Reply:set_status(<status> [ <reason>]) : Set the status and optionally the
reason. If no reason is provided, the default one corresponding to the status
code is used.
* Reply:add_header(<name>, <value>) : Add a header. For a given name, the
values are stored in an ordered list.
* Reply:del_header(<name>) : Removes all occurrences of a header name.
MINOR: lua: Add act:wake_time() function to set a timeout when an action yields
This function may be used to defined a timeout when a lua action returns
act:YIELD. It is a way to force to reexecute the script after a short time
(defined in milliseconds).
Unlike core:sleep() or core:yield(), the script is fully reexecuted if it
returns act:YIELD. With core functions to yield, the script is interrupted and
restarts from the yield point. When a script returns act:YIELD, it is finished
but the message analysis is blocked on the action waiting its end.
MINOR: lua: Create the global 'act' object to register all action return codes
ACT_RET_* code are now available from lua scripts. The gloabl object "act" is
used to register these codes as constant. Now, lua actions can return any of
following codes :
* act.CONTINUE for ACT_RET_CONT
* act.STOP for ACT_RET_STOP
* act.YIELD for ACT_RET_YIELD
* act.ERROR for ACT_RET_ERR
* act.DONE for ACT_RET_DONE
* act.DENY for ACT_RET_DENY
* act.ABORT for ACT_RET_ABRT
* act.INVALID for ACT_RET_INV
For instance, following script denied all requests :
core.register_action("deny", { "http-req" }, function (txn)
return act.DENY
end)
Thus "http-request lua.deny" do exactly the same than "http-request deny".
MINOR: lua: Get the action return code on the stack when an action finishes
When an action successfully finishes, the action return code (ACT_RET_*) is now
retrieve on the stack, ff the first element is an integer. In addition, in
hlua_txn_done(), the value ACT_RET_DONE is pushed on the stack before
exiting. Thus, when a script uses this function, the corresponding action still
finishes with the good code. Thanks to this change, the flag HLUA_STOP is now
useless. So it has been removed.
It is a mandatory step to allow a lua action to return any action return code.
CLEANUP: lua: Remove consistency check for sample fetches and actions
It is not possible anymore to alter the HTTP parser state from lua sample
fetches or lua actions. So there is no reason to still check for the parser
state consistency.
MEDIUM: http-rules: Support extra headers for HTTP return actions
It is now possible to append extra headers to the generated responses by HTTP
return actions, while it is not based on an errorfile. For return actions based
on errorfiles, these extra headers are ignored. To define an extra header, a
"hdr" argument must be used with a name and a value. The value is a log-format
string. For instance:
http-request status 200 hdr "x-src" "%[src]" hdr "x-dst" "%[dst]"
MEDIUM: http-rules: Add the return action to HTTP rules
Thanks to this new action, it is now possible to return any responses from
HAProxy, with any status code, based on an errorfile, a file or a string. Unlike
the other internal messages generated by HAProxy, these ones are not interpreted
as errors. And it is not necessary to use a file containing a full HTTP
response, although it is still possible. In addition, using a log-format string
or a log-format file, it is possible to have responses with a dynamic
content. This action can be used on the request path or the response path. The
only constraint is to have a responses smaller than a buffer. And to avoid any
warning the buffer space reserved to the headers rewritting should also be free.
When a response is returned with a file or a string as payload, it only contains
the content-length header and the content-type header, if applicable. Here are
examples:
MEDIUM: http: Add a ruleset evaluated on all responses just before forwarding
This patch introduces the 'http-after-response' rules. These rules are evaluated
at the end of the response analysis, just before the data forwarding, on ALL
HTTP responses, the server ones but also all responses generated by
HAProxy. Thanks to this ruleset, it is now possible for instance to add some
headers to the responses generated by the stats applet. Following actions are
supported :
MINOR: http-ana/http-rules: Use dedicated function to forward internal responses
Call http_forward_proxy_resp() function when an internal response is
returned. It concerns redirect, auth and error reponses. But also 100-Continue
and 103-Early-Hints responses. For errors, there is a subtlety. if the forward
fails, an HTTP 500 error is generated if it is not already an internal
error. For now http_forward_proxy_resp() cannot fail. But it will be possible
when the new ruleset applied on all responses will be added.
MINOR: http-ana: Add a function for forward internal responses
Operations performed when internal responses (redirect/deny/auth/errors) are
returned are always the same. The http_forward_proxy_resp() function is added to
group all of them under a unique function.
MINOR: http-ana: Rely on http_reply_and_close() to handle server error
The http_server_error() function now relies on http_reply_and_close(). Both do
almost the same actions. In addtion, http_server_error() sets the error flag and
the final state flag on the stream.
MINOR: http-rules: Handle the rule direction when a redirect is evaluated
The rule direction must be tested to do specific processing on the request
path. intercepted_req counter shoud be updated if the rule is evaluated on the
frontend and remaining request's analyzers must be removed. But only on the
request path. The rule direction must also be tested to set the right final
stream state flag.
This patch depends on the commit "MINOR: http-rules: Add a flag on redirect
rules to know the rule direction". Both must be backported to all stable
versions.
MINOR: http-rules: Add a flag on redirect rules to know the rule direction
HTTP redirect rules can be evaluated on the request or the response path. So
when a redirect rule is evaluated, it is important to have this information
because some specific processing may be performed depending on the direction. So
the REDIRECT_FLAG_FROM_REQ flag has been added. It is set when applicable on the
redirect rule during the parsing.
This patch is mandatory to fix a bug on redirect rule. It must be backported to
all stable versions.
BUG/MINOR: http-ana: Set HTX_FL_PROXY_RESP flag if a server perform a redirect
It is important to not forget to specify the HTX resposne was internally
generated when a server perform a redirect. This information is used by the H1
multiplexer to choose the right connexion mode when the response is sent to the
client.
BUG/MINOR: http-ana: Reset HTX first index when HAPRoxy sends a response
The first index in an HTX message is the HTX block index from which the HTTP
analysis must be performed. When HAProxy sends an HTTP response, on error or
redirect, this index must be reset because all pending incoming data are
considered as forwarded. For now, it is only a bug for 103-Early-Hints
response. For other responses, it is not a problem. But it will be when the new
ruleset applied on all responses will be added. For 103 responses, if the first
index is not reset, if there are rewritting rules on server responses, the
generated 103 responses, if any, are evaluated too.
This patch must be backported and probably adapted, at least for 103 responses,
as far as 1.9.