MINOR: tcp: Return TCP statistics like RTT and RTT variance

This patch adds 4 new sample fetches which returns the RTT of the
established connexion and the RTT variance. The established connection
can be between the client and HAProxy, and between HAProxy and the
server. This is very useful for statistics. A great use case is the
estimation of the TCP connection time of the client. Note that the
RTT of the server side is not so interesting because we already have
the connect() time.

BUG/MEDIUM: log: use function "escape_string" instead of "escape_chunk"

In function lf_text_len(), we used escape_chunk() to escape special
characters. There could be a problem if len is greater than the real src
string length (zero-terminated), eg. when calling lf_text_len() from
lf_text().

MINOR: standard: add function "escape_string"

Similar to "escape_chunk", this function tries to prefix all characters
tagged in the <map> with the <escape> character. The specified <string>
contains the input to be escaped.

BUG/MINOR: peers: don't count track-sc multiple times on errors

Ruoshan Huang found that the call to session_inc_http_err_ctr() in the
recent http-response patch was not a good idea because it also increments
counters that are already tracked (eg: http-request track-sc or previous
http-response track-sc). Better open-code the update, it's simple.

BUG/MINOR: peers: Fix peers data decoding issue

This error led to truncated data after decoding upon receipt.
It's specific to peers v2 and needs to be backported to 1.6.

MEDIUM: http: implement http-response track-sc* directive

This enables tracking of sticky counters from current response. The only
difference from "http-request track-sc" is the <key> sample expression
can only make use of samples in response (eg. res.*, status etc.) and
samples below Layer 6.

BUG/MEDIUM: lua: the function txn_done() from action wrapper can crash

If an action wrapper stops the processing of the transaction
with a txn_done() function, the return code of the action is
"continue". So the continue can implies the processing of other
like adding headers. However, the HTTP content is flushed and
a segfault occurs.

This patchs add a flag indicating that the Lua code want to
stop the processing, ths flags is forwarded to the haproxy core,
and other actions are ignored.

Must be backported in 1.6

BUG/MEDIUM: lua: the function txn_done() from sample fetches can crash

The function txn_done() ends a transaction. It does not make
sense to call this function from a lua sample-fetch wrapper,
because the role of a sample-fetch is not to terminate a
transaction.

This patch modify the role of the fucntion txn_done() if it
is called from a sample-fetch wrapper, now it just ends the
execution of the Lua code like the done() function.

Must be backported in 1.6

BUG/MINOR: Fix endiness issue in DNS header creation code

Alexander Lebedev reported that the response bit is set on SPARC when
DNS queries are sent. This has been tracked to the endianess issue, so
this patch makes the code portable.

Signed-off-by: Nenad Merdanovic <nmerdan@anine.io>

BUG/MEDIUM: dns: fix alignment issues in the DNS response parser

Alexander Lebedev reported that the DNS parser crashes in 1.6 with a bus
error on Sparc when it receives a response. This is obviously caused by
some alignment issues. The issue can also be reproduced on ARMv5 when
setting /proc/cpu/alignment to 4 (which helps debugging).

Two places cause this crash in turn, the first one is when the IP address
from the packet is compared to the current one, and the second place is
when the address is assigned because an unaligned address is passed to
update_server_addr().

This patch modifies these places to properly use memcpy() and memcmp()
to manipulate the unaligned data.

Nenad Merdanovic found another set of places specific to 1.7 in functions
in_net_ipv4() and in_net_ipv6(), which are used to compare networks. 1.6
has the functions but does not use them. There we perform a temporary copy
to a local variable to fix the problem. The type of the function's argument
is wrong since it's not necessarily aligned, so we change it for a const
void * instead.

This fix must be backported to 1.6. Note that in 1.6 the code is slightly
different, there's no rec[] array, the pointer is used directly from the
buffer.

BUG/MINOR: ssl: fix potential memory leak in ssl_sock_load_dh_params()

Roberto Guimaraes reported that Valgrind complains about a leak
in ssl_get_dh_1024().
This is caused caused by an oversight in ssl_sock_load_dh_params(),
where local_dh_1024 is always replaced by a new DH object even if
it already holds one. This patch simply checks whether local_dh_1024
is NULL before calling ssl_get_dh_1024().

DOC: spelling fixes

[wt: most of them are probably valid for 1.6 and 1.5 as well]

CLEANUP: connection: using internal struct to hold source and dest port.

Originally, tcphdr's source and dest from Linux were used to get the
source and port which led to a build issue on BSD oses.
To avoid side problems related to network then we just use an internal
struct as we need only those two fields.

Revert "BUG/MINOR: ssl: fix potential memory leak in ssl_sock_load_dh_params()"

This reverts commit 0ea4c23ca754c3e6c005b67403a0619ca17d4587.

Certain very simple confs randomly segfault upon startup with openssl 1.0.2
with this patch, which seems to indicate a use after free. Better drop it
and let valgrind complain about the potential leak.

Also it's worth noting that the man page for SSL_CTX_set_tmp_dh() makes no
mention about whether or not the element should be freed, and the example
provided does not use it either.

This fix should be backported to 1.6 and 1.5 where the patch was just
included.

CLEANUP: fixed some usages of realloc leading to memory leak

Changed all the cases where the pointer passed to realloc is overwritten
by the pointer returned by realloc. The new function my_realloc2 has
been used except in function register_name. If register_name fails to
add a new variable because of an "out of memory" error, all the existing
variables remain valid. If we had used my_realloc2, the array of variables
would have been freed.

MINOR: new function my_realloc2 = realloc + free upon failure

When realloc fails to allocate memory, the original pointer is not
freed. Sometime people override the original pointer with the pointer
returned by realloc which is NULL in case of failure. This results
in a memory leak because the memory pointed by the original pointer
cannot be freed.

BUG/MINOR: Rework slightly commit 9962f8fc to clean code and avoid mistakes

In commit 9962f8fc (BUG/MEDIUM: http: unbreak uri/header/url_param hashing), we
take care to update 'msg->sov' value when the parser changes to state
HTTP_MSG_DONE. This works when no filter is used. But, if a filter is used and
if it loops on 'http_end' callback, the following block is evaluated two times
consecutively:

    if (unlikely(!(chn->flags & CF_WROTE_DATA) || msg->sov > 0))
            msg->sov -= ret;

Today, in practice, because this happens when all data are parsed and forwarded,
the second test always fails (after the first update, msg->sov is always lower
or equal to 0). But it is useless and error prone. So to avoid misunderstanding
the code has been slightly changed. Now, in all cases, we try to update msg->sov
only once per iteration.

No backport is needed.

BUG/MEDIUM: http: unbreak uri/header/url_param hashing

Vedran Furac reported that "balance uri" doesn't work anymore in recent
1.7-dev versions. Dragan Dosen found that the first faulty commit was
dbe34eb ("MEDIUM: filters/http: Move body parsing of HTTP messages in
dedicated functions"), merged in 1.7-dev2.

After this patch, the hashing is performed on uninitialized data,
indicating that the buffer is not correctly rewound. In fact, all forms
of content-based hashing are broken since the commit above. Upon code
inspection, it appears that the new functions http_msg_forward_chunked_body()
and http_msg_forward_body() forget to rewind the buffer in the success
case, when the parser changes to state HTTP_MSG_DONE. The rewinding code
was reinserted in both functions and the fix was confirmed by two test,
with and without chunking.

No backport it needed.

CLEANUP: dumpstats: u64 field is an unsigned type.

The check will always be true, thus unnecessary.

DOC: add missing doc for http-request deny [deny_status <status>]

The feature was introduced in 1.6-dev2 by commit 108b1dd ("MEDIUM:
http: configurable http result codes for http-request deny") but the
doc was missing. Thanks to Cyril for noticing.

This must be backported into 1.6.

BUG/BUILD: don't automatically run "make" on "make install"

Kay Fuchs reported that the recent changes to automatically rebuild files
on config option changes caused "make install" to rebuild the whole code
with the wrong options. That's caused by the fact that the "install-bin"
target depends on the "haproxy" target, which detects the lack of options
and causes a rebuild with different ones.

This patch makes a simple change, it removes this automatic dependency
which was already wrong since it could cause some files to be built with
different options prior to these changes, and instead emits an error
message indicating that "make" should be run prior to "make install".

The patches were backported into 1.6 so this fix must go there as well.

BUG/MINOR: http: fix misleading error message for response captures

Kay Fuchs reported that the error message is misleading in response
captures because it suggests that "len" is accepted while it's not.

This needs to be backported to 1.6.

BUG/MINOR: ssl: close ssl key file on error

Explicitly close the FILE opened to read the ssl key file when parsing
fails to find a valid key.

This fix needs to be backported to 1.6.

BUG/MINOR: srv-state: fix incorrect output of state file

Eric Webster reported that the state file wouldn't reload in 1.6.5
while it used to work in 1.6.4. The issue is that headers are now
missing from the output when a specific backend is dumped since
commit 4c1544d ("BUG/MEDIUM: stats: show servers state may show an
empty or incomplete result"). This patch fixes this by introducing
a dump state.

It must be backported to 1.6.

BUG/MINOR: filters: Fix HTTP parsing when a filter loops on data forwarding

A filter can choose to loop on data forwarding. When this loop occurs in
HTTP_MSG_ENDING state, http_foward_data callbacks are called twice because of a
goto on the wrong label.

A filter can also choose to loop at the end of a HTTP message, in http_end
callback. Here the goto is good but the label is not at the right place. We must
be sure to upate msg->sov value.

BUG/MEDIUM: filters: Fix data filtering when data are modified

Filters can alter data during the parsing, i.e when http_data or tcp_data
callbacks are called. For now, the update must be done by hand. So we must
handle changes in the channel buffers, especially on the number of input bytes
pending (buf->i).
In addition, a filter can choose to switch channel buffers to do its
updates. So, during data filtering, we must always use the right buffer and not
use variable to reference them.

Without this patch, filters cannot safely alter data during the data parsing.

CLEANUP: external-check: don't block/unblock SIGCHLD when manipulating the list

There's no point in blocking/unblocking sigchld when removing entries
from the list since the code is called asynchronously.

Similarly the blocking/unblocking could be removed from the connect_proc_chk()
function but it happens that at high signal rates, fork() takes twice as much
time to execute as it is regularly interrupted by a signal, so in the end this
signal blocking is beneficial there for performance reasons.

BUG/MINOR: external-checks: do not unblock undesired signals

The external checks code makes use of block_sigchld() and unblock_sigchld()
to ensure nobody modifies the signals list while they're being manipulated.
It happens that these functions clear the list of blocked signals, so they
can possibly have a side effect if other signals are blocked. For now no
other signal is blocked but it may very well change in the future so rather
correctly use SIG_BLOCK/SIG_UNBLOCK instead of touching unrelated signals.

This fix should be backported to 1.6 for correctness.

BUG/MAJOR: external-checks: use asynchronous signal delivery

There are random segfaults occuring when using external checks. The
reason is that when receiving a SIGCHLD, a call to task_wakeup() is
performed. There are two situations where this causes trouble :
  - the scheduler is in process_running_tasks(), since task_wakeup()
    sets rq_next to NULL, when the former dereferences it to get the
    next pointer, the program crashes ;

  - when another task_wakeup() is being called and during eb_next()
    in process_running_tasks(), because the structure of the run queue
    tree changes while it is being processed.

The solution against this is to use asynchronous signal processing
thanks to the internal signal API. The signal handler is registered,
and upon delivery, the signal is added to the queue and processed
out of any other processing.

This code was stressed at 2500 forks/s and their respective signals
for quite some time and the segfault is now gone.

BUG/MEDIUM: external-checks: close all FDs right after the fork()

Lukas Erlacher reported an interesting problem : since we don't close
FDs after the fork() upon external checks, any script executed that
writes data on stdout/stderr will possibly send its data to wrong
places, very likely an existing connection.

After some analysis, the problem is even wider. It's not enough to
just close stdin/stdout/stderr, as all sockets are passed to the
sub-process, and as long as they're not closed, they are usable for
whatever mistake can be done. Furthermore with epoll, such FDs will
continue to be reported after a close() as the underlying file is
not closed yet.

CLOEXEC would be an acceptable workaround except that 1) it adds an
extra syscall on the fast path, and 2) we have no control over FDs
created by external libs (eg: openssl using /dev/crypto, libc using
/dev/random, lua using anything else), so in the end we still need
to close them all.

On some BSD systems there's a closefrom() syscall which could be
very useful for this.

Based on an insightful idea from Simon Horman, we don't close 0/1/2
when we're in verbose mode since they're properly connected to
stdin/stdout/stderr and can become quite useful during debugging
sessions to detect some script output errors or execve() failures.

This fix must be backported to 1.6.

BUG/MINOR: init: ensure that FD limit is raised to the max allowed

When the requested amount of FDs cannot be allocated, setrlimit() fails.
That's bad because if the limit is set to 1024 and we need 10000, we
stay on 1024 while we could possibly raise it to 4096 thanks to rlim_max.
This patch takes care of trying to assign rlim_cur to rlim_max on failure
so that we get as much as possible if we can't get all we need. The case
is particularly visible when starting haproxy as a non-privileged user
and a large maxconn is specified in the configuration.

Another point of doing this is that it is the only way to allow us to
close inherited FDs upon fork(), ie those between rlim_cur and rlim_max.

This patch may be backported to 1.6 and 1.5.

BUG/MINOR: init: always ensure that global.rlimit_nofile matches actual limits

global.rlimit_nofile contains the mxa number of file descriptors that
can be allocated, except if the user is not allowed to reach this limit,
where it still contains the initially requested value. It is important
that this value always matches what is really configured so that it is
properly reported in the stats and that we can use it later to close
all FDs without wasting time closing impossible FDs.

This fix may be backported to 1.6 and 1.5.

MINOR: tcp: add "tcp-request connection expect-netscaler-cip layer4"

This configures the client-facing connection to receive a NetScaler
Client IP insertion protocol header before any byte is read from the
socket. This is equivalent to having the "accept-netscaler-cip" keyword
on the "bind" line, except that using the TCP rule allows the PROXY
protocol to be accepted only for certain IP address ranges using an ACL.
This is convenient when multiple layers of load balancers are passed
through by traffic coming from public hosts.

MINOR: listener: add the "accept-netscaler-cip" option to the "bind" keyword

When NetScaler application switch is used as L3+ switch, informations
regarding the original IP and TCP headers are lost as a new TCP
connection is created between the NetScaler and the backend server.

NetScaler provides a feature to insert in the TCP data the original data
that can then be consumed by the backend server.

Specifications and documentations from NetScaler:
  https://support.citrix.com/article/CTX205670
  https://www.citrix.com/blogs/2016/04/25/how-to-enable-client-ip-in-tcpip-option-of-netscaler/

When CIP is enabled on the NetScaler, then a TCP packet is inserted just after
the TCP handshake. This is composed as:

  - CIP magic number : 4 bytes
    Both sender and receiver have to agree on a magic number so that
    they both handle the incoming data as a NetScaler Client IP insertion
    packet.

  - Header length : 4 bytes
    Defines the length on the remaining data.

  - IP header : >= 20 bytes if IPv4, 40 bytes if IPv6
    Contains the header of the last IP packet sent by the client during TCP
    handshake.

  - TCP header : >= 20 bytes
    Contains the header of the last TCP packet sent by the client during TCP
    handshake.

BUILD: ssl: fix typo causing a build failure in the multicert patch

I just noticed that SSL wouldn't build anymore since this afternoon's patch :

src/ssl_sock.c: In function 'ssl_sock_load_multi_cert':
src/ssl_sock.c:1982:26: warning: left-hand operand of comma expression has no effect [-Wunused-value]
    for (i = 0; i < fcount, i++)
                          ^
src/ssl_sock.c:1982:31: error: expected ';' before ')' token
    for (i = 0; i < fcount, i++)
                               ^
Makefile:791: recipe for target 'src/ssl_sock.o' failed
make: *** [src/ssl_sock.o] Error 1

MINOR: ssl: crt-list parsing factor

LINESIZE and MAX_LINE_ARGS are too low for parsing crt-list.

MEDIUM: ssl: support SNI filters with multicerts

SNI filters used to be ignored with multicerts (eg: those providing
ECDSA and RSA at the same time). This patch makes them work like
other certs.

Note: most of the changes in this patch are due to an extra level of
      indent, read it with "git show -b".

MINOR: systemd: Perform sanity check on config before reload

MINOR: systemd: Use variable for config and pidfile paths

Users can set the location of haproxy.cfg and pidfile files by providing
a systemd overwrite file /etc/systemd/system/haproxy.service.d/overwrite.conf
with the following content:

    [Service]
    Environment=CONFIG=/etc/foobar/haproxy.cfg

BUG/MINOR: fix http-response set-log-level parsing error

hi,
    `http-response set-log-level` doesn't work, as the config parsing always set the log level to -1.

From 2b183447c5b37c19aae5d596871fc0b9004c87b4 Mon Sep 17 00:00:00 2001
From: Ruoshan Huang <ruoshan.huang@gmail.com>
Date: Wed, 15 Jun 2016 22:07:58 +0800
Subject: [PATCH] BUG/MINOR: fix http-response set-log-level parsing error

http-response set-log-level can't parse the log level correctly,
as the level argument ptr is one byte ahead when passed to get_log_level
---
 src/proto_http.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

DOC: http: add documentation for url32 and url32+src

This patch needs to be backported to 1.6 and 1.5.

BUG/MINOR: http: url32+src should check cli_conn before using it

In function smp_fetch_url32_src(), it's better to check the value of
cli_conn before we go any further.

This patch needs to be backported to 1.6 and 1.5.

BUG/MINOR: http: url32+src should use the big endian version of url32

This is similar to the commit 5ad6e1dc ("BUG/MINOR: http: base32+src
should use the big endian version of base32"). Now we convert url32 to big
endian when building the binary block.

This patch needs to be backported to 1.6 and 1.5.

MEDIUM: dumpstats: make stats_tlskeys_list() yield-aware during tls-keys dump

The previous dump algorithm was not trying to yield when the buffer is
full, it's not a problem with the TLS_TICKETS_NO which is 3 by default
but it can become one if the buffer size is lowered and if the
TLS_TICKETS_NO is increased.

The index of the latest ticket dumped is now stored to ensure we can
resume the dump after a yield.

BUG/MEDIUM: dumpstats: undefined behavior in stats_tlskeys_list()

The function stats_tlskeys_list() can meet an undefined behavior when
called with appctx->st2 == STAT_ST_LIST, indeed the ref pointer is used
uninitialized.

However this function was using NULL in appctx->ctx.tlskeys.ref as a
flag to dump every tickets from every references.  A real flag
appctx->ctx.tlskeys.dump_all is now used for this behavior.

This patch delete the 'ref' variable and use appctx->ctx.tlskeys.ref
directly.

MINOR: stream: export the function 'smp_create_src_stkctr'

Could be useful outside of this file.

BUG/MINOR: ssl: fix potential memory leak in ssl_sock_load_dh_params()

Valgrind reports that the memory allocated in ssl_get_dh_1024() was leaking. Upon further inspection of openssl code, it seems that SSL_CTX_set_tmp_dh makes a copy of the data, so calling DH_free afterwards makes sense.

BUG/MEDIUM: http: add-header: buffer overwritten

If we use the action "http-request add-header" with a Lua sample-fetch or
converter, and the Lua function calls one of the Lua log function, the
header name is corrupted, it contains an extract of the last loggued data.

This is due to an overwrite of the trash buffer, because his scope is not
respected in the "add-header" function. The scope of the trash buffer must
be limited to the function using it. The build_logline() function can
execute a lot of other function which can use the trash buffer.

This patch fix the usage of the trash buffer. It limits the scope of this
global buffer to the local function, we build first the header value using
build_logline, and after we store the header name.

Thanks Michael Ezzell for the repporting.

This patch must be backported in 1.6 version

BUG/MINOR: http: add-header: header name copied twice

The header name is copied two time in the buffer. The first copy is a printf-like
function writing the name and the http separators in the buffer, and the second
form is a memcopy. This seems to be inherited from some changes. This patch
removes the printf like, format.

This patch must be backported in 1.6 and 1.5 versions

BUG/MEDIUM: lua: converters doesn't work

The number of arguments pushed in the stack are false, so we try to execute a
function out of the stack. This function is always a nil pointer, so the
following message is displayed.

   Lua converter 'testconv': runtime error: attempt to call a nil value.

Thanks Michael Ezzell for the repporting.

This patch must be backported in the 1.6 version.

BUILD/MEDIUM: force a full rebuild if some build options change

We now instrument the makefile to keep a copy of previous build options.
The goal is to ensure that we'll rebuild everything when build options
change. The options that are watched are TARGET, VERBOSE_CFLAGS, and
BUILD_OPTIONS. These ones are copied into a file ".build_opts" and
compared to the new ones upon each build. This file is referenced in
the DEP variable which all .o files depend on, and it depends on the
code which updates it only upon changes. This ensures that a new file
is regenerated and detected upon change and that everything is rebuilt.

BUILD/MEDIUM: rebuild everything when an include file is changed

Some users tend to get caught by incorrect builds when they try patches
that modify some include file after they forget to run "make clean".
While we can't blame users who are not developers, forcing developers
to rely on a painful autodepend is not nice either and will cause them
to test their changes less often. Here we propose a reasonable tradeoff.
This patch introduces a new "INCLUDES" variable which enumerates all
the ".h" files and sets them as a build dependency for all ".o" files.
This list is then copied into a "DEP" variable which can safely be
overridden if desired. This way by default all .c files are rebuilt if
any include file changes. This is the safe method for all users. And
developers can simply add "DEP=" to their quick build scripts to keep
the old fast and efficient behaviour.

BUG/MEDIUM: sticktables: segfault in some configuration error cases

When a stick table is tracked, and another one is used later on the
configuration, a segfault occurs.

The function "smp_create_src_stkctr" can return a NULL value, and
its value is not tested, so one other function try to dereference
a NULL pointer. This patch just add a verification of the NULL
pointer.

The problem is reproduced with this configuration:

   listen www
       mode http
       bind :12345
       tcp-request content track-sc0 src table IPv4
       http-request allow if { sc0_inc_gpc0(IPv6) gt 0 }
       server dummy 127.0.0.1:80
   backend IPv4
       stick-table type ip size 10 expire 60s store gpc0
   backend IPv6
       stick-table type ipv6 size 10 expire 60s store gpc0

Thank to kabefuna@gmail.com for the bug report.

This patch must be backported in the 1.6 and 1.5 version.

MEDIUM: tcp/http: new set-dst/set-dst-port actions

Like 'set-src' and 'set-src-port' but for destination address and port.
It's available in 'tcp-request connection' and 'http-request' actions.

MEDIUM: tcp/http: add 'set-src-port' action

set-src-port works the same way as 'set-src' but for the source port.
It's available in 'tcp-request connection' and 'http-request' actions.

MINOR: set the CO_FL_ADDR_FROM_SET flags with 'set-src'

When the 'set-src' action is used, the CO_FL_ADDR_FROM_SET wasn't set,
it can lead to address being rewritten.

MEDIUM: tcp: add 'set-src' to 'tcp-request connection'

The 'set-src' action was not available for tcp actions The action code
has been converted into a function in proto_tcp.c to be used for both
'http-request' and 'tcp-request connection' actions.

Both http and tcp keywords are registered in proto_tcp.c

DOC: update doc about tls-tickets-keys dump

The unix socket can now dump the tls-tickets-keys.

MEDIUM: dumpstats: 'show tls-keys' is now able to show secrets

You can now dump the tls-tickets-keys from the unix socket using the
file ID prefixed by an '#' or using a '*' to dump everything.

BUG/MEDIUM: fix risk of segfault with "show tls-keys"

The reference to the tls_keys_ref was not deleted from the
tlskeys_reference linked list.

When the SSL is malconfigured, it can lead to an access to freed memory
during a "show tls-keys" on the admin socked.

BUG/MEDIUM: stats: show servers state may show an servers from another backend

Olivier Doucet reported that "show servers state" was producing an invalid
output with some configurations where nbproc > 1.

Indeed, commit 76a99784f4 fixed some issues but unfortunately introduced a
regression when a backend bound to the same process as the stats socket and a
previous backend is bound to another one.

For example :
  global
    daemon
    nbproc 2
    stats socket /var/run/haproxy-1.sock process 1
    stats socket /var/run/haproxy-2.sock process 2

  listen proc1
    bind 127.0.0.1:9001
    bind-process 1
    server WRONG 127.0.0.1:80

  listen proc2
    bind 127.0.0.1:9002
    bind-process 2
    server RIGHT 127.0.0.1:80

Requesting "show servers state" on /var/run/haproxy-2.sock was producing a line
like :
3 proc2 1 WRONG 127.0.0.1 2 0 1 1 4 1 0 2 0 0 0 0

whereas the line below was awaited :
3 proc2 1 RIGHT 127.0.0.1 2 0 1 1 5 1 0 2 0 0 0 0

This was caused by the initialization of the server loop too early, before the
bind_proc filtering whereas it should be done after.

This fix should be backported to 1.6, where the regression has unfortunately
been backported.

BUG/MEDIUM: config: fix multiple declaration of section parsers

Ben Cabot reported that after commit 5e4261b ("CLEANUP: config:
detect double registration of a config section") recently introduced
in 1.7-dev, it's not possible anymore to load multiple configuration
files. Bryan Talbot provided a simple reproducer to exhibit the issue.

It turns out that function readcfgfile() registers new parsers for
section keywords for each new file. In addition to being useless, this
has the negative effect of wasting memory and slowing down the config
parser as the number of configuration files increases.

This fix only needs to be backported if/where the commit above is
backported.

BUILD: fix build on Solaris 11

htonll()/ntohll() already exist on Solaris 11 with a different declaration,
causing a build error as reported by Jonathan Fisher. They used to exist on
OSX with a #define which allowed us to detect them. It was a bad idea to give
these functions a name subject to conflicts like this. Simply rename them
my_htonll()/my_ntohll() to definitely get rid of the conflict.

This patch must be backported to 1.6.

BUG/MEDIUM: dns: unbreak DNS resolver after header fix

DNS requests (using the internal resolver) are corrupted since commit
e2f84977165a ("BUG/MINOR: dns: fix DNS header definition").

Fix it by defining the struct in network byte order, while complying
with RFC 2535, section 6.1.

First reported by Eduard Vopicka on discourse.

This must be backported to 1.6 (1.6.5 is affected).

MINOR: stick-table: change all stick-table converters' inputs to SMP_T_ANY

The stick-table converters used to take a string on input because it
was the only type that could be casted to from any other type. This is
inefficient and possibly inaccurate sometimes. For example in order to
look up an IP address, it must first be converted to a string then
converted back to an IP address.

We've had SMP_T_ANY introduced long ago in 1.6, but unfortunately it
was not propagated to these converters, so let's do it now.

It's important to note that a few direct type conversions which already
would not make any sense are not possible (for example, converting a
boolean to an IP address or an HTTP method to an integer). While this
would have caused the lookup to be performed on the wrong key, now the
lookup will fail and the converter will return no data. While there
should not be any case where this happens, it's probably best to avoid
backporting this change before a longer observation period.

BUG/MEDIUM: stick-tables: fix breakage in table converters

Baptiste reported that the table_conn_rate() converter would always
return zero in 1.6.5. In fact, commit bc8c404 ("MAJOR: stick-tables:
use sample types in place of dedicated types") broke all stick-table
converters because smp_to_stkey() now returns a pointer to the sample
instead of holding a copy of the key, and the converters used to
reinitialize the sample prior to performing the lookup. Only
"in_table()" continued to work.

The construct is still fragile, so some comments were added to a few
function to clarify their impacts. It's also worth noting that there
is no point anymore in forcing these converters to take a string on
input, but that will be changed in another commit.

The bug was introduced in 1.6-dev4, this fix must be backported to 1.6.

BUG/MAJOR: http: fix breakage of "reqdeny" causing random crashes

Commit 108b1dd ("MEDIUM: http: configurable http result codes for
http-request deny") introduced in 1.6-dev2 was incomplete. It introduced
a new field "rule_deny_status" into struct http_txn, which is filled only
by actions "http-request deny" and "http-request tarpit". It's then used
in the deny code path to emit the proper error message, but is used
uninitialized when the deny comes from a "reqdeny" rule, causing random
behaviours ranging from returning a 200, an empty response, or crashing
the process. Often upon startup only 200 was returned but after the fields
are used the crash happens. This can be sped up using -dM.

There's no need at all for storing this status in the http_txn struct
anyway since it's used immediately after being set. Let's store it in
a temporary variable instead which is passed as an argument to function
http_req_get_intercept_rule().

As an extra benefit, removing it from struct http_txn reduced the size
of this struct by 8 bytes.

This fix must be backported to 1.6 where the bug was detected. Special
thanks to Falco Schmutz for his detailed report including an exploitable
core and a reproducer.

DOC: Fix typo so fetch is properly parsed by Cyril's converter

Signed-off-by: Nenad Merdanovic <nmerdan@anine.io>

BUG/MINOR: fix listening IP address storage for frontends (cont)

Commit 6e6158 was incomplete. There was an additional aggregate copy
that may trigger a similar case in the future.

BUG/MAJOR: fix listening IP address storage for frontends

When compiled with GCC 6, the IP address specified for a frontend was
ignored and HAProxy was listening on all addresses instead. This is
caused by an incomplete copy of a "struct sockaddr_storage".

With the GNU Libc, "struct sockaddr_storage" is defined as this:

    struct sockaddr_storage
      {
        sa_family_t ss_family;
        unsigned long int __ss_align;
        char __ss_padding[(128 - (2 * sizeof (unsigned long int)))];
      };

Doing an aggregate copy (ss1 = ss2) is different than using memcpy():
only members of the aggregate have to be copied. Notably, padding can be
or not be copied. In GCC 6, some optimizations use this fact and if a
"struct sockaddr_storage" contains a "struct sockaddr_in", the port and
the address are part of the padding (between sa_family and __ss_align)
and can be not copied over.

Therefore, we replace any aggregate copy by a memcpy(). There is another
place using the same pattern. We also fix a function receiving a "struct
sockaddr_storage" by copy instead of by reference. Since it only needs a
read-only copy, the function is converted to request a reference.

SCRIPTS: make git-show-backports capable of limiting its history

When comparing very different branches, it can take a very long time
to scan all commits from the very old common ancestor (eg: haproxy
1.4 to 1.7). Now it is possible to specify a range of commits instead
of a specific branch, and the analysis will be limited to this range
for all commits. The user is responsible for ensuring that the range
covers all possible backports from base to ref, otherwise some of them
may be reported missing while they are not.

This also works with linux kernels, for example :

   git-show-backports -u -q -m -r v3.14.69 -b v3.14.65 v3.10.101..HEAD

SCRIPTS: teach git-show-backports how to report upstream commits

When git-show-backports is called with -u, instead of reporting the
commit IDs of the original branch on the left, it will display the
upstream commit IDs when such IDs are known, and will also display
them in the suggested "git cherry-pick" command line. This is useful
when the new branch is more recent than the one being checked and/or
it is desired to get rid of intermediary changes.

BUG/MEDIUM: init: don't use environment locale

This patch removes setlocale from the main function. It was introduced
by commit 379d9c7 ("MEDIUM: init: allow directory as argument of -f")
in 1.7-dev a few commits ago after a discussion on the mailing list.

Some regex may have different behaviours depending on the
locale. Some LUA scripts may change their behaviour too
(http://lua-users.org/wiki/LuaLocales).

Without this patch (haproxy is using setlocale) :

$ cat locale.cfg
defaults
  mode http

frontend test
  bind :9000
  mode http
  use_backend testbk if { hdr_reg(X-Test) ^\w+$ }

backend testbk
  mode http
  server s 127.0.0.1:80

$ LANG=fr_FR.UTF-8 ./haproxy -f locale.cfg
$ curl -i -H "X-Test: échec" localhost:9000
HTTP/1.1 200 OK
...

$ LANG=C ./haproxy -f locale.cfg
$ curl -i -H "X-Test: échec" localhost:9000
HTTP/1.0 503 Service Unavailable
...

DOC: filters: Update the filters documentation accordingly to recent changes

MEDIUM: filters: Add pre and post analyzer callbacks

'channel_analyze' callback has been removed. Now, there are 2 callbacks to
surround calls to analyzers:

  * channel_pre_analyze: Called BEFORE all filterable analyzers. it can be
    called many times for the same analyzer, once at each loop until the
    analyzer finishes its processing. This callback is resumable, it returns a
    negative value if an error occurs, 0 if it needs to wait, any other value
    otherwise.

  * channel_post_analyze: Called AFTER all filterable analyzers. Here, AFTER
    means when an analyzer finishes its processing. This callback is NOT
    resumable, it returns a negative value if an error occurs, any other value
    otherwise.

Pre and post analyzer callbacks are not automatically called. 'pre_analyzers'
and 'post_analyzers' bit fields in the filter structure must be set to the right
value using AN_* flags (see include/types/channel.h).

The flag AN_RES_ALL has been added (AN_REQ_ALL already exists) to ease the life
of filter developers. AN_REQ_ALL and AN_RES_ALL include all filterable
analyzers.

MINOR: filters: Simplify calls to analyzers using 2 new macros

Now, to call an analyzer in 'process_stream' function, we should use
FLT_ANALAYZE or ANALYZE macros, depending if this is a filterable analyzer or
not.

MEDIUM: filters: Move HTTP headers filtering in its own callback

Instead of calling 'channel_analyze' callback with the flag AN_FLT_HTTP_HDRS,
now we use the new callback 'http_headers'. This change is done because
'channel_analyze' callback will be removed in a next commit.

MINOR: log: add the %Td log-format specifier

As suggested by Pavlos, it's too bad that we didn't have a %Td log
format tag given that there are a few mentions of Td corresponding
to the data transmission time already in the doc, so this is now done.
Just like the other specifiers, we report -1 if the connection failed
before reaching the data transmission state.

CLEANUP: config: detect double registration of a config section

In an effort to make the config parser more robust, we should validate
that everything we register is not already registered. Most cfg_register_*
functions unfortunately return void and just perform a LIST_ADDQ(), so they
will have to change for this. At least cfg_register_section() does perform
a bit of checks and is easy to check for such errors, so let's start with
this one. Future patches will definitely have to focus on the remaining
functions and ensure unicity of all config parsers.

MEDIUM: init: allow directory as argument of -f

If -f argument is a directory add all the files (and only files) it
containes to the config files list.
These files are added in lexical order (respecting LC_COLLATE).
Only files with ".cfg" extension are added.
Only non hidden files (not prefixed with ".") are added.
Symlink are followed.
The -f order is still respected:

        $ tree -a rootdir
        rootdir
        |-- dir1
        |   |-- .6.cfg
        |   |-- 1.cfg
        |   |-- 2
        |   |-- 3.cfg
        |   |-- 4.cfg -> 1.cfg
        |   |-- 5 -> 1.cfg
        |   |-- 7.cfg -> .
        |   `-- dir4
        |       `-- 8.cfg
        |-- dir2
        |   |-- 10.cfg
        |   `-- 9.cfg
        |-- dir3
        |   `-- 11.cfg
        |-- link -> dir3/
        |-- root1
        |-- root2
        `-- root3

        $ ./haproxy -C rootdir -f root2 -f dir2 -f root3 -f dir1 \
                               -f link -f root1
        root2
        dir2/10.cfg
        dir2/9.cfg
        root3
        dir1/1.cfg
        dir1/3.cfg
        dir1/4.cfg
        link/11.cfg
        root1

This can be useful on systemd where you can't change the haproxy
commande line options on service reload.

MEDIUM: init: use list_append_word in haproxy.c

replace LIST_ADDQ with list_append_word

MINOR: add list_append_word function

int list_append_word(struct list *li, const char *str, char **err)

Append a copy of string <str> (inside a wordlist) at the end of
the list <li>.
The caller is responsible for freeing the <err> and <str> copy memory
area using free().

On failure : return 0 and <err> filled with an error message.

[RELEASE] Released version 1.7-dev3

Released version 1.7-dev3 with the following main changes :
    - MINOR: sample: Moves ARGS underlying type from 32 to 64 bits.
    - BUG/MINOR: log: Don't use strftime() which can clobber timezone if chrooted
    - BUILD: namespaces: fix a potential build warning in namespaces.c
    - MINOR: da: Using ARG12 macro for the sample fetch and the convertor.
    - DOC: add encoding to json converter example
    - BUG/MINOR: conf: "listener id" expects integer, but its not checked
    - DOC: Clarify tunes.vars.xxx-max-size settings
    - CLEANUP: chunk: adding NULL check to chunk_dup allocation.
    - CLEANUP: connection: fix double negation on memcmp()
    - BUG/MEDIUM: peers: fix incorrect age in frequency counters
    - BUG/MEDIUM: Fix RFC5077 resumption when more than TLS_TICKETS_NO are present
    - BUG/MAJOR: Fix crash in http_get_fhdr with exactly MAX_HDR_HISTORY headers
    - BUG/MINOR: lua: can't load external libraries
    - BUG/MINOR: prevent the dump of uninitialized vars
    - CLEANUP: map: it seems that the map were planed to be chained
    - MINOR: lua: move class registration facilities
    - MINOR: lua: remove some useless checks
    - CLEANUP: lua: Remove two same functions
    - MINOR: lua: refactor the Lua object registration
    - MINOR: lua: precise message when a critical error is catched
    - MINOR: lua: post initialization
    - MINOR: lua: Add internal function which strip spaces
    - MINOR: lua: convert field to lua type
    - DOC: "addr" parameter applies to both health and agent checks
    - DOC: timeout client: pointers to timeout http-request
    - DOC: typo on stick-store response
    - DOC: stick-table: amend paragraph blaming the loss of table upon reload
    - DOC: typo: ACL subdir match
    - DOC: typo: maxconn paragraph is wrong due to a wrong buffer size
    - DOC: regsub: parser limitation about the inability to use closing square brackets
    - DOC: typo: req.uri is now replaced by capture.req.uri
    - DOC: name set-gpt0 mismatch with the expected keyword
    - MINOR: http: sample fetch which returns unique-id
    - MINOR: dumpstats: extract stats fields enum and names
    - MINOR: dumpstats: split stats_dump_info_to_buffer() in two parts
    - MINOR: dumpstats: split stats_dump_fe_stats() in two parts
    - MINOR: dumpstats: split stats_dump_li_stats() in two parts
    - MINOR: dumpstats: split stats_dump_sv_stats() in two parts
    - MINOR: dumpstats: split stats_dump_be_stats() in two parts
    - MINOR: lua: dump general info
    - MINOR: lua: add class proxy
    - MINOR: lua: add class server
    - MINOR: lua: add class listener
    - BUG/MEDIUM: stick-tables: some sample-fetch doesn't work in the connection state.
    - MEDIUM: proxy: use dynamic allocation for error dumps
    - CLEANUP: remove unneeded casts
    - CLEANUP: uniformize last argument of malloc/calloc
    - DOC: fix "needed" typo
    - BUG/MINOR: dumpstats: fix write to global chunk
    - BUG/MINOR: dns: inapropriate way out after a resolution timeout
    - BUG/MINOR: dns: trigger a DNS query type change on resolution timeout
    - CLEANUP: proto_http: few corrections for gcc warnings.
    - BUG/MINOR: DNS: resolution structure change
    - BUG/MINOR : allow to log cookie for tarpit and denied request
    - BUG/MEDIUM: ssl: rewind the BIO when reading certificates
    - OPTIM/MINOR: session: abort if possible before connecting to the backend
    - DOC: http: rename the unique-id sample and add the documentation
    - BUG/MEDIUM: trace.c: rdtsc() is defined in two files
    - BUG/MEDIUM: channel: fix miscalculation of available buffer space (2nd try)
    - BUG/MINOR: server: risk of over reading the pref_net array.
    - BUG/MINOR: cfgparse: couple of small memory leaks.
    - BUG/MEDIUM: sample: initialize the pointer before parse_binary call.
    - DOC: fix discrepancy in the example for http-request redirect
    - MINOR: acl: Add predefined METH_DELETE, METH_PUT
    - CLEANUP: .gitignore cleanup
    - DOC: Clarify IPv4 address / mask notation rules
    - CLEANUP: fix inconsistency between fd->iocb, proto->accept and accept()
    - BUG/MEDIUM: fix maxaccept computation on per-process listeners
    - BUG/MINOR: listener: stop unbound listeners on startup
    - BUG/MINOR: fix maxaccept computation according to the frontend process range
    - TESTS: add blocksig.c to run tests with all signals blocked
    - MEDIUM: unblock signals on startup.
    - MINOR: filters: Print the list of existing filters during HA startup
    - MINOR: filters: Typo in an error message
    - MINOR: filters: Filters must define the callbacks struct during config parsing
    - DOC: filters: Add filters documentation
    - BUG/MEDIUM: channel: don't allow to overwrite the reserve until connected
    - BUG/MEDIUM: channel: incorrect polling condition may delay event delivery
    - BUG/MEDIUM: channel: fix miscalculation of available buffer space (3rd try)
    - BUG/MEDIUM: log: fix risk of segfault when logging HTTP fields in TCP mode
    - MINOR: Add ability for agent-check to set server maxconn
    - CLEANUP: Use server_parse_maxconn_change_request for maxconn CLI updates
    - MINOR: filters: add opaque data
    - BUG/MEDIUM: lua: protects the upper boundary of the argument list for converters/fetches.
    - MINOR: lua: migrate the argument mask to 64 bits type.
    - BUG/MINOR: dumpstats: Fix the "Total bytes saved" counter in backends stats
    - BUG/MINOR: log: fix a typo that would cause %HP to log <BADREQ>
    - BUG/MEDIUM: http: fix incorrect reporting of server errors
    - MINOR: channel: add new function channel_congested()
    - BUG/MEDIUM: http: fix risk of CPU spikes with pipelined requests from dead client
    - BUG/MAJOR: channel: fix miscalculation of available buffer space (4th try)
    - BUG/MEDIUM: stream: ensure the SI_FL_DONT_WAKE flag is properly cleared
    - BUG/MEDIUM: channel: fix inconsistent handling of 4GB-1 transfers
    - BUG/MEDIUM: stats: show servers state may show an empty or incomplete result
    - BUG/MEDIUM: stats: show backend may show an empty or incomplete result
    - MINOR: stats: fix typo in help messages
    - MINOR: stats: show stat resolvers missing in the help message
    - BUG/MINOR: dns: fix DNS header definition
    - BUG/MEDIUM: dns: fix alignment issue when building DNS queries
    - CLEANUP: don't ignore scripts in .gitignore
    - BUILD: add a few release and backport scripts in scripts/

BUILD: add a few release and backport scripts in scripts/

These ones have been used for several months already and are quite
convenient to emit new releases and backport fixes. I'm fed up with
having different versions on different machines, let's commit them
now.

CLEANUP: don't ignore scripts in .gitignore

We'll store a few maintenance scripts in scripts/ so don't ignore it.

BUG/MEDIUM: dns: fix alignment issue when building DNS queries

On some architectures, unaligned access is not authorized. On most
architectures, it is just slower. Therefore, we have to use memcpy()
when an unaligned access is needed, specifically when writing the qinfo.

Also remove the unaligned access when reading answer count when reading
the answer. It's likely that this instruction was optimized away by the
compiler since it is unneeded. Add a comment to explain why we use 7 as
an offset instead of 6. Not an unaligned offset since "resp" is
"unsigned char", then promoted to int.

BUG/MINOR: dns: fix DNS header definition

Conforming to RFC 2535, section 6.1. This is not an important bug as
those fields don't seem to be set to something else than 0 and to be
checked on answers.

MINOR: stats: show stat resolvers missing in the help message

The help message provided by the "help" command on the unix socket didn't
provide "show stat resolvers" in the list of supported commands.

This patch should be backported to 1.6

MINOR: stats: fix typo in help messages

The word "available" was mistakenly written "avalaible" in help messages and
comments.

This patch should be backported to 1.5 and 1.6

BUG/MEDIUM: stats: show backend may show an empty or incomplete result

This is the same issue as "show servers state", where the result is incorrect
it the data can't fit in one buffer. The similar fix is applied, to restart
the data processing where it stopped as buffers are sent to the client.

This fix should be backported to haproxy 1.6

BUG/MEDIUM: stats: show servers state may show an empty or incomplete result

It was reported that the unix socket command "show servers state" returned an
empty response while "show servers state <backend>" worked.
In fact, both cases can reproduce the issue. It happens when the response can't
fit in one buffer.

The fix consists in processing the response in several steps, as it is done in
some others commands, by restarting where it was stopped after the buffer is
sent to the client.

This fix should be backported to haproxy 1.6

BUG/MEDIUM: channel: fix inconsistent handling of 4GB-1 transfers

In 1.4-dev3, commit 31971e5 ("[MEDIUM] add support for infinite forwarding")
made it possible to configure the lower layer to forward data indefinitely
by setting the forward size to CHN_INFINITE_FORWARD (4GB-1). By then larger
chunk sizes were not supported so there was no confusion in the usage of the
function.

Since 1.5 we support 64-bit content-lengths and chunk sizes and the function
has grown to support 64-bit arguments, though it still limits a single pass
to 32-bit quantities (what fit in the channel's to_forward field). The issue
now becomes that a 4GB-1 content-length can be confused with infinite
forwarding (in fact it's 4GB-1+what was already in the buffer). It causes a
visible effect when transferring this exact size because the transfer rate
is lower than with other sizes due in part to the disabling of the Nagle
algorithm on the sendto() call.

In theory with keep-alive it should prevent a second request from being
processed after such a transfer, but since the analysers are still present,
the forwarding analyser properly counts down the remaining size to transfer
and ultimately the transaction gets correctly reset so there is no visible
effect.

Since the root cause of the issue is an API problem (lack of distinction
between a real valid length and a magic value), this patch modifies the API
to have a new dedicated function called channel_forward_forever() to program
a permanent forwarding. The existing function __channel_forward() was modified
to properly take care of the requested sizes and ensure it 1) never overflows
and 2) never reaches CHN_INFINITE_FORWARD by accident.

It is worth noting that the function used to have a bug causing a 2GB
forward to be scheduled if it was called with less data than what is present
in buf->i. Fortunately this bug couldn't be triggered with existing code.

This fix should be backported to 1.6 and 1.5. While it also theorically
affects 1.4, it's better not to backport it there, as the risk of breaking
large object transfers due to significant API differences is high, compared
to the fact that the largest supported objects (4GB-1) are just slower to
transfer.

BUG/MEDIUM: stream: ensure the SI_FL_DONT_WAKE flag is properly cleared

The previous buffer space bug has revealed an issue causing some stalled
connections to remain orphaned forever, preventing an old process from
dying. The issue is that once in a while a task may be woken up because
a disabled expiration timer has been reached despite no timeout being
reached. In this case we exit very early but the SI_FL_DONT_WAKE flag
wasn't cleared, resulting in new events not waking the task up. It may
be one of the reasons why a few people have already observed some peers
connections stuck in CLOSE_WAIT state.

This bug was introduced in 1.5-dev13 by commit 798f432 ("OPTIM: session:
don't process the whole session when only timers need a refresh"), so
the fix must be backported to 1.6 and 1.5.

BUG/MAJOR: channel: fix miscalculation of available buffer space (4th try)

Unfortunately, commit 169c470 ("BUG/MEDIUM: channel: fix miscalculation of
available buffer space (3rd try)") was still not enough to completely
address the issue. It fell into an integer comparison trap. Contrary to
expectations, chn->to_forward may also have the sign bit set when
forwarding regular data having a large content-length, resulting in
an incomplete check of the result and of the reserve because the with
to_forward very large, to_forward+o could become very small and also
the reserve could become positive again and make channel_recv_limit()
return a negative value.

One way to reproduce this situation is to transfer a large file (> 2GB)
with http-keep-alive or http-server-close, without splicing, and ensure
that the server uses content-length instead of chunks. The transfer
should stall very early after the first buffer has been transferred
to the client.

This fix now properly checks 1) for an overflow caused by summing o and
to_forward, and 2) for o+to_forward being smaller or larger than maxrw
before performing the subtract, so that all sensitive operations are
properly performed on 33-bit arithmetics.

The code was subjected again to a series of tests using inject+httpterm
scanning a wide range of object sizes (+10MB after each new request) :

  $ printf "new page 1\nget 127.0.0.1:8002 / s=%%s0m\n" | \
      inject64 -o 1 -u 1 -f /dev/stdin

With previous fix, the transfer would suddenly stop when reaching 2GB :

   hits ^hits hits/s  ^h/s     bytes  kB/s  last  errs  tout htime  sdht ptime
    203     1      2     1 216816173354 2710202 3144892     0     0 685.0 0.0 685.0
    205     2      2     2 219257283186 2706880 2441109     0     0 679.5 6.5 679.5
    205     0      2     0 219257283186 2673836     0     0     0 0.0 0.0 0.0
    205     0      2     0 219257283186 2641622     0     0     0 0.0 0.0 0.0
    205     0      2     0 219257283186 2610174     0     0     0 0.0 0.0 0.0

Now it's fine even past 4 GB.

Many thanks to Vedran Furac for reporting this issue early with a common
access pattern helping to troubleshoot this.

This fix must be backported to 1.6 and 1.5 where the commit above was
already backported.

BUG/MEDIUM: http: fix risk of CPU spikes with pipelined requests from dead client

Since client-side HTTP keep-alive was introduced in 1.4-dev, a good
number of corner cases had to be dealt with. One of them remained and
caused some occasional CPU spikes that Cyril Bonté had the opportunity
to observe from time to time and even recently to capture for deeper
analysis.

What happens is the following scenario :
  1) a client sends a first request which causes the server to respond
     using chunked encoding

  2) the server starts to respond with a large response that doesn't
     fit into a single buffer

  3) the response buffer fills up with the response

  4) the client reads the response while it is being sent by the server.

  5) haproxy eventually receives the end of the response from the server,
     which occupies the whole response buffer (must at least override the
     reserve), parses it and prepares to receive a second request while
     sending the last data blocks to the client. It then reinitializes the
     whole http_txn and calls http_wait_for_request(), which arms the
     http-request timeout.

  6) at this exact moment the client emits a second request, probably
     asking for an object referenced in the first page while parsing it
     on the fly, and silently disappears from the internet (internet
     access cut or software having trouble with pipelined request).

  7) the second request arrives into the request buffer and the response
     data stall in the response buffer, preventing the reserve from being
     used for anything else.

  8) haproxy calls http_wait_for_request() to parse the next request which
     has just arrived, but since it sees the response buffer is full, it
     refrains from doing so and waits for some data to leave or a timeout
     to strike.

  9) the http-request timeout strikes, causing http_wait_for_request() to
     be called again, and the function immediately returns since it cannot
     even produce an error message, and the loop is maintained here.

 10) the client timeout strikes, aborting the loop.

At first glance a check for timeout would be needed *before* considering
the buffer in http_wait_for_request(), but the issue is not there in fact,
because when doing so we see in the logs a client timeout error while
waiting for a request, which is wrong. The real issue is that we must not
consider the first transaction terminated until at least the reserve is
released so that http_wait_for_request() has no problem starting to process
a new request. This allows the first response to be reported in timeout and
not the second request. A more restrictive control could consist in not
considering the request complete until the response buffer has no more
outgoing data but this brings no added value and needlessly increases the
number of wake-ups when dealing with pipelining.

Note that the same issue exists with the request, we must ensure that any
POST data are finished being forwarded to the server before accepting a new
request. This case is much harder to trigger however as servers rarely
disappear and if they do so, they impact all their sessions at once.

No specific reproducer is usable because the bug is very hard to reproduce
and depends on the system as well with settings varying across reboots. On
a test machine, socket buffers were reduced to 4096 (tcp_rmem and tcp_wmem),
haproxy's buffers were set to 16384 and tune.maxrewrite to 12288. The proxy
must work in http-server-close mode, with a request timeout smaller than the
client timeout. The test is run like this :

  $ (printf "GET /15000 HTTP/1.1\r\n\r\n"; usleep 100000; \
     printf "GET / HTTP/1.0\r\n\r\n"; sleep 15) | nc6 --send-only 0 8002

The server returns chunks of the requested size (15000 bytes here, but
78000 in a previous test was the only working value). Strace must show the
last recvfrom() succeed and the last sendto() being shorter than desired or
better, not being called.

This fix must be backported to 1.6, 1.5 and 1.4. It was made in a way that
should make it possible to backport it. It depends on channel_congested()
which also needs to be backported. Further cleanup of http_wait_for_request()
could be made given that some of the tests are now useless.

MINOR: channel: add new function channel_congested()

This function returns non-zero if the channel is congested with data in
transit waiting for leaving, indicating to the caller that it should wait
for the reserve to be released before starting to process new data in
case it needs the ability to append data. This is meant to be used while
waiting for a clean response buffer before processing a request.

BUG/MEDIUM: http: fix incorrect reporting of server errors

Commit dbe34eb ("MEDIUM: filters/http: Move body parsing of HTTP
messages in dedicated functions") introduced a bug in function
http_response_forward_body() by getting rid of the while(1) loop.
The code immediately following the loop was only reachable on missing
data but now it's also reachable under normal conditions, which used
to be dealt with by the skip_resync_state label returning zero. The
side effect is that in http_server_close situations, the channel's
SHUTR flag is seen and considered as a server error which is reported
if any other error happens (eg: client timeout).

This bug is specific to 1.7, no backport is needed.

BUG/MINOR: log: fix a typo that would cause %HP to log <BADREQ>

Typo was introduced in 57bc891 ("BUG/MEDIUM: log: fix risk of
segfault when logging HTTP fields in TCP mode") which inverted the
condition in the test and caused <BADREQ> to be logged when using
%HP.

Signed-off-by: Nenad Merdanovic <nmerdan@anine.io>

BUG/MINOR: dumpstats: Fix the "Total bytes saved" counter in backends stats

Instead of subtracting ST_F_COMP_OUT (Compression out) from ST_F_COMP_IN
(Compressio in) in backends stats, ST_F_COMP_BYP (Compression bypass) was used.

MINOR: lua: migrate the argument mask to 64 bits type.

Recently the maximum number of arguments were ported to 12
due to the migration to 64 bits. So we allow lua to extend for
both converters and fetches.