MEDIUM: listener/proxy: make the listeners notify about proxy pause/resume

Till now, we used to call pause_proxy()/resume_proxy() to enable/disable
processing on a proxy, which is used during soft reloads. But since we want
to drive this process from the listeners themselves, we have to instead
proceed the other way around so that when we enable/disable a listener,
it checks if it changed anything for the proxy and notifies about updates
at this level.

The detection is made using li_ready=0 for pause(), and li_paused=0
for resume(). Note that we must not include any test for li_bound because
this state is seen by processes which share the listener with another one
and which must not act on it since the other process will do it. As such
the socket behind the FD will automatically be paused and resume without
its local state changing, but this is the limit of a multi-process system
with shared listeners.

MINOR: listeners: check the current listener earlier state in resume_listener()

It's quite confusing to have the test on LI_READY very low in the function
as it should be made much earlier. Just like with previous commit, let's
do it when entering. The additional states, however (limited, full) continue
to go through the whole function.

MINOR: listeners: check the current listener state in pause_listener()

It's better not to try to perform pause() actions on wrong states, so
let's check this and make sure that all callers are now safe. This
means that we must not try to pause a listener which is already paused
(e.g. it could possibly fail if the pause operation isn't idempotent at
the socket level), nor should we try it on earlier states.

MEDIUM: proxy: merge zombify_proxy() with stop_proxy()

The two functions don't need to be distinguished anymore since they have
all the necessary info to act as needed on their listeners. Let's just
pass via stop_proxy() and make it check for each listener which one to
close or not.

MEDIUM: proxy: remove start_proxies()

Its sole remaining purpose was to display "proxy foo started", which
has little benefit and pollutes output for those with plenty of proxies.
Let's remove it now.

The VTCs were updated to reflect this, because many of them had explicit
counts of dropped lines to match this message.

This is tagged as MEDIUM because some users may be surprized by the
loss of this quite old message.

MEDIUM: proxy: replace proxy->state with proxy->disabled

The remaining proxy states were only used to distinguish an enabled
proxy from a disabled one. Due to the initialization order, both
PR_STNEW and PR_STREADY were equivalent after startup, and they
would only differ from PR_STSTOPPED when the proxy is disabled or
shutdown (which is effectively another way to disable it).

Now we just have a "disabled" field which allows to distinguish them.
It's becoming obvious that start_proxies() is only used to print a
greeting message now, that we'd rather get rid of. Probably that
zombify_proxy() and stop_proxy() should be merged once their
differences move to the right place.

CLEANUP: peers: don't use the PR_ST* states to mark enabled/disabled

The enabled/disabled config options were stored into a "state" field
that is an integer but contained only PR_STNEW or PR_STSTOPPED, which
is a bit confusing, and causes a dependency with proxies. This was
renamed to "disabled" and is used as a boolean. The field was also
moved to the end of the struct to stop creating a hole and fill another
one.

MINOR: startup: don't rely on PR_STNEW to check for listeners

Instead of looking at listeners in proxies in PR_STNEW state, we'd
rather check for listeners in those not in PR_STSTOPPED as it's only
this state which indicates the proxy was disabled. And let's check
the listeners count instead of testing the list's head.

MEDIUM: proxy: remove state PR_STPAUSED

This state was used to mention that a proxy was in PAUSED state, as opposed
to the READY state. This was causing some trouble because if a listener
failed to resume (e.g. because its port was temporarily in use during the
resume), it was not possible to retry the operation later. Now by checking
the number of READY or PAUSED listeners instead, we can accurately know if
something went bad and try to fix it again later. The case of the temporary
port conflict during resume now works well:

  $ socat readline /tmp/sock1
  prompt
  > disable frontend testme3

  > disable frontend testme3
  All sockets are already disabled.

  > enable frontend testme3
  Failed to resume frontend, check logs for precise cause (port conflict?).

  > enable frontend testme3

  > enable frontend testme3
  All sockets are already enabled.

MEDIUM: proxy: remove the PR_STERROR state

This state is only set when a pause() fails but isn't even set when a
resume() fails. And we cannot recover from this state. Instead, let's
just count remaining ready listeners to decide to emit an error or not.
It's more accurate and will better support new attempts if needed.

MEDIUM: proxy: remove the unused PR_STFULL state

Since v1.4 or so, it's almost not possible anymore to set this state. The
only exception is by using the CLI to change a frontend's maxconn setting
below its current usage. This case makes no sense, and for other cases it
doesn't make sense either because "full" is a vague concept when only
certain listeners are full and not all. Let's just remove this unused
state and make it clear that it's not reported. The "ready" or "open"
states will continue to be reported without being misleading as they
will be opposed to "stop".

MINOR: proxy: maintain per-state counters of listeners

The proxy state tries to be synthetic but that doesn't work well with
many listeners, especially for transition phases or after a failed
pause/resume.

In order to address this, we'll instead rely on counters of listeners in
a given state for the 3 major states (ready, paused, listen) and a total
counter. We'll now be able to determine a proxy's state by comparing these
counters only.

MINOR: listeners: introduce listener_set_state()

This function is used as a wrapper to set a listener's state everywhere.
We'll use it later to maintain some counters in a consistent state when
switching state so it's capital that all state changes go through it.
No functional change was made beyond calling the wrapper.

CLEANUP: proxy: remove the first_to_listen hack in zombify_proxy()

This thing was needed for an optimization used in soft_stop() which
doesn't exist anymore, so let's remove it as it's cryptic and hinders
the listeners cleanup.

MINOR: listeners: do not uselessly try to close zombie listeners in soft_stop()

The loop doesn't match anymore since the non-started listeners are in
LI_INIT and even if it had ever worked the benefit of closing zombies
at this point looks void at best.

MEDIUM: listeners: remove the now unused ZOMBIE state

The zombie state is not used anymore by the listeners, because in the
last two cases where it was tested it couldn't match as it was covered
by the test on the process mask. Instead now the FD is either in the
LISTEN state or the INIT state. This also avoids forcing the listener
to be single-dimensional because actually belonging to another process
isn't totally exclusive with the other states, which explains some of
the difficulties requiring to check the proc_mask and the fd sometimes.

So let's get rid of it now not to be tempted to reuse it.

The doc on the listeners state was updated.

MEDIUM: deinit: close all receivers/listeners before scanning proxies

Because of the zombie state, proxies have a skewed vision of the state
of listeners, which explains why there are hacks switching the state
from ZOMBIE to INIT in the proxy cleaning loop. This is particularly
complicated and not needed, as all the information is now available
in the protocol list and the fdtab.

What we do here instead is to first close all active listeners or
receivers by protocol and clean their protocol parts. Then we scan the
fdtab to get rid of remaining ones that were necessarily in INIT state
after a previous invocation of delete_listener(). From this point, we
know the listeners are cleaned, the can safely be freed by scanning the
proxies.

MEDIUM: listeners: make unbind_listener() converge if needed

The ZOMBIE state on listener is a real mess. Listeners passing through
this state have lost their consistency with the proxy AND with the fdtab.
Plus this state is not used for all foreign listeners, only for those
belonging to a proxy that entirely runs on another process, otherwise it
stays in INIT state, which makes the usefulness extremely questionable.
But the real issue is that it's impossible to untangle the receivers
from the proxy state as long as we have this because of deinit()...

So what we do here is to start by making unbind_listener() support being
called more than once. This will permit to call it again to really close
the FD and finish the operations if it's called with an FD that's in a
fake state (such as INIT but with a valid fd).

MEDIUM: init: stop disabled proxies after initializing fdtab

During the startup process we don't have any fdtab nor fd_updt for quite
a long time, and as such some operations on the listeners are not
permitted, such as fd_want_*/fd_stop_* or fd_delete(). The latter is of
particular concern because it's used when stopping a disabled frontend,
and it's performed very early during check_config_validity() while there
is no fdtab yet. The trick till now relies on the listener's state which
is a bit brittle.

There is absolutely no valid reason for stopping a proxy's listeners this
early, we can postpone it after init_pollers() which will at least have
allocated fdtab.

MEDIUM: listeners: don't bounce listeners management between queues

During 2.1 development, commit f2cb16948 ("BUG/MAJOR: listener: fix
thread safety in resume_listener()") was introduced to bounce the
enabling/disabling of a listener's FD to one of its threads because
the remains of fd_update_cache() were fundamentally incompatible with
the need to call fd_want_recv() or fd_stop_recv() for another thread.

However since then we've totally dropped such code and it's totally
safe to use these functions on an FD that is solely used by another
thread (this is even used by the FD migration code). The only remaining
limitation concerning the wake up delay was addressed by previous commit
"MEDIUM: fd: always wake up one thread when enabling a foreing FD".

The current situation forces the FD management to remain in the
pause_listener() and resume_listener() functions just so that it can
bounce between threads, without having the ability to delegate it to
the suitable protocol layer.

So let's first remove this now unneeded workaround.

MEDIUM: fd: always wake up one thread when enabling a foreing FD

Since 2.2 it's safe to enable/disable another thread's FD but the fd_wake
calls will not immediately be considered because nothing wakes the other
threads up. This will have an impact on listeners when deciding to resume
them after they were paused, so at minima we want to wake up one of their
threads, just like the scheduler does on task_kill(). This is what this
patch does.

REGTESTS: mark abns_socket as broken

This test is inherently racy. It regularly pops up on the CI, and I've
spent one hour chasing a bug that apparently doesn't exist, just because
I'm running it 10 times in a row and it reports from 4 to 8 failures
when built at -O2 and generally even more at -O0. The logs are very
confusing, often reporting that it failed with status 0, with nothing
else wrong. I suspect it might sometimes be the shell command that fails
if it executes faster than haproxy finishes to start up, which would
also explain the relation with the optimization level. E.g:

>  Testing with haproxy version: 2.2.0
>  #    top  TEST reg-tests/seamless-reload/abns_socket.vtc FAILED (3.006) exit=2
>  #    top  TEST reg-tests/seamless-reload/abns_socket.vtc FAILED (3.006) exit=2
>  #    top  TEST reg-tests/seamless-reload/abns_socket.vtc FAILED (3.009) exit=2
>  #    top  TEST reg-tests/seamless-reload/abns_socket.vtc FAILED (3.008) exit=2
>  #    top  TEST reg-tests/seamless-reload/abns_socket.vtc FAILED (3.007) exit=2
>  #    top  TEST reg-tests/seamless-reload/abns_socket.vtc FAILED (3.007) exit=2
>  6 tests failed, 0 tests skipped, 4 tests passed

Some of the failures include this, suggesting that some barriers could
help:
  ---- h1   haproxy h1 PID file check failed:
       Could not read PID file '/tmp/haregtests-2020-10-09_11-19-40.kgsDB4/vtc.30539.04dbea7f/h1/pid

Since it has been causing false positives and consumed way more
troubleshooting time than it saved, let's mark it as broken so that it
doesn't waste more time. We can bring it back when someone manages to
figure what the problem is.

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.

BUG/MINOR: http: Fix content-length of the default 500 error

96 bytes is announce in the C-L header for a message of body of 97 bytes. This
bug was introduced by the patch 46a030cdd ("CLEANUP: assorted typo fixes in the
code and comments").

This patch must be backported in all versions where the patch above is (the 2.2
for now).

DOC: Fix typos in configuration.txt

This patch fixes small typos and grammar in configuration.txt for the
http-request return documentation.

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.

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.

DOC: Add missing stats fields in the management doc

Added latest fields: idle_conn_cur, safe_conn_cur, used_conn_cur, need_conn_est

CI: travis-ci: help Coverity to detect BUG_ON() as a real stop

Let's add DEBUG_STRICT=1 to coverity build definition. Hopefully,
it will resolve 1 coverity issue.

BUILD: makefile: Update feature flags for NetBSD

This updates the feature flags for NetBSD.

NetBSD 8 adds support for accept4().

Enable getaddrinfo().

BUG/MINOR: makefile: fix a tiny typo in the target list

Previous commit 382001b46 ("BUILD: Add a DragonFlyBSD target") introduced
a tiny typo in the target list ("iopenbs" vs "openbsd"). This will have to
be backported if that patch is backported.

BUILD: Add a DragonFlyBSD target

Add a target for DragonFlyBSD 4.3 and above.

DOC: fix a confusing typo on a regsub example

Sébastien reported a confusing example in the doc about regsub when used
with quotes. Nested quotes are already not trivial to grasp, but when
typos are there and result in something valid, it's even worse. The closing
quote ought to have been inside the brackets. However haproxy will not make
any difference because the single quotes delimit a word and the delimited
word remains the same. Let's just not add yet another level of confusion.

DOC: re-work log forward bind statement documentation.

This patch re-work the documentation about the bind statement
of log forward section.

BUG/MINOR: proxy/log: frontend/backend and log forward names must differ

This patch disallow to use same name for a log forward section
and a frontend/backend section.

MEDIUM: log: syslog TCP support on log forward section.

This patch re-introduce the "bind" statement on log forward
sections to handle syslog TCP listeners as defined in
rfc-6587.

As complement it introduce "maxconn", "backlog" and "timeout
client" statements to parameter those listeners.

MINOR: channel: new getword and getchar functions on channel.

This patch adds two new functions to get a char
or a word from a channel.

MINOR: stats: inc req counter on listeners.

This patch enables count of requests for listeners
if listener's counters are enabled.

BUG/MEDIUM: log: old processes with log foward section don't die on soft stop.

Old processes didn't die if a log foward section is declared and
a soft stop is requested.

This patch fix this issue and should be backpored in banches including
the log forward feature.

BUG/MINOR: proxy: inc req counter on new syslog messages.

Increase req counter instead of conn counter on
new syslog messages.

This should be backported on branches including the
syslog forward feature.

CLEANUP: sock-unix: Remove an unreachable goto clause

Coverity reported dead code in sock_unix_bind_receiver() function. A goto clause
is unreachable because of the preceeding if/else block.

This patch should fix the issue #865. No backport needed.

MINOR: mux-h1: Don't wakeup the H1C when output buffer become available

There is no reason to wake up the H1 connection when a new output buffer is
retrieved after an allocation failure because only the H1 stream will fill it.

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.

BUG/MINOR: mux-h1: Be sure to only set CO_RFL_READ_ONCE for the first read

The condition to set CO_RFL_READ_ONCE flag is not really accurate. We must check
the request state on frontend connection only and, in the opposite, the response
state on backend connection only. Only the parsed side must be considered, not
the opposite one.

This patch must be backported to 2.2.

CLEANUP: ssl: Release cached SSL sessions on deinit

On deinit, when the server SSL ctx is released, we must take care to release the
cached SSL sessions stored in the array <ssl_ctx.reused_sess>. There are
global.nbthread entries in this array, each one may have a pointer on a cached
session.

This patch should fix the issue #802. No backport needed.

CLEANUP: cache: Fix leak of cconf->c.name during config check

During the config check, the post parsing is not performed. Thus, cache filters
are not fully initialized and their cache name are never released. To be able to
release them, a flag is now set when a cache filter is fully initialized. On
deinit, if the flag is not set, it means the cache name must be freed.

The patch should fix #849. No backport needed.

[Cf: Tim is the patch author, but I added the commit message]

BUG/MINOR: proto_tcp: Report warning messages when listeners are bound

When a TCP listener is bound, in the tcp_bind_listener() function, a warning
message may be reported and should be displayed on verbose mode. But the warning
message is actually lost if the socket is successfully bound because we don't
fill the <errmsg> variable in this case.

This patch should fix the issue #863. No backport is needed.

DOC: update INSTALL with supported OpenBSD / FreeBSD versions

Update INSTALL with the OpenBSD / FreeBSD versions that are supported.

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.

DOC: ssl: fix typo about ocsp files

one of the last I saw in this section while working on github issue #872

might be backported in all still supported versions

Signed-off-by: William Dauchy <w.dauchy@criteo.com>

MINOR: stats: remove for loop declaration

Remove variable declaration inside a for-loop. This was introduced by my
patches serie of the implementation of dynamic stats. This is not
supported by older gcc, notably on the freebsd environment of the ci.

MINOR: dns/stats: integrate dns counters in stats

Use the new stats module API to integrate the dns counters in the
standard stats. This is done in order to avoid code duplication, keep
the code related to cli out of dns and use the full possibility of the
stats function, allowing to print dns stats in csv or json format.

MINOR: stats: add config "stats show modules"

By default, hide the extra statistics on the html page. Define a new
flag STAT_SHMODULES which is activated if the config "stats show
modules" is set.

MINOR: stats: display extra proxy stats on the html page

Integrate the additional proxy stats on the html stats page. For each
module, a new column is displayed with the individual stats available as
a tooltip.

MINOR: stats: support clear counters for dynamic stats

Add a boolean 'clearable' on stats module structure. If set, it forces
all the counters to be reset on 'clear counters' cli command. If not,
the counters are reset only when 'clear counters all' is used.

MEDIUM: stats: integrate static proxies stats in new stats

This is executed on startup with the registered statistics module. The
existing statistics have been merged in a list containing all
statistics for each domain. This is useful to print all available
statistics in a generic way.

Allocate extra counters for all proxies/servers/listeners instances.
These counters are allocated with the counters from the stats modules
registered on startup.

MEDIUM: stats: add abstract type to store counters

Implement a small API to easily add extra counters inside a structure
instance. This will be used to implement dynamic statistics linked on
every type of object as needed.

The counters are stored in a dynamic array inside the relevant objects.

MEDIUM: stats: define an API to register stat modules

A stat module can be registered to quickly add new statistics on
haproxy. It must be attached to one of the available stats domain. The
register must be done using INITCALL on STG_REGISTER.

The stat module has a name which should be unique for each new module in
a domain. It also contains a statistics list with their name/desc and a
pointer to a function used to fill the stats from the module counters.

The module also provides the initial counters values used on
automatically allocated counters. The offset for these counters
are stored in the module structure.

MEDIUM: stats: add delimiter for static proxy stats on csv

Use the character '-' to mark the end of static statistics on proxy
domain. After this marker, the order of the fields is not guaranteed and
should be parsed with care.

MINOR: stats: define additional flag px cap on domain

This flag can be used to determine on what type of proxy object the
statistics should be relevant. It will be useful when adding dynamic
statistics. Currently, this flag is not used.

MINOR: stats: define the concept of domain for statistics

The domain option will be used to have statistics attached to other
objects than proxies/listeners/servers. At the moment, only the PROXY
domain is available.

Add an argument 'domain' on the 'show stats' cli command to specify the
domain. Only 'domain proxy' is available now. If not specified, proxy
will be considered the default domain.

For HTML output, only proxy statistics will be displayed.

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.

REORG: stats: extract proxies dump loop in a function

Create a dedicated function to loop on proxies and dump them. This will
be clearer when other object will be dump as well.

This patch is needed to extend stat support to components other than
proxies objects.

REORG: stats: extract proxy json dump

Create a dedicated function to dump a proxy as a json content. This
patch will be needed when other types of objects will be available for
json dump.

This patch is needed to extend stat support to components other than
proxies objects.

MINOR: stats: hide px/sv/li fields in applet struct

Use an opaque pointer to store proxy instance. Regroup server/listener
as a single opaque pointer. This has the benefit to render the structure
more evolutive to support statistics on other types of objects in the
future.

This patch is needed to extend stat support for components other than
proxies objects.

The prometheus module has been adapted for these changes.

MINOR: stats: add stats size as a parameter for csv/json dump

Render the stats size parametric in csv/json dump functions. This is
needed for the future patch which provides dynamic stats. For now the
static value ST_F_TOTAL_FIELDS is provided.

Remove unused parameter px on stats_dump_one_line.

This patch is needed to extend stat support to components other than
proxies objects.

REORG: stats: export some functions

Un-mark stats_dump_one_line and stats_putchk as static and export them
in the header file. These functions will be reusable by other components to
print their statistics.

This patch is needed to extend stat support to components other than
proxies objects.

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.

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.

CLEANUP: ssl: "bundle" is not an OpenSSL wording

There is a confusion between the HAProxy bundle and OpenSSL. OpenSSL
does not have "bundles" but multiple certificates in the same store.

Fix a commentary in the crt-list code.

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+).

DOC: ssl: new "cert bundle" behavior

Update the documentation with the new bundle behavior which does not use
the same OpenSSL certificate store anymore but loads the PEM separately
as multiple "crt" were specified.

It should fix issue #872.

BUG/MINOR: tcpcheck: Set socks4 and send-proxy flags before the connect call

Since the health-check refactoring in the 2.2, the checks through a socks4 proxy
are broken. To fix this bug, CO_FL_SOCKS4 flag must be set on the connection
before calling the connect() callback function because this flags is checked to
use the right destination address. The same is done for the CO_FL_SEND_PROXY
flag for a consistency purpose.

A reg-test has been added to test the "check-via-socks4" directive.

This patch must be backported to 2.2.

MEDIUM: tcp-rules: Warn if a track-sc* content rule doesn't depend on content

The warning is only emitted for HTTP frontend. Idea is to encourage the usage of
"tcp-request session" rules to track counters that does not depend on the
request content. The documentation has been updated accordingly.

The warning is important because since the multiplexers were added in the
processing chain, the HTTP parsing is performed at a lower level. Thus parsing
errors are detected in the multiplexers, before the stream creation. In HTTP/2,
the error is reported by the multiplexer itself and the stream is never
created. This difference has a certain number of consequences, one of which is
that HTTP request counting in stick tables only works for valid H2 request, and
HTTP error tracking in stick tables never considers invalid H2 requests but only
invalid H1 ones. And the aim is to do the same with the mux-h1. This change will
not be done for the 2.3, but the 2.4. At the end, H1 and H2 parsing errors will
be caught by the multiplexers, at the session level. Thus, tracking counters at
the content level should be reserved for rules using a key based on the request
content or those using ACLs based on the request content.

To be clear, a warning will be emitted for the following rules :

  tcp-request content track-sc0 src
  tcp-request content track-sc0 src if ! { src 10.0.0.0/24 }
  tcp-request content track-sc0 src if { ssl_fc }

But not for the following ones :

  tcp-request content track-sc0 req.hdr(host)
  tcp-request content track-sc0 src if { req.hdr(host) -m found }

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.

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.

MINOR: ssl: remove uneeded check in crtlist_parse_file

this condition is never true as we either break or goto error, so those
two lines could be removed in the current state of the code.

this is fixing github issue #862

Signed-off-by: William Dauchy <w.dauchy@criteo.com>

MINOR: ssl: Add error if a crt-list might be truncated

Similar to warning during the parsing of the regular configuration file
that was added in 2fd5bdb439da29f15381aeb57c51327ba57674fc this patch adds
a warning to the parsing of a crt-list if the file does not end in a
newline (and thus might have been truncated).

The logic essentially just was copied over. It might be good to refactor
this in the future, allowing easy re-use within all line-based config
parsers.

see https://github.com/haproxy/haproxy/issues/860#issuecomment-693422936
see 0354b658f061d00d5ab4b728d7deeff2c8f1503a

This should be backported as a warning to 2.2.

CLEANUP: ssl: Use structured format for error line report during crt-list parsing

This reuses the known `parsing [%s:%d]:` from regular config file error
reporting.

BUILD: makefile: Enable closefrom() support on Solaris

Solaris 9 (released 2002) added support for closefrom().

I bumped the version in the comment to 10 as the default feature
flags already has event ports enabled which were introduced in
Solaris 10.

BUILD: tools: fix minor build issue on isspace()

Previous commit fa41cb679 ("MINOR: tools: support for word expansion
of environment in parse_line") introduced two new isspace() on a char
and broke the build on systems using an array disguised in a macro
instead of a function (like cygwin). Just use the usual cast.

MINOR: tools: support for word expansion of environment in parse_line

Allow the syntax "${...[*]}" to expand an environment variable
containing several values separated by spaces as individual arguments. A
new flag PARSE_OPT_WORD_EXPAND has been added to toggle this feature on
parse_line invocation. In case of an invalid syntax, a new error
PARSE_ERR_WRONG_EXPAND will be triggered.

This feature has been asked on the github issue #165.

BUILD: makefile: add an EXTRAVERSION variable to ease local naming

Sometimes it's desirable to append local version naming to packages,
and currently it can only be done using SUBVERS which is already set
by default to the git commit ID and patch count since last known tag,
making the addition a bit complicated.

Let's just add a new EXTRAVERSION field that is empty by default, and
systematically appended verbatim to the version string everywhere. This
way it becomes trivial to append some local strings, such as:

   make TARGET=foo EXTRAVERSION=+$(quilt applied|wc -l)
   -> 2.3-dev5-5018aa-15+1

or :

   make TARGET=foo EXTRAVERSION=-$(date +%F)
   -> 2.3-dev5-5018aa-15-20200110

Let's be careful not to add double quotes (used as the string delimiter)
nor spaces (which can confuse version parsers on the output). The extra
version is also used to name a tarball. It's always pre-initialized to an
empty string so that it's not accidently inherited from the environment.
It's not reported in "make version" to avoid fooling tools (it would be
pointless anyway).

As a side effect it also becomes possible to force VERSION and SUBVERS
to an empty string and use EXTRAVERSION alone to force a specific version
(could possibly be useful when bisecting from patch queues outside of Git
for example).

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.

OPTIM: backend: skip LB when we know the backend is full

For some algos (roundrobin, static-rr, leastconn, first) we know that
if there is any request queued in the backend, it's because a previous
attempt failed at finding a suitable server after trying all of them.
This alone is sufficient to decide that the next request will skip the
LB algo and directly reach the backend's queue. Doing this alone avoids
an O(N) lookup when load-balancing on a saturated farm of N servers,
which starts to be very expensive for hundreds of servers, especially
under the lbprm lock. This change alone has increased the request rate
from 110k to 148k RPS for 200 saturated servers on 8 threads, and
fwlc_reposition_srv() doesn't show up anymore in perf top. See github
issue #880 for more context.

It could have been the same for random, except that random is performed
using a consistent hash and it only considers a small set of servers (2
by default), so it may result in queueing at the backend despite having
some free slots on unknown servers. It's no big deal though since random()
only performs two attempts by default.

For hashing algorithms this is pointless since we don't queue at the
backend, except when there's no hash key found, which is the least of
our concerns here.

OPTIM: backend/random: never queue on the server, always on the backend

If random() returns a server whose maxconn is reached or the queue is
used, instead of adding the request to the server's queue, better add
it to the backend queue so that it can be served by any server (hence
the fastest one).

BUILD: makefile: Update feature flags for FreeBSD

This updates the feature flags for FreeBSD.

FreeBSD 10 adds support for accept4().

Enable getaddrinfo().

From the FreeBSD port / package.

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.

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".

REGTEST: make agent-check.vtc require 1.8

This one looks at the output of "show server state", which change slightly
in 1.8 (shows DNS fields).

REGTEST: the iif converter test requires 2.3

This one was recently added in 2.3.

REGTEST: make ssl_client_samples and ssl_server_samples requiret to 2.3

These ones added new sample fetches that are only available in 2.3 and
which fail on older versions.

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.

BUG/MINOR: ssl/crt-list: exit on warning out of crtlist_parse_line()

We should not exits on error out of the crtlist_parse_line() function.
The cfgerr error must be checked with the ERR_CODE mask.

Must be backported in 2.2.

DOC: crt: advise to move away from cert bundle

especially when starting to use `new ssl cert` runtime API, it might
become a bit confusing for users to mix bundle and single cert,
especially when it comes to use the commit command:
e.g.:
- start the process with `crt` loading a bundle
- use `set ssl cert my_cert.pem.ecdsa`: API detects it as a replacement
  of a bundle.
- `commit` has to be done on the bundle: `commit ssl cert my_cert.pem`

however:
- add a new cert: `new ssl cert my_cert.pem.rsa`: added as a single
  certificate
- `commit` has to be done on the certificate: `commit ssl cert
  my_cert.pem.rsa`

this should resolve github issue #872

this should probably be backported in >= v2.2 in order to encourage
people to move away from bundle certificates loading.

Signed-off-by: William Dauchy <w.dauchy@criteo.com>

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>

BUILD: makefile: Update feature flags for OpenBSD

Update the OpenBSD target features being enabled.

I updated the list of features after noticing
"BUILD: makefile: disable threads by default on OpenBSD".

The Makefile utilizing gcc(1) by default resulted in utilizing
our legacy and obsolete compiler (GCC 4.2.1) instead of the
proper system compiler (Clang), which does support TLS. With
"BUILD: makefile: change default value of CC from gcc to cc"
that is resolved.

REGTESTS: use "command" instead of "which" for better POSIX compatibility

for example, "which" is not installed by default in Fedora docker image.

[RELEASE] Released version 2.3-dev5

Released version 2.3-dev5 with the following main changes :
    - DOC: Fix typo in iif() example
    - CLEANUP: Update .gitignore
    - BUILD: introduce possibility to define ABORT_NOW() conditionally
    - CI: travis-ci: help Coverity to recognize abort()
    - BUG/MINOR: Fix type passed of sizeof() for calloc()
    - CLEANUP: Do not use a fixed type for 'sizeof' in 'calloc'
    - CLEANUP: tree-wide: use VAR_ARRAY instead of [0] in various definitions
    - BUILD: connection: fix build on clang after the VAR_ARRAY cleanup
    - BUG/MINOR: ssl: verifyhost is case sensitive
    - BUILD: makefile: change default value of CC from gcc to cc
    - CI: travis-ci: split asan step out of running tests
    - BUG/MINOR: server: report correct error message for invalid port on "socks4"
    - BUG/MEDIUM: ssl: Don't call ssl_sock_io_cb() directly.
    - BUG/MINOR: ssl/crt-list: crt-list could end without a \n
    - BUG/MINOR: log-forward: fail on unknown keywords
    - MEDIUM: log-forward: use "dgram-bind" instead of "bind" for the listener
    - BUG/MEDIUM: log-forward: always quit on parsing errors
    - MEDIUM: ssl: remove bundle support in crt-list and directories
    - MEDIUM: ssl/cli: remove support for multi certificates bundle
    - MINOR: ssl: crtlist_dup_ssl_conf() duplicates a ssl_bind_conf
    - MINOR: ssl: crtlist_entry_dup() duplicates a crtlist_entry
    - MEDIUM: ssl: emulates the multi-cert bundles in the crtlist
    - MEDIUM: ssl: emulate multi-cert bundles loading in standard loading
    - CLEANUP: ssl: remove test on "multi" variable in ckch functions
    - CLEANUP: ssl/cli: remove test on 'multi' variable in CLI functions
    - CLEANUP: ssl: remove utility functions for bundle
    - DOC: explain bundle emulation in configuration.txt
    - BUILD: fix build with openssl < 1.0.2 since bundle removal
    - BUG/MINOR: log: gracefully handle the "udp@" address format for log servers
    - BUG/MINOR: dns: gracefully handle the "udp@" address format for nameservers
    - MINOR: listener: create a new struct "settings" in bind_conf
    - MINOR: listener: move bind_proc and bind_thread to struct settings
    - MINOR: listener: move the interface to the struct settings
    - MINOR: listener: move the network namespace to the struct settings
    - REORG: listener: create a new struct receiver
    - REORG: listener: move the listening address to a struct receiver
    - REORG: listener: move the receiving FD to struct receiver
    - REORG: listener: move the listener's proto to the receiver
    - MINOR: listener: make sock_find_compatible_fd() check the socket type
    - REORG: listener: move the receiver part to a new file
    - MINOR: receiver: link the receiver to its settings
    - MINOR: receiver: link the receiver to its owner
    - MINOR: listener: prefer to retrieve the socket's settings via the receiver
    - MINOR: receiver: add a receiver-specific flag to indicate the socket is bound
    - MINOR: listener: move the INHERITED flag down to the receiver
    - MINOR: receiver: move the FOREIGN and V6ONLY options from listener to settings
    - MINOR: sock: make sock_find_compatible_fd() only take a receiver
    - MINOR: protocol: rename the ->bind field to ->listen
    - MINOR: protocol: add a new ->bind() entry to bind the receiver
    - MEDIUM: sock_inet: implement sock_inet_bind_receiver()
    - MEDIUM: tcp: make use of sock_inet_bind_receiver()
    - MEDIUM: udp: make use of sock_inet_bind_receiver()
    - MEDIUM: sock_unix: implement sock_unix_bind_receiver()
    - MEDIUM: uxst: make use of sock_unix_bind_receiver()
    - MEDIUM: sockpair: implement sockpair_bind_receiver()
    - MEDIUM: proto_sockpair: make use of sockpair_bind_receiver()
    - MEDIUM: protocol: explicitly start the receiver before the listener
    - MEDIUM: protocol: do not call proto->bind() anymore from bind_listener()
    - MINOR: protocol: add a new proto_fam structure for protocol families
    - MINOR: protocol: retrieve the family-specific fields from the family
    - CLEANUP: protocol: remove family-specific fields from struct protocol
    - MINOR: protocol: add a real family for existing FDs
    - CLEANUP: tools: make str2sa_range() less awful for fd@ and sockpair@
    - MINOR: tools: make str2sa_range() take more options than just resolve
    - MINOR: tools: add several PA_O_PORT_* flags in str2sa_range() callers
    - MEDIUM: tools: make str2sa_range() validate callers' port specifications
    - MEDIUM: config: remove all checks for missing/invalid ports/ranges
    - MINOR: tools: add several PA_O_* flags in str2sa_range() callers
    - MINOR: listener: remove the inherited arg to create_listener()
    - MINOR: tools: make str2sa_range() optionally return the fd
    - MINOR: log: detect LOG_TARGET_FD from the fd and not from the syntax
    - MEDIUM: tools: make str2sa_range() resolve pre-bound listeners
    - MINOR: config: do not test an inherited socket again
    - MEDIUM: tools: make str2sa_range() check for the sockpair's FD usability
    - MINOR: tools: start to distinguish stream and dgram in str2sa_range()
    - MEDIUM: tools: make str2sa_range() only report AF_CUST_UDP on listeners
    - MINOR: tools: remove the central test for "udp" in str2sa_range()
    - MINOR: cfgparse: add str2receiver() to parse dgram receivers
    - MINOR: log-forward: use str2receiver() to parse the dgram-bind address
    - MEDIUM: config: make str2listener() not accept datagram sockets anymore
    - MINOR: listener: pass the chosen protocol to create_listeners()
    - MINOR: tools: make str2sa_range() directly return the protocol
    - MEDIUM: tools: make str2sa_range() check that the protocol has ->connect()
    - MINOR: protocol: add the control layer type in the protocol struct
    - MEDIUM: protocol: store the socket and control type in the protocol array
    - MEDIUM: tools: make str2sa_range() use protocol_lookup()
    - MEDIUM: proto_udp: replace last AF_CUST_UDP* with AF_INET*
    - MINOR: tools: drop listener detection hack from str2sa_range()
    - BUILD: sock_unix: add missing errno.h
    - MINOR: sock_inet: report the errno string in binding errors
    - MINOR: sock_unix: report the errno string in binding errors
    - BUILD: sock_inet: include errno.h
    - MINOR: h2/trace: also display the remaining frame length in traces
    - BUG/MINOR: h2/trace: do not display "stream error" after a frame ACK
    - BUG/MEDIUM: h2: report frame bits only for handled types
    - BUG/MINOR: http-fetch: Don't set the sample type during the htx prefetch
    - BUG/MINOR: Fix memory leaks cfg_parse_peers
    - BUG/MINOR: config: Fix memory leak on config parse listen
    - 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
    - BUG/MEDIUM: listeners: do not pause foreign listeners
    - BUG/MINOR: listeners: properly close listener FDs
    - BUILD: trace: include tools.h

BUILD: trace: include tools.h

If the TRACE option is used when compiling the haproxy source,
the following error occurs on debian 9.13:

src/calltrace.o: In function `make_line':
.../src/calltrace.c:204: undefined reference to `rdtsc'
src/calltrace.o: In function `calltrace':
.../src/calltrace.c:277: undefined reference to `rdtsc'
collect2: error: ld returned 1 exit status
Makefile:866: recipe for target 'haproxy' failed