This function is used to move data which is located between ->w and ->r,
so it must not touch ->w, otherwise it will displace pending data which
is before the one we're actually overwriting. The issue arose in 1.4 with
some pipelined responses which cause some part of the previous one to
be chopped off when removing the connection: close header, thus
corrupting last response and shifting next one. Those are detected
in the logs because the next response will be a 502 with flags PH.
Note that this does not affect 1.3, still this is a bug that's better
fixed than blindly copy-pasted and woken up again.
(cherry picked from commit bbfa7938bd)
When using "option persist" or "force-persist", we want to know from the
logs if the cookie referenced a valid server or a down server. Till here
the flag reported a valid server even if the server was down, which is
misleading. Now we correctly report that the requested server was down.
We can typically see "--DI" when using "option persist" with redispatch,
ad "SCDN" when using force-persist on a down server.
(cherry picked from commit 2a6d88dafe)
This is used to force access to down servers for some requests. This
is useful when validating that a change on a server correctly works
before enabling the server again.
(cherry picked from commit 4de9149f87)
This can cause parts of responses to be truncated in case of
pipelined requests if the second request generates an error
before the first request is completely flushed.
(cherry picked from commit d5fd51c75b)
A check was performed in buffer_replace2() to compare buffer
length with its read pointer. This has been wrong for a long
time, though it only has an impact when dealing with keep-alive
requests/responses. In theory this should be backported but
the check has no impact without keep-alive.
(cherry picked from commit 43a7e6620b)
We can receive data with a notification of socket error. But we
must not check for the error before reading the data, because it
may be an asynchronous error notification that we check too early
while the response we're waiting for is available. If there is an
error, recv() will get it.
This should help with servers that close very fast after the response
and should also slightly lower the CPU usage during very fast checks
on massive amounts of servers since we eliminate one system call.
This should probably be backported to 1.3.
(cherry picked from commit a5aa1c86a5)
I've tried to follow all the pool_alloc2/pool_free2 calls in the code
to track memory leaks. I've found one which only happens when there's
already no more memory when allocating a new appsession cookie.
(cherry picked from commit 41689c22da)
Sometimes it can be desired to return a location which is the same
as the request with a slash appended when there was not one in the
request. A typical use of this is for sending a 301 so that people
don't reference links without the trailing slash. The name of the
new option is "append-slash" and it can be used on "redirect"
statements in prefix mode.
(cherry picked from commit 81e3b4f48d)
When using server redirection, it is possible to specify a path
consisting of only one slash. While this is discouraged (risk of
loop) it may sometimes be useful combined with content switching.
The prefixing of a '/' then causes two slashes to be returned in
the response. So we now do as with the other redirects, don't
prepend a slash if it's alone.
(cherry picked from commit dcb75c4a83)
The cookie parser could be fooled by spaces or commas in cookie names
and values, causing the persistence cookie not to be matched if located
just after such a cookie. Now spaces found in values are considered as
part of the value, and spaces, commas and semi-colons found in values
or names, are skipped till next cookie name.
This fix must be backported to 1.3.
(cherry picked from commit 305ae85957)
This option was disabled for frontends in the configuration because
it was useless in its initial implementation, though it was still
checked in the code. Let's officially enable it now.
(cherry picked from commit a31e5dff36)
This error was triggered by requests not starting at the beginning
of the buffer. It cannot happen with earlier versions though it might
be a good idea to fix it anyway.
(cherry picked from commit 019fd5bc93)
In many places where we perform header insertion, an error control
is performed but due to a mistake, it cannot match any error :
if (unlikely(error) < 0)
instead of
if (unlikely(error < 0))
This prevents error 400 responses from being sent when the buffer is
full due to many header additions. This must be backported to 1.3.
(cherry picked from commit 58cc872848)
If "balance url_param XXX check_post" is used, we must bound the
number of bytes analysed to the buffer's length.
(cherry picked from commit dc8017ced6)
The previous check was correct: the RFC states that it is required
to have a domain-name which contained a dot AND began with a dot.
However, currently some (all?) browsers do not obey this specification,
so such configuration might work.
This patch reverts 3d8fbb6658 but
changes the check from FATAL to WARNING and extends the message.
(cherry picked from commit 1a8bea9390)
Fix 500b8f0349 fixed the patch for the 64 bit
case but caused the opposite type issue to appear on 32 bit platforms. Cast
the difference and be done with it since gcc does not agree on type carrying
the difference between two pointers on 32 and 64 bit platforms.
(cherry picked from commit 3ccf94efd9)
src/cfgparse.c: In function 'readcfgfile':
src/cfgparse.c:4087: warning: format '%d' expects type 'int', but argument 5 has type 'long int'
(cherry picked from commit 500b8f0349)
Cyril Bonté found that when an error is detected in one config file, it
is also reported in all other ones, which is wrong. The fix obviously
consists in checking the return code from readcfgfile() and not the
accumulator.
(cherry picked from commit 25a67fae3e)
Today I was testing headers manipulation but I met a bug with my first test.
To reproduce it, add for example this line :
rspadd Cache-Control:\ max-age=1500
Check the response header, it will provide :
Cache-Control: max-age=15000 <= the last character is duplicated
This only happens when we use backslashes on the last line of the
configuration file, without returning to the line.
Also if the last line is like :
rspadd Cache-Control:\ max-age=1500\
the last backslash causes a segfault.
This is not due to rspadd but to a more general bug in cfgparse.c :
...
if (skip) {
memmove(line + 1, line + 1 + skip, end - (line + skip + 1));
end -= skip;
}
...
should be :
...
if (skip) {
memmove(line + 1, line + 1 + skip, end - (line + skip));
end -= skip;
}
...
I've reproduced it with haproxy 1.3.22 and the last 1.4 snapshot.
(cherry picked from commit dd1b01d027)
Cameron Simpson reported an annoying case where haproxy simply reports
"Error(s) found in configuration file" when the file is not found or
not readable.
Fortunately the parsing function still returns -1 in case of open
error, so we're able to detect the issue from the caller and report
the corresponding errno message.
(cherry picked from commit c438242878)
Right now, an HTTP server cannot track a TCP server and vice-versa.
This patch enables proxy tracking without relying on the proxy's mode
(tcp/http/health). It only requires a matching proxy name to exist. The
original function was renamed to findproxy_mode().
(cherry picked from commit 96532db923)
In some environments it is not possible to rely on any wildcard for a
domain name (eg: .com, .net, .fr...) so it is required to send multiple
domain extensions. (Un)fortunately the syntax check on the domain name
prevented that from being done the dirty way. So let's just build a
domain list when multiple domains are passed on the same line.
Gabriel Sosa reported that logs were appearing with BADREQ when
'option httplog' was used with a TCP proxy (eg: inherited via a
default instance). This patch detects it and falls back to tcplog
after emitting a warning.
Holger Just reported that running ACLs with too many args caused
a segfault during config parsing. This is caused by a wrong test
on argument count. In case of too many arguments on a config line,
the last one was not correctly zeroed. This is now done and we
report the error indicating what part had been truncated.
This patch has 2 goals :
1. I wanted to test the appsession feature with a small PHP code,
using PHPSESSID. The problem is that when PHP gets an unknown session
id, it creates a new one with this ID. So, when sending an unknown
session to PHP, persistance is broken : haproxy won't see any new
cookie in the response and will never attach this session to a
specific server.
This also happens when you restart haproxy : the internal hash becomes
empty and all sessions loose their persistance (load balancing the
requests on all backend servers, creating a new session on each one).
For a user, it's like the service is unusable.
The patch modifies the code to make haproxy also learn the persistance
from the client : if no session is sent from the server, then the
session id found in the client part (using the URI or the client cookie)
is used to associated the server that gave the response.
As it's probably not a feature usable in all cases, I added an option
to enable it (by default it's disabled). The syntax of appsession becomes :
appsession <cookie> len <length> timeout <holdtime> [request-learn]
This helps haproxy repair the persistance (with the risk of losing its
session at the next request, as the user will probably not be load
balanced to the same server the first time).
2. This patch also tries to reduce the memory usage.
Here is a little example to explain the current behaviour :
- Take a Tomcat server where /session.jsp is valid.
- Send a request using a cookie with an unknown value AND a path
parameter with another unknown value :
curl -b "JSESSIONID=12345678901234567890123456789012" http://<haproxy>/session.jsp;jsessionid=00000000000000000000000000000001
(I know, it's unexpected to have a request like that on a live service)
Here, haproxy finds the URI session ID and stores it in its internal
hash (with no server associated). But it also finds the cookie session
ID and stores it again.
- As a result, session.jsp sends a new session ID also stored in the
internal hash, with a server associated.
=> For 1 request, haproxy has stored 3 entries, with only 1 which will be usable
The patch modifies the behaviour to store only 1 entry (maximum).
Similar patch was merged in 1.4 with commit ID bf47aeb946.
I'm working on helping Arnaud update haproxy in Debian, and one of the
package build warnings I received was about "hyphen where a minus sign
was intended" in the man page - details:
http://lintian.debian.org/tags/hyphen-used-as-minus-sign.html
Patch included in my 1.3.20 Debian package is attached.
(cherry picked from commit 35928e84e2)
Released version 1.3.22 with the following main changes :
- [BUG] unix socket: don't try to dereference frontend/backends
- [MINOR] unix socket: report the socket path in case of bind error
- [CONTRIB] halog: support searching by response time
- [DOC] add a reminder about obsolete documents
haproxy-en.txt and haproxy-fr.txt are outdated but people still refer to
them quite often, generally causing a useless waste of time.
(cherry picked from commit a080eca533)
John Lauro reported a new crash on 1.3.21 due to a dereferencing bug
of the frontend which does not have any frontend. The bug was introduced
by commit a3e0e0767f.
Released version 1.3.21 with the following main changes :
- [DOC] add missing rate_lim and rate_max
- [BUG] check if rise/fall has an argument and it is > 0
- [MINOR] add "description", "node" and show-node"/"show-desc", remove "node-name", v2
- [DOC] Add information about http://haproxy.1wt.eu/contrib.html
- [MINOR] acl: don't report valid acls as potential mistakes
- [BUG] task.c: don't assing last_timer to node-less entries
- [MINOR] export the hostname variable so that all the code can access it
- [MINOR] stats: add a new node-name setting
- [MINOR] acl: add support for hdr_ip to match IP addresses in headers
- [CLEANUP] remove ifdef MSG_NOSIGNAL and define it instead
- [BUG] buffer_forward() would not correctly consider data already scheduled
- [MAJOR] http: add support for HTTP 1xx informational responses
- [BUILD] stream_interface: fix conflicting declaration
- [CLEANUP] include time.h from freq_ctr.h as it uses "now".
- [MINOR] report list of supported pollers with -vv
- [MEDIUM] new option "independant-streams" to stop updating read timeout on writes
- [BUG] don't refresh timeouts late after detected activity
- [MINOR] acl: add fe_conn, be_conn, queue, avg_queue
- [BUILD] add a 'make tags' target (cherry picked from commit ebe0af4b77)
I noticed that in __eb32_insert , if the tree is empty
(root->b[EB_LEFT] == NULL) , the node.bit is not defined.
However in __task_queue there are checks:
- if (last_timer->node.bit < 0)
- if (task->wq.node.bit < last_timer->node.bit)
which might rely upon an undefined value.
This is how I see it:
1. We insert eb32_node in an empty wait queue tree for a task (called by
process_runnable_tasks() ):
Inserting into empty wait queue &task->wq = 0x72a87c8, last_timer
pointer: (nil)
2. Then, we set the last timer to the same address:
Setting last_timer: (nil) to: 0x72a87c8
3. We get a new task to be inserted in the queue (again called by
process_runnable_tasks()) , before the __task_unlink_wq() is called for
the previous task.
4. At this point, we still have last_timer set to 0x72a87c8 , but since
it was inserted in an empty tree, it doesn't have node.bit and the
values above get dereferenced with undefined value.
The bug has no effect right now because the check for equality is still
made, so the next timer will still be queued at the right place anyway,
without any possible side-effect. But it's a pending bug waiting for a
small change somewhere to strike.
Iliya Polihronov
(cherry picked from commit 1d7a420c84)
These ACLs are used to check the number of active connections on the
frontend, backend or in a backend's queue. The avg_queue returns the
average number of queued connections per server, and for this, divides
the total number of queued connections by the number of alive servers.
The dst_conn ACL has been slightly changed to more reflect its name and
original usage, which is to return the number of connections on the
destination address/port (the socket) and not the whole frontend.
(cherry picked from commit a36af91951)
Commit 404e8ab461 introduced
smart checking for stupid acl typos. However, now haproxy shows
the warning even for valid acls, like this one:
acl Cookie-X-NoAccel hdr_reg(cookie) (^|\ |;)X-NoAccel=1(;|$)
(cherry picked from commit 4cdd8314e9)
In old versions, before 1.3.16, we had to refresh the timeouts after
each call to process_session() because the stream socket handler did
not do it. Now that the sockets can exchange data for a long period
without calling process_session(), we can detect an old activity and
refresh a timeout long after the last activity, causing too late a
detection of some timeouts.
The fix simply consists in not checking for activity anymore in
stream_sock_data_finish() but only set a timeout if it was not
previously set.
(cherry picked from commit fe8903cc76)
