The buffer pointer is now taken from the http_msg in the following
functions :
http_parse_chunk_size
http_forward_trailers
http_skip_chunk_crlf
Most internal pointers were converted to const as the result of the
operation.
The buffer pointer is now taken from the http_msg in the following functions :
- http_remove_header2
- http_header_add_tail
- http_header_add_tail2
- http_parse_connection_header
- http_change_connection_header
msg->sol is now a relative pointer just like all other ones. There is no
more absolute references to the buffer outside the struct buffer itself.
Next two cleanups should include removing buffer references to functions
which already have an msg, and removal of wrapping detection in request
and response parsing which cannot wrap by definition.
ACLs and patterns only rely on a struct http_msg and don't know the pointer
to the actual data. struct http_msg will soon only hold relative references
so that's not possible. We need http_msg to hold a reference to the struct
buffer before having relative pointers everywhere.
It is likely that doing so will also result in opportunities to simplify
a number of functions arguments. The following functions are already
candidate :
http_buffer_heavy_realign
http_capture_bad_message
http_change_connection_header
http_forward_trailers
http_header_add_tail
http_header_add_tail2
http_msg_analyzer
http_parse_chunk_size
http_parse_connection_header
http_remove_header2
http_send_name_header
http_skip_chunk_crlf
http_upgrade_v09_to_v10
These offsets were relative to the buffer itself. Now they're relative to
the buffer's origin (buf->p) which normally corresponds to the start of
current message.
This saves a big dependency between the HTTP message struct and the buffers.
It appeared during this change that ->col is not used anymore (it will have
to be removed). Next step is to turn ->eol and ->sol from absolute to relative.
The buffer's pointer <lr> was only used by HTTP parsers which also use a
struct http_msg to keep track of the parser's state. We've reached a point
where it makes no sense to keep ->lr in the buffer, as the split between
buffer and msg is only arbitrary for historical reasons.
This change ensures that touching buffers will not impact HTTP messages
anymore, making the buffers more content-agnostic. However, it becomes
very important not to forget to update msg->next when some data get
forwarded or moved (and in general each time buf->p is updated).
The new pointer in http_msg becomes relative to buffer->p so that
parsing multiple messages becomes easier. It is possible that at one
point ->som and ->next will be merged.
Note: http_parse_reqline() and http_parse_stsline() have been temporarily
modified to know the message starting point in the buffer (->p).
This change gets rid of buf->r which is always equal to buf->p + buf->i.
It removed some wrapping detection at a number of places, but required addition
of new relative offset computations at other locations. A large number of places
can be simplified now with extreme care, since most of the time, either the
pointer has to be computed once or we need a difference between the old ->w and
old ->r to compute free space. The cleanup will probably happen with the rewrite
of the buffer_input_* and buffer_output_* functions anyway.
buf->lr still has to move to the struct http_msg and be relative to buf->p
for the rework to be complete.
This change introduces the buffer's base pointer, which is the limit between
incoming and outgoing data. It's the point where the parsing should start
from. A number of computations have already been greatly simplified, but
more simplifications are expected to come from the removal of buf->r.
The changes appear good and have revealed occasional improper use of some
pointers. It is possible that this patch has introduced bugs or revealed
some, although preliminary testings tend to indicate that everything still
works as it should.
We don't have buf->l anymore. We have buf->i for pending data and
the total length is retrieved by adding buf->o. Some computation
already become simpler.
Despite extreme care, bugs are not excluded.
It's worth noting that msg->err_pos as set by HTTP request/response
analysers becomes relative to pending data and not to the beginning
of the buffer. This has not been completed yet so differences might
occur when outgoing data are left in the buffer.
Too many flags are stored in the transaction structure. Some flags are
clearly message-specific and exist in two versions (request and response).
Move them to a new "flags" field in the http_message struct instead.
There were a few unchecked write() calls in the debug code that cause
gcc 4.x to emit warnings on recent libc. We don't want to check them
as we can't make anything from the result, let's simply surround them
with an empty if statement.
Note that one of the warnings was for chdir("/") which normally cannot
fail since it follows a successful chroot (which means the perms are
necessarily there). Anyway let's move the call uppe to protect it too.
memprintf() is just like snprintf() except that it always returns a properly
sized allocated string that the caller is responsible for freeing. NULL is
returned on serious errors. It also supports stackable calls over the same
pointer since it offers support for automatically freeing a previous one :
memprintf(&err, "invalid argument: '%s'", arg);
...
memprintf(&err, "keyword parser said: <%s>", *err);
...
memprintf(&err, "line parser said: %s\n", *err);
...
free(*err);
The issue only happens when DEBUG_FULL is enabled, which causes
http_msg_analyzer() to complain if it's called twice with an invalid
message, for instance because of two consecutive ACLs using req_proto_http.
The code is commented out when DEBUG_FULL is disabled, so this is not a bug,
just an annoyance for the developer.
The three warnings below are totally wrong since the variables depend on another
one which is only turned on when the variables are initialized. Still this gcc-4.1.2
isn't able to see this and prefers to complain wrongly. So let's initialize the
variables to shut it up since we're not in the fast path.
src/proto_http.c: In function 'acl_fetch_any_cookie_cnt':
src/proto_http.c:8393: warning: 'val_end' may be used uninitialized in this function
src/proto_http.c: In function 'http_process_req_stat_post':
src/proto_http.c:2577: warning: 'st_next_param' may be used uninitialized in this function
src/proto_http.c:2577: warning: 'st_cur_param' may be used uninitialized in this function
It's very annoying that we have to deal with the crappy size_t and with ints
at some places because these ones don't mix well. Patch 6f61b2 changed the
chunk len to int but its size remains size_t and some functions are having
trouble being used by several callers depending on the type of their arguments.
Let's turn extract_cookie_value() to int for now on, and plan a massive cleanup
later to remove all size_t.
I have modified dumpstats.c to show additional information for the show
session <id> command on the statistics socket. This will dump the
public, frontend, backend, and server ip/tcp addresses and port. We
found it useful to have this information available in real time and
could not find another way of getting this information.
The wrong byte was checked for the session_id length in the payload. This
used to work when the session ID was absent because zero was found there,
but when a session ID is present, there is 1/256 chance that the inspected
data contains 0x20 (the actual session ID length), so it fails.
Thanks to Emmanuel Bézagu for reporting this bug.
This bug does not need backporting, it is 1.5 specific.
These callbacks are used to retrieve the source and destination address
of a socket. The address flags are not hold on the stream interface and
not on the session anymore. The addresses are collected when needed.
This still needs to be improved to store the IP and port separately so
that it is not needed to perform a getsockname() when only the IP address
is desired for outgoing traffic.
The Unique ID, is an ID generated with several informations. You can use
a log-format string to customize it, with the "unique-id-format" keyword,
and insert it in the request header, with the "unique-id-header" keyword.
%Fi: Frontend IP
%Fp: Frontend Port
%Si: Server IP
%Sp: Server Port
%Ts: Timestamp
%rt: HTTP request counter
%H: hostname
%pid: PID
+X: Hexadecimal represenation
The +X mode in logformat displays hexadecimal for the following flags
%Ci %Cp %Fi %Fp %Bi %Bp %Si %Sp %Ts %ct %pid
rename logformat_write_string() to lf_text()
Optimize size computation
* logformat functions now take a format linked list as argument
* build_logline() build a logline using a format linked list
* rename LOG_* by LOG_FMT_* in enum
* improve error management in build_logline()
The ACL matches rely on the extract_cookie_value() function as used for
for patterns. This permits ACLs to match cookie values based on the cookie
name instead of having to perform substring matching on the cookie header.
Sometimes it is desirable to forward a particular request to a specific
server without having to declare a dedicated backend for this server. This
can be achieved using the "use-server" rules. These rules are evaluated after
the "redirect" rules and before evaluating cookies, and they have precedence
on them. There may be as many "use-server" rules as desired. All of these
rules are evaluated in their declaration order, and the first one which
matches will assign the server.
memcmp()/strcmp() calls were needed in different parts of code to determine
the status code. Each new status code introduces new calls, which can become
inefficient and source of bugs.
This patch reorganizes the code to rely on a numeric status code internally
and to be hopefully more generic.
Proxy ids are unique whereas names can be used several times in the
configuration. In order to prevent the ambiguity, the HTML form now provides
the backend id instead of its name (the name can still be provided in the POST
data).
Previously, the stats admin page required POST parameters to be provided
exactly in the same order as the HTML form.
This patch allows to handle those parameters in any orders.
Also, note that haproxy won't alter server states anymore if backend or server
names are ambiguous (duplicated names in the configuration) to prevent
unexpected results (the same should probably be applied to the stats socket).
The hash of IPv6 addresses was not properly aligned and resulted in the
last quarter of the address not being hashed. In practice, this is rarely
detected since MAC addresses are used in the second half. But this becomes
very visible with IPv6-mapped IPv4 addresses such as ::FFFF:1.2.3.4 where
the IPv4 part is never hashed.
This bug has been there forever, since introduction of "balance source" in
v1.2.11. The fix must then be backported to all stable versions.
Thanks to Alex Markham for reporting this issue to the list !
Possible zero-pointer deference in sess_log().
Checks of return values in sess_log() fix the issue.
Fix bad computation in logformat_write_string().
This issue is 1.5-specific and was introduced just before 1.5-dev8.
No backport is needed.
Released version 1.5-dev8 with the following main changes :
- MINOR: patch for minor typo (ressources/resources)
- MEDIUM: http: add support for sending the server's name in the outgoing request
- DOC: mention that default checks are TCP connections
- BUG/MINOR: fix options forwardfor if-none when an alternative header name is specified
- CLEANUP: Make check_statuses, analyze_statuses and process_chk static
- CLEANUP: Fix HCHK spelling errors
- BUG/MINOR: fix typo in processing of http-send-name-header
- MEDIUM: log: Use linked lists for loggers
- BUILD: fix declaration inside a scope block
- REORG: log: split send_log function
- MINOR: config: Parse the string of the log-format config keyword
- MINOR: add ultoa, ulltoa, ltoa, lltoa implementations
- MINOR: Date and time fonctions that don't use snprintf
- MEDIUM: log: make http_sess_log use log_format
- DOC: log-format documentation
- MEDIUM: log: use log_format for mode tcplog
- MEDIUM: log-format: backend source address %Bi %Bp
- BUG/MINOR: log-format: fix %o flag
- BUG/MEDIUM: bad length in log_format and __send_log
- MINOR: logformat %st is signed
- BUILD/MINOR: fix the source URL in the spec file
- DOC: acl is http_first_req, not http_req_first
- BUG/MEDIUM: don't trim last spaces from headers consisting only of spaces
- MINOR: acl: add new matches for header/path/url length
- BUILD: halog: make halog build on solaris
- BUG/MINOR: don't use a wrong port when connecting to a server with mapped ports
- MINOR: remove the client/server side distinction in SI addresses
- MINOR: halog: add support for matching queued requests
- DOC: indicate that cookie "prefix" and "indirect" should not be mixed
- OPTIM/MINOR: move struct sockaddr_storage to the tail of structs
- OPTIM/MINOR: make it possible to change pipe size (tune.pipesize)
- BUILD/MINOR: silent a build warning in src/pipe.c (fcntl)
- OPTIM/MINOR: move the hdr_idx pools out of the proxy struct
- MEDIUM: tune.http.maxhdr makes it possible to configure the maximum number of HTTP headers
- BUG/MINOR: fix a segfault when parsing a config with undeclared peers
- CLEANUP: rename possibly confusing struct field "tracked"
- BUG/MEDIUM: checks: fix slowstart behaviour when server tracking is in use
- MINOR: config: tolerate server "cookie" setting in non-HTTP mode
- MEDIUM: buffers: add some new primitives and rework existing ones
- BUG: buffers: don't return a negative value on buffer_total_space_res()
- MINOR: buffers: make buffer_pointer() support negative pointers too
- CLEANUP: kill buffer_replace() and use an inline instead
- BUG: tcp: option nolinger does not work on backends
- CLEANUP: ebtree: remove a few annoying signedness warnings
- CLEANUP: ebtree: clarify licence and update to 6.0.6
- CLEANUP: ebtree: remove 4-year old harmless typo in duplicates insertion code
- CLEANUP: ebtree: remove another typo, a wrong initialization in insertion code
- BUG: ebtree: ebst_lookup() could return the wrong entry
- OPTIM: stream_sock: reduce the amount of in-flight spliced data
- OPTIM: stream_sock: save a failed recv syscall when splice returns EAGAIN
- MINOR: acl: add support for TLS server name matching using SNI
- BUG: http: re-enable TCP quick-ack upon incomplete HTTP requests
- BUG: proto_tcp: don't try to bind to a foreign address if sin_family is unknown
- MINOR: pattern: export the global temporary pattern
- CLEANUP: patterns: get rid of pattern_data_setstring()
- MEDIUM: acl: use temp_pattern to store fetched information in the "method" match
- MINOR: acl: include pattern.h to make pattern migration more transparent
- MEDIUM: pattern: change the pattern data integer from unsigned to signed
- MEDIUM: acl: use temp_pattern to store any integer-type information
- MEDIUM: acl: use temp_pattern to store any address-type information
- CLEANUP: acl: integer part of acl_test is not used anymore
- MEDIUM: acl: use temp_pattern to store any string-type information
- CLEANUP: acl: remove last data fields from the acl_test struct
- MEDIUM: http: replace get_ip_from_hdr2() with http_get_hdr()
- MEDIUM: patterns: the hdr() pattern is now of type string
- DOC: add minimal documentation on how ACLs work internally
- DOC: add a coding-style file
- OPTIM: halog: keep a fast path for the lines-count only
- CLEANUP: silence a warning when building on sparc
- BUG: http: tighten the list of allowed characters in a URI
- MEDIUM: http: block non-ASCII characters in URIs by default
- DOC: add some documentation from RFC3986 about URI format
- BUG/MINOR: cli: correctly remove the whole table on "clear table"
- BUG/MEDIUM: correctly disable servers tracking another disabled servers.
- BUG/MEDIUM: zero-weight servers must not dequeue requests from the backend
- MINOR: halog: add some help on the command line
- BUILD: fix build error on FreeBSD
- BUG: fix double free in peers config error path
- MEDIUM: improve config check return codes
- BUILD: make it possible to look for pcre in the default system paths
- MINOR: config: emit a warning when 'default_backend' masks servers
- MINOR: backend: rework the LC definition to support other connection-based algos
- MEDIUM: backend: add the 'first' balancing algorithm
- BUG: fix httplog trailing LF
- MEDIUM: increase chunk-size limit to 2GB-1
- BUG: queue: fix dequeueing sequence on HTTP keep-alive sessions
- BUG: http: disable TCP delayed ACKs when forwarding content-length data
- BUG: checks: fix server maintenance exit sequence
- BUG/MINOR: stream_sock: don't remove BF_EXPECT_MORE and BF_SEND_DONTWAIT on partial writes
- DOC: enumerate valid status codes for "observe layer7"
- MINOR: buffer: switch a number of buffer args to const
- CLEANUP: silence signedness warning in acl.c
- BUG: stream_sock: si->release was not called upon shutw()
- MINOR: log: use "%ts" to log term status only and "%tsc" to log with cookie
- BUG/CRITICAL: log: fix risk of crash in development snapshot
- BUG/MAJOR: possible crash when using capture headers on TCP frontends
- MINOR: config: disable header captures in TCP mode and complain
Olufemi Omojola provided a config and a core showing a possible crash
when captures are configured on a TCP-mode frontend which branches to
an HTTP backend. The reason is that being in TCP mode, the frontend
does not allocate capture pools for the request, but the HTTP backend
tries to use them and dies on the NULL.
While such a config has long been unlikely to happen, it looks like
people using websocket tend to do this more often now.
Change the control to use the pointer instead of the number of captures
to know when to log.
This bug was reported in 1.4.20, so it must be backported there.
The main stats page says "ressources" (French spelling) rather than
"resources" (English spelling).
One little patch attached (against v1.4.20).
Many thanks,
Adrian
__send_log(): the size of the buffer sent is wrong when the facility
is lower than 3 digits.
logformat_write_string(): computation of size is wrong
Note: this was introduced after 1.5-dev7, no backport needed.
Commit a1cc38 introduced a regression which was easy to trigger till ad4cd58
(snapshots 20120222 to 20120311 included). The bug was still present after
that but harder to trigger.
The bug is caused by the use of two distinct log buffers due to intermediary
changes. The issue happens when an HTTP request is logged just after a TCP
request during the same second and the HTTP request is too large for the buffer.
In this case, it happens that the HTTP request is logged into the TCP buffer
instead and that length controls can't detect anything.
Starting with bddd4f, the issue is still possible when logging too large an
HTTP request just after a send_log() call (typically a server status change).
We owe a big thanks to Sander Klein for testing several snapshots and more
specifically for taking significant risks in production by letting the buggy
version crash several times in order to provide an exploitable core ! The bug
could not have been found without this precious help. Thank you Sander !
This fix does not need to be backported, it did not affect any released version.
The difference could be seen when logging a request in HTTP mode with option
tcplog, as it would keep emitting 4 chars. Better use two distinct flags to
clear the confusion.
%Bi return the backend source IP
%Bp return the backend source port
Add a function pointer in logformat_type to do additional configuration
during the log-format variable parsing.
Merge http_sess_log() and tcp_sess_log() to sess_log() and move it to
log.c
A new field in logformat_type define if you can use a logformat
variable in TCP or HTTP mode.
doc: log-format in tcp mode
Note that due to the way log buffer allocation currently works, trying to
log an HTTP request without "option httplog" is still not possible. This
will change in the near future.
The ->release function of the stream interface is never called upon
a shutw() because it's placed after a return statement. It is possible
that it has impacted inter-process stick-table replication by preventing
a full resync after certain sequences of connection breakage. Since this
bug has been present since the introduction of the ->release() callback,
it cannot have caused regressions, just possibly non-working situations.
This was detected at Exceliance by Emeric Brun during a code review. It
is 1.5-specific.
The flags are one-shot but should be maintained over all send() operations
as long as send_max is not flushed. The flags were incidentely cleared once
a complete send() was performed, regardless of the fact that the send()
might have been on the first half of a buffer before a wrapping. The result
is that on wrapping data (eg: which happens often with chunked encoding),
many incomplete segments are transmitted instead of being aggregated.
The fix consists in only flushing the flags only once send_max is empty,
which was the expected behaviour.
This fix should be backported to 1.4 though it is not critical, just sub-optimal.
Recent commit 62c3be broke maintenance mode by fixing srv_is_usable().
Enabling a disabled server would not re-introduce it into the farm.
The reason is that in set_server_up(), the SRV_MAINTAIN flag is still
present when recounting the servers. The flag was removed late only to
adjust a log message. Keep a copy of the old flag instead and update
SRV_MAINTAIN earlier.
This fix must also be backported to 1.4 (but no release got the regression).
Commits 5c6209 and 072930 were aimed at avoiding undesirable PUSH flags
when forwarding chunked data, but had the undesired effect of causing
data advertised by content-length to be affected by the delayed ACK too.
This can happen when the data to be forwarded are small enough to fit into
a single send() call, otherwise the BF_EXPECT_MORE flag would be removed.
Content-length data don't need the BF_EXPECT_MORE flag since the low-level
forwarder already knows it can safely rely on bf->to_forward to set the
appropriate TCP flags.
Note that the issue is only observed in requests at the moment, though the
later introduction of server-side keep-alive could trigger the issue on the
response path too.
Special thanks to Randy Shults for reporting this issue with a lot of
details helping to reproduce it.
The fix must be backported to 1.4.
When a request completes on a server and the server connection is closed
while the client connection stays open, the HTTP engine releases all server
connection slots and scans the queues to offer the connection slot to
another pending request.
An issue happens when the released connection allows other requests to be
dequeued : may_dequeue_tasks() relies on srv->served which is only decremented
by sess_change_server() which itself is only called after may_dequeue_tasks().
This results in no connection being woken up until another connection terminates
so that may_dequeue_tasks() is called again.
This fix is minimalist and only moves sess_change_server() earlier (which is
safe). It should be reworked and the code factored out so that the same occurrence
in session.c shares the same code.
This bug has been there since the introduction of option-http-server-close and
the fix must be backported to 1.4.
Since commit 115acb97, chunk size was limited to 256MB. There is no reason for
such a limit and the comment on the code suggests a missing zero. However,
increasing the limit past 2 GB causes trouble due to some 32-bit subtracts
in various computations becoming negative (eg: buffer_max_len). So let's limit
the chunk size to 2 GB - 1 max.
commit a1cc3811 introduced an undesirable \0\n ending on HTTP log messages. This
is because of an extra character count passed to __send_log() which causes the LF
to be appended past the \0. Some syslog daemons thus log an extra empty line. The
fix is obvious. Fix the function comments to remind what they expect on their input.
This is past 1.5-dev7 regression so there's no backport needed.
The principle behind this load balancing algorithm was first imagined
and modeled by Steen Larsen then iteratively refined through several
work sessions until it would totally address its original goal.
The purpose of this algorithm is to always use the smallest number of
servers so that extra servers can be powered off during non-intensive
hours. Additional tools may be used to do that work, possibly by
locally monitoring the servers' activity.
The first server with available connection slots receives the connection.
The servers are choosen from the lowest numeric identifier to the highest
(see server parameter "id"), which defaults to the server's position in
the farm. Once a server reaches its maxconn value, the next server is used.
It does not make sense to use this algorithm without setting maxconn. Note
that it can however make sense to use minconn so that servers are not used
at full load before starting new servers, and so that introduction of new
servers requires a progressively increasing load (the number of servers
would more or less follow the square root of the load until maxconn is
reached). This algorithm ignores the server weight, and is more beneficial
to long sessions such as RDP or IMAP than HTTP, though it can be useful
there too.
http_sess_log now use the logformat linked list to make the log
string, snprintf is not used for speed issue.
CLF mode also uses logformat.
NOTE: as of now, empty fields in CLF now are "" not "-" anymore.
parse_logformat_string: parse the string, detect the type: text,
separator or variable
parse_logformat_var: dectect variable name
parse_logformat_var_args: parse arguments and flags
add_to_logformat_list: add to the logformat linked list
send_log function is now splited in 3 functions
* hdr_log: generate the syslog header
* send_log: send a syslog message with a printf format string
* __send_log: send a syslog message
When checking a configuration file using "-c -f xxx", sometimes it is
reported that a config is valid while it will later fail (eg: no enabled
listener). Instead, let's improve the return values :
- return 0 if config is 100% OK
- return 1 if config has errors
- return 2 if config is OK but no listener nor peer is enabled
If the local host is not found as a peer in a "peers" section, we have a
double free, and possibly a use-after-free because the peers section is
freed since it's aliased as the table's name.
Marcello Gorlani reported that commit 5e205524ad
(BUG: http: re-enable TCP quick-ack upon incomplete HTTP requests) broke build
on FreeBSD.
Moving the include lower fixes the issue. This must be backported to 1.4 too.
It was reported that a server configured with a zero weight would
sometimes still take connections from the backend queue. This issue is
real, it happens this way :
1) the disabled server accepts a request with a cookie
2) many cookie-less requests accumulate in the backend queue
3) when the disabled server completes its request, it checks its own
queue and the backend's queue
4) the server takes a pending request from the backend queue and
processes it. In response, the server's cookie is assigned to
the client, which ensures that some requests will continue to
be served by this server, leading back to point 1 above.
The fix consists in preventing a zero-weight server from dequeuing pending
requests from the backend. Making use of srv_is_usable() in such tests makes
the tests more robust against future changes.
This fix must be backported to 1.4 and 1.3.
In a config where server "s1" is marked disabled and "s2" tracks "s1",
s2 appears disabled on the stats but is still inserted into the LB farm
because the tracking is resolved too late in the configuration process.
We now resolve tracked servers before building LB maps and we also mark
the tracking server in maintenance mode, which previously was not done,
causing half of the issue.
Last point is that we also protect srv_is_usable() against electing a
server marked for maintenance. This is not absolutely needed but is a
safe choice and makes a lot of sense.
This fix must be backported to 1.4.
I downloaded version 1.4.19 this morning. While merging the code changes
to a custom build that we have here for our project I noticed a typo in
'session.c', in the new code for inserting the server name in the HTTP
header. The fix that I did is shown in the patch below.
[WT: the bug is harmless, it is only suboptimal]
Joe Price reported that "clear table xxx" sent on the CLI would only clear
the last entry. This is true, some code was missing to remove an entry from
within the loop, and only the final condition was able to remove an entry.
The fix is obvious. No backport is needed.
These ones are invalid and blocked unless "option accept-invalid-http-request"
is specified in the frontend. In any case, the faulty request is logged.
Note that some of the remaining invalid chars are still not checked against,
those are the invalid ones between 32 and 127 :
34 ('"'), 60 ('<'), 62 ('>'), 92 ('\'), 94 ('^'),
96 ('`'), 123 ('{'), 124 ('|'), 125 ('}')
Using a lookup table might be better at some point.
The HTTP request parser was considering that any non-LWS char was
par of the URI. Unfortunately, this allows control chars to be sent
in the URI, sometimes resulting in backend servers misbehaving, for
instance when they interprete \0 as an end of string and respond
with plain HTTP/0.9 without headers, that haproxy blocks as invalid
responses.
RFC3986 clearly states the list of allowed characters in a URI. Even
non-ASCII chars are not allowed. Unfortunately, after having run 10
years with these chars allowed, we can't block them right now without
an optional workaround. So the first step consists in only blocking
control chars. A later patch will allow non-ASCII only when an appropriate
option is enabled in the frontend.
Control chars are 0..31 and 127, with the exception of 9, 10 and 13
(\t, \n, \r).
On Solaris/sparc, getpid() returns pid_t which is not an int :
src/peers.c: In function `peer_io_handler':
src/peers.c:508: warning: int format, pid_t arg (arg 6)
New option "http-send-name-header" specifies the name of a header which
will hold the server name in outgoing requests. This is the name of the
server the connection is really sent to, which means that upon redispatches,
the header's value is updated so that it always matches the server's name.
This pattern previously was limited to type IP. With the new header
extraction function, it becomes possible to extract strings, so that
the header can be returned as a string. This will not change anything
to existing configs, as string will automatically be converted to IP
when needed. However, new configs will be able to use IPv6 addresses
from headers in stick-tables, as well as stick on any non-IP header
(eg: host, user-agent, ...).
The new function does not return IP addresses but header values instead,
so that the caller is free to make what it want of them. The conversion
is not quite clean yet, as the previous test which considered that address
0.0.0.0 meant "no address" is still used. A different IP parsing function
should be used to take this into account.
Now strings and data blocks are stored in the temp_pattern's chunk
and matched against this one.
The rdp_cookie currently makes extensive use of acl_fetch_rdp_cookie()
and will be a good candidate for the initial rework so that ACLs use
the patterns framework and not the other way around.
IPv4 and IPv6 addresses are now stored into temp_pattern instead of
the dirty hack consisting into storing them into the consumer's target
address.
Some refactoring should now be possible since the methods used to fetch
source and destination addresses are similar between patterns and ACLs.
All ACL fetches which return integer value now store the result into
the temporary pattern struct. All ACL matches which rely on integer
also get their value there.
Note: the pattern data types are not set right now.
This function was only used to call chunk_init_len() from another chunk,
which in the end consists in simply assigning the source chunk to the
destination chunk. Let's remove this indirection to make the code clearer.
Anyway it was the only place such a function was used.
This is 1.5-specific. It causes issues with transparent source binding involving
hdr_ip. We must not try to bind() to a foreign address when the family is not set,
and we must set the family when an address is set.
By default we disable TCP quick-acking on HTTP requests so that we
avoid sending a pure ACK immediately followed by the HTTP response.
However, if the client sends an incomplete request in a short packet,
its TCP stack might wait for this packet to be ACKed before sending
the rest of the request, delaying incoming requests by up to 40-200ms.
We can detect this undesirable situation when parsing the request :
- if an incomplete request is received
- if a full request is received and uses chunked encoding or advertises
a content-length larger than the data available in the buffer
In these situations, we re-enable TCP quick-ack if we had previously
disabled it.
Server Name Indication (SNI) is a TLS extension which makes a client
present the name of the server it is connecting to in the client hello.
It allows a transparent proxy to take a decision based on the beginning
of an SSL/TLS stream without deciphering it.
The new ACL "req_ssl_sni" matches the name extracted from the TLS
handshake against a list of names which may be loaded from a file if
needed.
When splice() returns EAGAIN, on old kernels it could be caused by a read
shutdown which was not detected. Due to this behaviour, we had to fall
back to recv(), which in turn says if it's a real EAGAIN or a shutdown.
Since this behaviour was fixed in 2.6.27.14, on more recent kernels we'd
prefer to avoid the fallback to recv() when possible. For this, we set a
variable the first time splice() detects a shutdown, to indicate that it
works. We can then rely on this variable to adjust our behaviour.
Doing this alone increased the overall performance by about 1% on medium
sized objects.
First, it's a waste not to call chk_snd() when spliced data are available,
because the pipe can almost always be transferred into the outgoing socket
buffers. Starting from now, when we splice data in, we immediately try to
send them. This results in less pipes used, and possibly less kernel memory
in use at once.
Second, if a pipe cannot be transferred into the outgoing socket buffers,
it means this buffer is full. There's no point trying again then, as space
will almost never be available, resulting in a useless syscall returning
EAGAIN.
Daniel Rankov reported that "option nolinger" is inefficient on backends.
The reason is that it is set on the file descriptor only, which does not
prevent haproxy from performing a clean shutdown() before closing. We must
set the flag on the stream_interface instead if we want an RST to be emitted
upon active close.
A number of primitives were missing for buffer management, and some
of them were particularly awkward to use. Specifically, the functions
used to compute free space could not always be used depending what was
wrapping in the buffers. Some documentation has been added about how
the buffers work and their properties. Some functions are still missing
such as a buffer replacement which would support wrapping buffers.
This patch settles the 2 loggers limitation.
Loggers are now stored in linked lists.
Using "global log", the global loggers list content is added at the end
of the current proxy list. Each "log" entries are added at the end of
the proxy list.
"no log" flush a logger list.
Up to now, if a cookie value was specified on a server when the proxy was
in TCP mode, it would cause a fatal error. Now we only report a warning,
since the cookie will be ignored. This makes it easier to generate configs
from scripts.
Ludovic Levesque reported and diagnosed an annoying bug. When a server is
configured to track another one and has a slowstart interval set, it's
assigned a minimal weight when the tracked server goes back up but keeps
this weight forever.
This is because the throttling during the warmup phase is only computed
in the health checking function.
After several attempts to resolve the issue, the only real solution is to
split the check processing task in two tasks, one for the checks and one
for the warmup. Each server with a slowstart setting has a warmum task
which is responsible for updating the server's weight after a down to up
transition. The task does not run in othe situations.
In the end, the fix is neither complex nor long and should be backported
to 1.4 since the issue was detected there first.
When reading the code, the "tracked" member of a server makes one
think the server is tracked while it's the opposite, it's a pointer
to the server being tracked. This is particularly true in constructs
such as :
if (srv->tracked) {
Since it's the second time I get caught misunderstanding it, let's
rename it to "track" to avoid the confusion.
Baptiste Assmann reported that a config where a non-existing peers
section is referenced by a stick-table causes a segfault after displaying
the error. This is caused by the freeing of the peers. Setting it to NULL
after displaying the error fixes the issue.
For a long time, the max number of headers was taken as a part of the buffer
size. Since the header size can be configured at runtime, it does not make
much sense anymore.
Nothing was making it necessary to have a static value, so let's turn this into
a tunable with a default value of 101 which equals what was previously used.
It makes no sense to have one pointer to the hdr_idx pool in each proxy
struct since these pools do not depend on the proxy. Let's have a common
pool instead as it is already the case for other types.
By default, pipes are the default size for the system. But sometimes when
using TCP splicing, it can improve performance to increase pipe sizes,
especially if it is suspected that pipes are not filled and that many
calls to splice() are performed. This has an impact on the kernel's
memory footprint, so this must not be changed if impacts are not understood.
Stream interfaces used to distinguish between client and server addresses
because they were previously of different types (sockaddr_storage for the
client, sockaddr_in for the server). This is not the case anymore, and this
distinction is confusing at best and has caused a number of regressions to
be introduced in the process of converting everything to full-ipv6. We can
now remove this and have a much cleaner code.
Nick Chalk reported that a connection to a server which has no port specified
used twice the port number. The reason is that the port number was taken from
the wrong part of the address, the client's destination address was used as the
base port instead of the server's configured address.
Thanks to Nick for his helpful diagnostic.
This patch introduces hdr_len, path_len and url_len for matching these
respective parts lengths against integers. This can be used to detect
abuse or empty headers.
Commit 588bd4 fixed header parsing so that trailing spaces were not part
of the returned string. Unfortunately, if a header only had spaces, the
last spaces were trimmed past the beginning of the value, causing a negative
length to be returned.
A quick code review shows that there should be no impact since the only
places where the vlen is used are either compared to a specific value or
with explicit contents (eg: digits).
This must be backported to 1.4.
These requests are mainly monitor requests, as well as stats requests when
the stats are processed by the frontend. Having this counter helps explain
the difference in number of sessions that is sometimes observed between a
frontend and a backend.
Passing -C <dir> causes haproxy to chdir to <dir> before loading
any file. The argument may be passed anywhere on the command line.
A typical use case is :
$ haproxy -C /etc/haproxy -f global.cfg -f haproxy.cfg
We now measure the work and idle times in order to report the idle
time in the stats. It's expected that we'll be able to use it at
other places later.
*_dom is mostly used for matching Host headers, and host headers may
include port numbers. To avoid having to create multiple rules with
and without :<port-number> in hdr_dom rules, change the *_dom
matching functions to also handle : as a delimiter.
Typically there are rules like this in haproxy.cfg:
acl is_foo hdr_dom(host) www.foo.com
Most clients send "Host: www.foo.com" in their HTTP header, but some
send "Host: www.foo.com:80" (which is allowed), and the above
rule will now work for those clients as well.
[Note: patch was edited before merge, any unexpected bug is mine /willy]
We already had the ability to kill a connection, but it was only
for the checks. Now we can do this for any session, and for this we
add a specific flag "K" to the logs.
The stats socket now allows the admin to disable, enable or shutdown a frontend.
This can be used when a bug is discovered in a configuration and it's desirable
to fix it but the rules in place don't allow to change a running config. Thus it
becomes possible to kill the frontend to release the port and start a new one in
a separate process.
This can also be used to temporarily make haproxy return TCP resets to incoming
requests to pretend the service is not bound. For instance, this may be useful
to quickly flush a very deep SYN backlog.
The frontend check and lookup code was factored with the "set maxconn" usage.
Upon an incoming soft restart request, we first pause all frontends and
peers. If the caller changes its mind and asks us to resume (eg: failed
binding), we must resume all the frontends and peers. Unfortunately the
peers were not resumed.
The code was arranged to avoid code duplication (which used to hide the
issue till now).
If a peers section has no peer named as the local peer, we must destroy
it, otherwise a NULL peer frontend remains in the lists and a segfault
can happen upon a soft restart.
We also now report the missing peer name in order to help troubleshooting.
Peers' frontends must have logging disabled by default, which was not
the case, so logs were randomly emitted upon restart, sometimes causing
a new process to fail to replace the old one.
This made sense a long time ago but since the maxconn is dynamically
computed from the tracking tables, it does not make any sense anymore
and will harm future changes.
The HTML page reports the current process connection rate, and the
"show info" command on the stats socket also reports the conn rate
limit and the max conn rate that was once reached.
Note that the max value can be cleared using "clear counters".
This one enforces a per-process connection rate limit, regardless of what
may be set per frontend. It can be a way to limit the CPU usage of a process
being severely attacked.
The side effect is that the global process connection rate is now measured
for each incoming connection, so it will be possible to report it.
This option permits to change the global maxconn setting within the
limit that was set by the initial value, which is now reported as the
hard maxconn value. This allows to immediately accept more concurrent
connections or to stop accepting new ones until the value passes below
the indicated setting.
The main use of this option is on systems where many haproxy instances
are loaded and admins need to re-adjust resource sharing at run time
to regain a bit of fairness between processes.
The way the unix socket is initialized is awkward. Some of the settings are put
in the sockets itself, other ones in the backend. And more importantly the
global.maxsock value is adjusted so that the stats socket evades the global
maxconn value. This complexifies maxsock computations for nothing, since the
stats socket is not supposed to receive hundreds of concurrent connections when
the global maxconn is very low. What is needed however is to ensure that there
are always connections left for the stats socket even when traffic sockets are
saturated, but this guarantee is not offered anymore by current code.
So as of now, the stats socket is subject to the global maxconn limitation just
as any other socket until a reservation mechanism is implemented.
Sometimes a bad content-length header is encountered and this causes
an abort. It's hard to debug without a trace, so let's take a capture
of the contents when this happens.
If a server starts to respond but stops before the body, then we
capture the truncated response. We don't do this on the request
because it would happen too often upon stupid attacks.
Trailing spaces after headers were not trimmed, only the leading ones
were. An issue was detected today with a content-length value which
was padded with spaces and which was rejected. Recent updates to the
http-bis draft made it a lot more clear that such spaces must be ignored,
so this is what this patch does.
It should be backported to 1.4.
Many inet_ntop calls were partially right, which was hard to detect given
the complex combinations. Some of them were relying on the listener's proto
instead of the address itself, which could have been different when dealing
with an accept-proxy connection.
The new addr_to_str() function does the dirty job and returns the family, which
makes it particularly suited to calls from switch/case statements. A large number
of if/else statements were removed and the stats output could even be cleaned up
in the case of session dump.
As a side effect of doing this, the resulting code is smaller by almost 1kB.
All changed parts have been tested and provided expected output.
A similar issue as the previous one causes port mapping to fail in some
combinations of client and server address families. Using the macros fixes
the issue.
In the number of switch/case statements added for IPv6 changes,
one was wrong and caused the check port to be ignored for outgoing
connection because the socket's family was not taken at the right
place. Use the set_host_port() macro instead to fix the issue.
The same cleanup could be performed at a number of other places
and should follow shortly.
Special thanks to Stephane Bakhos of Techboom for reporting a
detailed analysis of this bug.
Patch d5b9fd95 was missing an initialisation of "ctx.table.target", which caused
"show table" to segfault if it was issued after a "show errors" (target pointer == -1).
Some older libc don't define splice() and and don't define _syscall*()
either, which causes build errors if splicing is enabled.
To solve this, we now split the syscall redefinition into two layers :
- one file per syscall (epoll, splice)
- one common file to declare the _syscall*() macros
The code is cleaner because files using the syscalls just have to include
their respective file. It's not adviced to merge multiple syscall families
into a same file if all are not intended to be used simultaneously, because
defining unused static functions causes warnings to be emitted during build.
As a result, the new USE_MY_SPLICE parameter was added in order to be able
to define the splice() syscall separately.
If "option forwardfor" has the "if-none" argument, then the header is
only added when the request did not already have one. This option has
security implications, and should not be set blindly.
Manoj Kumar reported a case where haproxy would crash upon start-up. The
cause was an "http-check expect" statement declared in the defaults section,
which caused a NULL regex to be used during the check. This statement is not
allowed in defaults sections precisely because this requires saving a copy
of the regex in the default proxy. But the check was not made to prevent it
from being declared there, hence the issue.
Instead of adding code to detect its abnormal use, we decided to implement
it. It was not that much complex because the expect_str part was not used
with regexes, so it could hold the string form of the regex in order to
compile it again for every backend (there's no way to clone regexes).
This patch has been tested and works. So it's both a bugfix and a minor
feature enhancement.
It should be backported to 1.4 though it's not critical since the config
was not supposed to be supported.
"[MINOR] session: add a pointer to the new target into the session" (664beb8)
introduced a regression by changing the type of a peer's target from
TARG_TYPE_PROXY to TARG_TYPE_NONE. The effect of this is that during
a soft-restart the new process no longer tries to connect to the
old process to replicate its stick tables.
This patch sets the type of a peer's target as TARG_TYPE_PROXY and
replication on soft-restart works once again.
Adding health checks has become a real pain, with cross-references to all
checks everywhere because they're all a single bit. Since they're all
exclusive, let's change this to have a check number only. We reserve 4
bits allowing up to 16 checks (15+tcp), only 7 of which are currently
used. The code has shrunk by almost 1kB and we saved a few option bits.
The "dispatch" option has been moved to px->options, making a few tests
a bit cleaner.
This patch provides a new "option redis-check" statement to enable server health checks based on redis PING request (http://www.redis.io/commands/ping).
The new "set maxconn frontend XXX" statement on the stats socket allows
the admin to change a frontend's maxconn value. If some connections are
queued, they will immediately be accepted up to the new limit. If the
limit is lowered, new connections acceptation might be delayed. This can
be used to temporarily reduce or increase the impact of a specific frontend's
traffic on the whole process.
This global task is used to periodically check for end of resource shortage
and to try to enable queued listeners again. This is important in case some
temporary system-wide shortage is encountered, so that we don't have to wait
for an existing connection to be released before checking the queue again.
For situations where listeners are queued due to the global maxconn being
reached, the task is woken up at least every second. For situations where
a system resource shortage is detected (memory, sockets, ...) the task is
woken up at least every 100 ms. That way, recovery from severe events can
still be achieved under acceptable conditions.
This was revealed with one of the very latest patches which caused
the listener_queue not to be initialized on the stats socket frontend.
And in fact a number of other ones were missing too. This is getting so
boring that now we'll always make use of the same function to initialize
any proxy. Doing so has even saved about 500 bytes on the binary due to
the avoided code redundancy.
No backport is needed.
This function is finally not needed anymore, as it has been replaced with
a per-proxy task that is scheduled when some limits are encountered on
incoming connections or when the process is stopping. The savings should
be noticeable on configs with a large number of proxies. The most important
point is that the rate limiting is now enforced in a clean and solid way.
Peers were stopped on every call to maintain_proxies when stopping=1,
while they should only be stopped once upon call to soft_stop(). This
bug has little impact, mostly increased CPU usage. It's not needed to
backport it.
Instead of waking a listener up then making it sleep, we only wake them up
if we know their rate limit is fine. In the future we could improve on top
of that by deciding to wake a proxy-specific task in XX milliseconds to
take care of enabling the listeners again.
Patch d9bbe17b used to limit the rate-limit to off-by-one to avoid
a busy loop when the limit is reached. Now that the listeners are
automatically disabled and queued when a limit is reached, we don't
need this workaround anymore and can bring back the most accurate
computation.
Those states have been replaced with PR_STFULL and PR_STREADY respectively,
as it is what matches them the best now. Also, two occurrences of PR_STIDLE
in peers.c have been removed as this did not provide any form of error recovery
anyway.
Now maintain_proxies() only changes proxies states and does not affect their
listeners anymore since they are autonomous. A proxy will switch between the
PR_STIDLE and PR_STRUN states depending whether it's saturated or not. Next
step will consist in renaming PR_STIDLE to PR_STFULL. This state is now only
used to report the proxy state in the stats.
All listeners that are limited by a proxy-specific resource are now
queued at the proxy's and not globally. This allows finer-grained
wakeups when releasing resource.
When an accept() fails because of a connection limit or a memory shortage,
we now disable it and queue it so that it's dequeued only when a connection
is released. This has improved the behaviour of the process near the fd limit
as now a listener with a no connection (eg: stats) will not loop forever
trying to get its connection accepted.
The solution is still not 100% perfect, as we'd like to have this used when
proxy limits are reached (use a per-proxy list) and for safety, we'd need
to have dedicated tasks to periodically re-enable them (eg: to overcome
temporary system-wide resource limitations when no connection is released).
When a listeners encounters a resource shortage, it currently stops until
one re-enables it. This is far from being perfect as it does not yet handle
the case where the single connection from the listener is rejected (eg: the
stats page).
Now we'll have a special status for resource limited listeners and we'll
queue them into one or multiple lists. That way, each time we have to stop
a listener because of a resource shortage, we can enqueue it and change its
state, so that it is dequeued once more resources are available.
This patch currently does not change any existing behaviour, it only adds
the basic building blocks for doing that.
For listeners that are not bound to a frontend, the limit on the
number of accepted connections is tested at the end of the accept()
loop, but we don't break out of the loop, meaning that if more
connections than what the listener allows are available and if this
is less than the proxy's limits and within the size of a batch, then
they could be accepted. In practice, this problem currently cannot
appear since all listeners are bound to a frontend, and it's a very
minor issue anyway.
1.4 has the same issue (which cannot happen there either), but there
is some code after it, so it's the code cleanup which revealed it.
Managing listeners state is difficult because they have their own state
and can at the same time have theirs dictated by their proxy. The pause
is not done properly, as the proxy code is fiddling with sockets. By
introducing new functions such as pause_listener()/resume_listener(), we
make it a bit more obvious how/when they're supposed to be used. The
listen_proxies() function was also renamed to resume_proxies() since
it's only used for pause/resume.
This patch is the first in a series aiming at getting rid of the maintain_proxies
mess. In the end, proxies should not call enable_listener()/disable_listener()
anymore.
When an accept() returns -1 ENFILE du to system limits, it leaves the
connection pending in the backlog and epoll() comes back immediately
afterwards trying to make it accept it again. This causes haproxy to
remain at 100% CPU until something makes an accept() possible again.
Now upon such resource shortage, we mark the listener FULL so that we
only enable it again once at least one connection has been released.
In fact we only do that if there are some active connections on this
proxy, so that it has a chance to be marked not full again. This makes
haproxy remain idle when all resources are used, which helps a lot
releasing those resource as fast as possible.
Backport to 1.4 might be desirable but difficult and tricky.
By default on a single process, we accept 100 connections at once. This is too
much on recent CPUs where the cache is constantly thrashing, because we visit
all those connections several times. We should batch the processing slightly
less so that all the accepted session may remain in cache during their initial
processing.
Lowering the batch size from 100 to 32 has changed the connection rate for
concurrencies between 5-10k from 67 kcps to 94 kcps on a Core i5 660 (4M L3),
and forward rates from 30k to 39.5k.
Tests on this hardware show that values between 10 and 30 seem to do the job fine.
When we fail to create a session because of memory shortage, let's at
least try to send a 500 message directly on the socket. Even if we don't
have any buffers left, the kernel's orphans management will take care of
delivering the message as long as there are socket buffers left.
Patch af5149 introduced an issue which can be detected only on out of
memory conditions : a LIST_DEL() may be performed on an uninitialized
struct member instead of a LIST_INIT() during the accept() phase,
causing crashes and memory corruption to occur.
This issue was detected and diagnosed by the Exceliance R&D team.
This is 1.5-specific and very recent, so no existing deployment should
be impacted.
The motivation for this is that when soft-restart is merged
it will be come more important to free all relevant memory in deinit()
Discovered using valgrind.
The motivation for this is that when soft-restart is merged
it will be come more important to free all relevant memory in deinit()
Discovered using valgrind.
The motivation for this is that when soft-restart is merged
it will be come more important to free all relevant memory in deinit()
Discovered using valgrind.
The motivation for this is that when soft-restart is merged
it will be come more important to free all relevant memory in deinit()
Discovered using valgrind.
This is used to perform cookie-based stickiness with table replication
between multiple masters and across restarts. This partially overrides
some of the appsession capabilities.
Never add connections allocated to this sever to a stick-table.
This may be used in conjunction with backup to ensure that
stick-table persistence is disabled for backup servers.
This pattern fetch function extracts the value of the rdp cookie <name> as
a string and uses this value to match. This enables implementation of
persistence based on the mstshash cookie. This is typically done if there
is no msts cookie present.
This differs from "balance rdp-cookie" in that any balancing algorithm may
be used and thus the distribution of clients to backend servers is not
linked to a hash of the RDP cookie. It is envisaged that using a balancing
algorithm such as "balance roundrobin" or "balance leastconnect" will lead
to a more even distribution of clients to backend servers than the hash
used by "balance rdp-cookie".
Example :
listen tse-farm
bind 0.0.0.0:3389
# wait up to 5s for an RDP cookie in the request
tcp-request inspect-delay 5s
tcp-request content accept if RDP_COOKIE
# apply RDP cookie persistence
persist rdp-cookie
# Persist based on the mstshash cookie
# This is only useful makes sense if
# balance rdp-cookie is not used
stick-table type string size 204800
stick on rdp_cookie(mstshash)
server srv1 1.1.1.1:3389
server srv1 1.1.1.2:3389
apsession_refresh() and apsess_refressh are only used inside apsession.c
and thus can be made static.
The only use of apsession_refresh() is appsession_task_init().
These functions have been re-ordered to avoid the need for
a forward-declaration of apsession_refresh().
If a connection is closed by because the backend became unavailable
then log 'D' as the termination condition.
Signed-off-by: Simon Horman <horms@verge.net.au>
This adds the "on-marked-down shutdown-sessions" statement on "server" lines,
which causes all sessions established on a server to be killed at once when
the server goes down. The task's priority is reniced to the highest value
(1024) so that servers holding many tasks don't cause a massive slowdown due
to the wakeup storm.
The motivation for this is to allow iteration of all the connections
of a server without the expense of iterating over the global list
of connections.
The first use of this will be to implement an option to close connections
associated with a server when is is marked as being down or in maintenance
mode.
* The declaration of peer_session_create() does
not match its definition. As it is only
used inside of peers.c make it static.
* Make the declaration of peers_register_table()
match its definition.
* Also, make all functions in peers.c that
are not also in peers.h static
gcc (Debian 4.6.0-2) 4.6.1 20110329 (prerelease)
Copyright (C) 2011 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
...
src/proto_http.c:3029:14: warning: variable ‘del_cl’ set but not used [-Wunused-but-set-variable]
In file included from ebtree/eb64tree.c:23:0:
ebtree/eb64tree.h: In function ‘__eb64_lookup’:
ebtree/eb64tree.h:128:6: warning: variable ‘node_bit’ set but not used [-Wunused-but-set-variable]
ebtree/eb64tree.h: In function ‘__eb64i_lookup’:
ebtree/eb64tree.h:180:6: warning: variable ‘node_bit’ set but not used [-Wunused-but-set-variable]
In file included from ebtree/ebpttree.h:26:0,
from ebtree/ebimtree.c:23:
ebtree/eb64tree.h: In function ‘__eb64_lookup’:
ebtree/eb64tree.h:128:6: warning: variable ‘node_bit’ set but not used [-Wunused-but-set-variable]
ebtree/eb64tree.h: In function ‘__eb64i_lookup’:
ebtree/eb64tree.h:180:6: warning: variable ‘node_bit’ set but not used [-Wunused-but-set-variable]
In file included from ebtree/ebpttree.h:26:0,
from ebtree/ebistree.h:25,
from ebtree/ebistree.c:23:
ebtree/eb64tree.h: In function ‘__eb64_lookup’:
ebtree/eb64tree.h:128:6: warning: variable ‘node_bit’ set but not used [-Wunused-but-set-variable]
ebtree/eb64tree.h: In function ‘__eb64i_lookup’:
ebtree/eb64tree.h:180:6: warning: variable ‘node_bit’ set but not used [-Wunused-but-set-variable]
mysqld >= 5.5 want the client to announce 4.1+ authentication support, even if we have no password, so we do this.
I also check on a debian potato mysqld 3.22 and it works too so i assume we are good from 3.22 to 5.5.
[WT: this must be backported to 1.4]
The fullconn value is not easy to get right when doing dynamic regulation,
as it should depend on the maxconns of the frontends that can reach a
backend. Since the parameter is mandatory, many configs are found with
an inappropriate default value.
Instead of rejecting configs without a fullconn value, we now set it to
10% of the sum of the configured maxconns of all the frontends which are
susceptible to branch to the backend. That way if new frontends are added,
the backend's fullconn automatically adjusts itself.
Bashkim Kasa reported that the stats admin page did not work when colons
were used in server or backend names. This was caused by url-encoding
resulting in ':' being sent as '%3A'. Now we systematically decode the
field names and values to fix this issue.
Since version 1.0.0, it's forbidden to have a cookie specified without at
least one server. This test is useless and makes it complex to write APIs
to iteratively generate working configurations. Remove the test.
It's more expensive to call splice() on short payloads than to use
recv()+send(). One of the reasons is that doing a splice() involves
allocating a pipe. One other reason is that the kernel will have to
copy itself if we try to splice less than a page. So let's fix a
short offset of 4kB below which we don't splice.
A quick test shows that on chunked encoded data, with splice we had
6826 syscalls (1715 splice, 3461 recv, 1650 send) while with this
patch, the same transfer resulted in 5793 syscalls (3896 recv, 1897
send).
Fast-forwarding between file descriptors is nice but can be counter-productive
when only one part of the buffer is forwarded, because it can result in doubling
the number of send() syscalls. This is what happens on HTTP chunking, because
the chunk data are sent, then the CRLF + next chunk size are parsed and immediately
scheduled for forwarding. This results in two send() for the same block while a
single one would have done it.
Now that we support the http-no-delay mode, we can optimize HTTP
chunking again by always waiting for more data to come until the
last chunk is met.
This patch may or may not be backported to 1.4, it's not a big deal,
it will mainly help for chunks which are aligned with the buffer size.
There are some very rare server-to-server applications that abuse the HTTP
protocol and expect the payload phase to be highly interactive, with many
interleaved data chunks in both directions within a single request. This is
absolutely not supported by the HTTP specification and will not work across
most proxies or servers. When such applications attempt to do this through
haproxy, it works but they will experience high delays due to the network
optimizations which favor performance by instructing the system to wait for
enough data to be available in order to only send full packets. Typical
delays are around 200 ms per round trip. Note that this only happens with
abnormal uses. Normal uses such as CONNECT requests nor WebSockets are not
affected.
When "option http-no-delay" is present in either the frontend or the backend
used by a connection, all such optimizations will be disabled in order to
make the exchanges as fast as possible. Of course this offers no guarantee on
the functionality, as it may break at any other place. But if it works via
HAProxy, it will work as fast as possible. This option should never be used
by default, and should never be used at all unless such a buggy application
is discovered. The impact of using this option is an increase of bandwidth
usage and CPU usage, which may significantly lower performance in high
latency environments.
This change should be backported to 1.4 since the first report of such a
misuse was in 1.4. Next patch will also be needed.
The FL_TCP flag was a leftover from the old days we were using TCP_CORK.
With MSG_MORE it's not needed anymore so we can remove the condition and
sensibly simplify the test.
When sending is complete, it's preferred to systematically clear the flags
that were set for that transfer. What could happen is that the to_forward
counter had caused the MSG_MORE flag to be set and BF_EXPECT_MORE not to
be cleared, resulting in this flag being unexpectedly maintained for next
round.
The code has taken extreme care of not doing this till now, but it's not
acceptable that the caller has to know these precise semantics. So let's
unconditionnally clear the flag instead.
For the sake of safety, this fix should be backported to 1.4.
Commit 57f5c1 used to provide a nice improvement on chunked encoding since
it ensured that we did not set a PUSH flag for every chunk or buffer data
part of a chunked transfer.
Some applications appear to erroneously abuse HTTP chunking in order to
get interactive exchanges between a user agent and an origin server with
very small chunks. While it happens to work through haproxy, it's terribly
slow due to the latency added after passing each chunk to the system, who
could wait up to 200ms before pushing them onto the wire.
So we need an interactive mode for such usages. In the mean time, step back
on the optim, but not completely, so that we still keep the flag as long as
we know we're not finished with the current chunk.
This change should be backported to 1.4 too as the issue was discovered
with it.
This status code is used in response to requests matching "monitor-uri".
Some users need to adjust it to fit their needs (eg: make some strings
appear there). As it's already defined as a chunked string and used
exactly like other status codes, it makes sense to make it configurable
with the usual "errorfile", "errorloc", ...
Some people like to make the monitoring URL testable from unsafe locations.
Reporting haproxy's existence there can sometimes be problematic. This patch
should not be backported to 1.4 because it is possible, eventhough unlikely,
that some scripts rely on this word to appear there.
As reported by Lauri-Alo Adamson, version 1.5-dev6 doesn't support
stick-tables with a binary type.
This issue was introduced in the commit 4f92d32 where a line was erroneously
deleted, and is 1.5-specific.
Mark Brooks reported that commit 1b4b7c broke tproxy in 1.5-dev6. Nick
Chalk tracked the issue down to a missing address family setting in
tcp_bind_socket() which resulted in a failure to use get_addr_len().
This issue is 1.5-specific.
Christopher Blencowe reported that the httpchk_expect() function was
lacking a test for incomplete responses : if the server sends only the
headers in the first packet and the body in a subsequent one, there is
a risk that the check fails without waiting for more data. A failure
rate of about 1% was reported.
This fix must be backported to 1.4.
When doing fix 24581bae02 to correctly handle
response cookies, an unfortunate typo was inserted in the less likely code
path, resulting in a risk of crash when cookie-based persistence is enabled
and the server emits a cookie with several spaces around the equal sign.
This bug was noticed during a code backport. Its effects were never reported
because this situation is very unlikely to appear, but it can be provoked on
purpose by the server.
This patch must be backported to 1.4 versions which contain the fix above
(anything > 1.4.8), and to similar 1.3 versions > 1.3.25. 1.5-dev versions
after 1.5-dev2 are affected too.
John Helliwell reported a runtime issue on Solaris since 1.5-dev5. Traces
show that connect() returns EINVAL, which means the socket length is not
appropriate for the family. Solaris does not like being called with sizeof
and needs the address family's size on sockaddr_storage.
The fix consists in adding a get_addr_len() function which returns the
socket's address length based on its family. Tests show that this works
for both IPv4 and IPv6 addresses.
Since IPv6 is a different type than IPv4, the pattern fetch functions
src6 and dst6 were added. IPv6 stick-tables can also fetch IPv4 addresses
with src and dst. In this case, the IPv4 addresses are mapped to their
IPv6 counterpart, according to RFC 4291.
Patch 5ab04ec47c was incomplete,
because if the first send() fails on an empty buffer, we fail
to rearm the polling and we can't establish the connection
anymore.
The issue was reported by Ben Timby who provided large amounts
of traces of various tests helping to reliably reproduce the issue.
Since the latest additions to buffer_forward(), it became too large for
inlining, so let's uninline it. The code size drops by 3kB. Should be
backported to 1.4 too.
Despite much care around handling the content-length as a 64-bit integer,
forwarding was broken on 32-bit platforms due to the 32-bit nature of
the ->to_forward member of the "buffer" struct. The issue is that this
member is declared as a long, so while it works OK on 64-bit platforms,
32-bit truncate the content-length to the lower 32-bits.
One solution could consist in turning to_forward to a long long, but it
is used a lot in the critical path, so it's not acceptable to perform
all buffer size computations on 64-bit there.
The fix consists in changing the to_forward member to a strict 32-bit
integer and ensure in buffer_forward() that only the amount of bytes
that can fit into it is considered. Callers of buffer_forward() are
responsible for checking that their data were taken into account. We
arbitrarily ensure we never consider more than 2G at once.
That's the way it was intended to work on 32-bit platforms except that
it did not.
This issue was tracked down hard at Exosec with Bertrand Jacquin,
Thierry Fournier and Julien Thomas. It remained undetected for a long
time because files larger than 4G are almost always transferred in
chunked-encoded format, and most platforms dealing with huge contents
these days run on 64-bit.
The bug affects all 1.5 and 1.4 versions, and must be backported.
Since we now have the copy of the target in the session, use it instead
of relying on the SI for it. The SI drops the target upon unregister()
so applets such as stats were logged as "NOSRV".
Johannes Smith reported some wrong retries count in logs associated with bad
requests. The cause was that the conn_retries field in the stream interface
was only initialized when attempting to connect, but is used when logging,
possibly with an uninitialized value holding last connection's conn_retries.
This could have been avoided by making use of a stream interface initializer.
This bug is 1.5-specific.
Function gethostbyname is deprecated since IEEE Std 1003.1-2008 and
was replaced by getaddrinfo (available since IEEE Std 1003.1-2004).
Contrary to gethostbyname, getaddrinfo is specified to support both
IPv4 and IPv4 addresses.
Since some libc doesn't handle getaddrinfo properly, constant
USE_GETADDRINFO must be defined at compile time to enable use of
getaddrinfo.
It's always been a mess to debug wrong listening addresses because
the parsing function does not indicate the file and line number. Now
it does. Since the code was almost a duplicate of str2sa_range, it
now makes use of it and has been sensibly reduced.
The parser now distinguishes between pure addresses and address:port. This
is useful for some config items where only an address is required.
Raw IPv6 addresses are now parsed, but IPv6 host name resolution is still not
handled (gethostbyname does not resolve IPv6 names to addresses).
This option enables use of the PROXY protocol with the server, which
allows haproxy to transport original client's address across multiple
architecture layers.
Upon connection establishment, stream_sock is now able to send a PROXY
line before sending any data. Since it's possible that the buffer is
already full, and we don't want to allocate a block for that line, we
compute it on-the-fly when we need it. We just store the offset from
which to (re-)send from the end of the line, since it's assumed that
multiple outputs of the same proxy line will be strictly equivalent. In
practice, one call is enough. We just make sure to handle the case where
the first send() would indicate an incomplete output, eventhough it's
very unlikely to ever happen.
If the check fails for a low-level socket error (eg: address family not
supportd), we currently ignore the status. We must report the error and
declare a failed health check in this case. The only real reason for this
would be when an IPv6 check is required on an IPv4-only system.
And also rename "req_acl_rule" "http_req_rule". At the beginning that
was a bit confusing to me, especially the "req_acl" list which in fact
holds what we call rules. After some digging, it appeared that some
part of the code is 100% HTTP and not just related to authentication
anymore, so let's move that part to HTTP and keep the auth-only code
in auth.c.
Right now, http-request rules are not evaluated if the URL matches the
stats request. This is quite unexpected. For instance, in the config
below, an abuser present in the abusers list will not be prevented access
to the stats.
listen pub
bind :8181
acl abuser src -f abusers.lst
http-request deny if abuser
stats uri /stats
It is not a big deal but it's not documented as such either. For 1.5, let's
have both lists be evaluated in turn, until one blocks. For 1.4 we'll simply
update the doc to indicate that.
Also instead of duplicating the code, the patch factors out the list walking
code. The HTTP auth has been moved slightly earlier, because it was set after
the header addition code, but we don't need to add headers to a request we're
dropping.
It's very annoying that frontend and backend stats are merged because we
don't know what we're observing. For instance, if a "listen" instance
makes use of a distinct backend, it's impossible to know what the bytes_out
means.
Some points take care of not updating counters twice if the backend points
to the frontend, indicating a "listen" instance. The thing becomes more
complex when we try to add support for server side keep-alive, because we
have to maintain a pointer to the backend used for last request, and to
update its stats. But we can't perform such comparisons anymore because
the counters will not match anymore.
So in order to get rid of this situation, let's have both frontend AND
backend stats in the "struct proxy". We simply update the relevant ones
during activity. Some of them are only accounted for in the backend,
while others are just for frontend. Maybe we can improve a bit on that
later, but the essential part is that those counters now reflect what
they really mean.
This patch turns internal server addresses to sockaddr_storage to
store IPv6 addresses, and makes the connect() function use it. This
code already works but some caveats with getaddrinfo/gethostbyname
still need to be sorted out while the changes had to be merged at
this stage of internal architecture changes. So for now the config
parser will not emit an IPv6 address yet so that user experience
remains unchanged.
This change should have absolutely zero user-visible effect, otherwise
it's a bug introduced during the merge, that should be reported ASAP.
This one has been removed and is now totally superseded by ->target.
To get the server, one must use target_srv(&s->target) instead of
s->srv now.
The function ensures that non-server targets still return NULL.
s->prev_srv is used by assign_server() only, but all code paths leading
to it now take s->prev_srv from the existing s->srv. So assign_server()
can do that copy into its own stack.
If at one point a different srv is needed, we still have a copy of the
last server on which we failed a connection attempt in s->target.
When dealing with HTTP keep-alive, we'll have to know if we can reuse
an existing connection. For that, we'll have to check if the current
connection was made on the exact same target (referenced in the stream
interface).
Thus, we need to first assign the next target to the session, then
copy it to the stream interface upon connect(). Later we'll check for
equivalence between those two operations.
Since all of them are defined as proxy options, it's better to ensure
that at most one of them is enabled at once. The priority has been set
according to what is already performed in the backend :
1) dispatch
2) http_proxy
3) transparent
Till now we used the fact that the dispatch address was not null to use
the dispatch mode. This is very unconvenient, so let's have a dedicated
option.
This is in fact where those parts belong to. The old data_state was replaced
by applet.state and is now initialized when the applet is registered. It's
worth noting that the applet does not need to know the session nor the
buffer anymore since everything is brought by the stream interface.
It is possible that having a separate applet struct would simplify the
code but that's not a big deal.
With HTTP keep-alive, logging the right server name will be quite
complex because the assigned server will possibly change before we log.
Also, when we want to log accesses to an applet, it's not easy because
the applet becomes NULL again before logging.
The logged server's name is now taken from the target stored in the
stream interface. That way we can log an applet, a server name, or we
could even log a proxy or anything else if we wanted to. Ideally the
session should contain a desired target which is the one which should
be logged.
Now that we have the target pointer and type in the stream interface,
we don't need the applet.handler pointer anymore. That makes the code
somewhat cleaner because we know we're dealing with an applet by checking
its type instead of checking the pointer is not null.
When doing a connect() on a stream interface, some information is needed
from the server and from the backend. In some situations, we don't have
a server and only a backend (eg: peers). In other cases, we know we have
an applet and we don't want to connect to anything, but we'd still like
to have the info about the applet being used.
For this, we now store a pointer to the "target" into the stream interface.
The target describes what's on the other side before trying to connect. It
can be a server, a proxy or an applet for now. Later we'll probably have
descriptors for multiple-stage chains so that the final information may
still be found.
This will help removing many specific cases in the code. It already made
it possible to remove the "srv" and "be" parameters to tcpv4_connect_server().
I/O handlers are still delicate to manipulate. They have no type, they're
just raw functions which have no knowledge of themselves. Let's have them
declared as applets once for all. That way we can have multiple applets
share the same handler functions and we can store their names there. When
we later need to add more parameters (eg: usage stats), we'll be able to
do so in the applets themselves.
The CLI functions has been prefixed with "cli" instead of "stats" as it's
clearly what is going on there.
The applet descriptor in the stream interface should get all the applet
specific data (st0, ...) but this will be done in the next patch so that
we don't pollute this one too much.
Both Hank A. Paulson and Rob at pixsense reported a crash when
loading ACLs from a pattern file which contains empty lines.
From the tests, it appears that only files that contain nothing
but empty lines are causing that (in the past they would have had
their line feeds loaded as patterns).
The crash happens in the free_pattern() call which doesn't like to
be called with a NULL pattern. Let's make it accept it so that it's
more in line with the standard uses of free() which ignores NULLs.
Similar to the stats socket bug, we must check that the proxy is not disabled
before trying to enable/disable a server.
Even if a disabled proxy is not displayed, someone can inject a faulty proxy
name in the POST parameters. So, we must ensure that no disabled proxy can be
used.
As reported by Bryan Talbot, enabling and disabling a server in a disabled
proxy causes a segfault.
Changing the weight can also cause a similar segfault.
Bryan Talbot reported that POST requests with a query string were not
correctly processed if the hash parameter was the first one, because
the delimiter that was looked for to trigger the parsing was '&' instead
of '?'.
Also, while checking the code, it became apparent that it was enough for
a query string to be present in the request for POST parameters to be
ignored, even if the url_param was in the body and not in the URL.
The code has then been fixed like this :
1) look for URL param. If found, return it.
2) if no URL param was found and method is POST, then look it up into
the body
The code now seems to pass all request combinations.
This patch must be backported to 1.4 since 1.4 is equally broken right now.
Till now, the forwarding code was making use of the hdr_content_len member
to hold the size of the last chunk parsed. As such, it was reset after being
scheduled for forwarding. The issue is that this entry was reset before the
data could be viewed by backend.c in order to parse a POST body, so the
"balance url_param check_post" did not work anymore.
In order to fix this, we need two things :
- the chunk size (reset upon every forward)
- the total body size (not reset)
hdr_content_len was thus replaced by the former (hence the size of the patch)
as it makes more sense to have it stored that way than the way around.
This patch should be backported to 1.4 with care, considering that it affects
the forwarding code.
It seems like if a response message is chunked and the chunk size wraps
at the end of the buffer and the crlf sequence is incomplete, then we
can forward a wrong chunk size due to incorrect handling of the wrapped
size. It seems extremely unlikely to occur on real traffic (no reason to
have half of the CRLF after a chunk) but nothing prevents it from being
possible.
This fix must be backported to 1.4.
As reported by the Loadbalancer.org team, it was not possible to bind
more than 1024 ports. This is because the process' limits were set after
trying to bind the sockets, which defeats their purpose.
This fix must be backported to 1.4 and 1.3.
We used to only count one socket instead of one per listener. This makes
the socket count wrong, preventing from automatically computing the proper
number of sockets to bind.
This fix must be backported to 1.4 and 1.3.
req_acl was used instead of req_acl_final. As a matter of luck, both
happen to be the same at this point, but this is not granted in the
future.
This fix should be backported to 1.4.
Some browsers send POST requests in several packets, which was not supported
by the "stats admin" function.
This patch allows to wait for more data when they are not fully received
(we are still limited to a certain size defined by the buffer size minus its
reserved space).
It also adds support for the "Expect: 100-Continue" header.
Stefan Behte reported a strange case where depending on the position of
the Connection header in the header list, some headers added after it
were or were not usable in "balance hdr()". The reason is that when the
last header is removed, the list's tail was not updated, so any header
added after that one was not visible from the list.
This fix must be backported to 1.4 and possibly 1.3.
while working further on the changes to allow for dynamic
adding/removing of backend servers we noticed a potential problem: the
path given for the 'stats socket' global option may get truncated when
copying it into the sockaddr_un.sun_path field.
Attached patch checks the length, and reports an error if truncation
would happen.
This issue was noticed by Joerg Sonnenberger <joerg@NetBSD.org>.
src/frontend.c: In function 'frontend_accept':
src/frontend.c:110: warning: pointer targets in passing argument 5 of 'getsockopt' differ in signedness
The argument should be socklen_t and not int.
I have written a small patch to enable a correct PostgreSQL health check
It works similar to mysql-check with the very same parameters.
E.g.:
listen pgsql 127.0.0.1:5432
mode tcp
option pgsql-check user pgsql
server masterdb pgsql.server.com:5432 check inter 10000
It's better to avoid sticking on empty parameter values, as this almost
always indicates a missing parameter. Otherwise it's easy to enter a
situation where all new visitors stick to the same server.
Revert commits 035da6d1b0 and
f18b5f21ba.
These fixes were wrong. They worked but they were fixing the symptom
instead of the root cause of the problem. The real issue was in the
ebtree lookup code and it has been fixed now so these patches are not
needed anymore. It's better not to copy memory blocks when we don't
need to, so let's revert them.
Commit 035da6d1b0 was incorrect as it
could modify a live buffer. We must first ensure that we're on the
private buffer or perform a copy before modifying the data.
Gabriel Sosa reported that haproxy unexpectedly reports an error
when a pattern file loaded by an ACL contains an empty line. The
test was present but inefficient as it did not consider the '\n'
as the end of the line. This fix relies on the line length instead.
It should be backported to 1.4.
If a key to be looked up is extracted from data without being padded
and if it matches the beginning of another stored key, it is not
found in subsequent lookups because it does not end with a zero.
This bug was discovered and diagnosed by David Cournapeau.
One of the requirements we have is to run multiple instances of haproxy on a
single host; this is so that we can split the responsibilities (and change
permissions) between product teams. An issue we ran up against is how we
would distinguish between the logs generated by each instance. The solution
we came up with (please let me know if there is a better way) is to override
the application tag written to syslog. We can then configure syslog to write
these to different files.
I have attached a patch adding a global option 'log-tag' to override the
default syslog tag 'haproxy' (actually defaults to argv[0]).
