Christopher Faulet [Tue, 10 Nov 2020 17:45:34 +0000 (18:45 +0100)]
BUG/MAJOR: spoe: Be sure to remove all references on a released spoe applet
When a SPOE applet is used to send a frame, a reference on this applet is saved
in the spoe context of the offladed stream. But, if the applet is released
before receving the corresponding ack, we must be sure to remove this
reference. This was performed for fragmented frames only. But it must also be
performed for a spoe contexts in the applet waiting_queue and in the thread
waiting_queue (used in async mode).
This bug leads to a memory corruption when an offloaded stream try to update the
state of a released applet because it still have a reference on it. There are
many ways to trigger this bug. The easiest is probably during reloads. On the
old process, all applets are woken up to be released ASAP.
Many thanks to Maciej Zdeb to report the bug and to work on it for 2
months. Without his help, it would have been much more difficult to fix the
bug. It is always a huge pleasure to see how some users are enthousiast and
helpful. Thanks again Maciej !
This patch must be backported to all versions where the spoe is supported (>=
1.7).
(cherry picked from commit
cf181c76e341f2d49f6cae0ca8200158058073f1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
abe894d2c91da5f57ae7704eba59c41b409fc1a0)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
a2cf638010c841aa28c9d5a3ef57155fdc74cff5)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 13 Nov 2020 12:41:04 +0000 (13:41 +0100)]
BUG/MINOR: http-fetch: Fix calls w/o parentheses of the cookie sample fetches
req.cook, req.cook_val, req.cook_cnt and and their response counterparts may be
called without cookie name. In this case, empty parentheses may be used, or no
parentheses at all. In both, the result must be the same. But only the first one
works. The second one always returns a failure. This patch fixes this bug.
Note that on old versions (< 2.2), both cases fail.
This patch must be backported in all stable versions.
(cherry picked from commit
97fc8da2643531ade4163d6662f13f76fa59d677)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
934630a682fc4b5fb4902c254de17fcbe2ac84ee)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
d6e7b958dfde326932e63a5840e4f2623fb76505)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Maciej Zdeb [Fri, 13 Nov 2020 09:38:06 +0000 (09:38 +0000)]
BUG/MINOR: http-fetch: Extract cookie value even when no cookie name
HTTP sample fetches dealing with the cookies (req/res.cook,
req/res.cook_val and req/res.cook_cnt) must be prepared to be called
without cookie name. For the first two, the first cookie value is
returned, regardless its name. For the last one, all cookies are counted.
To do so, http_extract_cookie_value() may now be called with no cookie
name (cookie_name_l set to 0). In this case, the matching on the cookie
name is ignored and the first value found is returned.
Note this patch also fixes matching on cookie values in ACLs.
This should be backported in all stable versions.
(cherry picked from commit
dea7c209f8a77b471323dd97bdc1ac4d7a17b812)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
8cac341124e2b79c2a924ad186a114b39546298e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
02b89eadd7a7ddbaad88c5725b0a215d2d8c1d81)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Fri, 13 Nov 2020 13:10:20 +0000 (14:10 +0100)]
BUG/MEDIUM: peers: fix decoding of multi-byte length in stick-table messages
There is a bug in peer_recv_msg() due to an incorrect cast when trying
to decode the varint length of a stick-table message, causing lengths
comprised between 128 and 255 to consume one extra byte, ending in
protocol errors. The root cause of this is that peer_recv_msg() tries
hard to reimplement all the parsing and control that is already done in
intdecode() just to measure the length before calling it. And it got it
wrong.
Let's just get rid of this unneeded code duplication and solely rely on
intdecode() instead. The bug was introduced in 2.0 as part of a cleanup
pass on this code with commit
95203f218 ("MINOR: peers: Move high level
receive code to reduce the size of I/O handler."), so this patch must
be backported to 2.0.
Thanks to Yves Lafon for reporting the problem.
(cherry picked from commit
1dfd4f106f15bc4e6e992f8babbc863c12975b5a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
d4913917c6e471b1d18c0574711768b3b47ea151)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
13bd9d4ef093df43e74d44f14a24f0ad25b170d8)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Frédéric Lécaille [Thu, 12 Nov 2020 20:01:54 +0000 (21:01 +0100)]
BUG/MINOR: peers: Missing TX cache entries reset.
The TX part of a cache for a dictionary is made of an reserved array of ebtree nodes
which are pointers to dictionary entries. So when we flush the TX part of such a
cache, we must not only remove these nodes to dictionary entries from their ebtree.
We must also reset their values. Furthermore, the LRU key and the last lookup
result must also be reset.
(cherry picked from commit
ea875e62e6b2f69c50533c5cd52eb5284c69723f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
9650ee94b3cc5bc2e6c2a7cf67ef66c0273c1404)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
c65161466af0bbf1cce39727ffff1bbaefcfa9ec)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Frédéric Lécaille [Thu, 12 Nov 2020 18:53:11 +0000 (19:53 +0100)]
BUG/MINOR: peers: Do not ignore a protocol error for dictionary entries.
If we could not decode the ID of a dictionary entry from a peer update message,
we must inform the remote peer about such an error as this is done for
any other decoding error.
(cherry picked from commit
f9e51beec118f1bbd558ed689fdad35046160529)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
cd24d5cfbe58bf2b25a76d077a0923053cbf7838)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
c4b0ea55b105ae65d9580e7b915460645f0be2fa)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Thierry Fournier [Tue, 10 Nov 2020 19:38:20 +0000 (20:38 +0100)]
BUG/MINOR: lua: set buffer size during map lookups
This size is used by some pattern matching to determine if there
is sufficient room in the buffer to add final \0 if necessary.
If the size is not set, the conditions use uninitialized value.
Note: it seems this bug can't cause a crash.
Should be backported until 2.2 (at least)
(cherry picked from commit
91dc0c0d8fdc2fb091b49699ebb323d01aa1d9f6)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
66e9ff531481d6a043ea402cb303f36b3d9ca9e1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
4fa0bdfbb3357af1d08b55ed5f7db912225c25f3)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Thierry Fournier [Tue, 10 Nov 2020 19:51:36 +0000 (20:51 +0100)]
BUG/MINOR: pattern: a sample marked as const could be written
The functions add final 0 to string if the final 0 is not set,
but don't check the flag CONST. This patch duplicates the strings
if the final zero is not set and the string is CONST.
Should be backported until 2.2 (at least)
(cherry picked from commit
a68affeaa9377f88f773ef62a9bb2541dfb672d3)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
9633f444ac3ae540f6ce5dd0b0880fb40fabe9d5)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
b982485900e4f20f9c4a0ef2521da1c223acb30d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 6 Nov 2020 08:33:36 +0000 (09:33 +0100)]
BUG/MINOR: http-htx: Just warn if payload of an errorfile doesn't match the C-L
During startup, when an errorfile is parsed, if its payload does not match the
announced content-length, an error is triggered. This change was introduced in
the 2.3, which is ok. But for a stable release it may be seen as a
regression. Thus, now a warning is emitted, instead of an error. And the
content-length header is updated with the real payload length.
This patch depends on
58f55acf4e ("MINOR: http-htx: Add understandable errors
for the errorfiles parsing"). Both must be backported as far as 2.0. This bug
only exists in the 2.2, 2.1 and 2.0. Thus, there is no upstream commit ID for
this patch.
(cherry picked from commit
7bf3d81d3cf4b9f4587cee061c2ef4a533125002)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Thu, 5 Nov 2020 21:43:41 +0000 (22:43 +0100)]
MINOR: http-htx: Add understandable errors for the errorfiles parsing
No details are provided when an error occurs during the parsing of an errorfile,
Thus it is a bit hard to diagnose where the problem is. Now, when it happens, an
understandable error message is reported.
This patch is not a bug fix in itself. But it will be required to change an
fatal error into a warning in last stable releases. Thus it must be backported
as far as 2.0.
(cherry picked from commit
a66adf41ea28a0fa29437d1675f225b5cc589b59)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
3a10710b777370ef36763b8b6658ae63ef5c4ec4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
58f55acf4eafd19ed35ad8506064198229ba69c3)
[cf: Adapted for the 2.1]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Thu, 5 Nov 2020 14:40:11 +0000 (15:40 +0100)]
[RELEASE] Released version 2.1.10
Released version 2.1.10 with the following main changes :
- BUG/MEDIUM: ssl: crt-list negative filters don't work
- MINOR: ssl: reach a ckch_store from a sni_ctx
- BUILD: makefile: Fix building with closefrom() support enabled
- BUG/MINOR: Fix several leaks of 'log_tag' in init().
- DOC: tcp-rules: Refresh details about L7 matching for tcp-request content rules
- BUG/MEDIUM: queue: make pendconn_cond_unlink() really thread-safe
- MINOR: counters: fix a typo in comment
- BUG/MINOR: stats: fix validity of the json schema
- MINOR: hlua: Display debug messages on stderr only in debug mode
- BUG/MINOR: peers: Inconsistency when dumping peer status codes.
- BUG/MINOR: mux-h1: Always set the session on frontend h1 stream
- BUG/MEDIUM: mux-fcgi: Don't handle pending read0 too early on streams
- BUG/MEDIUM: mux-h2: Don't handle pending read0 too early on streams
- BUG/MINOR: http-htx: Expect no body for 204/304 internal HTTP responses
- BUG/MINOR: init: only keep rlim_fd_cur if max is unlimited
- BUG/MINOR: mux-h2: do not stop outgoing connections on stopping
- MINOR: fd: report an error message when failing initial allocations
- BUG/MEDIUM: task: bound the number of tasks picked from the wait queue at once
- BUG/MEDIUM: spoe: Unset variable instead of set it if no data provided
- BUG/MEDIUM: mux-h1: Get the session from the H1S when capturing bad messages
- BUG/MEDIUM: lb: Always lock the server when calling server_{take,drop}_conn
- BUG/MINOR: peers: Possible unexpected peer seesion reset after collisions.
- BUILD: ssl: make BoringSSL use its own version numbers
- BUG/MINOR: disable dynamic OCSP load with BoringSSL
- BUG/MEDIUM: ssl: OCSP must work with BoringSSL
- BUG/MINOR: queue: properly report redistributed connections
- BUG/MEDIUM: server: support changing the slowstart value from state-file
- BUG/MINOR: http-ana: Don't send payload for internal responses to HEAD requests
- BUG/MAJOR: mux-h2: Don't try to send data if we know it is no longer possible
- BUG/MINOR: extcheck: add missing checks on extchk_setenv()
- BUG/MINOR: log: fix memory leak on logsrv parse error
- BUG/MINOR: server: fix srv downtime calcul on starting
- BUG/MINOR: server: fix down_time report for stats
- BUG/MINOR: lua: initialize sample before using it
- MINOR: ist: Add a case insensitive istmatch function
- BUG/MINOR: cache: Manage multiple values in cache-control header value
- BUG/MINOR: cache: Inverted variables in http_calc_maxage function
- BUG/MEDIUM: filters: Don't try to init filters for disabled proxies
- BUG/MINOR: proxy/server: Skip per-proxy/server post-check for disabled proxies
- BUG/MINOR: server: Set server without addr but with dns in RMAINT on startup
- MINOR: server: Copy configuration file and line for server templates
- BUG/MEDIUM: mux-pt: Release the tasklet during an HTTP upgrade
- BUG/MINOR: filters: Skip disabled proxies during startup only
- BUG/MEDIUM: stick-table: limit the time spent purging old entries
- CLEANUP: mux-h2: Remove the h1 parser state from the h2 stream
Christopher Faulet [Tue, 3 Nov 2020 17:25:52 +0000 (18:25 +0100)]
CLEANUP: mux-h2: Remove the h1 parser state from the h2 stream
Since the h2 multiplexer no longer relies on the legacy HTTP representation, and
uses exclusively the HTX, the H1 parser state (h1m) is no longer used by the h2
streams. Thus it can be removed.
This patch may be backported as far as 2.1.
(cherry picked from commit
fafd1b0a5b3b6ee0c7d7dd288629c992fc63ef10)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
aa3c7001cb32cd9c5bb7b5258459bb971e956438)
[wt: include file is in common/h1.h in 2.1]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Tue, 3 Nov 2020 16:47:41 +0000 (17:47 +0100)]
BUG/MEDIUM: stick-table: limit the time spent purging old entries
An interesting case was reported with threads and moderately sized
stick-tables. Sometimes the watchdog would trigger during the purge.
It turns out that the stick tables were sized in the 10s of K entries
which is the order of magnitude of the possible number of connections,
and that threads were used over distinct NUMA nodes. While at first
glance nothing looks problematic there, actually there is a risk that
a thread trying to purge the table faces 100% of entries still in use
by a connection with (ts->ref_cnt > 0), and ends up scanning the whole
table, while other threads on the other NUMA node are causing the
cache lines to bounce back and forth and considerably slow down its
progress to the point of possibly spending hundreds of milliseconds
there, multiplied by the number of queued threads all failing on the
same point.
Interestingly, smaller tables would not trigger it because the scan
would be faster, and larger ones would not trigger it because plenty
of entries would be idle!
The most efficient solution is to increase the table size to be large
enough for this never to happen, but this is not reliable. We could
have a parallel list of idle entries but that would significantly
increase the storage and processing cost only to improve a few rare
corner cases.
This patch takes a more pragmatic approach, it considers that it will
not visit more than twice the number of nodes to be deleted, which
means that it accepts to fail up to 50% of the time. Given that very
small batches are programmed each time (1/256 of the table size), this
means the operation will finish quickly (128 times faster than now),
and will reduce the inter-thread contention. If this needs to be
reconsidered, it will probably mean that the batch size needs to be
fixed differently.
This needs to be backported to stable releases which extensively use
threads, typically 2.0.
Kudos to Nenad Merdanovic for figuring the root cause triggering this!
(cherry picked from commit
dfe79251dab0346fe4c71e7e3da7bb87d41c880c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Tue, 3 Nov 2020 15:40:37 +0000 (16:40 +0100)]
BUG/MINOR: filters: Skip disabled proxies during startup only
This partially reverts the patch
400829cd2 ("BUG/MEDIUM: filters: Don't try to
init filters for disabled proxies"). Disabled proxies must not be skipped in
flt_deinit() and flt_deinit_all_per_thread() when HAProxy is stopped because,
obvioulsy, at this step, all proxies appear as disabled (or stopped, it is the
same state). It is safe to do so because, during startup, filters declared on
disabled proxies are removed. Thus they don't exist anymore during shutdown.
This patch must be backported in all versions where the patch above is.
(cherry picked from commit
743bd6adc8a7657b324ac59376311d23d65bb4ed)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
59e7ab350d06542493d4da0467da8738c406dde8)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Tue, 3 Nov 2020 08:11:43 +0000 (09:11 +0100)]
BUG/MEDIUM: mux-pt: Release the tasklet during an HTTP upgrade
When a TCP connection is upgraded to HTTP, the passthrough multiplexer owning
the client connection is detroyed and replaced by an HTTP multiplexer. When it
happens, the connection context is changed (it is in fact the mux itself). Thus,
when the mux-pt is destroyed, the connection is not released. But, only the
connection must be kept. Everything else concerning the mux must be
released. Especially, the tasklet used for I/O subscriptions. In this part,
there was a bug and the tasklet was never released.
This patch should fix the issue #935. It must be backported as far as 2.0.
(cherry picked from commit
5a7ca29061dbc5736b53d45b9561e71807a0d05a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
827b4fc10101847da0953a556120884f96c38aa2)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Mon, 2 Nov 2020 21:04:55 +0000 (22:04 +0100)]
MINOR: server: Copy configuration file and line for server templates
When servers based on server templates are initialized, the configuration file
and line are now copied. This helps to emit understandable warning and alert
messages.
This patch may be backported if needed, as far as 1.8.
(cherry picked from commit
75bef00538eebc00c22309c7f9391c0f6d502ac0)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
97ae0cf4b20b50fa668e641126c6a65b587359d9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Mon, 26 Oct 2020 09:31:17 +0000 (10:31 +0100)]
BUG/MINOR: server: Set server without addr but with dns in RMAINT on startup
On startup, if a server has no address but the dns resolutions are configured,
"none" method is added to the default init-addr methods, in addition to "last"
and "libc". Thus on startup, this server is set to RMAINT mode if no address is
found. It is only performed if no other init-addr method is configured.
Setting the RMAINT mode on startup is important to inhibit the health checks.
For instance, following servers will now be set to RMAINT mode on startup :
server srv nofound.tld:80 check resolvers mydns
server srv _http._tcp.service.local check resolvers mydns
server-template srv 1-3 _http._tcp.service.local check resolvers mydns
while followings ones will trigger an error :
server srv nofound.tld:80 check
server srv nofound.tld:80 check resolvers mydns init-addr libc
server srv _http._tcp.service.local check
server srv _http._tcp.service.local check resolvers mydns init-addr libc
server-template srv 1-3 _http._tcp.service.local check resolvers mydns init-addr libc
This patch must be backported as far as 1.8.
(cherry picked from commit
ac1c60fd9c63dd9ac4bab7e9a9f24fb347a5fbdf)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
13704ca2d0de919935c91246ce582d7f22160051)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Mon, 2 Nov 2020 15:20:13 +0000 (16:20 +0100)]
BUG/MINOR: proxy/server: Skip per-proxy/server post-check for disabled proxies
per-proxy and per-server post-check callback functions must be skipped for
disabled proxies because most of the configuration validity check is skipped for
these proxies.
This patch must be backported as far as 2.1.
(cherry picked from commit
d5bd824b81cda4c02e967b05af920f455d4b3712)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
eb20100fe1be0f2641de331ddcc8b1246cd4db2b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Mon, 2 Nov 2020 15:08:09 +0000 (16:08 +0100)]
BUG/MEDIUM: filters: Don't try to init filters for disabled proxies
Configuration is parsed for such proxies but not validated. Concretely, it means
check_config_validity() function does almost nothing for such proxies. Thus, we
must be careful to not initialize filters for disabled proxies because the check
callback function is not called. In fact, to be sure to avoid any trouble,
filters for disabled proxies are released.
This patch fixes a segfault at startup if the SPOE is configured for a disabled
proxy. It must be backported as far as 1.7 (maybe with some adaptations).
(cherry picked from commit
400829cd2c6d734dbdd073d7a30d1e258fd2d1a0)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
9dd9e01e0d63bff1cb5592237db66cd22c04cf24)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Remi Tricot-Le Breton [Fri, 30 Oct 2020 13:26:13 +0000 (14:26 +0100)]
BUG/MINOR: cache: Inverted variables in http_calc_maxage function
The maxage and smaxage variables were inadvertently assigned the
Cache-Control s-maxage and max-age values respectively when it should
have been the other way around.
This can be backported on all branches after 1.8 (included).
(cherry picked from commit
8c2db71326addbcf9d6c8ae4d2048247031f9e4c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
2185f7e2b809889310efdf087da6777583607597)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Remi Tricot-Le Breton [Wed, 28 Oct 2020 10:35:15 +0000 (11:35 +0100)]
BUG/MINOR: cache: Manage multiple values in cache-control header value
If an HTTP request or response had a "Cache-Control" header that had
multiple comma-separated subparts in its value (like "max-age=1,
no-store" for instance), we did not process the values correctly and
only parsed the first one. That made us store some HTTP responses in the
cache when they were explicitely uncacheable.
This patch replaces the way the values are parsed by an http_find_header
loop that manages every sub part of the value independently.
This patch should be backported to 2.2 and 2.1. The bug also exists on
previous versions but since the sources changed, a new commit will have
to be created.
[wla: This patch requires bb4582c ("MINOR: ist: Add a case insensitive
istmatch function"). Backporting for < 2.1 is not a requirement since it
works well enough for most cases, it was a known limitation of the
implementation of non-htx version too]
(cherry picked from commit
40ed97b04b930c76c77c09c0cafc8781f94b66ee)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
23b37d0aa491a8c1bdd1a8bc204f7e4d86a25ac3)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Remi Tricot-Le Breton [Wed, 28 Oct 2020 10:35:14 +0000 (11:35 +0100)]
MINOR: ist: Add a case insensitive istmatch function
Add a helper function that checks if a string starts with another string
while ignoring case.
(cherry picked from commit
bb4582cf719bcf9facf4e9b459fd3a68a97fffbd)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
fce0402a3bebcfae35ce2f7bff00b2e24174d89e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Amaury Denoyelle [Thu, 29 Oct 2020 16:21:20 +0000 (17:21 +0100)]
BUG/MINOR: lua: initialize sample before using it
Memset the sample before using it through hlua_lua2smp. This function is
ORing the smp.flags, so this field need to be cleared before its use.
This was reported by a coverity warning.
Fixes the github issue #929.
This bug can be backported up to 1.8.
(cherry picked from commit
bc0af6a19947c7ab76bd226beaaaafedc0648be4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
42cd304165f9afa2050f169e7c852f2f637bcc70)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Amaury Denoyelle [Thu, 29 Oct 2020 14:59:05 +0000 (15:59 +0100)]
BUG/MINOR: server: fix down_time report for stats
Adjust condition used to report down_time for statistics. There was a
tiny probabilty to have a negative downtime if last_change was superior
to now. If this is the case, return only down_time.
This bug can backported up to 1.8.
(cherry picked from commit
e6ba7915ebdfb79d1d74eb402bbc2747c4a1c6dd)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
36f3e9bb61a06c1afce27474bbc357b52590ec96)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Amaury Denoyelle [Thu, 29 Oct 2020 14:59:04 +0000 (15:59 +0100)]
BUG/MINOR: server: fix srv downtime calcul on starting
When a server is up after a failure, its downtime was reset to 0 on the
statistics. This is due to a wrong condition that causes srv.down_time
to never be set. Fix this by updating down_time each time the server is in
STARTING state.
Fixes the github issue #920.
This bug can be backported up to 1.8.
(cherry picked from commit
fe2bf091f6f2c155b5d7b1372a43b73bb034002f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
774d5a3b69bf8c588c26ce09a6acbf29997a82ac)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Tue, 27 Oct 2020 08:51:37 +0000 (09:51 +0100)]
BUG/MINOR: log: fix memory leak on logsrv parse error
In case of parsing error on logsrv, we can leave parse_logsrv() without
releasing logsrv->ring_name or smp_rgs. Let's free them on the error path.
This should fix issue #926 detected by Coverity.
The impact is only a tiny leak just before reporting a fatal error, so it
will essentially annoy valgrind.
This can be backported to 2.0 (just drop the ring part).
(cherry picked from commit
ae32ac74dbc2cb93a07d7d29b4c20a075bc7741a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
ad6e92761d877d872414ccbcf408d5f5f92c3b91)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Sat, 24 Oct 2020 11:07:39 +0000 (13:07 +0200)]
BUG/MINOR: extcheck: add missing checks on extchk_setenv()
Issue #910 reports that we fail to check a few extchk_setenv() in the
child process. These are mostly harmless, but instead of counting on
the external check script to fail the dirty way, better fail cleanly
when detecting the failure.
This could probably be backported to all stable branches.
(cherry picked from commit
b3250a268b66ec686142f01ef04bef9e3db58532)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
f1c1481296da08a3ac7d4dcbd8ec7e035b89fbf8)
[cf: changes applied in src/checks.c because src/extcheck.c does not exist]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Thu, 22 Oct 2020 14:24:58 +0000 (16:24 +0200)]
BUG/MAJOR: mux-h2: Don't try to send data if we know it is no longer possible
In h2_send(), if we are in a state where we know it is no longer possible to
send data, we must exit the sending loop to avoid any possiblity to loop
forever. It may happen if the mbuf ring is released while the H2_CF_MUX_MFULL
flag is still set. Here is a possible scenario to trigger the bug :
1) The mbuf ring is full because we are unable to send data. The
H2_CF_MUX_MFULL flag is set on the H2 connection.
2) At this stage, the task timeout expires because the H2 connection is
blocked. We enter in h2_timeout_task() function. Because the mbuf ring is
full, we cannot send the GOAWAY frame. Thus the H2_CF_GOAWAY_FAILED flag is
set. The H2 connection is not released yet because there is still a stream
attached. Here we leave h2_timeout_task() function.
3) A bit later, the H2 connection is woken up. If h2_process(), nothing is
performed by the first attempt to send data, in h2_send(). Then, because
the H2_CF_GOAWAY_FAILED flag is set, the mbuf ring is released. But the
H2_CF_MUX_MFULL flag is still there. At this step a second attempt to send
data is performed.
4) In h2_send(), we try to send data in a loop. To exist this loop, done
variable must be set to 1. Because the H2_CF_MUX_MFULL flag is set, we
don't call h2_process_mux() and done is not updated. Because the mbuf ring
is now empty, nothing is sent and the H2_CF_MUX_MFULL flag is never
removed. Now, we loop forever... waiting for the watchdog.
To fix the bug, we now exit the loop if one of these conditions is true :
- The H2_CF_GOAWAY_FAILED flag is set on the H2 connection
- The CO_FL_SOCK_WR_SH flag is set on the underlying connection
- The H2 connection is in the H2_CS_ERROR2 state
This patch should fix the issue #912 and most probably #875. It must be
backported as far as the 1.8.
(cherry picked from commit
9a3d3fcb5d980bd1034e04222769452cc68ecc9b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
180a616caf65ba398a1c74e07e9b26196bd94fb3)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Mon, 19 Oct 2020 16:01:38 +0000 (18:01 +0200)]
BUG/MINOR: http-ana: Don't send payload for internal responses to HEAD requests
When an internal response is returned to a client, the message payload must be
skipped if it is a reply to a HEAD request. The payload is removed from the HTX
message just before the message forwarding.
This bugs has been around for a long time. It was already there in the pre-HTX
versions. In legacy HTTP mode, internal errors are not parsed. So this bug
cannot be easily fixed. Thus, this patch should only be backported in all HTX
versions, as far as 2.0. However, the code has significantly changed in the
2.2. Thus in the 2.1 and 2.0, the patch must be entirely reworked.
(cherry picked from commit
d6c48366b81e4f95676b038db32d23cdc541735e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
0471ef6514cc9f07e1096b878f3e47baf3692d08)
[cf: adapted for the 2.1, as expected. ]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Thu, 22 Oct 2020 09:30:59 +0000 (11:30 +0200)]
BUG/MEDIUM: server: support changing the slowstart value from state-file
If the slowstart value in a state file implies the latest state change
is within the slowstart period, we end up calling srv_update_status()
to reschedule the server's state change but its task is not yet
allocated and remains null, causing a crash on startup.
Make sure srv_update_status() supports being called with partially
initialized servers which do not yet have a task. If the task has to
be scheduled, it will necessarily happen after initialization since
it will result from a state change.
This should be backported wherever server-state is present.
(cherry picked from commit
1e690bb6c49590cd3b4fae11c2f85f37e958635e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
550a2f16abb0ef88e75b30c85319a989192de7cd)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Wed, 21 Oct 2020 09:54:38 +0000 (11:54 +0200)]
BUG/MINOR: queue: properly report redistributed connections
In commit
5cd4bbd7a ("BUG/MAJOR: threads/queue: Fix thread-safety issues
on the queues management") the counter of transferred connections was
accidently lost, so that when a server goes down with connections in its
queue, it will always be reported that 0 connection were transferred.
This should be backported as far as 1.8 since the patch above was
backported there.
(cherry picked from commit
ef71f0194cfdaa9c201bece65e8c1ab3d16ed8a0)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
e6985c91c3dd1100950987a5bf98eaeb863c97b8)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Emmanuel Hocdet [Mon, 26 Oct 2020 12:55:30 +0000 (13:55 +0100)]
BUG/MEDIUM: ssl: OCSP must work with BoringSSL
It's a regression from
b3201a3e "BUG/MINOR: disable dynamic OCSP load
with BoringSSL". The origin bug is link to 76b4a12 "BUG/MEDIUM: ssl:
memory leak of ocsp data at SSL_CTX_free()": ssl_sock_free_ocsp()
shoud be in #ifndef OPENSSL_IS_BORINGSSL.
To avoid long #ifdef for small code, the BoringSSL part for ocsp load
is isolated in a simple #ifdef.
This must be backported in 2.2 and 2.1
(cherry picked from commit
a73a222a9863e5f6763786845c1ff9e7e1038c3c)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
(cherry picked from commit
f495e5d6a597e2e1caa965e963ef16103da545db)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
Ilya Shipitsin [Sun, 18 Oct 2020 04:11:50 +0000 (09:11 +0500)]
BUG/MINOR: disable dynamic OCSP load with BoringSSL
it was accidently enabled on BoringSSL while
actually it is not supported
wla: Fix part of the issue mentionned in #895.
It fixes build of boringSSL versions prior to commit
https://boringssl.googlesource.com/boringssl/+/
49e9f67d8b7cbeb3953b5548ad1009d15947a523
Could be backported in 2.2, 2.1, 2.0, 1.8. Where the patch fcb69d7
("BUILD: ssl: make BoringSSL use its own version numbers") is also
relevant.
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
(cherry picked from commit
b3201a3e077198b3f75ebe8661aa45589b811552)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
Ilya Shipitsin [Sun, 18 Oct 2020 03:55:39 +0000 (08:55 +0500)]
BUILD: ssl: make BoringSSL use its own version numbers
BoringSSL is a fork of OpenSSL 1.1.0, however in
49e9f67d8b7cbeb3953b5548ad1009d15947a523 it has changed version to 1.1.1.
Should fix issue #895.
This must be backported to 2.2, 2.1, 2.0, 1.8
(cherry picked from commit
fcb69d768bc5fc3bde98131ae27d7f8de847ca57)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
(cherry picked from commit
2da43ac33319e05a30cbef5a9fd757c615b173ed)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
Frédéric Lécaille [Wed, 14 Oct 2020 09:50:26 +0000 (11:50 +0200)]
BUG/MINOR: peers: Possible unexpected peer seesion reset after collisions.
During a peers session collision (two peer sessions opened on both side) we must
mark the peer the session of which will be shutdown as alive, if not ->reconnect
timer will be set with a wrong value if the synchro task expires after the peer
has been reconnected. This possibly leads to unexpected deconnections during handshakes.
Furthermore, this patch cancels any heartbeat tranmimission when a reconnection
is prepared.
(cherry picked from commit
6e1a9c57c938360e95f445e800f0d22782bb6c22)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 16 Oct 2020 14:27:17 +0000 (16:27 +0200)]
BUG/MEDIUM: lb: Always lock the server when calling server_{take,drop}_conn
The server lock must be held when server_take_conn() and server_drop_conn()
lbprm callback functions are called. It is a documented prerequisite but it is
not always performed. It only affects leastconn and fas lb algorithm. Others
don't use these callback functions.
A race condition on the next pending effecive weight (next_eweight) may be
encountered with the leastconn lb algorithm. An agent check may set it to 0
while fwlc_srv_reposition() is called. The server is locked during the
next_eweight update. But because the server lock is not acquired when
fwlc_srv_reposition() is called, we may use it to recompute the server key,
leading to a division by 0.
This patch must be backported as far as 1.8.
(cherry picked from commit
26a52af642c3fcfd6a637aef019b78147c05e126)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
e50dd9f3dd622937e0b7bb4cbe7067c50e9a1f13)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Thu, 15 Oct 2020 15:19:46 +0000 (17:19 +0200)]
BUG/MEDIUM: mux-h1: Get the session from the H1S when capturing bad messages
It is not guaranteed that the backend connection has an owner. It is set when
the connection is created. But when the connection is moved in a server idle
list, the connection owner is set to NULL and may never be set again. On the
other hand, when a mux is created or when a CS is attached, the session is
always defined. The H1 stream always keep a reference on it when it is
created. Thus, when a bad message is captured we should not rely on the
connection owner to retrieve the session. Instead we should get it from the H1
stream.
(cherry picked from commit
db2c17da60e80091615ddbe8d97cb964d1f00ac7)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
9d99d5d06863c9ecb1d59de9f3964c8e3b35c298)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Thu, 15 Oct 2020 14:08:30 +0000 (16:08 +0200)]
BUG/MEDIUM: spoe: Unset variable instead of set it if no data provided
If an agent try to set a variable with the NULL data type, an unset is perform
instead to avoid undefined behaviors. Once decoded, such data are translated to
a sample with the type SMP_T_ANY. It is unexpected in HAProxy. When a variable
is set with such sample, no data are attached to the variable. Thus, when the
variable is retrieved later in the transaction, the sample data are
uninitialized, leading to undefined behaviors depending on how it is used. For
instance, it leads to a crash if the debug converter is used on such variable.
This patch should fix the issue #855. It must be backported as far as 1.8.
(cherry picked from commit
2469eba20fdc01f8ca95726a8c11feaaa8825027)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
dee726c36d3ee620f6564f1b6e53930c1f97481c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Fri, 16 Oct 2020 07:26:22 +0000 (09:26 +0200)]
BUG/MEDIUM: task: bound the number of tasks picked from the wait queue at once
There is a theorical problem in the wait queue, which is that with many
threads, one could spend a lot of time looping on the newly expired tasks,
causing a lot of contention on the global wq_lock and on the global
rq_lock. This initially sounds bening, but if another thread does just
a task_schedule() or task_queue(), it might end up waiting for a long
time on this lock, and this wait time will count on its execution budget,
degrading the end user's experience and possibly risking to trigger the
watchdog if that lasts too long.
The simplest (and backportable) solution here consists in bounding the
number of expired tasks that may be picked from the global wait queue at
once by a thread, given that all other ones will do it as well anyway.
We don't need to pick more than global.tune.runqueue_depth tasks at once
as we won't process more, so this counter is updated for both the local
and the global queues: threads with more local expired tasks will pick
less global tasks and conversely, keeping the load balanced between all
threads. This will guarantee a much lower latency if/when wakeup storms
happen (e.g. hundreds of thousands of synchronized health checks).
Note that some crashes have been witnessed with 1/4 of the threads in
wake_expired_tasks() and, while the issue might or might not be related,
not having reasonable bounds here definitely justifies why we can spend
so much time there.
This patch should be backported, probably as far as 2.0 (maybe with
some adaptations).
(cherry picked from commit
3cfaa8d1e075b5037b15775ec6b81ef16c4d81ce)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
563eede2418274f28a72a926362b11e21341c32f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Tue, 13 Oct 2020 13:45:07 +0000 (15:45 +0200)]
MINOR: fd: report an error message when failing initial allocations
When starting with a huge maxconn (say 1 billion), the only error seen
is "No polling mechanism available". This doesn't help at all to resolve
the problem. Let's add specific alerts for the failed mallocs. Now we can
get this instead:
[ALERT] 286/154439 (23408) : Not enough memory to allocate
2000000033 entries for fdtab!
This may be backported as far as 2.0 as it helps debugging bad configurations.
(cherry picked from commit
7c9f756dcc51b9974d5e81116e1d8a34a6152040)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
2e0c88d6418bd90877a30ef5db414fe412f5a1b6)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Tue, 13 Oct 2020 16:09:15 +0000 (18:09 +0200)]
BUG/MINOR: mux-h2: do not stop outgoing connections on stopping
There are reports of a few "SC" in logs during reloads when H2 is used
on the backend side. Christopher analysed this as being caused by the
proxy disabled test in h2_process(). As the comment says, this was done
for frontends only, and must absolutely not send a GOAWAY to the backend,
as all it will result in is to make newly queued streams fail.
The fix consists in simply testing the connection side before deciding
to send the GOAWAY.
This may be backported as far as 2.0, though for whatever reason it seems
to manifest itself only since 2.2 (probably due to changes in the outgoing
connection setup sequence).
(cherry picked from commit
b1e600c9c5e4a20a6de1f3b9593f20dc50f01315)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
403168c3787cc7098fe31640089bd508d3448115)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Tue, 13 Oct 2020 13:36:08 +0000 (15:36 +0200)]
BUG/MINOR: init: only keep rlim_fd_cur if max is unlimited
On some operating systems, RLIM_INFINITY is set to -1 so that when the
hard limit on the number of FDs is set to unlimited, taking the MAX
of both values keeps rlim_fd_cur and everything works. But on other
systems this values is defined as the highest positive integer. This
is what was observed on a 32-bit AIX 5.1. The effect is that maxsock
becomes 2^31-1 and that fdtab allocation fails.
Note that a simple workaround consists in manually setting maxconn in
the global section.
Let's ignore unlimited as soon as we retrieve rlim_fd_max so that all
systems behave consistently.
This may be backported as far as 2.0, though it doesn't seem like it
has annoyed anyone.
(cherry picked from commit
2bd0f8147b0682ec962f59a5c38f03314f43a4f5)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
bd2d932842afa5af41672f7e66483714537fcb3a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 9 Oct 2020 06:50:26 +0000 (08:50 +0200)]
BUG/MINOR: http-htx: Expect no body for 204/304 internal HTTP responses
204 and 304 HTTP responses must no contain message body. These status codes are
correctly handled when the responses are received from a server. But there is no
specific processing for internal HTTP reponses (errorfile and http replies).
Now, when errorfiles or an http replies are parsed during the configuration
parsing, an error is triggered if a 204/304 message contains a body. An extra
check is also performed to ensure the body length matches the announce
content-length.
This patch should fix the issue #891. It must be backported as far as 2.0. For
2.1 and 2.0, only the http_str_to_htx() function must be fixed.
http_parse_http_reply() function does not exist.
(cherry picked from commit
b8d148a93f0daff9821cb9ecc5934499f8d128a2)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
dde6c42da611badbf515c687a69c457aa8ecc40b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Thu, 8 Oct 2020 13:38:41 +0000 (15:38 +0200)]
BUG/MEDIUM: mux-h2: Don't handle pending read0 too early on streams
This patch is similar to the previous one on the fcgi. Same is true for the
H2. But the bug is far harder to trigger because of the protocol cinematic. But
it may explain strange aborts in some edge cases.
A read0 received on the connection must not be handled too early by H2 streams.
If the demux buffer is not empty, the pending read0 must not be considered. The
H2 streams must not be passed in half-closed remote state in
h2s_wake_one_stream() and the CS_FL_EOS flag must not be set on the associated
conn-stream in h2_rcv_buf(). To sum up, it means, if there are still data
pending in the demux buffer, no abort must be reported to the streams.
To fix the issue, a dedicated function has been added, responsible for detecting
pending read0 for a H2 connection. A read0 is reported only if the demux buffer
is empty. This function is used instead of conn_xprt_read0_pending() at some
places.
Note that the HREM stream state should not be used to report aborts. It is
performed on h2s_wake_one_stream() function and it is a legacy of the very first
versions of the mux-h2.
This patch should be backported as far as 2.0. In the 1.8, the code is too
different to apply it like that. But it is probably useless because the mux-h2
can only be installed on the client side.
(cherry picked from commit
aade4edc1a8d148447953e279c6a83bf3dea6661)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
36840d8ca8e298b4da4bd24d0f6128b46e0757e3)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Thu, 8 Oct 2020 13:26:33 +0000 (15:26 +0200)]
BUG/MEDIUM: mux-fcgi: Don't handle pending read0 too early on streams
A read0 received on the connection must not be handled too early by FCGI
streams. If the demux buffer is not empty, the pending read0 must not be
considered. The FCGI streams must not be passed in half-closed remote state in
fcgi_strm_wake_one_stream() and the CS_FL_EOS flag must not be set on the
associated conn-stream in fcgi_rcv_buf(). To sum up, it means, if there are
still data pending in the demux buffer, no abort must be reported to the
streams.
To fix the issue, a dedicated function has been added, responsible for detecting
pending read0 for a FCGI connection. A read0 is reported only if the demux
buffer is empty. This function is used instead of conn_xprt_read0_pending() at
some places.
This patch should fix the issue #886. It must be backported as far as 2.1.
(cherry picked from commit
6670e3e2bf64d4273c164cd5a70bb9acde2820b2)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
071c3f1d68c6b1b97e5cc93a6793d4106618f9f9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Wed, 30 Sep 2020 13:00:13 +0000 (15:00 +0200)]
BUG/MINOR: mux-h1: Always set the session on frontend h1 stream
The session is always defined for a frontend connection. When a new client
connection is established, the session is set for the first H1 stream. But on
keep-alived connections, it is not set for the followings H1 streams while it is
possible.
This patch is tagged as a bug because it fixes an inconsistency in the H1
streams creation. But it does not fixed a known bug.
This patch must be backported as far as 2.0.
(cherry picked from commit
e9da975aabb0f2137bde7afbd020b300029ef28f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
595a3f3b14ca1e5cae02d3e8530c88dc4bbbe6e4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Frédéric Lécaille [Mon, 5 Oct 2020 10:33:07 +0000 (12:33 +0200)]
BUG/MINOR: peers: Inconsistency when dumping peer status codes.
A peer connection status must be considered as valid only if there is an applet
which has been instantiated for the connection to the peer. So, ->statuscode
should be considered as the last known peer connection status from the last
connection to this peer if any. To reflect this, "statuscode" field of peer dump
is renamed to "last_statuscode".
This patch also add "active"/"inactive" field after the peer location type
("remote" or "local") if an applet has been instantiated for this peer connection
or not.
Thank you to Emeric for having noticed this issue.
Must be backported in >=1.9 version.
(cherry picked from commit
e7e2b21d274ef48da4e45d09786568d2239c9b2a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
186bee02a9722e22f0b95f150869ae8e1c3794a9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 2 Oct 2020 16:13:52 +0000 (18:13 +0200)]
MINOR: hlua: Display debug messages on stderr only in debug mode
Debug Messages emitted in lua using core.Debug() or core.log() are now only
displayed on stderr if HAProxy is started in debug mode (-d parameter on the
command line). There is no change for other message levels.
This patch should fix the issue #879. It may be backported to all stable
versions.
(cherry picked from commit
f98d821b942b4f392e77431c320aa895ca97369e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
604a647d4d7ff310d0ed1e9f98531b6a0c9e332e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Amaury Denoyelle [Fri, 2 Oct 2020 16:31:59 +0000 (18:31 +0200)]
BUG/MINOR: stats: fix validity of the json schema
The json schema seems to be invalid when checking using the validator
from https://www.jsonschemavalidator.net/. Correct it using the
following specification :
http://json-schema.org/draft/2019-09/json-schema-validation.html#rfc.section.9.1
The impact of the bug it not well known as I am not sure of how useful
the json schema is for users. It is probably not used at all or else
this bug would have been reported.
This should be backported up to 1.8.
(cherry picked from commit
a53ce4cc011250a0e561f4e5bb62f46719813534)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
a9299e1f56f65ce89d45690a2356d5f59eed6bdd)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Amaury Denoyelle [Fri, 2 Oct 2020 16:31:58 +0000 (18:31 +0200)]
MINOR: counters: fix a typo in comment
Wrong copy/paste comment, replace listeners/frontends by
servers/backends
This may be backported up to 1.7.
(cherry picked from commit
cd3de507794b4f063aa1e7f43b55b27998d7ee7d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
dc0b2880d87458ceafedbbeb5051ece149adfa6e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Fri, 2 Oct 2020 15:52:49 +0000 (17:52 +0200)]
BUG/MEDIUM: queue: make pendconn_cond_unlink() really thread-safe
A crash reported in github issue #880 looks impossible unless
pendconn_cond_unlink() occasionally sees a null leaf_p when attempting
to remove an entry, which seems to be confirmed by the reporter. What
seems to be happening is that depending on compiler optimizations,
this pointer can appear as null while pointers are moved if one of
the node's parents is removed from or inserted into the tree. There's
no explicit null of the pointer during these operations but those
pointers are rewritten in multiple steps and nothing prevents this
situation from happening, and there are no particular barrier nor
atomic ops around this.
This test was used to avoid unnecessary locking, for already deleted
entries, but looking at the code it appears that pendconn_free() already
resets s->pend_pos that's used as <p> there, and that the other call
reasons are after an error where the connection will be dropped as
well. So we don't save anything by doing this test, and make it
unsafe. The older code used to check for list emptiness there and
not inside pendconn_unlink(), which explains why the code has stayed
there. Let's just remove this now.
Thanks to @jaroslawr for reporting this issue in great details and for
testing the proposed fix.
This should be backpored to 1.8, where the test on LIST_ISEMPTY should
be moved to pendconn_unlink() instead (inside the lock, just like 2.0+).
(cherry picked from commit
fac0f645dff9fb1854db32da9fa7c317eaaeede1)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
3376d9f52e047214061aeedaf117a2006ce60d70)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 2 Oct 2020 09:38:46 +0000 (11:38 +0200)]
DOC: tcp-rules: Refresh details about L7 matching for tcp-request content rules
Because the parsing of HTTP message is now performed in the HTTP multiplexers,
the content is immediatly available when "tcp-request content" rules are
evaluated for an HTTP frontend. So, it is a good idea to make the documentation
explicit on this point. In addition, because in all cases, the parsing is
already performed, there is no reason to still use "tcp-request content" rules
based on L7 matching, although it is still valid. The recommended way is to use
"http-request" rules instead. Again, it is a good idea to update the
documentation on this point.
(cherry picked from commit
7ea509e15f6fe3390a969f6eec29d76b8732d3aa)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
e821fbde4a7b75ad52fb98a1ea3d77b3ff483054)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Eric Salama [Fri, 2 Oct 2020 09:58:19 +0000 (11:58 +0200)]
BUG/MINOR: Fix several leaks of 'log_tag' in init().
We use chunk_initstr() to store the program name as the default log-tag.
If we use the log-tag directive in the config file, this chunk will be
destroyed and replaced. chunk_initstr() sets the chunk size to 0 so we
will free the chunk itself, but not its content.
This happens for a global section and also for a proxy.
We fix this by using chunk_initlen() instead of chunk_initstr().
We also check that the memory allocation was successfull, otherwise we quit.
This fixes github issue #850.
It can be backported as far as 1.9, with minor adjustments to includes.
(cherry picked from commit
7cea6065aca04c91fc5109e581f124a46b2b5242)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
7566bcfa5a1d28e8c9d5f9ca1dfe0ebe791052eb)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Brad Smith [Wed, 30 Sep 2020 05:04:48 +0000 (01:04 -0400)]
BUILD: makefile: Fix building with closefrom() support enabled
I noticed the USE_CLOSEFROM define was not being passed along like the rest
during the build.
Looking around I see this was broken with the following two commits and related
series..
BUILD: Makefile: also report disabled options in the BUILD_OPTIONS variable
http://git.haproxy.org/?p=haproxy.git;a=commit;h=
05fd82da76d1bbc8d65d63ab246bda7cbcf8481a
BUILD: pass all "USE_*" variables as -DUSE_* to the compiler
http://git.haproxy.org/?p=haproxy.git;a=commit;h=
824cd00d3bda8f7f6d4c30baf77ba6c19ab47811
Looks like this should be back ported to 2.0, 2.1 and 2.2.
(cherry picked from commit
5018aacae5f2fb65365bce5a3a390c5126203952)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
4aafadb4d9a47e0cb3cc524d9306c35bd64ccd75)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Thu, 5 Mar 2020 09:17:47 +0000 (10:17 +0100)]
MINOR: ssl: reach a ckch_store from a sni_ctx
It was only possible to go down from the ckch_store to the sni_ctx but
not to go up from the sni_ctx to the ckch_store.
To allow that, 2 pointers were added:
- a ckch_inst pointer in the struct sni_ctx
- a ckckh_store pointer in the struct ckch_inst
(cherry picked from commit
cfca1422c7de05e34b72d2d57bc057b215aa9f8f)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
William Lallemand [Mon, 17 Aug 2020 12:31:19 +0000 (14:31 +0200)]
BUG/MEDIUM: ssl: crt-list negative filters don't work
The negative filters which are supposed to exclude a SNI from a
wildcard, never worked. Indeed the negative filters were skipped in the
code.
To fix the issue, this patch looks for negative filters that are on the
same line as a the wildcard that just matched.
This patch should fix issue #818. It must be backported in 2.2. The
problem also exists in versions > 1.8 but the infrastructure required to
fix this was only introduced in 2.1. In older versions we should
probably change the documentation to state that negative filters are
useless.
(cherry picked from commit
30f9e095f5ed695348f3041ef183f25739c2ace1)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
Willy Tarreau [Wed, 30 Sep 2020 05:20:26 +0000 (07:20 +0200)]
[RELEASE] Released version 2.1.9
Released version 2.1.9 with the following main changes :
- BUG/MEDIUM: mux-h1: Refresh H1 connection timeout after a synchronous send
- SCRIPTS: git-show-backports: make -m most only show the left branch
- SCRIPTS: git-show-backports: emit the shell command to backport a commit
- BUG/MINOR: ssl: fix memory leak at OCSP loading
- BUG/MEDIUM: ssl: memory leak of ocsp data at SSL_CTX_free()
- BUG/MEDIUM: map/lua: Return an error if a map is loaded during runtime
- MINOR: arg: Add an argument type to keep a reference on opaque data
- BUG/MINOR: converters: Store the sink in an arg pointer for debug() converter
- BUG/MINOR: lua: Duplicate map name to load it when a new Map object is created
- BUG/MINOR: arg: Fix leaks during arguments validation for fetches/converters
- BUG/MINOR: lua: Check argument type to convert it to IPv4/IPv6 arg validation
- BUG/MINOR: lua: Check argument type to convert it to IP mask in arg validation
- MINOR: hlua: Don't needlessly copy lua strings in trash during args validation
- BUG/MINOR: lua: Duplicate lua strings in sample fetches/converters arg array
- MEDIUM: lua: Don't filter exported fetches and converters
- BUG/MINOR: snapshots: leak of snapshots on deinit()
- BUG/MINOR: stats: use strncmp() instead of memcmp() on health states
- BUG/MEDIUM: htx: smp_prefetch_htx() must always validate the direction
- BUG/MINOR: reload: do not fail when no socket is sent
- MINOR: http-htx: Add an option to eval query-string when the path is replaced
- BUG/MINOR: http-rules: Replace path and query-string in "replace-path" action
- DOC: cache: Use '<name>' instead of '<id>' in error message
- BUG/MAJOR: contrib/spoa-server: Fix unhandled python call leading to memory leak
- BUG/MINOR: contrib/spoa-server: Ensure ip address references are freed
- BUG/MINOR: contrib/spoa-server: Do not free reference to NULL
- BUG/MINOR: contrib/spoa-server: Updating references to free in case of failure
- BUG/MEDIUM: contrib/spoa-server: Fix ipv4_address used instead of ipv6_address
- BUG/MINOR: startup: haproxy -s cause 100% cpu
- BUG/MEDIUM: doc: Fix replace-path action description
- Revert "BUG/MINOR: http-rules: Replace path and query-string in "replace-path" action"
- BUG/MEDIUM: ssl: check OCSP calloc in ssl_sock_load_ocsp()
- BUG/MINOR: threads: work around a libgcc_s issue with chrooting
- BUILD: thread: limit the libgcc_s workaround to glibc only
- MINOR: Commit .gitattributes
- CLEANUP: Update .gitignore
- BUG/MINOR: auth: report valid crypto(3) support depending on build options
- BUG/MEDIUM: mux-h1: always apply the timeout on half-closed connections
- BUILD: threads: better workaround for late loading of libgcc_s
- BUG/MEDIUM: pattern: Renew the pattern expression revision when it is pruned
- MINOR: arg: Use chunk_destroy() to release string arguments
- BUG/MEDIUM: http-ana: Don't wait to send 1xx responses received from servers
- BUG/MEDIUM: ssl: does not look for all SNIs before chosing a certificate
- BUG/MINOR: ssl: verifyhost is case sensitive
- BUG/MINOR: server: report correct error message for invalid port on "socks4"
- BUG/MINOR: h2/trace: do not display "stream error" after a frame ACK
- BUG/MINOR: http-fetch: Don't set the sample type during the htx prefetch
- BUG/MEDIUM: h2: report frame bits only for handled types
- MINOR: h2/trace: also display the remaining frame length in traces
- BUG/MINOR: Fix memory leaks cfg_parse_peers
- MINOR: backend: make the "whole" option of balance uri take only one bit
- MINOR: backend: add a new "path-only" option to "balance uri"
- REGTESTS: add a few load balancing tests
- DOC: spoa-server: fix false friends `actually`
- BUG/MINOR: config: Fix memory leak on config parse listen
- BUG/MEDIUM: listeners: do not pause foreign listeners
- DOC: agent-check: fix typo in "fail" word expected reply
- REGTEST: fix host part in balance-uri-path-only.vtc
- REGTEST: make abns_socket.vtc require 1.8
- REGTEST: make map_regm_with_backref require 1.7
Willy Tarreau [Tue, 29 Sep 2020 09:00:51 +0000 (11:00 +0200)]
REGTEST: make map_regm_with_backref require 1.7
map_regm was only introduced in 1.7, I don't know why the require field
was set to 1.6, probably tha the test evolved and didn't start with
map_regm.
(cherry picked from commit
73cc5457cb7608b5b7f797d8d78d655a6fcab9eb)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
091a9a865ea738cc323bf533c15b125b4eac19f2)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Tue, 29 Sep 2020 08:58:44 +0000 (10:58 +0200)]
REGTEST: make abns_socket.vtc require 1.8
It's not as much an abns test as it is a seamless reload test, which
appeared in 1.8 due to "expose-fd listeners".
(cherry picked from commit
f8d46de80640a691f6b33c72c6efb07111d5e0a3)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
528e26e7426ce64f889fb963d5af8fccfebae396)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Tue, 29 Sep 2020 07:58:23 +0000 (09:58 +0200)]
REGTEST: fix host part in balance-uri-path-only.vtc
We used to set it to ${h1_px_addr} but it randomly fails on certain
hosts (FreeBSD and OSX) where the address is surprisingly set to "::1"
while the Host field contains 127.0.0.1 (hence two different address
families). While this is likely a minor issue in vtest, we don't need
to depend on this and can easily hard-code 127.0.0.1 which is already
used in other tests.
(cherry picked from commit
2aa3dd2b4318eddf54e3958e7cd5751b0e25db74)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
8e61a9d656dcfbb2a6047c176ca39b6e0573c99f)
Signed-off-by: Willy Tarreau <w@1wt.eu>
William Dauchy [Sat, 26 Sep 2020 11:35:51 +0000 (13:35 +0200)]
DOC: agent-check: fix typo in "fail" word expected reply
`tcpcheck_agent_expect_reply` expects "fail" not "failed"
This should fix github issue #876
This can be backported to all maintained versions (i.e >= 1.6) as of
today.
Signed-off-by: William Dauchy <w.dauchy@criteo.com>
(cherry picked from commit
f8e795ca04a2e89b801c40843b1b79fd97251606)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
df223005a3f61d19c140e8bffbb17ec8507cc486)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Wed, 23 Sep 2020 15:17:22 +0000 (17:17 +0200)]
BUG/MEDIUM: listeners: do not pause foreign listeners
There's a nasty case with listeners that belong to foreign processes.
If a proxy is defined this way:
global
nbproc 2
frontend f
bind :1111 process 1
bind :2222 process 2
and if stats expose-fd listeners is set, the listeners' FDs will not
be closed on the processes that don't use them. At this point it's not
a big deal, except that they're shared between processes and that a
"disable frontend f" issued on one process will pause all of them and
cause the other process to see accept() fail, turning its own listener
to state LI_LIMITED to try to leave it some time to recover. But it
will never recover, even after an enable.
The root cause of the issue is that the ZOMBIE state doesn't cover
this situation since it's only for a proxy being entirely bound to a
process.
What we do here to address this is that we refrain from pausing a
file descriptor that belongs to a foreign process in pause_listener().
This definitely solves the problem. A similar test is present in
resume_listener() and is the reason why the FD doesn't recover upon the
"enable" action by the way.
This ought to be backported to 1.8 where seamless reload was integrated.
The config above should be sufficient to validate that the fix works;
after a pair of "disable/enable frontend" no process will handle the
traffic to one of the ports anymore.
(cherry picked from commit
02e1975c29eb19da98205164f23154f903802644)
[wt: context adjustment; bind_proc is in l->bind_conf in 2.2; test ok]
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
c7239d5da5801f2238b56601bfae29441153d4b8)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Amaury Denoyelle [Fri, 18 Sep 2020 13:59:39 +0000 (15:59 +0200)]
BUG/MINOR: config: Fix memory leak on config parse listen
This memory leak happens if there is two or more defaults section. When
the default proxy is reinitialized, the structure member containing the
config filename must be freed.
Fix github issue #851.
Should be backported as far as 1.6.
(cherry picked from commit
36b536652f23c8b9b430d7a7cbb42a696d06a929)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
a9becb5273d67bd44a6939fac7f2610624d2717b)
[wt: minor context adjustment]
Signed-off-by: Willy Tarreau <w@1wt.eu>
William Dauchy [Sat, 1 Aug 2020 14:28:52 +0000 (16:28 +0200)]
DOC: spoa-server: fix false friends `actually`
it seems like `actually` was wrongly used instead of `currently`
Signed-off-by: William Dauchy <w.dauchy@criteo.com>
(cherry picked from commit
4896d279aa159ba1fc9f7728a30120993c7ac577)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
96bcd2110b6d1219236ace2bdb0fa32ee0c092f4)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Wed, 23 Sep 2020 07:02:13 +0000 (09:02 +0200)]
REGTESTS: add a few load balancing tests
This adds "balance-rr" to test round robin, "balance-uri" to test the
default balance-uri method, and "balance-uri-path-only" which mixes H1
and H2 through "balance uri path-only" and verifies that they reach
the same server.
Note that for the latter, "proto h2" explicitly had to be placed on
the listening socket otherwise it would timeout. This may indicate an
issue in the H1->H2 upgrade depending how the H2 preface is sent maybe.
(cherry picked from commit
19490907b3836369215a9860f1bd6d2bc3297937)
[wt: marked the balance-uri-path-only for 2.2 since already backported]
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
6a551a0ea58218ab02f45b0fb1eb26248b7a5263)
[wt: marked the balance-uri-path-only for 2.1 since already backported]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Wed, 23 Sep 2020 06:56:29 +0000 (08:56 +0200)]
MINOR: backend: add a new "path-only" option to "balance uri"
Since we've fixed the way URIs are handled in 2.1, some users have started
to experience inconsistencies in "balance uri" between requests received
over H1 and the same ones received over H2. This is caused by the fact
that H1 rarely uses absolute URIs while H2 always uses them. Similar
issues were reported already around replace-uri etc, leading to "pathq"
recently being introduced, so this isn't new.
Here what this patch does is add a new option to "balance uri" to indicate
that the hashing should only start at the path and not cover the authority.
This makes H1 relative URIs and H2 absolute URI hashes equally again.
Some extra options could be added to normalize URIs by always hashing the
authority (or host) in front of them, which would make sure that both
absolute and relative requests provide the same hash. This is left for
later if needed.
(cherry picked from commit
57a374131cd3c1c46f9d78ea9c0f2ad027d0be97)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
1d5961c9fbc95059cb3e8ad50f9f740370093d37)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Wed, 23 Sep 2020 06:05:47 +0000 (08:05 +0200)]
MINOR: backend: make the "whole" option of balance uri take only one bit
We'll want to add other boolean options on "balance uri", so let's make
some room aside "whole" and make it take only one bit and not one int.
(cherry picked from commit
3d1119d2257692f1508dd42f32c1840de0e16173)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
0df04e4b0f99ad22ad95ea5554a4d361e8df489d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Eric Salama [Fri, 18 Sep 2020 09:55:17 +0000 (11:55 +0200)]
BUG/MINOR: Fix memory leaks cfg_parse_peers
When memory allocation fails in cfg_parse_peers or when an error occurs
while parsing a stick-table, the temporary table and its id must be freed.
This fixes github issue #854. It should be backported as far as 2.0.
(cherry picked from commit
1aab911017e0634c109325eff73d5d49325babed)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
7623beb5192955bdc40aaad218df497e3378d716)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Fri, 18 Sep 2020 05:39:29 +0000 (07:39 +0200)]
MINOR: h2/trace: also display the remaining frame length in traces
It's often missing when debugging, even though it's often zero for
control frames or after data are consumed.
(cherry picked from commit
8520d87198fa0682816f1d8ebcc831ecd0bfba5d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
563be0e196c32685514d55331281a23c918cb1c5)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Fri, 18 Sep 2020 05:43:38 +0000 (07:43 +0200)]
BUG/MEDIUM: h2: report frame bits only for handled types
As part of his GREASE experiments on Chromium, Bence Béky reported in
https://lists.w3.org/Archives/Public/ietf-http-wg/2020JulSep/0202.html
and https://bugs.chromium.org/p/chromium/issues/detail?id=1127060 that
a certain combination of frame type and frame flags was causing an error
on app.slack.com. It turns out that it's haproxy that is causing this
issue because the frame type is wrongly assumed to support padding, the
frame flags indicate padding is present, and the frame is too short for
this, resulting in an error.
The reason why only some frame types are affected is due to the frame
type being used in a bit shift to match against a mask, and where the
5 lower bits of the frame type only are used to compute the frame bit.
If the resulting frame bit matches a DATA, HEADERS or PUSH_PROMISE frame
bit, then padding support is assumed and the test is enforced, resulting
in a PROTOCOL_ERROR or FRAME_SIZE_ERROR depending on the payload size.
We must never match any such bit for unsupported frame types so let's
add a check for this. This must be backported as far as 1.8.
Thanks to Cooper Bethea for providing enough context to help narrow the
issue down and to Bence Béky for creating a simple reproducer.
(cherry picked from commit
3ca2365904062f883751a4099e2eb5e47ecf11b7)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
1ec08a4d48d2a13859063712b86c82db7920e06e)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Fri, 18 Sep 2020 08:19:02 +0000 (10:19 +0200)]
BUG/MINOR: http-fetch: Don't set the sample type during the htx prefetch
A subtle bug was introduced by the commit
a6d9879e6 ("BUG/MEDIUM: htx:
smp_prefetch_htx() must always validate the direction"), for the "method"
sample fetch only. The sample data type and the method id are always
overwritten because smp_prefetch_htx() function is called later in the
sample fetch evaluation. The bug is in the smp_prefetch_htx() function but
it is only visible for the "method" sample fetch, for an unknown method.
In fact, when smp_prefetch_htx() is called, the sample object is
altered. The data type is set to SMP_T_BOOL and, on success, the data value
is set to 1. Thus, if the caller has already set some infos into the sample
object, they may be lost. AFAIK, there is no reason to do so. It is
inherited from the legacy HTTP code and I honestely don't known why it was
done this way. So, instead of fixing the "method" sample fetch to set useful
info after the call to smp_prefetch_htx() function, I prefer to not alter
the sample object in smp_prefetch_htx().
This patch must be backported as far as 2.0. On the 2.0, only the HTX part
must be fixed.
(cherry picked from commit
d2414a23c4c20b14c54ee9279e99fb96c81a29b1)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
b8aa5f28cea612eadfbd6d775d8402e45a6547c6)
[wt: adjusted ctx]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Fri, 18 Sep 2020 05:41:28 +0000 (07:41 +0200)]
BUG/MINOR: h2/trace: do not display "stream error" after a frame ACK
When sending a frame ACK, the parser state is not equal to H2_CS_FRAME_H
and we used to report it as an error, which is not true. In fact we should
only indicate when we skip remaining data.
This may be backported as far as 2.1.
(cherry picked from commit
bba7a4dafdabf6e4b669e905778ab1904eb4784d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
e6fb02032ed3166abfdd99b172adbb6868b0887d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Tue, 15 Sep 2020 09:25:54 +0000 (11:25 +0200)]
BUG/MINOR: server: report correct error message for invalid port on "socks4"
The socks4 keyword parser was a bit too much copy-pasted, it only checks
for a null port and reports "invalid range". Let's properly check for the
1-65535 range and report the correct error.
It may be backported everywhere "socks4" is present (2.0).
(cherry picked from commit
9743f709d03c036b7a6da2fb260d0e77f9e04a3d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
e96489ea5e0408d852fc1dd4318926923916c230)
Signed-off-by: Willy Tarreau <w@1wt.eu>
William Lallemand [Mon, 14 Sep 2020 13:20:10 +0000 (15:20 +0200)]
BUG/MINOR: ssl: verifyhost is case sensitive
In bug #835, @arjenzorgdoc reported that the verifyhost option on the
server line is case-sensitive, that shouldn't be the case.
This patch fixes the issue by replacing memcmp by strncasecmp and strcmp
by strcasecmp. The patch was suggested by @arjenzorgdoc.
This must be backported in all versions supporting the verifyhost
option.
(cherry picked from commit
2d6fd0a90df8d3ab77f13c59e5f1efa3d271c42c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
656c331cbd02b4bef43e6ceac347b6e9bd440296)
[wt: adjusted context]
Signed-off-by: Willy Tarreau <w@1wt.eu>
William Lallemand [Fri, 14 Aug 2020 12:43:35 +0000 (14:43 +0200)]
BUG/MEDIUM: ssl: does not look for all SNIs before chosing a certificate
In bug #810, the SNI are not matched correctly, indeed when trying to
match a certificate type in ssl_sock_switchctx_cbk() all SNIs were not
looked up correctly.
In the case you have in a crt-list:
wildcard.subdomain.domain.tld.pem.rsa *.subdomain.domain.tld record.subdomain.domain.tld
record.subdomain.domain.tld.pem.ecdsa record.subdomain.domain.tld another-record.subdomain.domain.tld
If the client only supports RSA and requests
"another-record.subdomain.domain.tld", HAProxy will find the single
ECDSA certificate and won't try to look up for a wildcard RSA
certificate.
This patch fixes the code so we look for all single and
wildcard before chosing the certificate type.
This bug was introduced by commit 3777e3a ("BUG/MINOR: ssl: certificate
choice can be unexpected with openssl >= 1.1.1").
It must be backported as far as 1.8 once it is heavily tested.
(cherry picked from commit
94bd319b264a64f73202a82bdd2d0bcf23722246)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
fa434af0ac47304e6c060d864ba08202b4030ca9)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Mon, 31 Aug 2020 09:07:07 +0000 (11:07 +0200)]
BUG/MEDIUM: http-ana: Don't wait to send 1xx responses received from servers
When an informational response (1xx) is received, we must be sure to send it
ASAP. To do so, CF_SEND_DONTWAIT flag must be set on the response channel to
instruct the stream-interface to not set the CO_SFL_MSG_MORE flag on the
transport layer. Otherwise the response delivery may be delayed, because of the
commit
8945bb6c0 ("BUG/MEDIUM: stream-int: fix loss of CO_SFL_MSG_MORE flag in
forwarding").
Note that a previous patch (
cf6898cd ["BUG/MINOR: http-ana: Don't wait to send
1xx responses generated by HAProxy"]) add this flag on 1xx responses generated
by HAProxy but not on responses coming from servers.
This patch must be backported to 2.2 and may be backported as far as 1.9, for
HTX part only. But this part has changed in the 2.2, so it may be a bit
tricky. Note it does not fix any known bug on 2.1 and below because the
CO_SFL_MSG_MORE flag is ignored by the h1 mux.
(cherry picked from commit
7d518454bb501659e8b13f38c22d216f5485be34)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
4777db423493d10cb97607b601c8ba9821b9a149)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Fri, 7 Aug 2020 09:45:18 +0000 (11:45 +0200)]
MINOR: arg: Use chunk_destroy() to release string arguments
This way, all fields of the buffer structure are reset when a string argument
(ARGT_STR) is released. It is also a good way to explicitly specify this kind
of argument is a chunk. So .data and .size fields must be set.
This patch may be backported to ease backports.
(cherry picked from commit
6ad7df423b71470babef545506a1af569074fc59)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
1c67ab418b2adfd99e0c9d2796af382a86ce4a0f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Wed, 9 Sep 2020 14:09:44 +0000 (16:09 +0200)]
BUG/MEDIUM: pattern: Renew the pattern expression revision when it is pruned
It must be done to expire patterns cached in the LRU cache. Otherwise it is
possible to retrieve an already freed pattern, attached to a released pattern
expression.
When a specific pattern is deleted (->delete() callback), the pattern expression
revision is already renewed. Thus it is not affected by this bug. Only prune
action on the pattern expression is concerned.
In addition, for a pattern expression, in ->prune() callbacks when the pattern
list is released, a missing LIST_DEL() has been added. It is not a real issue
because the list is reinitialized at the end and all elements are released and
should never be reused. But it is less confusing this way.
This bug may be triggered when a map is cleared from the cli socket. A
workaround is to set the pattern cache size (tune.pattern.cache-size) to 0 to
disable it.
This patch should fix the issue #844. It must be backported to all supported
versions.
(cherry picked from commit
6cfc8516741c57fb9f879b364568cfa79a6b42a1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
d585ffd50c020529c145a680aae87363ded00c13)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Wed, 9 Sep 2020 15:07:54 +0000 (17:07 +0200)]
BUILD: threads: better workaround for late loading of libgcc_s
Commit
77b98220e ("BUG/MINOR: threads: work around a libgcc_s issue with
chrooting") tried to address an issue with libgcc_s being loaded too late.
But it turns out that the symbol used there isn't present on armhf, thus
it breaks the build.
Given that the issue manifests itself during pthread_exit(), the safest
and most portable way to test this is to call pthread_exit(). For this
we create a dummy thread which exits, during the early boot. This results
in the relevant library to be loaded if needed, making sure that a later
call to pthread_exit() will still work. It was tested to work fine under
linux on the following platforms:
glibc:
- armhf
- aarch64
- x86_64
- sparc64
- ppc64le
musl:
- mipsel
Just running the code under strace easily shows the call in the dummy
thread, for example here on armhf:
$ strace -fe trace=file ./haproxy -v 2>&1 | grep gcc_s
[pid 23055] open("/lib/libgcc_s.so.1", O_RDONLY|O_CLOEXEC) = 3
The code was isolated so that it's easy to #ifdef it out if needed.
This should be backported where the patch above is backported (likely
2.0).
(cherry picked from commit
f734ebfac4c406f245347527bd0e5831a251cc61)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
10c627ab2ebeb0a38ae8df477a5f2d870ad77f7c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Tue, 8 Sep 2020 13:40:57 +0000 (15:40 +0200)]
BUG/MEDIUM: mux-h1: always apply the timeout on half-closed connections
The condition in h1_refresh_timeout() seems insufficient to properly
take care of the half-closed timeout, because depending on the ordering
of operations when performing the last send() to a client, the stream
may or may not still be there and we may fail to shrink the client
timeout on our last opportunity to do so.
Here we want to make sure that the timeout is always reduced when the
last chunk was sent and the shutdown completed, regardless of the
presence of a stream or not. This is what this patch does.
This should be backported as far as 2.0, and should fix the issue
reported in #541.
(cherry picked from commit
4313d5ae98b90613318da7e1181a6c4a1db29799)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
ee6ed47bba75896b6bbdd15f6a622e88a8168390)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Victor Kislov [Thu, 6 Aug 2020 16:21:39 +0000 (19:21 +0300)]
BUG/MINOR: auth: report valid crypto(3) support depending on build options
Since 1.8 with commit
e8692b41e ("CLEANUP: auth: use the build options list
to report its support"), crypt(3) is always reported as being supported in
"haproxy -vv" because no test on USE_LIBCRYPT is made anymore when
producing the output.
This reintroduces the distinction between with and without USE_LIBCRYPT
in the output by indicating "yes" or "no". It may be backported as far
as 1.8, though the code differs due to a number of include files cleanups.
(cherry picked from commit
ec00251c88a91615cc0446c8501dccd290484395)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
0688433d7fce12c885afd96d40b66aee0cbe67ab)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Tim Duesterhus [Sun, 6 Sep 2020 16:34:12 +0000 (18:34 +0200)]
CLEANUP: Update .gitignore
This excludes the currently tracked files from being ignored.
(cherry picked from commit
c14d20eda38e48ff3db588e9dad34f56967f72e6)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
397a7076ba2aaa470866d4a7c3d7da0a32f6f048)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Tim Duesterhus [Sat, 5 Sep 2020 10:46:11 +0000 (12:46 +0200)]
MINOR: Commit .gitattributes
Commit .gitattributes to the repository to allow GitHub downloads to
properly fill in the SUBVERS and VERDATE files. It also prevents the
engineer in charge of the release to forget creating it when a new branch
is released.
see https://www.mail-archive.com/haproxy@formilux.org/msg38107.html
No functional change, may be backported everywhere.
(cherry picked from commit
78b9c54c666b88abd0c2a001ba23f420be29ea8c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
43bf9ecea6864f90f227190682a1dc4355ef1fb0)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Wed, 2 Sep 2020 07:53:47 +0000 (09:53 +0200)]
BUILD: thread: limit the libgcc_s workaround to glibc only
Previous commit
77b98220e ("BUG/MINOR: threads: work around a libgcc_s
issue with chrooting") broke the build on cygwin. I didn't even know we
supported threads on cygwin. But the point is that it's actually the
glibc-based libpthread which requires libgcc_s, so in absence of other
reports we should not apply the workaround on other libraries.
This should be backported along with the aforementioned patch.
(cherry picked from commit
06a1806083dd9f0d21e045baad4c61559211f956)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
e6a4e318db089df3dbadcbed286ed12a3742a7e5)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Wed, 2 Sep 2020 06:04:35 +0000 (08:04 +0200)]
BUG/MINOR: threads: work around a libgcc_s issue with chrooting
Sander Hoentjen reported another issue related to libgcc_s in issue #671.
What happens is that when the old process quits, pthread_exit() calls
something from libgcc_s.so after the process was chrooted, and this is
the first call to that library, causing an attempt to load it. In a
chroot, this fails, thus libthread aborts. The behavior widely differs
between operating systems because some decided to use a static build for
this library.
In 2.2 this was resolved as a side effect of a workaround for the same issue
with the backtrace() call, which is also in libgcc_s. This was in commit
0214b45 ("MINOR: debug: call backtrace() once upon startup"). But backtraces
are not necessarily enabled, and we need something for older versions.
By inspecting a significant number of ligcc_s on various gcc versions and
platforms, it appears that a few functions have been present since gcc 3.0,
one of which, _Unwind_Find_FDE() has no side effect (it only returns a
pointer). What this patch does is that in the thread initialization code,
if built with gcc >= 3.0, a call to this function is made in order to make
sure that libgcc_s is loaded at start up time and that there will be no
need to load it upon exit.
An easy way to check which libs are loaded under Linux is :
$ strace -e trace=openat ./haproxy -v
With this patch applied, libgcc_s now appears during init.
Sander confirmed that this patch was enough to put an end to the core
dumps on exit in 2.0, so this patch should be backported there, and maybe
as far as 1.8.
(cherry picked from commit
77b98220e81207aad854e480e6207d501d476666)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
c2678c60e02f0bc9ba22ee3e2b8fe78e714e869a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Fri, 31 Jul 2020 09:43:20 +0000 (11:43 +0200)]
BUG/MEDIUM: ssl: check OCSP calloc in ssl_sock_load_ocsp()
Check the return of the calloc in ssl_sock_load_ocsp() which could lead
to a NULL dereference.
This was introduced by commit be2774d ("MEDIUM: ssl: Added support for
Multi-Cert OCSP Stapling").
Could be backported as far as 1.7.
(cherry picked from commit
a560c06af7d9ba68cb0e82d1d25652b9a6a5a336)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
ea22337f040fd563b5bd5457976d90568a0b4697)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Wed, 2 Sep 2020 09:10:38 +0000 (11:10 +0200)]
Revert "BUG/MINOR: http-rules: Replace path and query-string in "replace-path" action"
This reverts commit
4b9c0d1fc08388bf44c6ebbd88f786032dd010fc.
Actually, the "replace-path" action is ambiguous. "set-path" action preserves
the query-string. The "path" sample fetch does not contain the query-string. But
"replace-path" action is documented to handle the query-string. It is probably
not the expected behavior. So instead of fixing the code, we will fix the
documentation to make "replace-path" action consistent with other parts of the
code. In addition actions and sample fetches to handle the path with the
query-string will be added.
If the commit above is ever backported, this one must be as well.
(cherry picked from commit
1fa0cc18e1c6dd5460492128b5b400d808f3c823)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
34b0840ec15e9c869f1bcbdfb4cfca119cb7a874)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Wed, 2 Sep 2020 12:16:59 +0000 (14:16 +0200)]
BUG/MEDIUM: doc: Fix replace-path action description
The description of the replace-path action does not reflect what the code
do. When the request path is replaced, the query-string is preserved. But the
documentation stated the query-string is part of the replacement, if any is
present. Most of time, when the doc and the code differ, the code is fixed. But
here, the replace-path action is pretty confusing because the set-path action is
only applied on the path. The query-string is left intact. And the path sample
fetch also ignores the query-string. In addition, the replace-path action is
quite recent. It was added in the 2.2. Thus, exceptionally, the documentation is
fixed instead.
Note that set-pathq and replace-pathq actions and pathq sample fetch will be
added to manipulate the path with the query-string.
This patch must be backported as far as 2.0.
(cherry picked from commit
82c8332be058d79ce0b97151cd312ea5e43a4356)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
7c621164176de7d9bb736f08f75a11f75f8e488c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Wed, 2 Sep 2020 14:12:23 +0000 (16:12 +0200)]
BUG/MINOR: startup: haproxy -s cause 100% cpu
It was reported in bug #837 that haproxy -s causes a 100% CPU.
However this option does not exist and haproxy must exit with the
usage message.
The parser was not handling the case where -s is not followed by 't' or
'f' which are the only two valid cases.
This bug was introduced by df6c5a ("BUG/MEDIUM: mworker: fix the copy of
options in copy_argv()") which was backported as far as 1.8.
This fix must be backported as far as 1.8.
(cherry picked from commit
398da62c38014c76006571e0caa52a3799f41815)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
93dec6abdbb23501802f988f95de717feb8d8a8d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Gilchrist Dadaglo [Mon, 24 Aug 2020 19:21:35 +0000 (19:21 +0000)]
BUG/MEDIUM: contrib/spoa-server: Fix ipv4_address used instead of ipv6_address
Typo leads to checking the wrong object for memory issues
This patch must be backported as far as 2.0.
(cherry picked from commit
d00ce06d9dfe3fd7cd277faf0d8da9ee21dd3765)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
dadf4315e1eb0768c8eb57dad7a3a39048b65618)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Gilchrist Dadaglo [Mon, 24 Aug 2020 19:21:34 +0000 (19:21 +0000)]
BUG/MINOR: contrib/spoa-server: Updating references to free in case of failure
When we encounter a failure, all previously borrowed references should
be freed. Especially if the program is not failing immediately
This patch must be backported as far as 2.0.
(cherry picked from commit
2417ebcc013b23ac961afb436b9b2dead32c28b0)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
9fe95de0df2748362fddc4acf84dd19c40de6a9f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Gilchrist Dadaglo [Mon, 24 Aug 2020 19:21:33 +0000 (19:21 +0000)]
BUG/MINOR: contrib/spoa-server: Do not free reference to NULL
As per https://docs.python.org/3/c-api/refcounting.html, Py_DECREF
should not be called on NULL objects.
This patch must be backported as far as 2.0.
(cherry picked from commit
c7d303a0a8962f955e9dd271ee9775dd8eb88cfe)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
dcda9a0cf1d8c381208c426cbf500c7e5f9a3468)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Gilchrist Dadaglo [Mon, 24 Aug 2020 19:21:32 +0000 (19:21 +0000)]
BUG/MINOR: contrib/spoa-server: Ensure ip address references are freed
IP addresses references passed in argument for ps_python are not freed after
they have been used. Leading to a small chance of mem leak if a lot of ip
addresses are passed around
This patch must be backported as far as 2.0.
(cherry picked from commit
9f0c984cf50fee3a2282993b573be2f049efe5ab)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
a77f10426189fe48bca2c3543e65510933b95272)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Gilchrist Dadaglo [Mon, 24 Aug 2020 19:21:31 +0000 (19:21 +0000)]
BUG/MAJOR: contrib/spoa-server: Fix unhandled python call leading to memory leak
The result from spoa evaluation of the user provided python code is
never passed back to the main spoa process nor freed.
Same for the keyword list passed.
This results into the elements never freed by Python as reference count
never goes down.
https://docs.python.org/3/extending/extending.html#reference-counting-in-python
This patch must be backported as far as 2.0.
(cherry picked from commit
222f060be379126fa0890f6cd2820c4bd8d5ac90)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
85e705ece1519756fae0b610793cdf602cf25bf6)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
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>