Tim Duesterhus [Tue, 18 Aug 2020 20:06:51 +0000 (22:06 +0200)]
DOC: cache: Use '<name>' instead of '<id>' in error message
When the cache name is left out in 'filter cache' the error message refers
to a missing '<id>'. The name of the cache is called 'name' within the docs.
Adjust the error message for consistency.
The error message was introduced in
99a17a2d91f9044ea20bba6617048488aed80555.
This commit first appeared in 1.9, thus the patch must be backported to 1.9+.
(cherry picked from commit
ea969f6f26adfa081df0d439723813cfcd4862c4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
a2acefe080df9b2430d823129173591df2c9f4a3)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Mon, 31 Aug 2020 14:27:42 +0000 (16:27 +0200)]
BUG/MINOR: http-rules: Replace path and query-string in "replace-path" action
The documentation stated the "replace-path" action replaces the path, including
the query-string if any is present. But in the code, only the path is
replaced. The query-string is preserved. So, now, instead of relying on the same
action code than "set-uri" action (1), a new action code (4) is used for
"replace-path" action. In http_req_replace_stline() function, when the action
code is 4, we call http_replace_req_path() setting the last argument (with_qs)
to 1. This way, the query-string is not skipped but included to the path to be
replaced.
This patch relies on the commit
b8ce505c6 ("MINOR: http-htx: Add an option to
eval query-string when the path is replaced"). Both must be backported as far as
2.0. It should fix the issue #829.
(cherry picked from commit
4b9c0d1fc08388bf44c6ebbd88f786032dd010fc)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
984435789b5aee02671005ad9f098847bbb60a61)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Mon, 31 Aug 2020 14:11:57 +0000 (16:11 +0200)]
MINOR: http-htx: Add an option to eval query-string when the path is replaced
The http_replace_req_path() function now takes a third argument to evaluate the
query-string as part of the path or to preserve it. If <with_qs> is set, the
query-string is replaced with the path. Otherwise, only the path is replaced.
This patch is mandatory to fix issue #829. The next commit depends on it. So be
carefull during backports.
(cherry picked from commit
b8ce505c6f0b202ccdb8e512fed0888652ed5b12)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
0ba7161614d5641a865165b1159a8133c910e21d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Fri, 28 Aug 2020 16:45:01 +0000 (18:45 +0200)]
BUG/MINOR: reload: do not fail when no socket is sent
get_old_sockets() mistakenly sets ret=0 instead of ret2=0 before leaving
when the old process announces zero FD. So it will return an error
instead of success. This must be particularly rare not to have a
single socket to offer though!
A few comments were added to make it more obvious what to expect in
return.
This must be backported to 1.8 since the bug has always been there.
(cherry picked from commit
febbce87baf15b610d30172e37b98133e92159cc)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
899f15532e9770ccc3fa51b6ae237ffed8e1b5dc)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Wed, 12 Aug 2020 12:04:52 +0000 (14:04 +0200)]
BUG/MEDIUM: htx: smp_prefetch_htx() must always validate the direction
It is possible to process a channel based on desynchronized info if a
request fetch is called from a response and conversely. However, the
code in smp_prefetch_htx() already makes sure the analysis has already
started before trying to fetch from a buffer, so the problem effectively
lies in response rules making use of request expressions only.
Usually it's not a problem as extracted data are checked against the
current HTTP state, except when it comes to the start line, which is
usually accessed directly from sample fetch functions such as status,
path, url, url32, query and so on. In this case, trying to access the
request buffer from the response path will lead to unpredictable
results. When building with DEBUG_STRICT, a process violating these
rules will simply die after emitting:
FATAL: bug condition "htx->first == -1" matched at src/http_htx.c:67
But when this is not enabled, it may or may not crash depending on what
the pending request buffer data look like when trying to spot a start
line there. This is typically what happens in issue #806.
This patch adds a test in smp_prefetch_htx() so that it does not try
to parse an HTX buffer in a channel belonging to the wrong direction.
There's one special case on the "method" sample fetch since it can
retrieve info even without a buffer, from the other direction, as
long as the method is one of the well known ones. Three, we call
smp_prefetch_htx() only if needed.
This was reported in 2.0 and must be backported there (oldest stable
version with HTX).
(cherry picked from commit
a6d9879e69d65e8821347860394829c4dc72f937)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
05f3910f58a8d5e0710a4a8eea82f8bc24861e32)
[wt: adjusted context; chn already checked; 3 args to smp_prefetch_htx()]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Tue, 11 Aug 2020 08:26:36 +0000 (10:26 +0200)]
BUG/MINOR: stats: use strncmp() instead of memcmp() on health states
The reports for health states are checked using memcmp() in order to
only focus on the first word and possibly ignore trailing %d/%d etc.
This makes gcc unhappy about a potential use of "" as the string, which
never happens since the string is always set. This resulted in commit
c4e6460f6 ("MINOR: build: Disable -Wstringop-overflow.") to silence
these messages. However some lengths are incorrect (though cannot cause
trouble), and in the end strncmp() is just safer and cleaner.
This can be backported to all stable branches as it will shut a warning
with gcc 8 and above.
(cherry picked from commit
7b52485f1afb7d420633d7794549ec0549d1e2d7)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
af400cf0ec9a418ac1f127d9ed696e7807c2f33b)
Signed-off-by: Willy Tarreau <w@1wt.eu>
William Lallemand [Fri, 7 Aug 2020 12:48:37 +0000 (14:48 +0200)]
BUG/MINOR: snapshots: leak of snapshots on deinit()
Free the snapshots on deinit() when they were initialized in a proxy
upon an error.
This was introduced by c55015e ("MEDIUM: snapshots: dynamically allocate
the snapshots").
Should be backported as far as 1.9.
(cherry picked from commit
efc5a9d55ba46a934f1474802b90c679461ebc92)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
147355c50f5b09bd4251a0f886792f4d7cb82aea)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Thu, 6 Aug 2020 08:32:28 +0000 (10:32 +0200)]
MEDIUM: lua: Don't filter exported fetches and converters
Thanks to previous commits, it is now safe to use from lua the sample fetches
and sample converters that convert arguments, especially the strings
(ARGT_STR). So now, there are all exported to the lua. They was filtered on the
validation functions. Only fetches without validation functions or with val_hdr
or val_payload_lv functions were exported, and converters without validation
functions.
This patch depends on following commits :
*
aec27ef44 "BUG/MINOR: lua: Duplicate lua strings in sample fetches/converters arg array"
*
fdea1b631 "MINOR: hlua: Don't needlessly copy lua strings in trash during args validation"
It must be backported as far as 2.1 because the date() and http_date()
converters are no longer exported because of the filter on the validation
function, since the commit
ae6f125c7 ("MINOR: sample: add us/ms support to
date/http_date)".
(cherry picked from commit
05e2d7744189e52dbd1625a168d6f8888b9658b8)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
4cf32912f80ca2d45eeb5d2dfdb5339a425f401a)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Thu, 6 Aug 2020 06:54:25 +0000 (08:54 +0200)]
BUG/MINOR: lua: Duplicate lua strings in sample fetches/converters arg array
Strings in the argument array used by sample fetches and converters must be
duplicated. This is mandatory because, during the arguments validations, these
strings may be converted and released. It works this way during the
configuration parsing and there is no reason to adapt this behavior during the
runtime when a sample fetch or a sample converter is called from the lua. In
fact, there is a reason to not change the behavior. It must reamain simple for
everyone to add new fetches or converters.
Thus, lua strings are duplicated. It is only performed at the end of the
hlua_lua2arg_check() function, if the argument is still a ARGT_STR. Of course,
it requires a cleanup loop after the call or when an error is triggered.
This patch depends on following commits:
*
959171376 "BUG/MINOR: arg: Fix leaks during arguments validation for fetches/converters"
*
fdea1b631 "MINOR: hlua: Don't needlessly copy lua strings in trash during args validation"
It may be backported to all supported versions, most probably as far as 2.1
only.
(cherry picked from commit
aec27ef443a8263bd1914acf449e30851feb86df)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
844b403f96adc80b83b887bf86cf5ef2b354ce47)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Thu, 6 Aug 2020 06:29:18 +0000 (08:29 +0200)]
MINOR: hlua: Don't needlessly copy lua strings in trash during args validation
Lua strings are NULL terminated. So in the hlua_lua2arg_check() function, used
to check arguments against the sample fetches specification, there is no reason
to copy these strings in a trash to add a terminating null byte.
In addition, when the array of arguments is built from lua values, we must take
care to count this terminating null bytes in the size of the buffer where a
string is stored. The same must be done when a sample is built from a lua value.
This patch may be backported to easy backports.
(cherry picked from commit
fdea1b6319bc014320a18ba7b7043ba0694dd0dd)
[wt: backported for next commit]
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
f657d400b442b83d1565579c2b78eba9d4e12abc)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Fri, 7 Aug 2020 07:11:22 +0000 (09:11 +0200)]
BUG/MINOR: lua: Check argument type to convert it to IP mask in arg validation
In hlua_lua2arg_check() function, before converting an argument to an IPv4 or
IPv6 mask, we must be sure to have an integer or a string argument (ARGT_SINT or
ARGT_STR).
This patch must be backported to all supported versions.
(cherry picked from commit
e663a6e326c3d4d465d5aa66e4fbe3acde69aac3)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
2ef95fa486eae674cb32c491c7f5ca5da3299fee)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Fri, 7 Aug 2020 07:07:26 +0000 (09:07 +0200)]
BUG/MINOR: lua: Check argument type to convert it to IPv4/IPv6 arg validation
In hlua_lua2arg_check() function, before converting a string to an IP address,
we must be to sure to have a string argument (ARGT_STR).
This patch must be backported to all supported versions.
(cherry picked from commit
8e09ac8592cb35e11bff0f821db19369e0941e35)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
af6bd3010aac6565e98e61d6bb1ea973f0f271d4)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Wed, 5 Aug 2020 21:07:07 +0000 (23:07 +0200)]
BUG/MINOR: arg: Fix leaks during arguments validation for fetches/converters
Some sample fetches or sample converters uses a validation functions for their
arguments. In these function, string arguments (ARGT_STR) may be converted to
another type (for instance a regex, a variable or a integer). Because these
strings are allocated when the argument list is built, they must be freed after
a conversion. Most of time, it is done. But not always. This patch fixes these
minor memory leaks (only on few strings, during the configuration parsing).
This patch may be backported to all supported versions, most probably as far as
2.1 only. If this commit is backported, the previous one
73292e9e6 ("BUG/MINOR:
lua: Duplicate map name to load it when a new Map object is created") must also
be backported. Note that some validation functions does not exists on old
version. It should be easy to resolve conflicts.
(cherry picked from commit
959171376f4974b883b2aa4228b3f8c610ca2ae9)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
0e7c530357024c8aa20886a42570071c06595193)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Thu, 6 Aug 2020 06:40:09 +0000 (08:40 +0200)]
BUG/MINOR: lua: Duplicate map name to load it when a new Map object is created
When a new map is created, the sample_load_map() function is called. To do so,
an argument array is created with the name as first argument. Because it is a
lua string, owned by the lua, it must be duplicated. The sample_load_map()
function will convert this argument to a map. In theory, after the conversion,
it must release the original string. It is not performed for now and it is a bug
that will be fixed in the next commit.
This patch may be backported to all supported versions, most probably as far as
2.1 only. But it must be backported with the next commit "BUG/MINOR: arg: Fix
leaks during arguments validation for fetches/converters".
(cherry picked from commit
73292e9e660b98467ed48266d25684fa1e0bcebd)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
2c8b3c9893bbff3afc9c29dbee2cb2b52e3f63ff)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Fri, 7 Aug 2020 12:00:23 +0000 (14:00 +0200)]
BUG/MINOR: converters: Store the sink in an arg pointer for debug() converter
The debug() converter uses a string to reference the sink where to send debug
events. During the configuration parsing, this string is converted to a sink
object but it is still store as a string argument. It is a problem on deinit
because string arguments are released. So the sink pointer will be released
twice.
To fix the bug, we keep a reference on the sink using an ARGT_PTR argument. This
way, it will not be freed on the deinit.
This patch depends on the commit
e02fc4d0d ("MINOR: arg: Add an argument type to
keep a reference on opaque data"). Both must be backported as far as 2.1.
(cherry picked from commit
b45bf8eb70789264b9fd38fd6c1bc1c1c723ffe3)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
c89077713915f605eb5d716545f182c8d0bf5581)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Fri, 7 Aug 2020 11:56:00 +0000 (13:56 +0200)]
MINOR: arg: Add an argument type to keep a reference on opaque data
The ARGT_PTR argument type may now be used to keep a reference to opaque data in
the argument array used by sample fetches and converters. It is a generic way to
point on data. I guess it could be used for some other arguments, like proxy,
server, map or stick-table.
(cherry picked from commit
e02fc4d0dd57c92bbe96f3ff2ae0b890405458f2)
[wt: needed by next commit]
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
bf1fcab0670c3a94270d19c50a23a88b3d0edfba)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Wed, 5 Aug 2020 21:23:37 +0000 (23:23 +0200)]
BUG/MEDIUM: map/lua: Return an error if a map is loaded during runtime
In sample_load_map() function, the global mode is now tested to be sure to be in
the starting mode. If not, an error is returned.
At first glance, this patch may seem useless because maps are loaded during the
configuration parsing. But in fact, it is possible to load a map from the lua,
using Map:new() method. And, there is nothing to forbid to call this method at
runtime, during a script execution. It must never be done because it may perform
an filesystem access for unknown maps or allocation for known ones. So at
runtime, it means a blocking call or a memroy leak. Note it is still possible to
load a map from the lua, but in the global part of a script only. This part is
executed during the configuration parsing.
This patch must be backported in all stable versions.
(cherry picked from commit
0eb967d122d3bbc682ad407bff9f21893cd25157)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
ebfb14e32803dd78da5d42f218a32ce40da8313c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
William Lallemand [Tue, 4 Aug 2020 15:41:39 +0000 (17:41 +0200)]
BUG/MEDIUM: ssl: memory leak of ocsp data at SSL_CTX_free()
This bug affects all version of HAProxy since the OCSP data is not free
in the deinit(), but leaking on exit() is not really an issue. However,
when doing dynamic update of certificates over the CLI, those data are
not free'd upon the free of the SSL_CTX.
3 leaks are happening, the first leak is the one of the ocsp_arg
structure which serves the purpose of containing the pointers in the
case of a multi-certificate bundle. The second leak is the one ocsp
struct. And the third leak is the one of the struct buffer in the
ocsp_struct.
The problem lies with SSL_CTX_set_tlsext_status_arg() which does not
provide a way to free the argument upon an SSL_CTX_free().
This fix uses ex index functions instead of registering a
tlsext_status_arg(). This is really convenient because it allows to
register a free callback which will free the ex index content upon a
SSL_CTX_free().
A refcount was also added to the ocsp_response structure since it is
stored in a tree and can be reused in another SSL_CTX.
Should fix part of the issue #746.
This must be backported in 2.2 and 2.1.
(cherry picked from commit
76b4a12591b6cea2fdfe80a5b2175a2d44c02a85)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
3b70eb66948234396bbb81eb7463745d601718bd)
Signed-off-by: Willy Tarreau <w@1wt.eu>
William Lallemand [Thu, 6 Aug 2020 22:44:32 +0000 (00:44 +0200)]
BUG/MINOR: ssl: fix memory leak at OCSP loading
Fix a memory leak when loading an OCSP file when the file was already
loaded elsewhere in the configuration.
Indeed, if the OCSP file already exists, a useless chunk_dup() will be
done during the load.
To fix it we reverts "ocsp" to "iocsp" like it was done previously.
This was introduced by commit 246c024 ("MINOR: ssl: load the ocsp
in/from the ckch").
Should fix part of the issue #746.
It must be backported in 2.1 and 2.2.
(cherry picked from commit
86e4d63316de32b964c8b6b453b549532611e7e5)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
742556f1ae840df317533c0154c5393e09a4d120)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Fri, 31 Jul 2020 14:47:44 +0000 (16:47 +0200)]
SCRIPTS: git-show-backports: emit the shell command to backport a commit
It's cumbersome to copy-paste a commit ID into another window after having
typed "git cherry-pick -sx", so let's have the suggested output format of
git-show prepare this line just before the subject line, it remains at a
stable position on the terminal when searching for "/^commit". One just
has to copy-paste the line into another terminal will result in the commit
being properly picked.
(cherry picked from commit
1f927d1bc228c78c322c94c8c2d5247297c6f6c4)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
2d6847aa7e2f7704c44c57982ef383c00f77a7c6)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Fri, 31 Jul 2020 14:38:57 +0000 (16:38 +0200)]
SCRIPTS: git-show-backports: make -m most only show the left branch
We've never used the output of the rightmost branch with this tool,
and it systematically causes two identical outputs making the job
harder during backport sessions. Let's simply remove the right part
when it's identical to the left one. This also adds a few line feeds
to make the output more readable.
(cherry picked from commit
f456f6f2a36f0e2ce82ef80819974cb9dc8b4e86)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
2a53511500609a57c40bf0a33219954d45d6a31d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Wed, 5 Aug 2020 09:31:16 +0000 (11:31 +0200)]
BUG/MEDIUM: mux-h1: Refresh H1 connection timeout after a synchronous send
The H1 multiplexer is able to perform synchronous send. When a large body is
transfer, if nothing is received and if no error or shutdown occurs, it is
possible to not go down at the H1 connection level to do I/O for a long
time. When this happens, we must still take care to refresh the H1 connection
timeout. Otherwise it is possible to hit the connection timeout during the
transfer while it should not expire.
This bug exists because only h1_process() refresh the H1 connection timeout. To
fix the bug, h1_snd_buf() must also refresh this timeout. To make things more
readable, a dedicated function has been introduced and called to refresh the
timeout.
This bug exists on all HTX versions. But it is harder to hit it on 2.1 and below
because when a H1 mux is initialized, we actively try to read data instead of
subscribing for receiving. So there is at least one call to h1_process().
This patch should fix the issue #790. It must be backported as far as 2.0.
(cherry picked from commit
7a145d682379a99c443c7888baa9c4c68847eddc)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
f20b950140929db875e165f848cff910470ac2d2)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Fri, 31 Jul 2020 11:34:55 +0000 (13:34 +0200)]
[RELEASE] Released version 2.1.8
Released version 2.1.8 with the following main changes :
- BUG/MEDIUM: log: don't hold the log lock during writev() on a file descriptor
- BUG/MEDIUM: pattern: fix thread safety of pattern matching
- BUILD: make dladdr1 depend on glibc version and not __USE_GNU
- REGTESTS: Add missing OPENSSL to REQUIRE_OPTIONS for lua/txn_get_priv
- REGTESTS: Add missing OPENSSL to REQUIRE_OPTIONS for compression/lua_validation
- BUG/MINOR: ssl: fix ssl-{min,max}-ver with openssl < 1.1.0
- BUG/MEDIUM: ssl: crt-list must continue parsing on ERR_WARN
- MINOR: http: Add 410 to http-request deny
- MINOR: http: Add 404 to http-request deny
- BUG/MINOR: http: make smp_fetch_body() report that the contents may change
- BUG/MINOR: tcp-rules: tcp-response must check the buffer's fullness
- BUG/MEDIUM: ebtree: use a byte-per-byte memcmp() to compare memory blocks
- BUG/MINOR: spoe: add missing key length check before checking key names
- BUG/MINOR: cli: allow space escaping on the CLI
- BUG/MINOR: mworker/cli: fix the escaping in the master CLI
- BUG/MINOR: mworker/cli: fix semicolon escaping in master CLI
- REGTEST: http-rules: test spaces in ACLs
- REGTEST: http-rules: test spaces in ACLs with master CLI
- MEDIUM: map: make the "clear map" operation yield
- BUG/MINOR: systemd: Wait for network to be online
- REGTEST: Add a simple script to tests errorfile directives in proxy sections
- BUG/MINOR: spoe: correction of setting bits for analyzer
- BUG/MINOR: http_ana: clarify connection pointer check on L7 retry
- MINOR: spoe: Don't systematically create new applets if processing rate is low
- REGTEST: ssl: tests the ssl_f_* sample fetches
- REGTEST: ssl: add some ssl_c_* sample fetches test
- BUG/MEDIUM: fetch: Fix hdr_ip misparsing IPv4 addresses due to missing NUL
- MINOR: cli: make "show sess" stop at the last known session
- DOC: ssl: add "allow-0rtt" and "ciphersuites" in crt-list
- BUG/MEDIUM: pattern: Add a trailing \0 to match strings only if possible
- BUG/MINOR: proxy: fix dump_server_state()'s misuse of the trash
- BUG/MINOR: proxy: always initialize the trash in show servers state
- DOC: configuration: add missing index entries for tune.pool-{low,high}-fd-ratio
- DOC: configuration: fix alphabetical ordering for tune.pool-{high,low}-fd-ratio
- BUILD: haproxy: fix build error when RLIMIT_AS is not set
- BUG/MINOR: http_act: don't check capture id in backend (2)
- BUG/MINOR: mux-h1: Fix the splicing in TUNNEL mode
- BUG/MINOR: mux-h1: Don't read data from a pipe if the mux is unable to receive
- BUG/MINOR: mux-h1: Disable splicing only if input data was processed
- BUG/MEDIUM: mux-h1: Disable splicing for the conn-stream if read0 is received
- MINOR: mux-h1: Improve traces about the splicing
- BUG/MEDIUM: mux-h1: Subscribe rather than waking up in h1_rcv_buf()
- MINOR: connection: move the CO_FL_WAIT_ROOM cleanup to the reader only
- BUG/MEDIUM: connection: Continue to recv data to a pipe when the FD is not ready
- BUG/MINOR: backend: Remove CO_FL_SESS_IDLE if a client remains on the last server
- MINOR: http: Add support for http 413 status
- DOC: configuration: remove obsolete mentions of H2 being converted to HTTP/1.x
- BUG/MINOR: sample: Free str.area in smp_check_const_bool
- BUG/MINOR: sample: Free str.area in smp_check_const_meth
- BUG/MEDIUM: lists: add missing store barrier on MT_LIST_BEHEAD()
- BUG/MEDIUM: lists: add missing store barrier in MT_LIST_ADD/MT_LIST_ADDQ
- CONTRIB: da: fix memory leak in dummy function da_atlas_open()
- BUG/MEDIUM: mux-h1: Continue to process request when switching in tunnel mode
- BUG/MINOR: mux-fcgi: Handle empty STDERR record
- BUG/MINOR: mux-fcgi: Set conn state to RECORD_P when skipping the record padding
- BUG/MINOR: mux-fcgi: Set flags on the right stream field for empty FCGI_STDOUT
- BUG/MEDIUM: log: issue mixing sampled to not sampled log servers.
- BUG/MEDIUM: fcgi-app: fix memory leak in fcgi_flt_http_headers
- BUG/MEDIUM: server: resolve state file handle leak on reload
- BUG/MEDIUM: server: fix possibly uninitialized state file on close
- BUG/MEDIUM: channel: Be aware of SHUTW_NOW flag when output data are peeked
- BUILD: ebtree: fix build on libmusl after recent introduction of eb_memcmp()
- REGEST: Add reg tests about error files
- BUG/MINOR: threads: Don't forget to init each thread toremove_lock.
- MINOR: pools: increase MAX_BASE_POOLS to 64
- BUILD: thread: add parenthesis around values of locking macros
- BUG/MINOR: cfgparse: don't increment linenum on incomplete lines
- BUG/MEDIUM: resolve: fix init resolving for ring and peers section.
- BUG/MAJOR: dns: Make the do-resolve action thread-safe
- BUG/MEDIUM: dns: Release answer items when a DNS resolution is freed
- BUG/MINOR: mux-fcgi: Don't url-decode the QUERY_STRING parameter anymore
- BUG/MEDIUM: mux-h1: Wakeup the H1C in h1_rcv_buf() if more data are expected
- BUG/MEDIUM: mux-h1: Disable the splicing when nothing is received
- BUILD: tools: fix build with static only toolchains
- BUG/MINOR: debug: Don't dump the lua stack if it is not initialized
- MEDIUM: lua: Add support for the Lua 5.4
- BUG/MEDIUM: dns: Don't yield in do-resolve action on a final evaluation
- BUG/MINOR: tcp-rules: Set the inspect-delay when a tcp-response action yields
- MINOR: connection: Preinstall the mux for non-ssl connect
- MINOR: stream-int: Be sure to have a mux to do sends and receives
- SCRIPTS: announce-release: add the link to the wiki in the announce messages
- BUG/MEDIUM: backend: always attach the transport before installing the mux
Willy Tarreau [Fri, 31 Jul 2020 06:39:31 +0000 (08:39 +0200)]
BUG/MEDIUM: backend: always attach the transport before installing the mux
In connect_server(), we can enter in a stupid situation:
- conn_install_mux_be() is called to install the mux. This one
subscribes for receiving and quits ;
- then we discover that a handshake is required on the connection
(e.g. send-proxy), so xprt_add_hs() is called and subscribes as
well.
- we crash in conn_subscribe() which gets a different subscriber.
And if BUG_ON is disabled, we'd likely lose one event.
Note that it doesn't seem to happen by default, but definitely does
if connect() rightfully performs fd_cant_recv(), so it's a matter of
who does what and in what order.
A simple reproducer consists in adding fd_cant_recv() after fd_cant_send()
in tcp_connect_server() and running it on this config, as discussed in issue
listen foo
bind :8181
mode http
server srv1 127.0.0.1:8888 send-proxy-v2
The root cause is that xprt_add_hs() installs an xprt layer underneath
the mux without taking over its subscriptions. Ideally if we want to
support this, we'd need to steal the connection's wait_events and
replace them by new ones. But there doesn't seem to be any case where
we're interested in doing this so better simply always install the
transport layer before installing the mux, that's safer and simpler.
This needs to be backported to 2.1 which is constructed the same way
and thus suffers from the same issue, though the code is slightly
different there.
(cherry picked from commit
a3b17563e16ba0dd1dd9a617424fe198d7687bac)
[wt: adjusted context]
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
964ad9d6a23374e8d6584c315a2b2af0fc5faecc)
[wt: adjusted context]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Thu, 30 Jul 2020 15:41:42 +0000 (17:41 +0200)]
SCRIPTS: announce-release: add the link to the wiki in the announce messages
Let's add the link to the wiki to the announce messages, plenty of
users don't even know it exists.
(cherry picked from commit
be789dfc5d31120d96b8b067105e2b624add3532)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
e549826653a98a67fb0c4a14b9b79678c54907ad)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Thu, 30 Jul 2020 07:26:46 +0000 (09:26 +0200)]
MINOR: stream-int: Be sure to have a mux to do sends and receives
In si_cs_send() and si_cs_recv(), we explicitly test the connection's mux is
defined to proceed. For si_cs_recv(), it is probably a bit overkill. But
opportunistic sends are possible from the moment the server connection is
created. So it is safer to do this test.
This patch may be backported as far as 1.9 if necessary.
(cherry picked from commit
e96993b1f203a14d2ef1eaa70be595c7d5b98643)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
17c13f0fdc42e53e1034a7e4ef0147184afd4221)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Thu, 30 Jul 2020 07:10:36 +0000 (09:10 +0200)]
MINOR: connection: Preinstall the mux for non-ssl connect
In the connect_server() function, there is an optim to install the mux as soon
as possible. It is possible if we can determine the mux to use from the
configuration only. For instance if the mux is explicitly specified or if no ALPN
is set. This patch adds a new condition to preinstall the mux for non-ssl
connection. In this case, by default, we always use the mux_pt for raw
connections and the mux-h1 for HTTP ones.
This patch is related to the issue #762. It may be backported to 2.2 (and
possibly as far as 1.9 if necessary).
(cherry picked from commit
b4de4204721fc2fb9a0781efc45b957b76ece065)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
cbe1c43508f42fc129ab9766fc321a92220fd375)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Wed, 29 Jul 2020 10:00:23 +0000 (12:00 +0200)]
BUG/MINOR: tcp-rules: Set the inspect-delay when a tcp-response action yields
On a tcp-response content ruleset evaluation, the inspect-delay is engaged when
rule's conditions are not validated but not when the rule's action yields.
This patch must be backported to all supported versions.
(cherry picked from commit
54f3e183c87fa3694a25b3116c724222fc95676b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
d3cd85d2c9cc514d84b782e14c33e2bc3215adaa)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Tue, 28 Jul 2020 08:21:54 +0000 (10:21 +0200)]
BUG/MEDIUM: dns: Don't yield in do-resolve action on a final evaluation
When an action is evaluated, flags are passed to know if it is the first call
(ACT_OPT_FIRST) and if it must be the last one (ACT_OPT_FINAL). For the
do-resolve DNS action, the ACT_OPT_FINAL flag must be handled because the
action may yield. It must never yield when this flag is set. Otherwise, it may
lead to a wakeup loop of the stream because the inspected-delay of a tcp-request
content ruleset was reached without stopping the rules evaluation.
This patch is related to the issue #222. It must be backported as far as 2.0.
(cherry picked from commit
385101e53816dc1b7bc1fc957adc512ce8a07cb4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
5c038f759959adf95b4b347aba9d97e60ab87e93)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Tue, 28 Jul 2020 08:33:25 +0000 (10:33 +0200)]
MEDIUM: lua: Add support for the Lua 5.4
On Lua 5.4, some API changes make HAProxy compilation to fail. Among other
things, the lua_resume() function has changed and now takes an extra argument in
Lua 5.4 and the error LUA_ERRGCMM was removed. Thus the LUA_VERSION_NUM macro is
now tested to know the lua version is used and adapt the code accordingly.
Here are listed the incompatibilities with the previous Lua versions :
http://www.lua.org/manual/5.4/manual.html#8
This patch comes from the HAproxy's fedora RPM, committed by Tom Callaway :
https://src.fedoraproject.org/rpms/haproxy/blob/
db970613/f/haproxy-2.2.0-lua-5.4.patch
This patch should fix the issue #730. It must be backported to 2.2 and probably
as far as 2.0.
(cherry picked from commit
08ed98fd7963968de49593304fdd9234812845a4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
dc2e6f544fee393543cdeaae7bbb4fe57d49b409)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 24 Jul 2020 17:08:05 +0000 (19:08 +0200)]
BUG/MINOR: debug: Don't dump the lua stack if it is not initialized
When the watchdog is fired because of the lua, the stack of the corresponding
lua context is dumped. But we must be sure the lua context is fully initialized
to do so. If we are blocked on the global lua lock, during the lua context
initialization, the lua stask may be NULL.
This patch should fix the issue #776. It must be backported as far as 2.0.
(cherry picked from commit
471425f51d71d1214abeee40439a51a2a3217102)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
f9f875213809d0d08857def5ce4f76d6b747fbd6)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Baruch Siach [Fri, 24 Jul 2020 04:52:20 +0000 (07:52 +0300)]
BUILD: tools: fix build with static only toolchains
uClibc toolchains built with no dynamic library support don't provide
the dlfcn.h header. That leads to build failure:
CC src/tools.o
src/tools.c:15:10: fatal error: dlfcn.h: No such file or directory
#include <dlfcn.h>
^~~~~~~~~
Enable dladdr on Linux platforms only when USE_DL is defined.
This should be backported wherever
109201fc5 ("BUILD: tools: rely on
__ELF__ not USE_DL to enable use of dladdr()") is backported (currently
only 2.2 and 2.1).
(cherry picked from commit
e1651b2970a699c517e185115e8f866dd5a04c92)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
c28833ecacd3cc315527003dca5b1ee7ac06a1c3)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 24 Jul 2020 14:13:12 +0000 (16:13 +0200)]
BUG/MEDIUM: mux-h1: Disable the splicing when nothing is received
When nothing is received when xprt->rcv_pipe() is called, and if the pipe is
empty, the mux rcv_buf() callback is called. Because the splicing is enabled,
nothing is performed and the H1 connection is woken up. Unfortunately, the H1
connection will do the same with the H1 stream. Thus, while nothing is received,
a ping-pong loop between the H1 connection and a H1 stream will consume all the
CPU. To avoid this loop, the splicing is disabled in this case. This way, the H1
connection will try to read data and it will wake up the H1 stream only if it
reads something.
This bug was previously fixed by the commit a1dde23 ("BUG/MEDIUM: mux-h1:
Subscribe rather than waking up in h1_rcv_buf()"). But it was reverted because
it relies on not-backported changes on the connection layer.
This patch must be backported to 2.0.
Christopher Faulet [Fri, 24 Jul 2020 14:31:14 +0000 (16:31 +0200)]
BUG/MEDIUM: mux-h1: Wakeup the H1C in h1_rcv_buf() if more data are expected
After the input processing, in h1_rcv_buf(), if more data are expected and the
H1C is not subscribed on receives, we now wake up the H1C to try to read more
data ASAP. This reverts commit
a1dde23e. Depending on how data are received,
subscribing on receives at this stage may freeze the H1 connection because a
read event is missing.
This bug seems to no exist in the 2.2 because of changes on the connection
subscriptions. But we must be careful because I don't really know why the read
event is missed.
Moreover, the bug fixed by the commit
a1dde23e will be fixed another way in
the next commit.
This patch should fix the issue #774. It must be backported to 2.0.
Christopher Faulet [Thu, 23 Jul 2020 13:44:37 +0000 (15:44 +0200)]
BUG/MINOR: mux-fcgi: Don't url-decode the QUERY_STRING parameter anymore
In the CGI/1.1 specification, it is specified the QUERY_STRING must not be
url-decoded. However, this parameter is sent decoded because it is extracted
after the URI's path decoding. Now, the query-string is first extracted, then
the script part of the path is url-decoded. This way, the QUERY_STRING parameter
is no longer decoded.
This patch should fix the issue #769. It must be backported as far as 2.1.
(cherry picked from commit
0f17a4444e641b6e16bff49e3bd99466f637b272)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
327bbfc3b81a8c22e2bb645ee6f9bb43b70c0276)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Wed, 22 Jul 2020 13:55:49 +0000 (15:55 +0200)]
BUG/MEDIUM: dns: Release answer items when a DNS resolution is freed
When a DNS resolution is freed, the remaining items in .ar_list and .answer_list
are also released. It must be done to avoid a memory leak. And it is the last
chance to release these objects. I've honestly no idea if there is a better
place to release them earlier. But at least, there is no more leak.
This patch should solve the issue #222. It must be backported, at least, as far
as 2.0, and probably, with caution, as far as 1.8 or 1.7.
(cherry picked from commit
010ab35a9118daf17a670fb2b42e40447f967f7c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
c58ac80d00284886b108b209a5bf993de5ab38ed)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Wed, 22 Jul 2020 09:46:32 +0000 (11:46 +0200)]
BUG/MAJOR: dns: Make the do-resolve action thread-safe
The do-resolve HTTP action, performing a DNS resolution of a sample expression
output, is not thread-safe at all. The resolver object used to do the resolution
must be locked when the action is executed or when the stream is released
because its curr or wait resolution lists and the requester list inside a
resolution are updated. It is also important to not wake up a released stream
(with a destroyed task).
Of course, because of this bug, various kind of crashes may be observed.
This patch should fix the issue #236. It must be backported as far as 2.0.
(cherry picked from commit
5098a08c2fafb0d9513996729d2a30c9785378f3)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
99f4623952cbbad2bcae451abdd0f3133bcbe75c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Emeric Brun [Tue, 21 Jul 2020 14:54:36 +0000 (16:54 +0200)]
BUG/MEDIUM: resolve: fix init resolving for ring and peers section.
Reported github issue #759 shows there is no name resolving
on server lines for ring and peers sections.
This patch introduce the resolving for those lines.
This patch adds boolean a parameter to parse_server function to specify
if we want the function to perform an initial name resolving using libc.
This boolean is forced to true in case of peers or ring section.
The boolean is kept to false in case of classic servers (from
backend/listen)
This patch should be backported in branches where peers sections
support 'server' lines.
(cherry picked from commit
d3db3846c5d2f3a2a27a6055d28d146838b7189d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
de103017907a09d257fb0d3d75cc1dcef8588ada)
[wt: dropped ring changes for 2.1]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Thu, 25 Jun 2020 07:37:54 +0000 (09:37 +0200)]
BUG/MINOR: cfgparse: don't increment linenum on incomplete lines
When fgets() returns an incomplete line we must not increment linenum
otherwise line numbers become incorrect. This may happen when parsing
files with extremely long lines which require a realloc().
The bug has been present since unbounded line length was supported, so
the fix should be backported to older branches.
(cherry picked from commit
40cb26f6ec8cf9d8ba54706d95df4cfed3a7b332)
[wt: adjusted context wrt missing fatal++]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Fri, 12 Jun 2020 09:42:25 +0000 (11:42 +0200)]
BUILD: thread: add parenthesis around values of locking macros
clang just failed on fd.c with this error:
src/fd.c:491:9: error: logical not is only applied to the left hand side of this comparison [-Werror,-Wlogical-not-parentheses]
while (HA_SPIN_TRYLOCK(OTHER_LOCK, &log_lock) != 0) {
^ ~~
That's because this expands to this:
while (!pl_try_s(&log_lock) != 0) {
Let's just add parenthesis in the TRYLOCK macros to avoid this.
This may need to be backported if commit
df187875d ("BUG/MEDIUM: log:
don't hold the log lock during writev() on a file descriptor") is
backported as well as it seems to be the first one to trigger it.
(cherry picked from commit
db57a142c31016ff3e0dd533cb2b4de14445781e)
[wt: applied by hand to common/hathread.h since context significantly differed]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Tue, 30 Jun 2020 12:29:02 +0000 (14:29 +0200)]
MINOR: pools: increase MAX_BASE_POOLS to 64
When not sharing pools (i.e. when building with -DDEBUG_DONT_SHARE_POOLS)
we have about 47 pools right now, while MAX_BASE_POOLS is only 32, meaning
that only the first 32 ones will benefit from a per-thread cache entry.
This totally kills performance when pools are not shared (roughly -20%).
Let's double the limit to gain some margin, and make it possible to set
it as a build option.
It might be useful to backport this to stable versions as they're likely
to be affected as well.
(cherry picked from commit
daf8aa62a8d0210f81db5c93235da19d5ed22ab3)
[wt: applied to include/common/memory.h]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Olivier Houchard [Mon, 29 Jun 2020 15:48:27 +0000 (17:48 +0200)]
BUG/MINOR: threads: Don't forget to init each thread toremove_lock.
Don't forget to use HA_SPIN_INIT() on each toremove_lock, or DEBUG_THREAD may
not work reliably with it.
This should be backported to 2.1 and 2.0.
(cherry picked from commit
f21695bd8b59d582efc99c85f9a1afac200eda81)
[wt: the lock is called toremove_lock[i] in 2.1]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Mon, 20 Jan 2020 12:49:48 +0000 (13:49 +0100)]
REGEST: Add reg tests about error files
2 reg tests are added. The first one ensures the declaration of errors in a
proxy is fonctionnal. It declares http-errors sections and declare error files
using the errorfile and the errorfiles directives, both in the default section
and the frontend sections. The second one ensures it is possible to use a custom
error file for an HTTP deny rule.
(cherry picked from commit
a5afb0bf3668851d130d75ccd7dbf4fcadd3a2b7)
[wt: needed since the backport of
60837d340 ("REGTEST: Add a simple
script to tests errorfile directives in proxy sections")]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Mon, 20 Jul 2020 19:40:14 +0000 (21:40 +0200)]
BUILD: ebtree: fix build on libmusl after recent introduction of eb_memcmp()
In order to address a corner case with memory compares, commit 853926a
("BUG/MEDIUM: ebtree: use a byte-per-byte memcmp() to compare memory
blocks") introduced a new eb_memcmp() function, which uses ssize_t for
an offset. However this one is not known by default in ebtree for versions
prior to 2.2, and it depends on the libs. Musl requires unistd to be included
to have it.
This patch just adds this harmless include, which was verified to address
issue #760. There is no mainline commit ID as 2.2 and above already include
unistd.h as part of haproxy/api.h. It must be backported to older versions
(2.0 already has the commit above).
Christopher Faulet [Thu, 16 Jul 2020 09:43:46 +0000 (11:43 +0200)]
BUG/MEDIUM: channel: Be aware of SHUTW_NOW flag when output data are peeked
The CF_SHUTW_NOW flag must be handled the same way than the CF_SHUTW flag in
co_getblk_nc() and co_getline_nc() functions. It is especally important when we
try to peek a line from outgoing data. In this case, an unfinished line is
blocked an nothing is peeked if the CF_SHUTW_NOW flag is set. But the blocked
data pevent the transition to CF_SHUTW.
The above functions are only used by LUA cosockets. Because of this bug, we may
experienced wakeups in loop of the cosocket's io handler if we try to read a
line on a closed socket with a pending unfinished line (no LF found at the end).
This patch should fix issue #744. It must be backported to all supported
versions.
(cherry picked from commit
f706a794d8b8d854cba4414adb6f9a1297c6fd02)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
b95c6fe4a0b23b7ed69ea99e48680d29997fbdd7)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Thu, 16 Jul 2020 04:40:27 +0000 (06:40 +0200)]
BUG/MEDIUM: server: fix possibly uninitialized state file on close
Previous fix
dc6e8a9a7 ("BUG/MEDIUM: server: resolve state file handle
leak on reload") traded a bug for another one, now we get this warning
when building server.c, which is valid since f is not necessarily
initialized (e.g. if no global state file is passed):
src/server.c: In function 'apply_server_state':
src/server.c:3272:3: warning: 'f' may be used uninitialized in this function [-Wmaybe-uninitialized]
fclose(f);
^~~~~~~~~
Let's initialize it first. This whole code block should really be
splitted, cleaned up and reorganized as it's possible that other
similar bugs are hidden in it.
This must be backported to the same branches the commit above is
backported to (likely 2.2 and 2.1).
(cherry picked from commit
2d067f93fbbecc2b25bb0374a1cd2552299f19f0)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
4881a40915f618a8b7455f5229f46724e6f9a70b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Ilya Shipitsin [Wed, 15 Jul 2020 21:02:40 +0000 (02:02 +0500)]
BUG/MEDIUM: server: resolve state file handle leak on reload
During reload, server state file is read and file handle is not released
this was indepently reported in #738 and #660.
partially resolves #660. This should be backported to 2.2 and 2.1.
(cherry picked from commit
dc6e8a9a7b19052b8fd8ab7f9a84a4a95b4a6d5b)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
d74774bca53a2f4c30995d01eeb028dde313fc29)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Harris Kaufmann [Wed, 15 Jul 2020 14:26:13 +0000 (16:26 +0200)]
BUG/MEDIUM: fcgi-app: fix memory leak in fcgi_flt_http_headers
When the loop is continued early, the memory for param_rule is not freed. This
can leak memory per request, which will eventually consume all available memory
on the server.
This patch should fix the issue #750. It must be backported as far as 2.1.
(cherry picked from commit
b605a736b0766030746b50c322879d929fcebacf)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
4f472bedc8c696f9c18da0eb16904a98a37f076c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Emeric Brun [Fri, 10 Jul 2020 13:47:11 +0000 (15:47 +0200)]
BUG/MEDIUM: log: issue mixing sampled to not sampled log servers.
A boolean was mistakenly declared 'static THREAD_LOCAL' causing
the probe of a log to a 'not sampled' log server conditionned by
the last evaluated 'sampled log' server test on the same thread.
This results to unpredictable drops of logs on 'not sampled'
log servers as soon a 'sampled' log server is declared.
This patch removes the static THREAD_LOCAL attribute from this
boolean, fixing the issue and allowing to mix 'sampled' and
'not sampled' servers.
This fix should be backported in any branches which includes
the log sampling feature.
(cherry picked from commit
2f4cc28e0f1c8c3deca8b1b7fa446c6523dcc93c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
9ce61f6d578876f1eda258d4cfad997e09c140a4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Wed, 15 Jul 2020 14:04:49 +0000 (16:04 +0200)]
BUG/MINOR: mux-fcgi: Set flags on the right stream field for empty FCGI_STDOUT
In fcgi_strm_handle_empty_stdout(), the FCGI_SF_ES_RCVD flag is set on "->state"
stream field instead of "->flags". It is obviously wrong. This bug is not
noticeable because the right state is set in the fcgi_process_demux() function a
bit later.
This patch must be backported as far as 2.1.
(cherry picked from commit
3b3096ede1b52007fa49a563436df8ee9323d78c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
ca5e8c058e157ee7d549014705e23e3be6fc505c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Wed, 15 Jul 2020 13:55:52 +0000 (15:55 +0200)]
BUG/MINOR: mux-fcgi: Set conn state to RECORD_P when skipping the record padding
When the padding of a "stream" record (STDOUT or STDERR) is skipped, we must set
the connection state to RECORD_P. It is especially important if the padding is
not fully received.
This patch must be backported as far as 2.1.
(cherry picked from commit
6c99d3baeabef9aa3a798b2251215f1026b48751)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
82cffcd95ae7dc81387344e69432c1776d02102c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Wed, 15 Jul 2020 13:46:30 +0000 (15:46 +0200)]
BUG/MINOR: mux-fcgi: Handle empty STDERR record
As mentionned in the FastCGI specification, FCGI "streams" are series of
non-empty stream records (length != 0), followed by an empty one. It is properly
handled for FCGI_STDOUT records, but not for FCGI_STDERR ones. If an empty
FCGI_STDERR record is received, the connection is blocked waiting for data which
will never come.
To fix the bug, when an empty FCGI_STDERR record is received, we drop it, eating
the padding if any.
This patch should fix the issue #743. It must be backported as far as 2.1.
(cherry picked from commit
7f85433a912529c5cda1629001b34fd2b2e54758)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
bd6dc07038f4c6b552559ccde41d31ff9a7dbf6d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 10 Jul 2020 08:01:26 +0000 (10:01 +0200)]
BUG/MEDIUM: mux-h1: Continue to process request when switching in tunnel mode
When input data are processed, if the request is switched in tunnel mode on a
protocol upgrade, we must continue the processing. Otherwise, pending input data
will only be processed on the next wakeup. So when new input data are received,
on a timeout expiration or shutdown. Worst, if the input buffer is full when it
happens, only a timeout or a shutdown will unblock the situation.
This patch should fix the issue #737. It must be backported as far as 1.9. The
bug does not seem to affect the 2.0 and 1.9 because, on a protocol upgrade, the
request is switched in tunnel mode when the response is sent to the client. But
the bug is present, so the backport remains necessary.
(cherry picked from commit
23021ad7f1d9d0e924aa5e5c6c4103b51a805af0)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
4814c74842d13dcf8d948910902f248263f80eed)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Sun, 12 Jul 2020 07:12:07 +0000 (09:12 +0200)]
CONTRIB: da: fix memory leak in dummy function da_atlas_open()
The dummy function takes care of doing a bit of work using a malloc()
to avoid returning a constant but it doesn't free the tested pointer,
which coverity noticed in issue #741. Let's free it before testing it
for the return value.
This may be backported but is not important since this code is only
present to allow to build the device detection code and not to actually
run it.
(cherry picked from commit
62fd12149f73feb4e9c93a961670d527436a6e33)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
7bd62cad8b711ed0b39ca00bf74ca326909ff73d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Thu, 9 Jul 2020 03:01:27 +0000 (05:01 +0200)]
BUG/MEDIUM: lists: add missing store barrier in MT_LIST_ADD/MT_LIST_ADDQ
The torture test run for previous commit 787dc20 ("BUG/MEDIUM: lists: add
missing store barrier on MT_LIST_BEHEAD()") finally broke again after 34M
connections. It appeared that MT_LIST_ADD and MT_LIST_ADDQ were suffering
from the same missing barrier when restoring the original pointers before
giving up, when checking if the element was already added. This is indeed
something which seldom happens with the shared scheduler, in case two
threads simultaneously try to wake up the same tasklet.
With a store barrier there after reverting the pointers, the torture test
survived 750M connections on the NanoPI-Fire3, so it looks good this time.
Probably that MT_LIST_BEHEAD should be added to test-list.c since it seems
to be more sensitive to concurrent accesses with MT_LIST_ADDQ.
It's worth noting that there is no barrier between the last two pointers
update, while there is one in MT_LIST_POP and MT_LIST_BEHEAD, the latter
having shown to be needed, but I cannot demonstrate why we would need
one here. Given that the code seems solid here, let's stick to what is
shown to work.
This fix should be backported to 2.1, just for the sake of safety since
the issue couldn't be triggered there, but it could change with the
compiler or when backporting a fix for example.
(cherry picked from commit
3b8f9b7b880397db03bfa54b26c8b6dbf6b5dc5e)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
ae8b8361ce2d50df0fb8c2d4b4d23e13ff497029)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Wed, 8 Jul 2020 17:45:50 +0000 (19:45 +0200)]
BUG/MEDIUM: lists: add missing store barrier on MT_LIST_BEHEAD()
When running multi-threaded tests on my NanoPI-Fire3 (8 A53 cores), I
managed to occasionally get either a bus error or a segfault in the
scheduler, but only when running at a high connection rate (injecting
on a tcp-request connection reject rule). The bug is rare and happens
around once per million connections. I could never reproduce it with
less than 4 threads nor on A72 cores.
Haproxy 2.1.0 would also fail there but not 2.1.7.
Every time the crash happened with the TL_URGENT task list corrupted,
though it was not immediately after the LIST_SPLICE() call, indicating
background activity survived the MT_LIST_BEHEAD() operation. This queue
is where the shared runqueue is transferred, and the shared runqueue
gets fast inter-thread tasklet wakeups from idle conn takeover and new
connections.
Comparing the MT_LIST_BEHEAD() and MT_LIST_DEL() implementations, it's
quite obvious that a few barriers are missing from the former, and these
will simply fail on weakly ordered caches.
Two store barriers were added before the break() on failure, to match
what is done on the normal path. Missing them almost always results in
a segfault which is quite rare but consistent (after ~3M connections).
The 3rd one before updating n->prev seems intuitively needed though I
coudln't make the code fail without it. It's present in MT_LIST_DEL so
better not be needlessly creative. The last one is the most important
one, and seems to be the one that helps a concurrent MT_LIST_ADDQ()
detect a late failure and try again. With this, the code survives at
least 30M connections.
Interestingly the exact same issue was addressed in 2.0-dev2 for MT_LIST_DEL
with commit
690d2ad4d ("BUG/MEDIUM: list: add missing store barriers when
updating elements and head").
This fix must be backported to 2.1 as MT_LIST_BEHEAD() is also used there.
It's only tagged as medium because it will only affect entry-level CPUs
like Cortex A53 (x86 are not affected), and requires load levels that are
very hard to achieve on such machines to trigger it. In practice it's
unlikely anyone will ever hit it.
(cherry picked from commit
787dc20952aecdfb9322fb511f07d1d21b5733c7)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
826c7b73820b44b210f97b0c24320c2aa6990bf5)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Tim Duesterhus [Sat, 4 Jul 2020 09:49:46 +0000 (11:49 +0200)]
BUG/MINOR: sample: Free str.area in smp_check_const_meth
Given the following example configuration:
listen foo
mode http
bind *:8080
http-request set-var(txn.leak) meth(GET)
server x example.com:80
Running a configuration check with valgrind reports:
==25992== 4 bytes in 1 blocks are definitely lost in loss record 1 of 344
==25992== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==25992== by 0x4E239D: my_strndup (tools.c:2261)
==25992== by 0x581E20: make_arg_list (arg.c:253)
==25992== by 0x4DE91D: sample_parse_expr (sample.c:890)
==25992== by 0x58E304: parse_store (vars.c:772)
==25992== by 0x566A3F: parse_http_req_cond (http_rules.c:95)
==25992== by 0x4A4CE6: cfg_parse_listen (cfgparse-listen.c:1339)
==25992== by 0x494C59: readcfgfile (cfgparse.c:2049)
==25992== by 0x545145: init (haproxy.c:2029)
==25992== by 0x421E42: main (haproxy.c:3175)
After this patch is applied the leak is gone as expected.
This is a fairly minor leak, but it can add up for many uses of the `bool()`
sample fetch. The bug most likely exists since the `bool()` sample fetch was
introduced in commit
cc103299c75c530ab3637a1698306145bdc85552. The fix may
be backported to HAProxy 1.6+.
(cherry picked from commit
041a626a8acfd53aa3710cd3d620f7f9f01fe893)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
761200b597cb36378527ba8cca1ad10a5d09a922)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Tim Duesterhus [Sat, 4 Jul 2020 09:49:45 +0000 (11:49 +0200)]
BUG/MINOR: sample: Free str.area in smp_check_const_bool
Given the following example configuration:
listen foo
mode http
bind *:8080
http-request set-var(txn.leak) bool(1)
server x example.com:80
Running a configuration check with valgrind reports:
==24233== 2 bytes in 1 blocks are definitely lost in loss record 1 of 345
==24233== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==24233== by 0x4E238D: my_strndup (tools.c:2261)
==24233== by 0x581E10: make_arg_list (arg.c:253)
==24233== by 0x4DE90D: sample_parse_expr (sample.c:890)
==24233== by 0x58E2F4: parse_store (vars.c:772)
==24233== by 0x566A2F: parse_http_req_cond (http_rules.c:95)
==24233== by 0x4A4CE6: cfg_parse_listen (cfgparse-listen.c:1339)
==24233== by 0x494C59: readcfgfile (cfgparse.c:2049)
==24233== by 0x545135: init (haproxy.c:2029)
==24233== by 0x421E42: main (haproxy.c:3175)
After this patch is applied the leak is gone as expected.
This is a fairly minor leak, but it can add up for many uses of the `bool()`
sample fetch. The bug most likely exists since the `bool()` sample fetch was
introduced in commit
cc103299c75c530ab3637a1698306145bdc85552. The fix may
be backported to HAProxy 1.6+.
(cherry picked from commit
c7d8a86f2f8ccdcaada18a5e84bd5cb8c27dac51)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
300f65dfa5d5bdb51cd3283d92bf84793f959b86)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Tue, 7 Jul 2020 13:55:23 +0000 (15:55 +0200)]
DOC: configuration: remove obsolete mentions of H2 being converted to HTTP/1.x
The first H2 implementation in version 1.8 used to turn HTTP/2 requests
to HTTP/1.1, causing many limitations. This is not true anymore and we
don't suffer from the lack of server-side H2 nor are we forced to close
mode anymore, so let's remove such obsolete mentions.
This could be backported to 2.0.
(cherry picked from commit
253c2519c2d3e394cc71630331dd79ceb14c57ec)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Anthonin Bonnefoy [Mon, 22 Jun 2020 07:17:01 +0000 (09:17 +0200)]
MINOR: http: Add support for http 413 status
Add 413 http "payload too large" status code. This will allow 413 to be
used in deny_status and errorfile.
(cherry picked from commit
85048f80c9c1ca22732951e83c01ab0b31f5371f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
[Cf: A "connection: close" has been added in the HTTP message to be consistent
with other error codes]
Christopher Faulet [Wed, 1 Jul 2020 16:56:30 +0000 (18:56 +0200)]
BUG/MINOR: backend: Remove CO_FL_SESS_IDLE if a client remains on the last server
When a connection is picked from the session server list because the proxy or
the session are marked to use the last requested server, if it is idle, we must
marked it as used removing the CO_FL_SESS_IDLE flag and decrementing the session
idle_conns counter.
This patch must be backported as far as 1.9.
(cherry picked from commit
e91a526c8fe7bf3b655057d41e8b72a2011b7046)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Tue, 7 Jul 2020 09:57:12 +0000 (11:57 +0200)]
BUG/MEDIUM: connection: Continue to recv data to a pipe when the FD is not ready
Don't wait a FD is ready to receive data to try to get data to a pipe to be sure
to not be blocked. Indeed, after a call to raw_sock_to_pipe(), the readiness for
a FD may be disabled because we read enough data (fd_done_recv) or because
nothing was received (fd_cant_recv). In the first case, the readiness is not
re-enabled after a successful send on the opposite side.
On the 2.2, calls to fd_done_recvt() was removed from raw_sock functions in the
commit
1113116b4 ("MEDIUM: raw-sock: remove obsolete calls to
fd_{cant,cond,done}_{send,recv}"). But it is a bit dangerous to directely
backport it in a stable version because many changes were perforned on the
connection layer in the 2.2. So to unblock the situation, it is safer to just
remove the test on the FD at the beginning of raw_sock_to_pipe(). At worst, we
have an extra syscall from time to time.
This patch must be backported to 2.0.
Willy Tarreau [Fri, 17 Jan 2020 08:59:40 +0000 (09:59 +0100)]
MINOR: connection: move the CO_FL_WAIT_ROOM cleanup to the reader only
CO_FL_WAIT_ROOM is set by the splicing function in raw_sock, and cleared
by the stream-int when splicing is disabled, as well as in
conn_refresh_polling_flags() so that a new call to ->rcv_pipe() could
be attempted by the I/O callbacks called from conn_fd_handler(). This
clearing in conn_refresh_polling_flags() makes no sense anymore and is
in no way related to the polling at all.
Since we don't call them from there anymore it's better to clear it
before attempting to receive, and to set it again later. So let's move
this operation where it should be, in raw_sock_to_pipe() so that it's
now symmetric. It was also placed in raw_sock_to_buf() so that we're
certain that it gets cleared if an attempt to splice is replaced with
a subsequent attempt to recv(). And these were currently already achieved
by the call to conn_refresh_polling_flags(). Now it could theorically be
removed from the stream-int.
(cherry picked from commit
e2a0eeca778115adc2e6dd7c89bb36181f349ca1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
[Cf: This patch is mandatory in the 2.1 to remove the flag CO_FL_WAIT_ROOM
because it is not removed after a successfull send.]
Christopher Faulet [Fri, 3 Jul 2020 13:38:11 +0000 (15:38 +0200)]
BUG/MEDIUM: mux-h1: Subscribe rather than waking up in h1_rcv_buf()
At the end of h1_rcv_buf(), if the message was not fully processed (not in
HTTP_MSG_DONE state), it means some data are missing. But instead of waking up
the H1 connection, it is safer to subscribe for reads. Especially if the
splicing is enabled (or if we are waiting the input buffer is flushed). Because
in that case, waking up the H1 connection leads to a ping-pong loop with
h1_recv() function. Indeed, in h1_recv(), if the splicing is enabled, no receive
is performed and the H1 stream is woken up.
This bug is specific to the 2.1. Thus, there is no upstream commit ID for this
patch. But in the 2.2, it is part of the commit
2c1f37d35 ("OPTIM: mux-h1:
subscribe rather than waking up at a few other places"). The 2.0 must be
evaluated to know if this patch must be backported or not.
Christopher Faulet [Fri, 3 Jul 2020 13:08:49 +0000 (15:08 +0200)]
MINOR: mux-h1: Improve traces about the splicing
Trace messages have been added when the CS_FL_MAY_SPLICE flag is set or unset
and when the splicing is really enabled for the H1 connection.
This patch may be backpored to 2.1 to ease debugging.
(cherry picked from commit
27182297c763fb6f4818c9872692df51d4032d6b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Tue, 7 Jul 2020 08:56:40 +0000 (10:56 +0200)]
BUG/MEDIUM: mux-h1: Disable splicing for the conn-stream if read0 is received
The CS_FL_MAY_SPLICE flag must be unset for the conn-stream if a read0 is
received while reading on the kernel pipe. It is mandatory when some data was
also received. Otherwise, this flag prevent the call to the h1 rcv_buf()
callback. Thus the read0 will never be handled by the h1 multiplexer leading to
a freeze of the session until a timeout is reached.
This patch must be backported to 2.1 and 2.0.
(cherry picked from commit
a131a8fe934b40324fd8a16d237616331ea0088f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 3 Jul 2020 13:12:00 +0000 (15:12 +0200)]
BUG/MINOR: mux-h1: Disable splicing only if input data was processed
In h1_rcv_buf(), the splicing is systematically disabled if it was previously
enabled. When it happens, if the splicing is enabled it means the channel's
buffer was empty before calling h1_rcv_buf(). Thus, the only reason to disable
the splicing at this step is when some input data have just been processed.
This patch may be backported to 2.1 and 2.0.
(cherry picked from commit
7b7016bf6e69f61074684e9145e1a6784b6a9d03)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 3 Jul 2020 13:02:25 +0000 (15:02 +0200)]
BUG/MINOR: mux-h1: Don't read data from a pipe if the mux is unable to receive
In h1_rcv_pipe(), if the mux is unable to receive data, for instance because the
multiplexer is blocked on input waiting the other side (BUSY mode), no receive
must be performed.
This patch must be backported to 2.1 and 2.0.
(cherry picked from commit
0060be9ccfc76f8c6a9c29ce99d18a37d85bcc1d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 3 Jul 2020 12:51:15 +0000 (14:51 +0200)]
BUG/MINOR: mux-h1: Fix the splicing in TUNNEL mode
In the commit
17ccd1a35 ("BUG/MEDIUM: connection: add a mux flag to indicate
splice usability"), The CS_FL_MAY_SPLICE flags was added to notify the upper
layer that the mux is able to use the splicing. But this was only done for the
payload in a message, in HTTP_MSG_DATA state. But the splicing is also possible
in TUNNEL mode, in HTTP_MSG_TUNNEL state. In addition, the splicing ability is
always disabled for chunked messages.
This patch must be backported to 2.1 and 2.0.
(cherry picked from commit
2eaf30955f3619e2f262ba948313b8c2cb40ee81)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Tim Duesterhus [Fri, 3 Jul 2020 11:43:42 +0000 (13:43 +0200)]
BUG/MINOR: http_act: don't check capture id in backend (2)
Please refer to commit
19a69b3740702ce5503a063e9dfbcea5b9187d27 for all the
details. This follow up commit fixes the `http-response capture` case, the
previous one only fixed the `http-request capture` one. The documentation was
already updated and the change to `check_http_res_capture` is identical to
the `check_http_req_capture` change.
This patch must be backported together with
19a69b3740702ce5503a063e9dfbcea5b9187d27.
Most likely this is 1.6+.
(cherry picked from commit
f3f4aa0266809c729ca98171a34de8c9bdd2c9f3)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Thu, 2 Jul 2020 13:38:35 +0000 (15:38 +0200)]
BUILD: haproxy: fix build error when RLIMIT_AS is not set
As reported in issue #724, openbsd fails to build in haproxy.c
due to a faulty comma in the middle of a warning message. This code
is only compiled when RLIMIT_AS is not defined, which seems to be
rare these days.
This may be backported to older versions as the problem was likely
introduced when strict limits were added.
(cherry picked from commit
ab8b6a45be2f76efb28f7f0ad18b69549c77b4d6)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Wed, 1 Jul 2020 16:30:16 +0000 (18:30 +0200)]
DOC: configuration: fix alphabetical ordering for tune.pool-{high,low}-fd-ratio
In addition they were in the wrong alphabetical order in the doc. They
were added in 2.0 by commit
88698d966 ("MEDIUM: connections: Add a way
to control the number of idling connections.") so this must be backported
to 2.0.
(cherry picked from commit
83ca305ddc2222ec269e18a9df6d519ed0f08ae8)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Wed, 1 Jul 2020 16:27:16 +0000 (18:27 +0200)]
DOC: configuration: add missing index entries for tune.pool-{low,high}-fd-ratio
These two keywords didn't have an entry in the index. They were added in
2.0 by commit
88698d966 ("MEDIUM: connections: Add a way to control the
number of idling connections.") so this must be backported to 2.0.
(cherry picked from commit
a8e2d9790548f395c5f3782117ef2eef350c7c39)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Wed, 1 Jul 2020 05:09:39 +0000 (07:09 +0200)]
BUG/MINOR: proxy: always initialize the trash in show servers state
Actually the cleanup in commit
6ff8143f7 ("BUG/MINOR: proxy: fix
dump_server_state()'s misuse of the trash") allowed to spot that the
trash is never reset when dumping a servers state. I couldn't manage
to make it dump garbage even with large setups but didn't find either
where it's cleared between successive calls while other handlers do
explicitly invoke chunk_reset(), so it seems to happen a bit by luck.
Let's use chunk_printf() here for each turn, it makes things clearer.
This could be backported along with previous patch, especially if any
user reports occasional garbage appearing in the show servers output.
(cherry picked from commit
df2a0305f24a58f56d44e1d12ea247a5ff243757)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Wed, 1 Jul 2020 05:02:42 +0000 (07:02 +0200)]
BUG/MINOR: proxy: fix dump_server_state()'s misuse of the trash
dump_server_state() claims to dump into a buffer but instead it writes
into a buffer then dumps the trash into the channel, so it only supports
being called with buf=&trash and doesn't need this buffer. There doesn't
seem to be any current impact of this mistake since the function is called
from one location only.
A backport may be performed if it helps fixing other bugs but it will not
fix an existing bug by itself.
(cherry picked from commit
6ff8143f7c87ad40d7369119467d779642702ad5)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Tue, 30 Jun 2020 16:52:32 +0000 (18:52 +0200)]
BUG/MEDIUM: pattern: Add a trailing \0 to match strings only if possible
In pat_match_str() and pat_math_beg() functions, a trailing zero is
systematically added at the end of the string, even if the buffer is not large
enough to accommodate it. It is a possible buffer overflow. For instance, when
the alpn is matched against a list of strings, the sample fetch is filled with a
non-null terminated string returned by the SSL library. No trailing zero must be
added at the end of this string, because it is outside the buffer.
So, to fix the bug, a trailing zero is added only if the buffer is large enough
to accommodate it. Otherwise, the sample fetch is duplicated. smp_dup() function
adds a trailing zero to the duplicated string, truncating it if it is too long.
This patch should fix the issue #718. It must be backported to all supported
versions.
(cherry picked from commit
b4cf7ab9bc413bbb956e225f903959bff17e4049)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Tue, 30 Jun 2020 14:11:36 +0000 (16:11 +0200)]
DOC: ssl: add "allow-0rtt" and "ciphersuites" in crt-list
Support for "allow-0rtt" and "ciphersuites" exists for crt-list.
Fix issue #721.
Should be backported as far as 1.8.
(cherry picked from commit
5d03639ba6fa9e7eee8af8fe489101de65d7f6f1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Sat, 27 Jun 2020 23:24:12 +0000 (01:24 +0200)]
MINOR: cli: make "show sess" stop at the last known session
"show sess" and particularly "show sess all" can be very slow when dumping
lots of information, and while dumping, new sessions might appear, making
the output really endless. When threads are used, this causes a double
problem:
- all threads are paused during the dump, so an overly long dump degrades
the quality of service ;
- since all threads are paused, more events get postponed, possibly
resulting in more streams to be dumped on next invocation of the dump
function.
This patch addresses this long-lasting issue by doing something simple:
the CLI's stream is moved at the end of the steams list, serving as an
identifiable marker to end the dump, because all entries past it were
added after the command was entered. As a result, the CLI's stream always
appears as the last one.
It may make sense to backport this to stable branches where dumping live
streams is difficult as well.
(cherry picked from commit
c6e7a1b8e9da7813661096b096c65b137b72e35f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Tim Duesterhus [Fri, 26 Jun 2020 13:44:48 +0000 (15:44 +0200)]
BUG/MEDIUM: fetch: Fix hdr_ip misparsing IPv4 addresses due to missing NUL
The IPv4 code did not take into account that the header value might not
contain the trailing NUL byte, possibly reading stray data after the header
value, failing the parse and testing the IPv6 branch. That one adds the
missing NUL, but fails to parse IPv4 addresses.
Fix this issue by always adding the trailing NUL.
The bug was reported on GitHub as issue #715.
It's not entirely clear when this bug started appearing, possibly earlier
versions of smp_fetch_hdr guaranteed the NUL termination. However the
addition of the NUL in the IPv6 case was added together with IPv6 support,
hinting that at that point in time the NUL was not guaranteed.
The commit that added IPv6 support was
69fa99292e689e355080d83ab19db4698b7c502b
which first appeared in HAProxy 1.5. This patch should be backported to
1.5+, taking into account the various buffer / chunk changes and the movement
across different files.
(cherry picked from commit
5cd00873f46f49f78587d4cc770e5ce7e979cc9c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Fri, 26 Jun 2020 10:03:45 +0000 (12:03 +0200)]
REGTEST: ssl: add some ssl_c_* sample fetches test
Test the following ssl sample fetches:
ssl_c_der, ssl_c_sha1,hex, ssl_c_notafter, ssl_c_notbefore,
ssl_c_sig_alg, ssl_c_i_dn, ssl_c_s_dn, ssl_c_serial,hex, ssl_c_key_alg,
ssl_c_version
This reg-test could be used as far as haproxy 1.6.
(cherry picked from commit
86bc9539f29df77231584ba17cc1796680fce92f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Fri, 26 Jun 2020 09:29:43 +0000 (11:29 +0200)]
REGTEST: ssl: tests the ssl_f_* sample fetches
Test the following ssl sample fetches:
ssl_f_der, ssl_f_sha1,hex, ssl_f_notafter, ssl_f_notbefore,
ssl_f_sig_alg, ssl_f_i_dn, ssl_f_s_dn, ssl_f_serial,hex, ssl_f_key_alg,
ssl_f_version
This reg-test could be used as far as haproxy 1.5.
(cherry picked from commit
6e56b2cd8a673ace60e9fa3fcde8fd1ecef3d22d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Mon, 22 Jun 2020 13:32:14 +0000 (15:32 +0200)]
MINOR: spoe: Don't systematically create new applets if processing rate is low
When an event must be processed, we decide to create a new SPOE applet if there
is no idle applet at all or if the processing rate is lower than the number of
waiting events. But when the processing rate is very low (< 1 event/second), a
new applet is created independently of the number of idle applets.
Now, when there is at least one idle applet when there is only one event to
process, no new applet is created.
This patch is related to the issue #690.
(cherry picked from commit
eccfa612e8c9fd566f94dea9b0213b8b0d2d0531)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Tue, 23 Jun 2020 03:58:20 +0000 (05:58 +0200)]
BUG/MINOR: http_ana: clarify connection pointer check on L7 retry
Coverity reports a possible null deref in issue #703. It seems this
cannot happen as in order to have a CF_READ_ERROR we'd need to have
attempted a recv() which implies a conn_stream, thus conn cannot be
NULL anymore. But at least one line tests for conn and the other one
not, which is confusing. So let's add a check for conn before
dereferencing it.
This needs to be backported to 2.1 and 2.0. Note that in 2.0 it's
in proto_htx.c.
(cherry picked from commit
ee99aaf1f9bad06475aaf8c5dc3c0986a9899952)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Miroslav Zagorac [Fri, 19 Jun 2020 20:17:17 +0000 (22:17 +0200)]
BUG/MINOR: spoe: correction of setting bits for analyzer
When a SPOE filter starts the response analyze, the wrong flag is tested on the
pre_analyzers bit field. AN_RES_INSPECT must be tested instead of
SPOE_EV_ON_TCP_RSP.
This patch must be backported to all versions with the SPOE support, i.e as far
as 1.7.
(cherry picked from commit
88403266e5c38b5fbe278a25304cbdc735ae50fe)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Thu, 11 Jun 2020 11:43:20 +0000 (13:43 +0200)]
REGTEST: Add a simple script to tests errorfile directives in proxy sections
This script is compatible with all HAProxy versions. It does not depend on 2.2
features.
(cherry picked from commit
60837d340ccf0c715b0b06db347d66109c20accf)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Ryan O'Hara [Mon, 15 Jun 2020 16:34:54 +0000 (11:34 -0500)]
BUG/MINOR: systemd: Wait for network to be online
Change systemd service file to wait for network to be completely
online. This solves two problems:
If haproxy is configured to bind to IP address(es) that are not yet
assigned, haproxy would previously fail. The workaround is to use
"option transparent".
If haproxy us configured to use a resolver to resolve servers via DNS,
haproxy would previously fail due to the fact that the network is not
fully online yet. This is the most compelling reason for this patch.
Signed-off-by: Ryan O'Hara <rohara@redhat.com>
Acked-by: Lukas Tribus <lukas@ltri.eu>
(cherry picked from commit
f49a6049b88b7a9f0f0a18b076f34ab83820445a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Fri, 20 Dec 2019 17:22:02 +0000 (18:22 +0100)]
MEDIUM: map: make the "clear map" operation yield
As reported in issue #419, a "clear map" operation on a very large map
can take a lot of time and freeze the entire process for several seconds.
This patch makes sure that pat_ref_prune() can regularly yield after
clearing some entries so that the rest of the process continues to work.
The first part, the removal of the patterns, can take quite some time
by itself in one run but it's still relatively fast. It may block for
up to 100ms for 16M IP addresses in a tree typically. This change needed
to declare an I/O handler for the clear operation so that we can get
back to it after yielding.
The second part can be much slower because it deconstructs the elements
and its users, but it iterates progressively so we can yield less often
here.
The patch was tested with traffic in parallel sollicitating the map being
released and showed no problem. Some traffic will definitely notice an
incomplete map but the filling is already not atomic anyway thus this is
not different.
It may be backported to stable versions once sufficiently tested for side
effects, at least as far as 2.0 in order to avoid the watchdog triggering
when the process is frozen there. For a better behaviour, all these
prune_* functions should support yielding so that the callers have a
chance to continue also yield in turn.
(cherry picked from commit
d1d005d7f6b894ed243c177937626d888b3d03e1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Thu, 18 Jun 2020 16:56:44 +0000 (18:56 +0200)]
REGTEST: http-rules: test spaces in ACLs with master CLI
Do the tests for spaces on the CLI with the master CLI.
Could be backported as far as 2.0 once the required patches are applied.
(cherry picked from commit
5bb21b1d29f58875fbac05a9fdfd49f84de991d8)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Thu, 18 Jun 2020 16:52:18 +0000 (18:52 +0200)]
REGTEST: http-rules: test spaces in ACLs
This reg-test tests the spaces in an ACL file, it tries to add new
entries with spaces from the CLI
This reg-test could backported in all stable branches if the fix for
spaces on the CLI was backported.
(cherry picked from commit
398c5f39ee768e1b0a97b13e1a6c31ce19a6e80c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Thu, 18 Jun 2020 16:45:04 +0000 (18:45 +0200)]
BUG/MINOR: mworker/cli: fix semicolon escaping in master CLI
Fix the semicolon escaping which must be handled in the master CLI,
the commands were wrongly splitted and could be forwarded partially to
the target CLI.
(cherry picked from commit
02c255e64bd528dfa9f77dc15912d692c2213cc5)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Thu, 18 Jun 2020 16:03:57 +0000 (18:03 +0200)]
BUG/MINOR: mworker/cli: fix the escaping in the master CLI
The master CLI must not do the escaping since it forwards the commands
to another CLI. It should be able to split into words by taking care of
the escaping, but must not remove the forwarded backslashes.
This fix do the same thing as the previous patch applied to the
cli_parse_request() function, by taking care of the escaping during the
word split, but it also remove the part which was removing the
backslashes from the forwarded command.
(cherry picked from commit
fe249c3df53127d53d55c3fda8e6be7f22cfa58b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Yves Lafon [Mon, 8 Jun 2020 14:08:06 +0000 (16:08 +0200)]
BUG/MINOR: cli: allow space escaping on the CLI
It was not possible to escape spaces over the CLI, making impossible the
insertion of new ACL entries with spaces from the CLI.
This patch fixes the escaping of spaces over the CLI.
It is now possible to launch "add acl agents.acl My\ User\ Agent" over
the CLI.
Could be backported in all stable branches.
Should fix issue #400.
(cherry picked from commit
b08c6d06e712b148bf396dcf13f7da590a6104c9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Tue, 16 Jun 2020 15:58:14 +0000 (17:58 +0200)]
BUG/MINOR: spoe: add missing key length check before checking key names
The spoe parser fails to check that the decoded key length is large
enough to match a given key but it uses the returned length in memcmp().
So returning "ver" could match "version" for example. In addition this
makes clang 10's ASAN complain because the second argument to memcmp()
is the static key which is shorter than the decoded buffer size, which
in practice has no impact.
I'm still not 100% sure the parser is entirely correct because even
with this fix it cannot parse a key whose name matches the beginning
of another one, but in practice this does not happen. Ideally a
preliminary length check before the comparison would be safer.
This needs to be backported as far as 1.7.
(cherry picked from commit
da21ed1662ff7f1610db14b3ce48f19ec37a695b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Tue, 16 Jun 2020 09:10:53 +0000 (11:10 +0200)]
BUG/MEDIUM: ebtree: use a byte-per-byte memcmp() to compare memory blocks
As reported in issue #689, there is a subtle bug in the ebtree code used
to compared memory blocks. It stems from the platform-dependent memcmp()
implementation. Original implementations used to perform a byte-per-byte
comparison and to stop at the first non-matching byte, as in this old
example:
https://www.retro11.de/ouxr/211bsd/usr/src/lib/libc/compat-sys5/memcmp.c.html
The ebtree code has been relying on this to detect the first non-matching
byte when comparing keys. This is made so that a zero-terminated string
can fail to match against a longer string.
Over time, especially with large busses and SIMD instruction sets,
multi-byte comparisons have appeared, making the processor fetch bytes
past the first different byte, which could possibly be a trailing zero.
This means that it's possible to read past the allocated area for a
string if it was allocated by strdup().
This is not correct and definitely confuses address sanitizers. In real
life the problem doesn't have visible consequences. Indeed, multi-byte
comparisons are implemented so that aligned words are loaded (e.g. 512
bits at once to process a cache line at a time). So there is no way such
a multi-byte access will cross a page boundary and end up reading from
an unallocated zone. This is why it was never noticed before.
This patch addresses this by implementing a one-byte-at-a-time memcmp()
variant for ebtree, called eb_memcmp(). It's optimized for both small and
long strings and guarantees to stop after the first non-matching byte. It
only needs 5 instructions in the loop and was measured to be 3.2 times
faster than the glibc's AVX2-optimized memcmp() on short strings (1 to
257 bytes), since that latter one comes with a significant setup cost.
The break-even seems to be at 512 bytes where both version perform
equally, which is way longer than what's used in general here.
This fix should be backported to stable versions and reintegrated into
the ebtree code.
(cherry picked from commit
853926a9ac7f8a98f22121ea596034660e3ae537)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Mon, 15 Jun 2020 16:08:07 +0000 (18:08 +0200)]
BUG/MINOR: tcp-rules: tcp-response must check the buffer's fullness
It's unclear why the buffer length wasn't considered when tcp-response
rules were added in 1.5-dev3 with commit
97679e790 ("[MEDIUM] Implement
tcp inspect response rules"). But it's impossible to write working
tcp-response content rules as they're always waiting for the expiration
and do not consider the fact that the buffer is full. It's likely that
tcp-response content rules were only used with HTTP traffic.
This may be backported to stable versions, though it's not very
important considering that that nobody reported this in 10 years.
(cherry picked from commit
55ae1ab9e46bb8c10931cadfe685319e4fa9170c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Mon, 15 Jun 2020 16:01:10 +0000 (18:01 +0200)]
BUG/MINOR: http: make smp_fetch_body() report that the contents may change
The req_body and res_body sample fetch functions forgot to set the
SMP_F_MAY_CHANGE flag, making them unusable in tcp content rules. Now we
set the flag as long as the channel is not full and nothing indicates
the end was reached.
This is marked as a bug because it's unusual for a sample fetch function
to return a final verdict while data my change, but this results from a
limitation that was affecting the legacy mode where it was not possible
to know whether the end was reached without de-chunking the message. In
HTX there is no more reason to limit this. This fix could be backported
to 2.1, and to 2.0 if really needed, though it will only be doable for
HTX, and legacy cannot be fixed.
(cherry picked from commit
9dc92b2650a83a17db9779eadf63cb011be4f8ff)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Florian Tham [Wed, 8 Jan 2020 12:35:30 +0000 (13:35 +0100)]
MINOR: http: Add 404 to http-request deny
This patch adds http status code 404 Not Found to http-request deny. See
issue #80.
(cherry picked from commit
9205fea13a5e1711a54e6eca68b94128d47c391a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Florian Tham [Wed, 8 Jan 2020 09:19:05 +0000 (10:19 +0100)]
MINOR: http: Add 410 to http-request deny
This patch adds http status code 410 Gone to http-request deny. See
issue #80.
(cherry picked from commit
272e29b5cc55baacce96350a5921a5ae204f43d3)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Mon, 15 Jun 2020 12:37:19 +0000 (14:37 +0200)]
BUG/MEDIUM: ssl: crt-list must continue parsing on ERR_WARN
The original crt-list parsing was stopping at any non-zero value in
the cfgerr variable, even warnings.
This is an issue as it could lead to a crt-list parsing stopped at the
first warning, then HAProxy launched with a partial crt-list.
A ERR_WARN must continue the parsing. The parsing must be only
stopped on an ERR_CODE.
This commit is 2.1 only since it was fixed
in 2.2 by commit 2954c47 ("MEDIUM: ssl: allow crt-list caching")
and accidently in 2.0 by commit b131c87 ("CLEANUP: ssl: make
ssl_sock_load_cert*() return real error codes") as well as in 1.9 and
1.8.
William Lallemand [Thu, 11 Jun 2020 15:34:00 +0000 (17:34 +0200)]
BUG/MINOR: ssl: fix ssl-{min,max}-ver with openssl < 1.1.0
In bug #676, it was reported that ssl-min-ver SSLv3 does not work in
Amazon environments with OpenSSL 1.0.2.
The reason for this is a patch of Amazon OpenSSL which sets
SSL_OP_NO_SSLv3 in SSL_CTX_new(). Which is kind of a problem with our
implementation of ssl-{min,max}-ver in old openSSL versions, because it
does not try to clear existing version flags.
This patch fixes the bug by cleaning versions flags known by HAProxy in
the SSL_CTX before applying the right ones.
Should be backported as far as 1.8.
(cherry picked from commit
d0712f3873546a0c24f3204ad75dd7eacd689602)
[wla: context edit]
Signed-off-by: William Lallemand <wlallemand@haproxy.org>