]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
9 years agoMINOR: stick-table: change all stick-table converters' inputs to SMP_T_ANY
Willy Tarreau [Wed, 25 May 2016 15:16:38 +0000 (17:16 +0200)] 
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.

9 years agoBUG/MEDIUM: stick-tables: fix breakage in table converters
Willy Tarreau [Wed, 25 May 2016 15:07:56 +0000 (17:07 +0200)] 
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.

9 years agoBUG/MAJOR: http: fix breakage of "reqdeny" causing random crashes
Willy Tarreau [Wed, 25 May 2016 14:23:59 +0000 (16:23 +0200)] 
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.

9 years agoDOC: Fix typo so fetch is properly parsed by Cyril's converter
Nenad Merdanovic [Tue, 17 May 2016 01:31:21 +0000 (03:31 +0200)] 
DOC: Fix typo so fetch is properly parsed by Cyril's converter

Signed-off-by: Nenad Merdanovic <nmerdan@anine.io>
9 years agoBUG/MINOR: fix listening IP address storage for frontends (cont)
Vincent Bernat [Thu, 19 May 2016 09:29:43 +0000 (11:29 +0200)] 
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.

9 years agoBUG/MAJOR: fix listening IP address storage for frontends
Vincent Bernat [Wed, 18 May 2016 14:17:44 +0000 (16:17 +0200)] 
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.

9 years agoSCRIPTS: make git-show-backports capable of limiting its history
Willy Tarreau [Mon, 16 May 2016 15:01:12 +0000 (17:01 +0200)] 
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

9 years agoSCRIPTS: teach git-show-backports how to report upstream commits
Willy Tarreau [Mon, 16 May 2016 14:39:38 +0000 (16:39 +0200)] 
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.

9 years agoBUG/MEDIUM: init: don't use environment locale
Maxime de Roucy [Wed, 18 May 2016 21:13:38 +0000 (23:13 +0200)] 
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
...

9 years agoDOC: filters: Update the filters documentation accordingly to recent changes
Christopher Faulet [Wed, 11 May 2016 15:29:14 +0000 (17:29 +0200)] 
DOC: filters: Update the filters documentation accordingly to recent changes

9 years agoMEDIUM: filters: Add pre and post analyzer callbacks
Christopher Faulet [Wed, 11 May 2016 15:13:39 +0000 (17:13 +0200)] 
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.

9 years agoMINOR: filters: Simplify calls to analyzers using 2 new macros
Christopher Faulet [Wed, 11 May 2016 15:06:28 +0000 (17:06 +0200)] 
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.

9 years agoMEDIUM: filters: Move HTTP headers filtering in its own callback
Christopher Faulet [Wed, 11 May 2016 14:48:33 +0000 (16:48 +0200)] 
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.

9 years agoMINOR: log: add the %Td log-format specifier
Willy Tarreau [Tue, 17 May 2016 15:55:27 +0000 (17:55 +0200)] 
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.

9 years agoCLEANUP: config: detect double registration of a config section
Willy Tarreau [Tue, 17 May 2016 14:16:09 +0000 (16:16 +0200)] 
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.

9 years agoMEDIUM: init: allow directory as argument of -f
Maxime de Roucy [Fri, 13 May 2016 21:52:56 +0000 (23:52 +0200)] 
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.

9 years agoMEDIUM: init: use list_append_word in haproxy.c
Maxime de Roucy [Fri, 13 May 2016 21:52:55 +0000 (23:52 +0200)] 
MEDIUM: init: use list_append_word in haproxy.c

replace LIST_ADDQ with list_append_word

9 years agoMINOR: add list_append_word function
Maxime de Roucy [Fri, 13 May 2016 21:52:54 +0000 (23:52 +0200)] 
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.

9 years ago[RELEASE] Released version 1.7-dev3 v1.7-dev3
Willy Tarreau [Tue, 10 May 2016 13:36:58 +0000 (15:36 +0200)] 
[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/

9 years agoBUILD: add a few release and backport scripts in scripts/
Willy Tarreau [Tue, 10 May 2016 10:04:13 +0000 (12:04 +0200)] 
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.

9 years agoCLEANUP: don't ignore scripts in .gitignore
Willy Tarreau [Tue, 10 May 2016 10:02:27 +0000 (12:02 +0200)] 
CLEANUP: don't ignore scripts in .gitignore

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

9 years agoBUG/MEDIUM: dns: fix alignment issue when building DNS queries
Vincent Bernat [Fri, 8 Apr 2016 20:17:45 +0000 (22:17 +0200)] 
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.

9 years agoBUG/MINOR: dns: fix DNS header definition
Vincent Bernat [Fri, 8 Apr 2016 20:17:44 +0000 (22:17 +0200)] 
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.

9 years agoMINOR: stats: show stat resolvers missing in the help message
Cyril Bonté [Fri, 6 May 2016 10:18:51 +0000 (12:18 +0200)] 
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

9 years agoMINOR: stats: fix typo in help messages
Cyril Bonté [Fri, 6 May 2016 10:18:50 +0000 (12:18 +0200)] 
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

9 years agoBUG/MEDIUM: stats: show backend may show an empty or incomplete result
Cyril Bonté [Fri, 6 May 2016 10:18:49 +0000 (12:18 +0200)] 
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

9 years agoBUG/MEDIUM: stats: show servers state may show an empty or incomplete result
Cyril Bonté [Fri, 6 May 2016 10:18:48 +0000 (12:18 +0200)] 
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

9 years agoBUG/MEDIUM: channel: fix inconsistent handling of 4GB-1 transfers
Willy Tarreau [Wed, 4 May 2016 12:05:58 +0000 (14:05 +0200)] 
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.

9 years agoBUG/MEDIUM: stream: ensure the SI_FL_DONT_WAKE flag is properly cleared
Willy Tarreau [Wed, 4 May 2016 08:18:37 +0000 (10:18 +0200)] 
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.

9 years agoBUG/MAJOR: channel: fix miscalculation of available buffer space (4th try)
Willy Tarreau [Tue, 3 May 2016 15:46:24 +0000 (17:46 +0200)] 
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.

9 years agoBUG/MEDIUM: http: fix risk of CPU spikes with pipelined requests from dead client
Willy Tarreau [Mon, 2 May 2016 14:07:07 +0000 (16:07 +0200)] 
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.

9 years agoMINOR: channel: add new function channel_congested()
Willy Tarreau [Mon, 2 May 2016 14:05:10 +0000 (16:05 +0200)] 
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.

9 years agoBUG/MEDIUM: http: fix incorrect reporting of server errors
Willy Tarreau [Mon, 2 May 2016 13:25:15 +0000 (15:25 +0200)] 
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.

9 years agoBUG/MINOR: log: fix a typo that would cause %HP to log <BADREQ>
Nenad Merdanovic [Mon, 25 Apr 2016 23:39:02 +0000 (01:39 +0200)] 
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>
9 years agoBUG/MINOR: dumpstats: Fix the "Total bytes saved" counter in backends stats
Christopher Faulet [Thu, 28 Apr 2016 13:09:31 +0000 (15:09 +0200)] 
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.

9 years agoMINOR: lua: migrate the argument mask to 64 bits type.
David Carlier [Wed, 27 Apr 2016 15:21:56 +0000 (16:21 +0100)] 
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.

9 years agoBUG/MEDIUM: lua: protects the upper boundary of the argument list for converters...
David Carlier [Wed, 27 Apr 2016 15:14:50 +0000 (16:14 +0100)] 
BUG/MEDIUM: lua: protects the upper boundary of the argument list for converters/fetches.

When a converter or sample is called from within a Lua code, there is a risk
of invalid argument string data usage when the upper boundary is reached.
Would be kind to port to 1.6 if possible.

9 years agoMINOR: filters: add opaque data
Thierry Fournier [Wed, 13 Apr 2016 16:27:51 +0000 (18:27 +0200)] 
MINOR: filters: add opaque data

Add opaque data between the filter keyword registrering and the parsing
function. This opaque data allow to use the same parser with differents
registered keywords. The opaque data is used for giving data which mainly
makes difference between the two keywords.

It will be used with Lua keywords registering.

9 years agoCLEANUP: Use server_parse_maxconn_change_request for maxconn CLI updates
Nenad Merdanovic [Sun, 24 Apr 2016 21:10:07 +0000 (23:10 +0200)] 
CLEANUP: Use server_parse_maxconn_change_request for maxconn CLI updates

9 years agoMINOR: Add ability for agent-check to set server maxconn
Nenad Merdanovic [Sun, 24 Apr 2016 21:10:06 +0000 (23:10 +0200)] 
MINOR: Add ability for agent-check to set server maxconn

This is very useful in complex architecture systems where HAproxy
is balancing DB connections for example. We want to keep the maxconn
high in order to avoid issues with queueing on the LB level when
there is slowness on another part of the system. Example is a case of
an architecture where each thread opens multiple DB connections, which
if get stuck in queue cause a snowball effect (old connections aren't
closed, new ones cannot be established). These connections are mostly
idle and the DB server has no problem handling thousands of them.

Allowing us to dynamically set maxconn depending on the backend usage
(LA, CPU, memory, etc.) enables us to have high maxconn for situations
like above, but lowering it in case there are real issues where the
backend servers become overloaded (cache issues, DB gets hit hard).

9 years agoBUG/MEDIUM: log: fix risk of segfault when logging HTTP fields in TCP mode
Willy Tarreau [Mon, 25 Apr 2016 15:09:40 +0000 (17:09 +0200)] 
BUG/MEDIUM: log: fix risk of segfault when logging HTTP fields in TCP mode

David Torgerson faced an issue when using HTTP fields in log-format in TCP
sections. The txn is dereferenced while it's null, resulting in a crash of
the process. Such configurations are invalid and a warning is emitted, but
nevertheless the process must not crash. As found by Lukas Tribus, this is
a side effect of the split between the stream and the HTTP transaction that
happened in 1.6, making it possible to have txn==NULL there.

The fix consists in checking that txn is valid before using it. Fortunately
it's easy since almost all places already used to check for the existence
of a field (eg: txn->uri).

This patch should be backported to 1.6.

9 years agoBUG/MEDIUM: channel: fix miscalculation of available buffer space (3rd try)
Willy Tarreau [Wed, 20 Apr 2016 16:05:17 +0000 (18:05 +0200)] 
BUG/MEDIUM: channel: fix miscalculation of available buffer space (3rd try)

Latest fix 8a32106 ("BUG/MEDIUM: channel: fix miscalculation of available
buffer space (2nd try)") did happen to fix some observable issues but not
all of them in fact, some corner cases still remained and at least one user
reported a busy loop that appeared possible, though not easily reproducible
under experimental conditions.

The remaining issue is that we still consider min(i, to_fwd) as the number
of bytes in transit, but in fact <i> is not relevant here. Indeed, what
matters is that we can read everything we want at once provided that at
the end, <i> cannot be larger than <size-maxrw> (if it was not already).

This is visible in two cases :
  - let's have i=o=max/2 and to_fwd=0. Then i+o >= max indicates that the
    buffer is already full, while it is not since once <o> is forwarded,
    some space remains.

  - when to_fwd is much larger than i, it's obvious that we can fill the
    buffer.

The only relevant part in fact is o + to_fwd. to_fwd will ensure that at
least this many bytes will be moved from <i> to <o> hence will leave the
buffer, whatever the number of rounds it takes.

Interestingly, the fix applied here ensures that channel_recv_max() will
now equal (size - maxrw - i + to_fwd), which is indeed what remains
available below maxrw after to_fwd bytes are forwarded from i to o and
leave the buffer.

Additionally, the latest fix made it possible to meet an integer overflow
that was not caught by the range test when forwarding in TCP or tunnel
mode due to to_forward being added to an existing value, causing the
buffer size to be limited when it should not have been, resulting in 2
to 3 recv() calls when a single one was enough. The first one was limited
to the unreserved buffer size, the second one to the size of the reserve
minus 1, and the last one to the last byte. Eg with a 2kB buffer :

recvfrom(22, "HTTP/1.1 200\r\nConnection: close\r"..., 1024, 0, NULL, NULL) = 1024
recvfrom(22, "23456789.123456789.123456789.123"..., 1023, 0, NULL, NULL) = 1023
recvfrom(22, "5", 1, 0, NULL, NULL)     = 1

This bug is still present in 1.6 and 1.5 so the fix should be backported
there.

9 years agoBUG/MEDIUM: channel: incorrect polling condition may delay event delivery
Willy Tarreau [Thu, 21 Apr 2016 10:12:45 +0000 (12:12 +0200)] 
BUG/MEDIUM: channel: incorrect polling condition may delay event delivery

The condition to poll for receive as implemented in channel_may_recv()
is still incorrect. If buf->o is null and buf->i is slightly larger than
chn->to_forward and at least as large as buf->size - maxrewrite, then
reading will be disabled. It may slightly delay some data delivery by
having first to forward pending bytes, but may also cause some random
issues with analysers that wait for some data before starting to forward
what they correctly parsed. For instance, a body analyser may be prevented
from seeing the data that only fits in the reserve.

This bug may also prevent an applet's chk_rcv() function from being called
when part of a buffer is released. It is possible (though not verified)
that this participated to some peers frozen session issues some people
have been facing.

This fix should be backported to 1.6 and 1.5 to ensure better coherency
with channel_recv_limit().

9 years agoBUG/MEDIUM: channel: don't allow to overwrite the reserve until connected
Willy Tarreau [Wed, 20 Apr 2016 18:09:22 +0000 (20:09 +0200)] 
BUG/MEDIUM: channel: don't allow to overwrite the reserve until connected

Commit 9c06ee4 ("BUG/MEDIUM: channel: don't schedule data in transit for
leaving until connected") took care of an issue involving POST in conjunction
with http-send-name-header, where we absolutely never want to touch the
reserve until we're sure not to touch the buffer contents anymore, which
is indicated by the output stream-interface being connected.

But channel_may_recv() was not equipped with such a test, so in some
situations it might decide that it is possible to poll for reads, and
later channel_recv_limit() will decide it's not possible to read,
causing a loop. So we must add a similar test there.

Since the fix above was backported to 1.6 and 1.5, this fix must as well.

9 years agoDOC: filters: Add filters documentation
Christopher Faulet [Thu, 7 Apr 2016 13:30:10 +0000 (15:30 +0200)] 
DOC: filters: Add filters documentation

The configuration documention has been updated. Doc about the filter line has
been added and a new chapter (§. 9) has been created to list and document
supported filters (for now, flt_trace and flt_http_comp).

The developer documentation about filters has also been added. The is a "pre"
version. Incoming changes in the filter API will require an update.
This documentation requires a deeper review and some TODO need to be complete.

9 years agoMINOR: filters: Filters must define the callbacks struct during config parsing
Christopher Faulet [Tue, 19 Apr 2016 15:00:44 +0000 (17:00 +0200)] 
MINOR: filters: Filters must define the callbacks struct during config parsing

9 years agoMINOR: filters: Typo in an error message
Christopher Faulet [Mon, 4 Apr 2016 08:51:17 +0000 (10:51 +0200)] 
MINOR: filters: Typo in an error message

9 years agoMINOR: filters: Print the list of existing filters during HA startup
Christopher Faulet [Mon, 7 Mar 2016 11:46:38 +0000 (12:46 +0100)] 
MINOR: filters: Print the list of existing filters during HA startup

This is done  in verbose/debug mode and when build options are reported.

9 years agoMEDIUM: unblock signals on startup.
Willy Tarreau [Wed, 20 Apr 2016 08:33:15 +0000 (10:33 +0200)] 
MEDIUM: unblock signals on startup.

A problem was reported recently by some users of programs compiled
with Go 1.5 which by default blocks all signals before executing
processes, resulting in haproxy not receiving SIGUSR1 or even SIGTERM,
causing lots of zombie processes.

This problem was apparently observed by users of consul and kubernetes
(at least).

This patch is a workaround for this issue. It consists in unblocking
all signals on startup. Since they're normally not blocked in a regular
shell, it ensures haproxy always starts under the same conditions.

Quite useful information reported by both Matti Savolainen and REN
Xiaolei actually helped find the root cause of this problem and this
workaround. Thanks to them for this.

This patch must be backported to 1.6 and 1.5 where the problem is
observed.

9 years agoTESTS: add blocksig.c to run tests with all signals blocked
Willy Tarreau [Wed, 20 Apr 2016 08:20:22 +0000 (10:20 +0200)] 
TESTS: add blocksig.c to run tests with all signals blocked

A problem was reported recently by some users of programs compiled
with Go 1.5 which by default blocks all signals before executing
processes, resulting in haproxy not receiving SIGUSR1 or even SIGTERM.

This program mimmicks this behaviour to make it easier to run tests.
It also displays the current signal mask. A simple test consists in
running it through itself.

9 years agoBUG/MINOR: fix maxaccept computation according to the frontend process range
Cyril Bonté [Fri, 15 Apr 2016 05:58:43 +0000 (07:58 +0200)] 
BUG/MINOR: fix maxaccept computation according to the frontend process range

commit 7c0ffd23 is only considering the explicit use of the "process" keyword
on the listeners. But at this step, if it's not defined in the configuration,
the listener bind_proc mask is set to 0. As a result, the code will compute
the maxaccept value based on only 1 process, which is not always true.

For example :
  global
    nbproc 4

  frontend test
    bind-process 1-2
    bind :80

Here, the maxaccept value for the "test" frontend was set to the global
tune.maxaccept value (default to 64), whereas it should consider 2 processes
will accept connections. As of the documentation, the value should be divided
by twice the number of processes the listener is bound to.

To fix this, we can consider that if no mask is set to the listener, we take
the frontend mask.

This is not critical but it can introduce unfairness distribution of the
incoming connections across the processes.

It should be backported to the same branches as commit 7c0ffd23 (1.6 and 1.5
were in the scope).

9 years agoBUG/MINOR: listener: stop unbound listeners on startup
Willy Tarreau [Thu, 14 Apr 2016 10:05:02 +0000 (12:05 +0200)] 
BUG/MINOR: listener: stop unbound listeners on startup

When a listener is not bound to a process its frontend belongs to, it
is only paused and not stopped. This creates confusion from the outside
as "netstat -ltnp" for example will report only the parent process as
the listener instead of the effective one. "ss -lnp" will report that
all processes are listening to all sockets.

This is confusing enough to suggest a fix. Now we simply stop the unused
listeners. Example with this simple config :

  global
      nbproc 4

  frontend haproxy_test
      bind-process 1-40
      bind :12345 process 1
      bind :12345 process 2
      bind :12345 process 3
      bind :12345 process 4

Before the patch :
  $ netstat -ltnp
  Proto Recv-Q Send-Q Local Address           Foreign Address         State       PID/Program name
  tcp        0      0 0.0.0.0:12345           0.0.0.0:*               LISTEN      30457/./haproxy
  tcp        0      0 0.0.0.0:12345           0.0.0.0:*               LISTEN      30457/./haproxy
  tcp        0      0 0.0.0.0:12345           0.0.0.0:*               LISTEN      30457/./haproxy
  tcp        0      0 0.0.0.0:12345           0.0.0.0:*               LISTEN      30457/./haproxy

After the patch :
  $ netstat -ltnp
  Proto Recv-Q Send-Q Local Address           Foreign Address         State       PID/Program name
  tcp        0      0 0.0.0.0:12345           0.0.0.0:*               LISTEN      30504/./haproxy
  tcp        0      0 0.0.0.0:12345           0.0.0.0:*               LISTEN      30503/./haproxy
  tcp        0      0 0.0.0.0:12345           0.0.0.0:*               LISTEN      30502/./haproxy
  tcp        0      0 0.0.0.0:12345           0.0.0.0:*               LISTEN      30501/./haproxy

This patch may be backported to 1.6 and 1.5, but it relies on commit
7a798e5 ("CLEANUP: fix inconsistency between fd->iocb, proto->accept
and accept()") since it will expose an API inconsistency by including
listener.h in the .c.

9 years agoBUG/MEDIUM: fix maxaccept computation on per-process listeners
Willy Tarreau [Thu, 14 Apr 2016 09:47:38 +0000 (11:47 +0200)] 
BUG/MEDIUM: fix maxaccept computation on per-process listeners

Christian Ruppert reported a performance degradation when binding a
single frontend to many processes while only one bind line was being
used, bound to a single process.

The reason comes from the fact that whenever a listener is bound to
multiple processes, the it is assigned a maxaccept value which equals
half the global maxaccept value divided by the number of processes the
frontend is bound to. The purpose is to ensure that no single process
will drain all the incoming requests at once and ensure a fair share
between all listeners. Usually this works pretty well, when a listener
is bound to all the processes of its frontend. But here we're in a
situation where the maxaccept of a listener which is bound to a single
process is still divided by a large value.

The fix consists in taking into account the number of processes the
listener is bound do and not only those of the frontend. This way it
is perfectly possible to benefit from nbproc and SO_REUSEPORT without
performance degradation.

1.6 and 1.5 normally suffer from the same issue.

9 years agoCLEANUP: fix inconsistency between fd->iocb, proto->accept and accept()
Willy Tarreau [Thu, 14 Apr 2016 09:13:20 +0000 (11:13 +0200)] 
CLEANUP: fix inconsistency between fd->iocb, proto->accept and accept()

There's quite some inconsistency in the internal API. listener_accept()
which is the main accept() function returns void but is declared as int
in the include file. It's assigned to proto->accept() for all stream
protocols where an int is expected but the result is never checked (nor
is it documented by the way). This proto->accept() is in turn assigned
to fd->iocb() which is supposed to return an int composed of FD_WAIT_*
flags, but which is never checked either.

So let's fix all this mess :
  - nobody checks accept()'s return
  - nobody checks iocb()'s return
  - nobody sets a return value

=> let's mark all these functions void and keep the current ones intact.

Additionally we now include listener.h from listener.c to ensure we won't
silently hide this incoherency in the future.

Note that this patch could/should be backported to 1.6 and even 1.5 to
simplify debugging sessions.

9 years agoDOC: Clarify IPv4 address / mask notation rules
Daniel Schneller [Tue, 12 Apr 2016 22:26:52 +0000 (00:26 +0200)] 
DOC: Clarify IPv4 address / mask notation rules

Adds some examples regarding shorthand IPv4 address notation which might
be confused with RFC 4632 CIDR notation, leading to different than
expected results.

9 years agoCLEANUP: .gitignore cleanup
Vincent Bernat [Tue, 12 Apr 2016 08:56:55 +0000 (10:56 +0200)] 
CLEANUP: .gitignore cleanup

.gitignore is an odd beast. All the stuff at the beginning is useless
since in the bottom part starts with /.* and /*. Therefore, the top part
is useless. Moreover, the bottom part makes unignore *.o and
friends. Add it back at the bottom.

9 years agoMINOR: acl: Add predefined METH_DELETE, METH_PUT
Daniel Schneller [Mon, 11 Apr 2016 15:45:29 +0000 (17:45 +0200)] 
MINOR: acl: Add predefined METH_DELETE, METH_PUT

Adds the missing HTTP verbs DELETE and PUT as predefined ACLs, similar
to GET, POST etc.

9 years agoDOC: fix discrepancy in the example for http-request redirect
Coen Rosdorff [Mon, 11 Apr 2016 09:33:49 +0000 (11:33 +0200)] 
DOC: fix discrepancy in the example for http-request redirect

Commit c8f0e78 ("DOC: typo: req.uri is now replaced by capture.req.uri")
fixed a discrepancy in the doc but the scheme is still missing, resulting
in a redirect loop. Let's fix this as well. This should be backported to
1.5.

9 years agoBUG/MEDIUM: sample: initialize the pointer before parse_binary call.
David Carlier [Fri, 8 Apr 2016 09:37:02 +0000 (10:37 +0100)] 
BUG/MEDIUM: sample: initialize the pointer before parse_binary call.

parse_binary line 2025 checks the nullity of binstr parameter.
Other calls of parse_binary properly zeroify this parameter.
[wt: this could result in random failures of the const parser]

9 years agoBUG/MINOR: cfgparse: couple of small memory leaks.
David Carlier [Fri, 8 Apr 2016 09:35:26 +0000 (10:35 +0100)] 
BUG/MINOR: cfgparse: couple of small memory leaks.

During the config parse in some code paths, there is some
forgotten pointers freeing, and as often, during errors handlings.

9 years agoBUG/MINOR: server: risk of over reading the pref_net array.
David Carlier [Fri, 8 Apr 2016 09:26:44 +0000 (10:26 +0100)] 
BUG/MINOR: server: risk of over reading the pref_net array.

dns_option struct pref_net field is an array of 5. The issue
here shows that pref_net_nb can go up to 5 as well which might lead
to read outside of this array.

9 years agoBUG/MEDIUM: channel: fix miscalculation of available buffer space (2nd try)
Willy Tarreau [Mon, 11 Apr 2016 15:04:02 +0000 (17:04 +0200)] 
BUG/MEDIUM: channel: fix miscalculation of available buffer space (2nd try)

Commit 999f643 ("BUG/MEDIUM: channel: fix miscalculation of available buffer
space.") introduced a bug which made output data to be ignored when computing
the remaining room in a buffer. The problem is that channel_may_recv()
properly considers them and may declare that the FD may be polled for read
events, but once the even strikes, channel_recv_limit() called before recv()
says the opposite. In 1.6 and later this case is automatically caught by
polling loop detection at the connection level and is harmless. But the
backport in 1.5 ends up with a busy polling loop as soon as it becomes
possible to have a buffer with this conflict. In order to reproduce it, it
is necessary to have less than [maxrewrite] bytes available in a buffer, no
forwarding enabled (end of transfer) and [buf->o >= maxrewrite - free space].

Since this heavily depends on socket buffers, it will randomly strike users.
On 1.5 with 8kB buffers it was possible to reproduce it with httpterm using
the following command line :

   $ (printf "GET /?s=675000 HTTP/1.0\r\n\r\n"; sleep 60) | \
       nc6 --rcvbuf-size 1 --send-only 127.0.0.1 8002

This bug is only medium in 1.6 and later but is major in the 1.5 backport,
so it must be backported there.

Thanks to Nenad Merdanovic and Janusz Dziemidowicz for reporting this issue
with enough elements to help understand it.

9 years agoBUG/MEDIUM: trace.c: rdtsc() is defined in two files
William Lallemand [Sat, 9 Apr 2016 19:27:21 +0000 (21:27 +0200)] 
BUG/MEDIUM: trace.c: rdtsc() is defined in two files

The rdtsc() function provided in standard.h forbid trace.c to compile
because it's already defined there.

9 years agoDOC: http: rename the unique-id sample and add the documentation
Thierry Fournier [Thu, 7 Apr 2016 13:47:40 +0000 (15:47 +0200)] 
DOC: http: rename the unique-id sample and add the documentation

This patch renames the ssample fetch from "uniqueid" to "unique-id".
It also adds the documentation associated with this sample fetch.

9 years agoOPTIM/MINOR: session: abort if possible before connecting to the backend
Frederik Deweerdt [Thu, 7 Apr 2016 16:01:04 +0000 (09:01 -0700)] 
OPTIM/MINOR: session: abort if possible before connecting to the backend

Depending on the path that led to sess_update_stream_int(), it's
possible that we had a read error on the frontend, but that we haven't
checked if we may abort the connection.

This was seen in particular the following setup: tcp mode, with
abortonclose set, frontend using ssl. If the ssl connection had a first
successful read, but the second read failed, we would stil try to open a
connection to the backend, although we had enough information to close
the connection early.

sess_update_stream_int() had some logic to handle that case in the
SI_ST_QUE and SI_ST_TAR, but that was missing in the SI_ST_ASS case.

This patches addresses the issue by verifying the state of the req
channel (and the abortonclose option) right before opening the
connection to the backend, so we have the opportunity to close the
connection there, and factorizes the shared SI_ST_{QUE,TAR,ASS} code.

9 years agoBUG/MEDIUM: ssl: rewind the BIO when reading certificates
Willy Tarreau [Wed, 6 Apr 2016 17:02:38 +0000 (19:02 +0200)] 
BUG/MEDIUM: ssl: rewind the BIO when reading certificates

Emeric found that some certificate files that were valid with the old method
(the one with the explicit name involving SSL_CTX_use_PrivateKey_file()) do
not work anymore with the new one (the one trying to load multiple cert types
using PEM_read_bio_PrivateKey()). With the last one, the private key couldn't
be loaded.

The difference was related to the ordering in the PEM file was different. The
old method would always work. The new method only works if the private key is
at the top, or if it appears as an "EC" private key. The cause in fact is that
we never rewind the BIO between the various calls. So this patch moves the
loading of the private key as the first step, then it rewinds the BIO, and
then it loads the cert and the chain. With this everything works.

No backport is needed, this issue came with the recent addition of the
multi-cert support.

9 years agoBUG/MINOR : allow to log cookie for tarpit and denied request
Bertrand Paquet [Wed, 6 Apr 2016 09:58:31 +0000 (11:58 +0200)] 
BUG/MINOR : allow to log cookie for tarpit and denied request

The following patch allow to log cookie for tarpit and denied request.
This minor bug affect at least 1.5, 1.6 and 1.7 branch.

The solution is not perfect : may be the cookie processing
(manage_client_side_cookies) can be moved into http_process_req_common.

9 years agoBUG/MINOR: DNS: resolution structure change
Baptiste Assmann [Tue, 5 Apr 2016 19:19:51 +0000 (21:19 +0200)] 
BUG/MINOR: DNS: resolution structure change

060e57301db98853eef91b344b6ace187b657190 introduced a bug, related to a
dns option structure change and an improper rebase.

Thanks Lukas Tribus for reporting it.

backport: 1.7 and above

9 years agoCLEANUP: proto_http: few corrections for gcc warnings.
David Carlier [Mon, 4 Apr 2016 10:54:42 +0000 (11:54 +0100)] 
CLEANUP: proto_http: few corrections for gcc warnings.

first, we modify the signatures of http_msg_forward_body and
http_msg_forward_chunked_body as they are declared as inline
below. Secondly, just verify the returns of the chunk initialization
which holds the Authorization Method (althought it is unlikely to fail  ...).
Both from gcc warnings.

9 years agoBUG/MINOR: dns: trigger a DNS query type change on resolution timeout
Baptiste Assmann [Wed, 6 Jan 2016 01:01:59 +0000 (02:01 +0100)] 
BUG/MINOR: dns: trigger a DNS query type change on resolution timeout

After Cedric Jeanneret reported an issue with HAProxy and DNS resolution
when multiple servers are in use, I saw that the implementation of DNS
query type update on resolution timeout was not implemented, even if it
is documented.

backport: 1.6 and above

9 years agoBUG/MINOR: dns: inapropriate way out after a resolution timeout
Baptiste Assmann [Wed, 6 Jan 2016 00:53:46 +0000 (01:53 +0100)] 
BUG/MINOR: dns: inapropriate way out after a resolution timeout

A bug leading HAProxy to stop DNS resolution when multiple servers are
configured and one is in timeout, the request is not resent.
Current code fix this issue.

backport status: 1.6 and above

9 years agoBUG/MINOR: dumpstats: fix write to global chunk
Conrad Hoffmann [Fri, 1 Apr 2016 18:40:58 +0000 (20:40 +0200)] 
BUG/MINOR: dumpstats: fix write to global chunk

This just happens to work as it is the correct chunk, but should be whatever
gets passed in as argument.

Signed-off-by: Conrad Hoffmann <conrad@soundcloud.com>
9 years agoDOC: fix "needed" typo
Thiago Farina [Fri, 1 Apr 2016 19:43:50 +0000 (16:43 -0300)] 
DOC: fix "needed" typo

While at it use "You" instead of "They" as in the context
it seems to make more sense to refer to "you", as it is
you that are going to be running the command, there is no
"they".

Signed-off-by: Thiago Farina <tfarina@chromium.org>
9 years agoCLEANUP: uniformize last argument of malloc/calloc
Vincent Bernat [Sun, 3 Apr 2016 11:48:43 +0000 (13:48 +0200)] 
CLEANUP: uniformize last argument of malloc/calloc

Instead of repeating the type of the LHS argument (sizeof(struct ...))
in calls to malloc/calloc, we directly use the pointer
name (sizeof(*...)). The following Coccinelle patch was used:

@@
type T;
T *x;
@@

  x = malloc(
- sizeof(T)
+ sizeof(*x)
  )

@@
type T;
T *x;
@@

  x = calloc(1,
- sizeof(T)
+ sizeof(*x)
  )

When the LHS is not just a variable name, no change is made. Moreover,
the following patch was used to ensure that "1" is consistently used as
a first argument of calloc, not the last one:

@@
@@

  calloc(
+ 1,
  ...
- ,1
  )

9 years agoCLEANUP: remove unneeded casts
Vincent Bernat [Sun, 3 Apr 2016 11:48:42 +0000 (13:48 +0200)] 
CLEANUP: remove unneeded casts

In C89, "void *" is automatically promoted to any pointer type. Casting
the result of malloc/calloc to the type of the LHS variable is therefore
unneeded.

Most of this patch was built using this Coccinelle patch:

@@
type T;
@@

- (T *)
  (\(lua_touserdata\|malloc\|calloc\|SSL_get_app_data\|hlua_checkudata\|lua_newuserdata\)(...))

@@
type T;
T *x;
void *data;
@@

  x =
- (T *)
  data

@@
type T;
T *x;
T *data;
@@

  x =
- (T *)
  data

Unfortunately, either Coccinelle or I is too limited to detect situation
where a complex RHS expression is of type "void *" and therefore casting
is not needed. Those cases were manually examined and corrected.

9 years agoMEDIUM: proxy: use dynamic allocation for error dumps
Willy Tarreau [Thu, 31 Mar 2016 11:45:10 +0000 (13:45 +0200)] 
MEDIUM: proxy: use dynamic allocation for error dumps

There are two issues with error captures. The first one is that the
capture size is still hard-coded to BUFSIZE regardless of any possible
tune.bufsize setting and of the fact that frontends only capture request
errors and that backends only capture response errors. The second is that
captures are allocated in both directions for all proxies, which start to
count a lot in configs using thousands of proxies.

This patch changes this so that error captures are allocated only when
needed, and of the proper size. It also refrains from dumping a buffer
that was not allocated, which still allows to emit all relevant info
such as flags and HTTP states. This way it is possible to save up to
32 kB of RAM per proxy in the default configuration.

9 years agoBUG/MEDIUM: stick-tables: some sample-fetch doesn't work in the connection state.
Thierry Fournier [Tue, 29 Mar 2016 19:27:36 +0000 (21:27 +0200)] 
BUG/MEDIUM: stick-tables: some sample-fetch doesn't work in the connection state.

The sc_* sample fetch can work without the struct strm, because the
tracked counters are also stored in the session. So, this patchs
removes the check for the strm existance.

This bug is recent and was introduced in 1.7-dev2 by commit 6204cd9
("BUG/MAJOR: vars: always retrieve the stream and session from the sample")

This bugfix must be backported in 1.6.

9 years agoMINOR: lua: add class listener
Thierry Fournier [Thu, 25 Feb 2016 07:36:46 +0000 (08:36 +0100)] 
MINOR: lua: add class listener

This class provides the access to the listener struct, it allows
some manipulations and retrieve informations.

9 years agoMINOR: lua: add class server
Thierry Fournier [Mon, 22 Feb 2016 07:21:39 +0000 (08:21 +0100)] 
MINOR: lua: add class server

This class provides the access to the server struct, it allows
some manipulations and retrieve informations.

9 years agoMINOR: lua: add class proxy
Thierry Fournier [Fri, 19 Feb 2016 19:56:00 +0000 (20:56 +0100)] 
MINOR: lua: add class proxy

This class provides the access to the proxy struct, it allows
some manipulations and retrieve informations.

9 years agoMINOR: lua: dump general info
Thierry Fournier [Fri, 18 Mar 2016 07:47:13 +0000 (08:47 +0100)] 
MINOR: lua: dump general info

This patch adds function able to dump general haproxy information.

9 years agoMINOR: dumpstats: split stats_dump_be_stats() in two parts
Thierry Fournier [Fri, 25 Mar 2016 07:21:51 +0000 (08:21 +0100)] 
MINOR: dumpstats: split stats_dump_be_stats() in two parts

This patch splits the function stats_dump_be_stats() in two parts. The
part is called stats_fill_be_stats(), and just fill the stats buffer.
This split allows the usage of preformated stats in other parts of HAProxy
like the Lua.

9 years agoMINOR: dumpstats: split stats_dump_sv_stats() in two parts
Thierry Fournier [Fri, 25 Mar 2016 07:21:21 +0000 (08:21 +0100)] 
MINOR: dumpstats: split stats_dump_sv_stats() in two parts

This patch splits the function stats_dump_sv_stats() in two parts. The
extracted part is called stats_fill_sv_stats(), and just fill the stats buffer.
This split allows the usage of preformated stats in other parts of HAProxy
like the Lua.

9 years agoMINOR: dumpstats: split stats_dump_li_stats() in two parts
Thierry Fournier [Fri, 25 Mar 2016 07:20:49 +0000 (08:20 +0100)] 
MINOR: dumpstats: split stats_dump_li_stats() in two parts

This patch splits the function stats_dump_li_stats() in two parts. The
extracted part is called stats_fill_li_stats(), and just fill the stats buffer.
This split allows the usage of preformated stats in other parts of HAProxy
like the Lua.

9 years agoMINOR: dumpstats: split stats_dump_fe_stats() in two parts
Thierry Fournier [Fri, 25 Mar 2016 07:20:11 +0000 (08:20 +0100)] 
MINOR: dumpstats: split stats_dump_fe_stats() in two parts

This patch splits the function stats_dump_fe_stats() in two parts. The
extracted part is called stats_fill_fe_stats(), and just fill the stats buffer.
This split allows the usage of preformated stats in other parts of HAProxy
like the Lua.

9 years agoMINOR: dumpstats: split stats_dump_info_to_buffer() in two parts
Thierry Fournier [Fri, 25 Mar 2016 07:19:23 +0000 (08:19 +0100)] 
MINOR: dumpstats: split stats_dump_info_to_buffer() in two parts

This patch splits the function stats_dump_info_to_buffer() in two parts. The
extracted part is called stats_fill_info(), and just fill the stats buffer.
This split allows the usage of preformated stats in other parts of HAProxy
like the Lua.

9 years agoMINOR: dumpstats: extract stats fields enum and names
Thierry Fournier [Wed, 23 Mar 2016 15:25:49 +0000 (16:25 +0100)] 
MINOR: dumpstats: extract stats fields enum and names

These field names can be used outside of the dumpstats file.
This will be useful for exporting stats in Lua.

9 years agoMINOR: http: sample fetch which returns unique-id
Thierry Fournier [Tue, 29 Mar 2016 15:23:51 +0000 (17:23 +0200)] 
MINOR: http: sample fetch which returns unique-id

This patch adds a sample fetch which returns the unique-id if it is
configured. If the unique-id is not yet generated, it build it. If
the unique-id is not configured, it returns none.

9 years agoDOC: name set-gpt0 mismatch with the expected keyword
Thierry Fournier [Tue, 29 Mar 2016 17:34:37 +0000 (19:34 +0200)] 
DOC: name set-gpt0 mismatch with the expected keyword

replace set-gpt0 by sc-set-gpt0

must be backported in 1.6

9 years agoDOC: typo: req.uri is now replaced by capture.req.uri
Baptiste Assmann [Sun, 6 Mar 2016 22:42:52 +0000 (23:42 +0100)] 
DOC: typo: req.uri is now replaced by capture.req.uri

A configuration example was not updated after the switch from req.uri to
capture.req.uri.

backport: 1.5 and above

9 years agoDOC: regsub: parser limitation about the inability to use closing square brackets
Baptiste Assmann [Sun, 6 Mar 2016 22:36:48 +0000 (23:36 +0100)] 
DOC: regsub: parser limitation about the inability to use closing square brackets

We can't match range in regsub, since the closing bracket is evaluated
by the configuration parser.

backport: 1.6 and above

9 years agoDOC: typo: maxconn paragraph is wrong due to a wrong buffer size
Baptiste Assmann [Sun, 6 Mar 2016 22:34:31 +0000 (23:34 +0100)] 
DOC: typo: maxconn paragraph is wrong due to a wrong buffer size

HAProxy allocates 2 tune.bufsize, which is by default 16kB.

backport: 1.4 and above

9 years agoDOC: typo: ACL subdir match
Baptiste Assmann [Sun, 6 Mar 2016 22:32:10 +0000 (23:32 +0100)] 
DOC: typo: ACL subdir match

ACL subdir match is "-m dir"

backport: 1.5 and above

9 years agoDOC: stick-table: amend paragraph blaming the loss of table upon reload
Baptiste Assmann [Sun, 6 Mar 2016 22:29:28 +0000 (23:29 +0100)] 
DOC: stick-table: amend paragraph blaming the loss of table upon reload

This statement is not true anymore since we have the peers in HAProxy.

backport: 1.6 and above

9 years agoDOC: typo on stick-store response
Baptiste Assmann [Sun, 6 Mar 2016 22:27:24 +0000 (23:27 +0100)] 
DOC: typo on stick-store response

It is used to store responses, and not requests.

backport: 1.5 and above

9 years agoDOC: timeout client: pointers to timeout http-request
Baptiste Assmann [Sun, 6 Mar 2016 22:24:12 +0000 (23:24 +0100)] 
DOC: timeout client: pointers to timeout http-request

It worth mentionning "timeout http-request" in the "timeout client"
documentation paragraph, to ensure nobody misses this important setting.

backport: 1.5 and above

9 years agoDOC: "addr" parameter applies to both health and agent checks
Baptiste Assmann [Sun, 6 Mar 2016 22:14:36 +0000 (23:14 +0100)] 
DOC: "addr" parameter applies to both health and agent checks

It was not obvious in the documentation that the server's "addr"
parameter applies to both the agent and the health check.

backport: 1.5 and above

9 years agoMINOR: lua: convert field to lua type
Thierry Fournier [Wed, 16 Mar 2016 17:29:13 +0000 (18:29 +0100)] 
MINOR: lua: convert field to lua type

This function converts a field used by stats in lua type.

9 years agoMINOR: lua: Add internal function which strip spaces
Thierry Fournier [Wed, 24 Feb 2016 07:06:32 +0000 (08:06 +0100)] 
MINOR: lua: Add internal function which strip spaces

Some internal HAproxy error message are provided with a final '\n'.
Its typically for the integration in the CLI. Sometimes, these messages
are returned as Lua string. These string must be without "\n" or final
spaces.

This patch adds a function whoch removes unrequired parameters.

9 years agoMINOR: lua: post initialization
Thierry Fournier [Fri, 19 Feb 2016 19:53:30 +0000 (20:53 +0100)] 
MINOR: lua: post initialization

This patch adds a Lua post initialisation wrapper. It already exists for
pure Lua function, now it executes also C. It is useful for doing things
when the configuration is ready to use. For example we can can browse and
register all the proxies.