Some converters will need one or several arguments. It's not possible
to write a simple generic parser for that, so let's add the ability
for each converter to support its own argument parser, and call it
to get the arguments when it's specified. If unspecified, the arguments
are passed unmodified as string+len.
The pattern type converters currently support a string arg and a length.
Sometimes we'll prefer to pass them a list or a structure. So let's convert
the string and length into a generic void* and int that each converter may
use as it likes.
Released version 1.4-dev8 with the following main changes :
- [CLEANUP] Keep in sync "defaults" support between documentation and code
- [MEDIUM] http: add support for Proxy-Connection header
- [CRITICAL] buffers: buffer_insert_line2 must not change the ->w entry
- [MINOR] http: remove a copy-paste typo in transaction cleaning
- [BUG] http: trim any excess buffer data when recycling a connection
Hi Willy,
I've made a quick pass on the "defaults" column in the Proxy keywords matrix (chapter 4.1. in the documentation).
This patch resyncs the code and the documentation. I let you decide if some keywords that still work in the "defaults" section should be forbidden.
- default_backend : in the matrix, "defaults" was not supported but the keyword details say it is.
Tests also shows it works, then I've updated the matrix.
- capture cookie : in the keyword details, we can read `It is not possible to specify a capture in a "defaults" section.'.
Ok, even if the tests worked, I've added an alert in the configuration parser (as it is for capture request/response header).
- description : not supported in "defaults", I added an alert in the parser.
I've also noticed that this keyword doesn't appear in the documentation.
There's one "description" entry, but for the "global" section, which is for a different use (the patch doesn't update the documentation).
- grace : even if this is maybe useless, it works in "defaults". Documentation is updated.
- redirect : alert is added in the parser.
- rsprep : alert added in the parser.
--
Cyril Bonté
We must trim any excess data from the response buffer when recycling
a keep-alive connection, because we may have blocked an invalid response
from a server that we don't want to accidentely forward once we disable
the analysers, nor do we want those data to come along with next response.
A typical example of such data would be from a buggy server responding to
a HEAD with some data, or sending more than the advertised content-length.
For deciding to set the BF_EXPECT_MORE, we reused the same code as in
http_wait_for_request(), but here we must ignore buf->lr which is not
yet set and useless. This might only have caused random sub-optimal
behaviours.
Krzysztof Oledzki reported that 1.4-dev7 would regularly crash
on an apparently very common workload. The cores he provided
showed some inter-buffer data corruption, exactly similar to
what was fixed by the following recent commit :
bbfa7938bd [BUG] buffer_replace2 must never change the ->w entry
In fact, it was buffer_insert_line2() which was still modifying the
->w pointer, causing issues with pipelined responses in keep-alive
mode if some headers were to be added.
The bug requires a remote client, a near server, large server buffers
and small client buffers to be reproduced, with response header
insertion. Still, it's surprizing that it did not trigger earlier.
Now after 100k pipelined requests it did not trigger anymore.
Despite what is explicitly stated in HTTP specifications,
browsers still use the undocumented Proxy-Connection header
instead of the Connection header when they connect through
a proxy. As such, proxies generally implement support for
this stupid header name, breaking the standards and making
it harder to support keep-alive between clients and proxies.
Thus, we add a new "option http-use-proxy-header" to tell
haproxy that if it sees requests which look like proxy
requests, it should use the Proxy-Connection header instead
of the Connection header.
Released version 1.4-dev7 with the following main changes :
- [BUG] appsession: possible memory leak in case of out of memory condition
- [MINOR] config: don't accept 'appsession' in defaults section
- [MINOR] Add function to parse a size in configuration
- [MEDIUM] Add stick table (persistence) management functions and types
- [MEDIUM] Add pattern fetch management types and functions
- [MEDIUM] Add src dst and dport pattern fetches.
- [MEDIUM] Add stick table configuration and init.
- [MEDIUM] Add stick and store rules analysers.
- [MINOR] add option "mysql-check" to use MySQL health checks
- [BUG] health checks: fix requeued message
- [OPTIM] remove SSP_O_VIA and SSP_O_STATUS
- [BUG] checks: fix newline termination
- [MINOR] acl: add fe_id/so_id to match frontend's and socket's id
- [BUG] appsession's sessid must be reset at end of transaction
- [BUILD] appsession did not build anymore under gcc-2.95
- [BUG] server redirection used an uninitialized string.
- [MEDIUM] http: fix handling of message pointers
- [MINOR] http: fix double slash prefix with server redirect
- [MINOR] http redirect: add the ability to append a '/' to the URL
- [BUG] stream_interface: fix retnclose and remove cond_close
- [MINOR] http redirect: don't explicitly state keep-alive on 1.1
- [MINOR] http: move appsession 'sessid' from session to http_txn
- [OPTIM] reorder http_txn to optimize cache lines placement
- [MINOR] http: differentiate waiting for new request and waiting for a complete requst
- [MINOR] http: add a separate "http-keep-alive" timeout
- [MINOR] config: remove undocumented and buggy 'timeout appsession'
- [DOC] fix various too large lines
- [DOC] remove several trailing spaces
- [DOC] add the doc about stickiness
- [BUILD] remove a warning in standard.h on AIX
- [BUG] checks: chars are unsigned on AIX, check was always true
- [CLEANUP] stream_sock: MSG_NOSIGNAL is only for send(), not recv()
- [BUG] check: we must not check for error before reading a response
- [BUG] buffers: remove remains of wrong obsolete length check
- [OPTIM] stream_sock: don't shutdown(write) when the socket is in error
- [BUG] http: don't count req errors on client resets or t/o during keep-alive
- [MEDIUM] http: don't switch to tunnel mode upon close
- [DOC] add documentation about connection header processing
- [MINOR] http: add http_remove_header2() to remove a header value.
- [MINOR] tools: add a "word_match()" function to match words and ignore spaces
- [MAJOR] http: rework request Connection header handling
- [MAJOR] http: rework response Connection header handling
- [MINOR] add the ability to force kernel socket buffer size.
- [BUG] http_server_error() must not purge a previous pending response
- [OPTIM] http: don't delay response if next request is incomplete
- [MINOR] add the "force-persist" statement to force persistence on down servers
- [MINOR] http: logs must report persistent connections to down servers
- [BUG] buffer_replace2 must never change the ->w entry
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 arises 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.
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.
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.
We use to delay the response if there is a new request in the buffer.
However, if the pending request is incomplete, we should not delay the
pending responses.
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.
Sometimes we need to be able to change the default kernel socket
buffer size (recv and send). Four new global settings have been
added for this :
- tune.rcvbuf.client
- tune.rcvbuf.server
- tune.sndbuf.client
- tune.sndbuf.server
Those can be used to reduce kernel memory footprint with large numbers
of concurrent connections, and to reduce risks of write timeouts with
very slow clients due to excessive kernel buffering.
This one is the next step of previous patch. It correctly computes
the response mode and the Connection flag transformations depending
on the request mode and version, and the response version and headers.
We're now also able to add "Connection: keep-alive", and to convert
server's close during a keep-alive connection to a server-close
connection.
We need to improve Connection header handling in the request for it
to support the upcoming keep-alive mode. Now we have two flags which
keep in the session the information about the presence of a
Connection: close and a Connection: keep-alive headers in the initial
request, as well as two others which keep the current state of those
headers so that we don't have to parse them again. Knowing the initial
value is essential to know when the client asked for keep-alive while
we're forcing a close (eg in server-close mode). Also the Connection
request parser is now able to automatically remove single header values
at the same time they are parsed. This provides greater flexibility and
reliability.
All combinations of listen/front/back in all modes and with both
1.0 and 1.1 have been tested.
Some header values might be delimited with spaces, so it's not enough to
compare "close" or "keep-alive" with strncasecmp(). Use word_match() for
that.
Calling this function after http_find_header2() automatically deletes
the current value of the header, and removes the header itself if the
value is the only one. The context is automatically adjusted for a
next call to http_find_header2() to return the next header. No other
change nor test should be made on the transient context though.
The connection header is complex to handle, especially in the response
path, depending on request and response HTTP versions, desired mode,
etc... Let's document it. Note that only a subset of this document is
currently implemented.
The close mode of a transaction would be switched to tunnel mode
at the end of the processing, letting a lot of pending data pass
in the other direction if any. Let's fix that by checking for the
close mode during state resync too.
We must set the error flags when detecting that a client has reset
a connection or timed out while waiting for a new request on a keep-alive
connection, otherwise process_session() sets it itself and counts one
request error.
That explains why some sites were showing an increase in request errors
with the keep-alive.
We get a lot of those, especially with web crawlers :
recv(2, 0x810b610, 7000, 0) = -1 ECONNRESET (Connection reset by peer)
shutdown(2, 1 /* send */) = -1 ENOTCONN (Transport endpoint is not connected)
close(2) = 0
There's no need to perform the shutdown() here, the socket is already
in error so it is down.
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.
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.
While waiting in a keep-alive state for a request, we want to silently
close if we don't get anything. However if we get a partial request it's
different because that means the client has started to send something.
This requires a new transaction flag. It will be used to implement a
distinct timeout for keep-alive and requests.
