Olivier Houchard [Fri, 28 Dec 2018 13:44:41 +0000 (14:44 +0100)]
BUG/MEDIUM: mux_h2: Don't add to the idle list if we're full.
In h2_detach(), don't add the connection to the idle list if nb_streams
is at the max. This can happen if we already closed that stream before, so
its slot became available and was used by another stream.
Jérôme Magnin [Fri, 28 Dec 2018 13:49:08 +0000 (14:49 +0100)]
BUG/MINOR: htx: send the proper authenticate header when using http-request auth
When we use htx and http-request auth rules, we need to send WWW-Authenticate
with a 401 and Proxy-Authenticate with a 407. We only sent Proxy-Authenticate
regardless of status, with htx enabled.
Olivier Houchard [Mon, 24 Dec 2018 12:32:13 +0000 (13:32 +0100)]
BUG/MEDIUM: servers: Don't try to reuse connection if we switched server.
In connect_server(), don't attempt to reuse the old connection if it's
targetting a different server than the one we're supposed to access, or
we will never be able to connect to a server if the first one we tried failed.
Willy Tarreau [Sat, 22 Dec 2018 19:19:26 +0000 (20:19 +0100)]
MEDIUM: mux-h2: handle decoding of CONTINUATION frames
Now that the HEADERS frame decoding is retryable, we can safely try to
fold CONTINUATION frames into a HEADERS frame when the END_OF_HEADERS
flag is missing. In order to do this, h2c_decode_headers() moves the
frames payloads in-situ and leaves a hole that is plugged when leaving
the function. There is no limit to the number of CONTINUATION frames
handled this way provided that all of them fit into the buffer. The
error reported when meeting isolated CONTINUATION frames has now changed
from INTERNAL_ERROR to PROTOCOL_ERROR.
Now there is only one (unrelated) remaining failure in h2spec.
Willy Tarreau [Sat, 22 Dec 2018 18:19:50 +0000 (19:19 +0100)]
MINOR: buffers: add a new b_move() function
This function will be used to move parts of a buffer to another place
in the same buffer, even if the parts overlap. In order to keep things
under reasonable control, it only uses a length and absolute offsets
for the source and destination, and doesn't consider head nor data.
Willy Tarreau [Sun, 23 Dec 2018 17:30:44 +0000 (18:30 +0100)]
MINOR: mux-h2: fail stream creation more cleanly using RST_STREAM
The H2 demux only checks for too many streams in h2c_frt_stream_new(),
then refuses to create a new stream and causes the connection to be
aborted by sending a GOAWAY frame. This will also happen if any error
happens during the stream creation (e.g. memory allocation).
RFC7540#5.1.2 says that attempts to create streams in excess should
instead be dealt with using an RST_STREAM frame conveying either the
PROTOCOL_ERROR or REFUSED_STREAM reason (the latter being usable only
if it is guaranteed that the stream was not processed). In theory it
should not happen for well behaving clients, though it may if we
configure a low enough h2.max_concurrent_streams limit. This error
however may definitely happen on memory shortage.
Previously it was not possible to use RST_STREAM due to the fact that
the HPACK decompressor would be desynchronized. But now we first decode
and only then try to allocate the stream, so the decompressor remains
synchronized regardless of policy or resources issues.
With this patch we enforce stream termination with RST_STREAM and
REFUSED_STREAM if this protocol violation happens, as well as if there
is a temporary condition like a memory allocation issue. It will allow
a client to recover cleanly.
This could possibly be backported to 1.9. Note that this requires that
these five previous patches are merged as well :
MINOR: h2: add a bit-based frame type representation
MEDIUM: mux-h2: remove padlen during headers phase
MEDIUM: mux-h2: decode HEADERS frames before allocating the stream
MINOR: mux-h2: make h2c_send_rst_stream() use the dummy stream's error code
MINOR: mux-h2: add a new dummy stream for the REFUSED_STREAM error code
Willy Tarreau [Sun, 23 Dec 2018 17:29:12 +0000 (18:29 +0100)]
MINOR: mux-h2: add a new dummy stream for the REFUSED_STREAM error code
This patch introduces a new dummy stream, h2_refused_stream, in CLOSED
status with the aforementioned error code. It will be usable to reject
unexpected extraneous streams.
Willy Tarreau [Sun, 23 Dec 2018 17:26:26 +0000 (18:26 +0100)]
MINOR: mux-h2: make h2c_send_rst_stream() use the dummy stream's error code
We currently have 2 dummy streams allowing us to send an RST_STREAM
message with an error code matching this one. However h2c_send_rst_stream()
still enforces the STREAM_CLOSED error code for these dummy streams,
ignoring their respective errcode fields which however are properly
set.
Let's make the function always use the stream's error code. This will
allow to create other dummy streams for different codes.
Willy Tarreau [Sun, 23 Dec 2018 10:30:42 +0000 (11:30 +0100)]
MEDIUM: mux-h2: decode HEADERS frames before allocating the stream
It's hard to recover from a HEADERS frame decoding error after having
already created the stream, and it's not possible to recover from a
stream allocation error without dropping the connection since we can't
maintain the HPACK context, so let's decode it before allocating the
stream, into a temporary buffer that will then be offered to the newly
created stream.
Willy Tarreau [Fri, 21 Dec 2018 14:34:50 +0000 (15:34 +0100)]
MEDIUM: mux-h2: remove padlen during headers phase
Three types of frames may be padded : DATA, HEADERS and PUSH_PROMISE.
Currently, each of these independently deals with padding and needs to
wait for and skip the initial padlen byte. Not only this complicates
frame processing, but it makes it very hard to process CONTINUATION
frames after a padded HEADERS frame, and makes it complicated to perform
atomic calls to h2s_decode_headers(), which are needed if we want to be
able to maintain the HPACK decompressor's context even when dropping
streams.
This patch takes a different approach : the padding is checked when
parsing the frame header, the padlen byte is waited for and parsed,
and the dpl value is updated with this padlen value. This will allow
the frame parsers to decide to overwrite the padding if needed when
merging adjacent frames.
Willy Tarreau [Sun, 23 Dec 2018 19:43:58 +0000 (20:43 +0100)]
BUG/MEDIUM: mux-h2: mark that we have too many CS once we have more than the max
Since commit f210191 ("BUG/MEDIUM: h2: don't accept new streams if
conn_streams are still in excess") we're refraining from reading input
frames if we've reached the limit of number of CS. The problem is that
it prevents such situations from working fine. The initial purpose was
in fact to prevent from reading new HEADERS frames when this happens,
and causes some occasional transfer hiccups and pauses with large
concurrencies.
Given that we now properly reject extraneous streams before checking
this value, we can be sure never to have too many streams, and that
any higher value is only caused by a scheduling reason and will go
down after the scheduler calls the code.
This fix must be backported to 1.9 and possibly to 1.8. It may be
tested using h2spec this way with an h2spec config :
while :; do
h2spec -o 5 -v -t -S -k -h 127.0.0.1 -p 4443 http2/5.1.2
done
Willy Tarreau [Sun, 23 Dec 2018 07:13:59 +0000 (08:13 +0100)]
BUG/MINOR: mux-h2: make empty HEADERS frame return a connection error
We were returning a stream error of type PROTOCOL_ERROR on empty HEADERS
frames, but RFC7540#4.2 stipulates that we should instead return a
connection error of type FRAME_SIZE_ERROR.
This may be backported to 1.9 and 1.8 though it's unlikely to have any
real life effect.
Willy Tarreau [Sun, 23 Dec 2018 08:58:41 +0000 (09:58 +0100)]
REGTESTS: remove the expected window updates from H2 handshakes
These ones are not needed anymore since commit 97aaa67 ("MINOR: mux-h2:
only increase the connection window with the first update"). The tests
should now be more reliable. It might be worth simply removing all the
explicit handshake though it doesn't hurt and still serves as documentation.
Willy Tarreau [Sun, 23 Dec 2018 08:49:04 +0000 (09:49 +0100)]
MINOR: mux-h2: only increase the connection window with the first update
Commit dc57236 ("BUG/MINOR: mux-h2: advertise a larger connection window
size") caused a WINDOW_UPDATE message to be sent early with the connection
to increase the connection's window size. It turns out that it causes some
minor trouble that need to be worked around :
- varnishtest cannot transparently cope with the WU frames during the
handshake, forcing all tests to explicitly declare the handshake
sequence ;
- some vtc scripts randomly fail if the WU frame is sent after another
expected response frame, adding uncertainty to some tests ;
- h2spec doesn't correctly identify these WU at the connection level
that it believes are the responses to some purposely erroneous frames
it sends, resulting in some errors being reported
None of these are a problem with real clients but they add some confusion
during troubleshooting.
Since the fix above was intended to increase the upload bandwidth, we
have another option which is to increase the window size with the first
WU frame sent for the connection. This way, no WU frame is sent until
one is really needed, and this first frame will adjust the window to
the maximum value. It will make the window increase slightly later, so
the client will experience the first round trip when uploading data,
but this should not be perceptible, and is not worth the extra hassle
needed to maintain our debugging abilities. As an extra bonus, a few
extra bytes are saved for each connection until the first attempt to
upload data.
This should possibly be backported to 1.9 and 1.8.
Willy Tarreau [Sat, 22 Dec 2018 10:19:45 +0000 (11:19 +0100)]
[RELEASE] Released version 2.0-dev0
Released version 2.0-dev0 with the following main changes :
- BUG/MAJOR: connections: Close the connection before freeing it.
- REGTEST: Require the option LUA to run lua tests
- REGTEST: script: Process script arguments before everything else
- REGTEST: script: Evaluate the varnishtest command to allow quoted parameters
- REGTEST: script: Add the option --clean to remove previous log direcotries
- REGTEST: script: Add the option --debug to show logs on standard ouput
- REGTEST: script: Add the option --keep-logs to keep all log directories
- REGTEST: script: Add the option --use-htx to enable the HTX in regtests
- REGTEST: script: Print only errors in the results report
- REGTEST: Add option to use HTX prefixed by the macro 'no-htx'
- REGTEST: Make reg-tests target support argument.
- REGTEST: Fix a typo about barrier type.
- REGTEST: Be less Linux specific with a syslog regex.
- REGTEST: Missing enclosing quotes for ${tmpdir} macro.
- REGTEST: Exclude freebsd target for some reg tests.
- BUG/MEDIUM: h2: Don't forget to quit the sending_list if SUB_CALL_UNSUBSCRIBE.
- BUG/MEDIUM: mux-h2: Don't forget to quit the send list on error reports
- BUG/MEDIUM: dns: Don't prevent reading the last byte of the payload in dns_validate_response()
- BUG/MEDIUM: dns: overflowed dns name start position causing invalid dns error
- BUG/MINOR: compression/htx: Don't compress responses with unknown body length
- BUG/MINOR: compression/htx: Don't add the last block of data if it is empty
- MEDIUM: mux_h1: Implement h1_show_fd.
- REGTEST: script: Add support of alternatives in requited options list
- REGTEST: Add a basic test for the compression
- BUG/MEDIUM: mux-h2: don't needlessly wake up the demux on short frames
- REGTEST: A basic test for "http-buffer-request"
- BUG/MEDIUM: server: Also copy "check-sni" for server templates.
- MINOR: ssl: Add ssl_sock_set_alpn().
- MEDIUM: checks: Add check-alpn.
Olivier Houchard [Fri, 21 Dec 2018 18:47:01 +0000 (19:47 +0100)]
MEDIUM: checks: Add check-alpn.
Add a way to configure the ALPN used by check, with a new "check-alpn"
keyword. By default, the checks will use the server ALPN, but it may not
be convenient, for instance because the server may use HTTP/2, while checks
are unable to do HTTP/2 yet.
Willy Tarreau [Fri, 21 Dec 2018 15:09:41 +0000 (16:09 +0100)]
BUG/MEDIUM: mux-h2: don't needlessly wake up the demux on short frames
In some situations, if too short a frame header is received, we may leave
h2_process_demux() waking up the task again without checking that we were
already subscribed.
In order to avoid this once for all, let's introduce an h2_restart_reading()
function which performs the control and calls the task up. This way we won't
needlessly wake the task up if it's already waiting for I/O.
REGTEST: script: Add support of alternatives in requited options list
It is now possible to specify a list of "alternatives" for a required
option. This must be done by separating options by a pipe ('|'). A test will be
executed if at least one of them is available. For instance:
#REQUIRED_OPTIONS=ZLIB|SLZ,LUA,OPENSSL
The function _findtest() has also been sligthly simplified.
BUG/MINOR: compression/htx: Don't add the last block of data if it is empty
In HTX, when the compression filter analyze the EOM, it flushes the compression
context and add the last block of compressed data. But, this block can be
empty. In this case, we must ignore it.
Nikhil Agrawal [Thu, 20 Dec 2018 05:20:59 +0000 (10:50 +0530)]
BUG/MEDIUM: dns: overflowed dns name start position causing invalid dns error
In dns_read_name() when dns name is used with compression and start position of
name is greater than 255 name read is incorrect and causes invalid dns error.
eg: 0xc11b c specifies name compression being used. 11b represent the start
position of name but currently we are using only 1b for start position.
Jérôme Magnin [Thu, 20 Dec 2018 15:47:31 +0000 (16:47 +0100)]
BUG/MEDIUM: dns: Don't prevent reading the last byte of the payload in dns_validate_response()
A regression was introduced with efbbdf72 BUG: dns: Prevent out-of-bounds
read in dns_validate_dns_response() as it prevented from taking into account
the last byte of the payload. this patch aims at fixing it.
Olivier Houchard [Thu, 20 Dec 2018 10:56:28 +0000 (11:56 +0100)]
BUG/MEDIUM: h2: Don't forget to quit the sending_list if SUB_CALL_UNSUBSCRIBE.
In mux_h2_unsubscribe, don't forget to leave the sending_list if
SUB_CALL_UNSUBSCRIBE was set. SUB_CALL_UNSUBSCRIBE means we were about
to be woken up for writing, unless the mux was too full to get more data.
If there's an unsubscribe call in the meanwhile, we should leave the list,
or we may be put back in the send_list.
REGTEST: script: Add the option --use-htx to enable the HTX in regtests
A check is done on HAProxy version to be sure it is supported. Internally, by
default, the macro 'no-htx' is set to "#". So in regtests, we can prefix
http-use-htx option line with this macro to inhibit the HTX. Concretly, this
line can be added in the HAProxy configuration:
${no-htx} option http-use-htx
When the option --use-htx is used, the macro 'no-htx' is unset.
REGTEST: script: Add the option --keep-logs to keep all log directories
By default a log directory is kept only if the test fails. With this option it
is possible to always keep it. If this option is used, the result of all tests
are displayed (and not only the failing ones).
REGTEST: script: Add the option --clean to remove previous log direcotries
Running regtests several times leaves many direcotries. It is painful to remove
them by hand. This command do it for you. It ask a confirmation to proceed to be
sure. The template used to create log direcotries has been changed. The prefix
'haregtests-' has been added to help the cleanup function to find existing log
direcotries.
REGTEST: script: Evaluate the varnishtest command to allow quoted parameters
Instead of directly executing varnishtest command, we use a variable to build
the command line and we execute it with the 'eval' builtin. This way it is
possible to have quoted parameters. For instance:
And the variable 'varnishtestparams' is also move at the end of the command
line, just before the list of test files. So it is possible to override all
default varnish options set by the script.
REGTEST: script: Process script arguments before everything else
The help message is now in the function _help(). It exits the script with the
status 0 to not run regtests when the help is displayed. So it is also handy to
process script arguments before anything else. This avoids to start printing
messages related to regtests execution when it is not appropriate. Another
change, when it detects an invalid argument, the script exits with an error.
Olivier Houchard [Wed, 19 Dec 2018 22:21:46 +0000 (23:21 +0100)]
BUG/MAJOR: connections: Close the connection before freeing it.
In si_release_endpoint(), if the end point is a connection, because we don't
know which mux to use it, make sure we close the connection before freeing it,
or else, we'd have a fd left for polling, which would point to a now free'd
connection.
Willy Tarreau [Wed, 19 Dec 2018 18:13:17 +0000 (19:13 +0100)]
[RELEASE] Released version 1.9.0
Released version 1.9.0 with the following main changes :
- BUG/MEDIUM: compression: Use the right buffer pointers to compress input data
- BUG/MINOR: mux_pt: Set CS_FL_WANT_ROOM when count is zero in rcv_buf() callback
- BUG/MEDIUM: connection: Add a new CS_FL_ERR_PENDING flag to conn_streams.
- CONTRIB: debug: teach the "flags" utility about new conn_stream flags
- BUG/MEDIUM: stream-int: always clear CS_FL_WANT_ROOM before receiving
- BUG/MEDIUM: mux-h2: also restart demuxing when data are pending in demux
- BUG/MEDIUM: mux-h2: restart demuxing as soon as demux data are available
- BUG/MEDIUM: h2: fix aggregated cookie length computation in HTX mode
- MINOR: mux-h2: report more h2c, last h2s and cs information on "show fd"
- CONTRIB: debug: report stream-int's flag SI_FL_CLEAN_ABRT
- MINOR: cli/stream: add the conn_stream in "show sess" output
- BUG/MINOR: mux-h2: don't report a fantom h2s in "show fd"
- BUG/MINOR: cli/fd: don't isolate the thread for each individual fd
- MINOR: objtype: report a few missing types in names and base pointers
- BUG/MEDIUM: mux-h2: make sure to report synchronous errors after EOS
- BUG/MEDIUM: mux-h2: report asynchronous errors in h2_wake_some_streams()
- BUG/MEDIUM: mux-h2: make sure the demux also wakes streams up on errors
- BUG/MINOR: mux-h1: report the correct frontend in error captures
- BUG/MEDIUM: stream-int: also wake the stream up on end of transfer
- MEDIUM: h2: properly check and deduplicate the content-length header in HTX
- BUG/MEDIUM: stream: Forward the right amount of data before infinite forwarding
- BUG/MINOR: proto_htx: Call the HTX version of the function managing client cookies
- BUG/MEDIUM: lua/htx: Handle EOM in receive/get_line calls in HTTP applets
- BUG/MINOR: lua: Return an error if a legacy HTTP applet doesn't send anything
- MINOR: compression: Remove the thread_local variable buf_output
- CLEANUP: connection: rename subscription events values and event field
- CLEANUP: connection: rename conn->mux_ctx to conn->ctx
- MINOR: connection: remove an unwelcome dependency on struct stream
- CLEANUP: stream-int: consistently call the si/stream_int functions
- BUG/MEDIUM: h1: Don't shutw/shutr the connection if we have keepalive.
- BUG/MEDIUM: H2: Make sure htx is set even on empty frames.
- BUG/MEDIUM: mux-h2: pass CS_FL_ERR_PENDING to h2_wake_some_streams()
- MEDIUM: stream-int: always consider all CS errors on the send side
- BUG/MEDIUM: h2: Make sure we don't set CS_FL_ERROR if there's still data.
- CLEANUP: mux-h2: implement h2s_notify_{send,recv} to report events to subscribers
- MINOR: mux-h2: add a new function h2s_alert() to call the data layer
- BUG/MEDIUM: mux-h2: make use of h2s_alert() to report aborts
- MINOR: connection: add cs_set_error() to set the error bits
- CLEANUP: mux-h2: make use of cs_set_error()
- BUG/MINOR: mux-h2: make sure we check the conn_stream in early data
- BUG/MEDIUM: h2: Don't wait for flow control if the connection had a shutr.
- MINOR: cli/show_fd: report that a connection is back or not
- SCRIPTS: add the slack channel URL to the announce script
- CLEANUP: remove my name and address from the copyright banner
- DOC: mention in the readme that 1.9 is a stable version now
Willy Tarreau [Wed, 19 Dec 2018 18:07:04 +0000 (19:07 +0100)]
CLEANUP: remove my name and address from the copyright banner
First, it's a pain to always have to think about updating this date,
second for a long time I've not been the only developer there, and third,
some users contact me hoping to get help that I can't deliver. It's about
time to redirect them to the main site where all the useful links should
be.
Olivier Houchard [Wed, 19 Dec 2018 17:16:17 +0000 (18:16 +0100)]
BUG/MEDIUM: h2: Don't wait for flow control if the connection had a shutr.
In h2_snd_buf(), if we couldn't send the data because of flow control, and
the connection got a shutr, then add CS_FL_ERROR (or CS_FL_ERR_PENDING). We
will never get any window update, so we will never be unlocked, anyway.
Willy Tarreau [Wed, 19 Dec 2018 17:33:16 +0000 (18:33 +0100)]
BUG/MINOR: mux-h2: make sure we check the conn_stream in early data
When dealing with early data we scan the list of stream to notify them.
We're not supposed to have h2s->cs == NULL here but it doesn't cost much
to make the scan more robust and verify it before notifying.
Willy Tarreau [Wed, 19 Dec 2018 16:44:55 +0000 (17:44 +0100)]
BUG/MEDIUM: mux-h2: make use of h2s_alert() to report aborts
If we had no pending read, it could be complicated to report an
RST_STREAM to a sender since we used to only report it via the
rx side if subscribed. Similarly in h2_wake_some_streams() we
now try all methods, hoping to catch all possible events.
Willy Tarreau [Wed, 19 Dec 2018 16:36:48 +0000 (17:36 +0100)]
MINOR: mux-h2: add a new function h2s_alert() to call the data layer
In order to report an error to the data layer, we have different ways
depending on the situation. At a lot of places it's open-coded and not
always correct. Let's create a new function h2s_alert() to handle this
task. It tries to wake on recv() first, then on send(), then using
wake().
Olivier Houchard [Wed, 19 Dec 2018 13:49:39 +0000 (14:49 +0100)]
BUG/MEDIUM: h2: Make sure we don't set CS_FL_ERROR if there's still data.
In the mux h2, make sure we set CS_FL_ERR_PENDING and wake the recv task,
instead of setting CS_FL_ERROR, if CS_FL_EOS is not set, so if there's
potentially still some data to be sent.
Willy Tarreau [Wed, 19 Dec 2018 16:17:10 +0000 (17:17 +0100)]
MEDIUM: stream-int: always consider all CS errors on the send side
We still have an issue with asynchronous errors, which is that while
they don't truncate reads anymore, they might be missed during a
send() attempt. This can happen for example when processing a request
followed by undesired data for which the stream doesn't try to receive,
while the send side experiences an error (transfer aborted by the client).
In this case we definitely want all send() attempts to fail as soon as
the error was reported, even if it's only pending. This way we leave an
opportunity to the stream interface to try to receive the last data
pending in the buffer but it cannot send anymore and knows that there
is an error when trying to do so.
Olivier Houchard [Wed, 19 Dec 2018 13:49:39 +0000 (14:49 +0100)]
BUG/MEDIUM: mux-h2: pass CS_FL_ERR_PENDING to h2_wake_some_streams()
Commiy 8519357c ("BUG/MEDIUM: mux-h2: report asynchronous errors in
h2_wake_some_streams()") addressed an issue with synchronous errors
but forgot to fix the call places to also pass CS_FL_ERR_PENDING
instead of CS_FL_ERROR.
Olivier Houchard [Wed, 19 Dec 2018 14:55:23 +0000 (15:55 +0100)]
BUG/MEDIUM: h1: Don't shutw/shutr the connection if we have keepalive.
In h1_shutw() and h1_shutr(), don't attempt to shutdown() the connection
if we're using keepalive and the connection has no error, or we will close
the connection too soon.
Willy Tarreau [Wed, 19 Dec 2018 14:19:27 +0000 (15:19 +0100)]
CLEANUP: stream-int: consistently call the si/stream_int functions
As long-time changes have accumulated over time, the exported functions
of the stream-interface were almost all prefixed "si_<something>" while
most private ones (mostly callbacks) were called "stream_int_<something>".
There were still a few confusing exceptions, which were addressed to
follow this shcme :
- stream_sock_read0(), only used internally, was renamed stream_int_read0()
and made static
- stream_int_notify() is only private and was made static
- stream_int_{check_timeouts,report_error,retnclose,register_handler,update}
were renamed si_<something>.
Now it is clearer when checking one of these if it risks to be used outside
or not.
Willy Tarreau [Wed, 19 Dec 2018 13:36:29 +0000 (14:36 +0100)]
MINOR: connection: remove an unwelcome dependency on struct stream
There was a reference to struct stream in conn_free() for the case
where we're freeing a connection that doesn't have a mux attached.
For now we know it's always a stream, and we only need to do it to
put a NULL in s->si[1].end.
Let's do it better by storing the pointer to si[1].end in the context
and specifying that this pointer is always nulled if the mux is null.
This way it allows a connection to detach itself from wherever it's
being used. Maybe we could even get rid of the condition on the mux.
Willy Tarreau [Wed, 19 Dec 2018 13:12:10 +0000 (14:12 +0100)]
CLEANUP: connection: rename conn->mux_ctx to conn->ctx
We most often store the mux context there but it can also be something
else while setting up the connection. Better call it "ctx" and know
that it's the owner's context than misleadingly call it mux_ctx and
get caught doing suspicious tricks.
Willy Tarreau [Wed, 19 Dec 2018 12:59:17 +0000 (13:59 +0100)]
CLEANUP: connection: rename subscription events values and event field
The SUB_CAN_SEND/SUB_CAN_RECV enum values have been confusing a few
times, especially when checking them on reading. After some discussion,
it appears that calling them SUB_RETRY_SEND/SUB_RETRY_RECV more
accurately reflects their purpose since these events may only appear
after a first attempt to perform the I/O operation has failed or was
not completed.
In addition the wait_reason field in struct wait_event which carries
them makes one think that a single reason may happen at once while
it is in fact a set of events. Since the struct is called wait_event
it makes sense that this field is called "events" to indicate it's the
list of events we're subscribed to.
Last, the values for SUB_RETRY_RECV/SEND were swapped so that value
1 corresponds to recv and 2 to send, as is done almost everywhere else
in the code an in the shutdown() call.
BUG/MEDIUM: lua/htx: Handle EOM in receive/get_line calls in HTTP applets
In HTTP applets, the request's EOM was removed like other blocks when receive or
get_line was called from lua scripts. So it was impossible to stop receiving
data on successive calls when all the request body was already consumed,
blocking infinitly the applet.
Now, we never consume the EOM. So it is easy to interrupt receive/get_line
calls. In all cases, this block is consumed when the applet ends.
BUG/MEDIUM: stream: Forward the right amount of data before infinite forwarding
Before setting the infinite forward, we first forward all remaining input data
from the channel. Of course for HTX streams, this must be done using the amount
of data in the HTX message not in the channel (which appears as full because of
the HTX).
Willy Tarreau [Wed, 19 Dec 2018 12:08:08 +0000 (13:08 +0100)]
MEDIUM: h2: properly check and deduplicate the content-length header in HTX
When producing an HTX message, we can't rely on the next-level H1 parser
to check and deduplicate the content-length header, so we have to do it
while parsing a message. The algorithm is the exact same as used for H1
messages.
Willy Tarreau [Wed, 19 Dec 2018 10:00:00 +0000 (11:00 +0100)]
BUG/MEDIUM: stream-int: also wake the stream up on end of transfer
There is an issue with some medium sized transfers occasionally not
shutting down at the end. Olivier tracked this to being caused by a
missing wakeup of process_stream(). What happens is that one of the
analysers sets CF_WAKE_WRITE to be woken up at the end of the transfer
to take note of the end of transaction, but a failed si_cs_send() at
the end of process_stream causes the call to be attempted again, with
CF_WAKE_WRITE lost. Then stream_int_notify() doesn't find any valid
condition to wake up process_stream(), and the stream stays there,
idling till the timeout.
In fact, CF_WAKE_WRITE has been designed for calling the analysers
to complete an operation without closing (keep-alive HTTP transfer
for instance). It only applies once the buffer is empty and there
is nothing left to be forwarded. In case the channel is closed, the
wakeup is already granted. So what we need here is to make sure to
wake process_stream() up in case the channel will not be closed and
it doesn't have anything left to be transferred. This is detected by
the lack of CF_AUTO_CLOSE and the emptiness of the buffer + to_forward
after a write activity. So now we take care of always waking the stream
up on end of transfers even if the analysers didn't subscribe to this
or if their subscription was lost.
CF_WAKE_WRITE should probably be killed now, though this first requires
careful inspection.
No backport is needed.
Cc: Olivier Houchard <ohouchard@haproxy.com> Cc: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Tue, 18 Dec 2018 15:52:44 +0000 (16:52 +0100)]
BUG/MEDIUM: mux-h2: make sure the demux also wakes streams up on errors
Today the demux only wakes a stream up after receiving some contents, but
not necessarily on close or error. Let's do it based on both error flags
and both EOS flags. With a bit of refinement we should be able to only do
it when the pending bits are there but not the static ones.
Willy Tarreau [Tue, 18 Dec 2018 15:44:28 +0000 (16:44 +0100)]
BUG/MEDIUM: mux-h2: report asynchronous errors in h2_wake_some_streams()
This function is called when dealing with a connection error or a GOAWAY
frame. It used to report a synchronous error instead of an asycnhronous
error, which can lead to data truncation since whatever is still available
in the rxbuf will be ignored. Let's correctly use CS_FL_ERR_PENDING instead
and only fall back to CS_FL_ERROR if CS_FL_EOS was already delivered.
Willy Tarreau [Tue, 18 Dec 2018 15:39:21 +0000 (16:39 +0100)]
BUG/MEDIUM: mux-h2: make sure to report synchronous errors after EOS
If EOS has already been reported on the conn_stream, there won't be
any read anymore to turn ERR_PENDING into ERROR, so we have to do
report it directly.
Willy Tarreau [Tue, 18 Dec 2018 15:29:48 +0000 (16:29 +0100)]
MINOR: objtype: report a few missing types in names and base pointers
Types DNS_SRVRQ and CS were not referenced in the type to string
conversions, causing possibly misleading outputs in session dumps.
Now instead of showing "NONE" for unknown invalid types names, we
display "!INVAL!" to clear the confusion that may exist in case of
memory corruption for example.
Willy Tarreau [Tue, 18 Dec 2018 14:45:11 +0000 (15:45 +0100)]
BUG/MINOR: cli/fd: don't isolate the thread for each individual fd
It takes ages to proceed with "show fd" when there is sustained activity
because it uses the rendez-vous point for each and every file descriptor
in the loop. It's very common to see socat timeout there.
Instead of doing this, let's just isolate the function when entering the
loop. Its duration is limited by the number of FDs that may be emitted in
a single buffer anyway, so it's much lighter and responds much faster.
Willy Tarreau [Tue, 18 Dec 2018 13:34:41 +0000 (14:34 +0100)]
BUG/MINOR: mux-h2: don't report a fantom h2s in "show fd"
The h2s pointer was used to scan fctl lists prior to being used to scan
the send list by ID, so it could appear non-null eventhough the list is
empty, resulting in misleading information on empty connections.
Willy Tarreau [Tue, 18 Dec 2018 09:32:05 +0000 (10:32 +0100)]
MINOR: mux-h2: report more h2c, last h2s and cs information on "show fd"
Most of the time when we issue "show fd" to dump a mux's state, it's
to figure why a transfer is frozen. Connection, stream and conn_stream
states are critical there. And most of the time when this happens there
is a single stream left in the H2 mux, so let's always dump the last
known stream on show fd, as most of the time it will be the one of
interest.
Willy Tarreau [Tue, 18 Dec 2018 10:00:41 +0000 (11:00 +0100)]
BUG/MEDIUM: h2: fix aggregated cookie length computation in HTX mode
Cyril Bonté reported a bug in the way the cookie length is computed
when aggregating multiple cookies : the first cookie name was counted
as part of the value length, causing random contents to be placed there,
possibly leading to bad requests.
Willy Tarreau [Tue, 18 Dec 2018 09:29:54 +0000 (10:29 +0100)]
BUG/MEDIUM: mux-h2: restart demuxing as soon as demux data are available
Commit 7505f94f9 ("MEDIUM: h2: Don't use a wake() method anymore.")
changed the conditions to restart demuxing so that this happens as soon
as something is read. But similar to previous fix, at an end of stream
we may be woken up with nothing to read but data still available in the
demux buffer, so we must also use this as a valid condition for demuxing.
Willy Tarreau [Tue, 18 Dec 2018 09:27:18 +0000 (10:27 +0100)]
BUG/MEDIUM: mux-h2: also restart demuxing when data are pending in demux
Commit 082f559d3 ("BUG/MEDIUM: h2: restart demuxing after releasing
buffer space") tried to address a situation where transfers could stall
after a read, but the condition was not completely covered : some stalls
may still happen at end of stream because there's nothing anymore to
receive and the last data lie in the demux buffer. Thus we must also
consider this state as a valid condition to restart demuxing.
Willy Tarreau [Tue, 18 Dec 2018 08:15:43 +0000 (09:15 +0100)]
BUG/MEDIUM: stream-int: always clear CS_FL_WANT_ROOM before receiving
Commit d94f877cd ("BUG/MINOR: mux_pt: Set CS_FL_WANT_ROOM when count is
zero in rcv_buf() callback") triggered a pending issue with this flag,
which is that it's cleared too late and sometimes causes some Rx
transfers to stall. We need to clear it before attempting to receive
otherwise we may risk to see an earlier copy of the flag.
Note that it should probably be defined that this flag could be purged
on each invocation of mux->rcv_buf(), which would make sense.
Olivier Houchard [Mon, 17 Dec 2018 13:16:46 +0000 (14:16 +0100)]
BUG/MEDIUM: connection: Add a new CS_FL_ERR_PENDING flag to conn_streams.
Add a new flag to conn_streams, CS_FL_ERR_PENDING. This is to be set instead
of CS_FL_ERR in case there's still more data to be read, so that we read all
the data before closing.
BUG/MINOR: mux_pt: Set CS_FL_WANT_ROOM when count is zero in rcv_buf() callback
When count is zero in the function mux_pt_rcv_buf(), it means the channel's
buffer is full. So we need to set the CS_FL_WANT_ROOM on the
conn_stream. Otherwise, while the channel is full, we will try to receive in
loop more data.
BUG/MEDIUM: compression: Use the right buffer pointers to compress input data
A bug was introduced when the buffers API was refactored. It was when wrapping
input data were compressed. the pointer b_peek(in, 0) was used instead of
"b_orig(in)". b_peek(in, 0) is in fact the same as b_head(in).
Willy Tarreau [Sun, 16 Dec 2018 21:35:06 +0000 (22:35 +0100)]
[RELEASE] Released version 1.9-dev11
Released version 1.9-dev11 with the following main changes :
- BUG/MEDIUM: connection: Don't use the provided conn_stream if it was tried.
- REGTEST/MINOR: remove double body specification for server txresp
- BUG/MEDIUM: connections: Remove error flags when retrying.
- REGTEST/MINOR: skip seamless-reload test with abns socket on freebsd
- REGTEST/MINOR: remove health-check that can make the test fail
- DOC: clarify that check-sni needs an argument.
- DOC: refer to check-sni in the documentation of sni
- BUG/MEDIUM: mux-h2: fix encoding of non-GET/POST methods
- BUG/MINOR: mux-h1: Fix conn_mode processing for headerless outgoing messages
- BUG/MEDIUM: mux-h1: Add a BUSY mode to not loop on pipelinned requests
- BUG/MEDIUM: mux-h1: Don't loop on the headers parsing if the read0 was received
- BUG/MEDIUM: htx: Always do a defrag if a block value is replace by a bigger one
- BUG/MEDIUM: mux-h2: Don't forget to set the CS_FL_EOS flag with htx.
- BUG/MINOR: hpack: fix off-by-one in header name encoding length calculation
- CLEANUP: hpack: no need to include chunk.h, only include buf.h
- MINOR: hpack: simplify the len to bytes conversion
- MINOR: hpack: use ist2bin() to copy header names in hpack_encode_header()
- MINOR: hpack: optimize header encoding for short names
- CONTRIB: hpack: add a compressed stream generator for the encoder
- MEDIUM: hpack: make it possible to encode any static header name
- MINOR: hpack: move the length computation and encoding functions to .h
- MINOR: hpack: provide a function to encode a short indexed header
- MINOR: hpack: provide a function to encode a long indexed header
- MINOR: hpack: provide new functions to encode the ":status" header
- MEDIUM: mux-h2: make use of standard HPACK encoding functions for the status
- MINOR: hpack: provide a function to encode an HTTP method
- MEDIUM: mux-h2: make use of hpack_encode_method() to encode the method
- MINOR: hpack: provide a function to encode an HTTP scheme
- MEDIUM: mux-h2: make use of hpack_encode_scheme() to encode the scheme
- MINOR: hpack: provide a function to encode an HTTP path
- MEDIUM: mux-h2: make use of hpack_encode_path() to encode the path
- REGTEST: add the HTTP rules test involving HTX processing
- REORG: connection: centralize the conn_set_{tos,mark,quickack} functions
- MEDIUM: cli: rework the CLI proxy parser
- MINOR: cli: parse prompt command in the CLI proxy
- MINOR: cli: implements 'quit' in the CLI proxy
- BUG/MINOR: cli: wait for payload data even without prompt
- MEDIUM: cli: handle payload in CLI proxy
- MINOR: cli: use pcli_flags for prompt activation
- MINOR: compression: Rename the function check_legacy_http_comp_flt()
- MINOR: cache/htx: Don't use the same cache on HTX and legacy HTTP proxies
- MINOR: cache: Register the cache as a data filter only if response is cacheable
- MEDIUM: cache/htx: Add the HTX support into the cache
- MINOR: cache: Improve and simplify the cache configuration check
- MINOR: filters: Export the name of known filters
- MEDIUM: cache/compression: Add a way to safely combined compression and cache
- MEDIUM: cache: Require an explicit filter declaration if other filters are used
- REORG: htx: merge types+proto into common/htx.h
- REORG: http: create http_msg.c to place there some legacy HTTP parts
- REORG: h1: move legacy http functions to http_msg.c
- REORG: h1: move the h1_state definition to proto_http
- CLEANUP: h1: remove some occurrences of unneeded h1.h inclusions
- REORG: h1: merge types+proto into common/h1.h
- CLEANUP: stream: remove SF_TUNNEL, SF_INITIALIZED, SF_CONN_TAR
- MEDIUM: mux-h1: implement true zero-copy of DATA blocks
- MINOR: config: round up global.tune.bufsize to the next multiple of 2 void*
- BUG/MINOR: mux-h2: refrain from muxing during the preface
- BUG/MINOR: mux-h2: advertise a larger connection window size
- DOC: master CLI documentation in management.txt
- MINOR: mux-h2: avoid copying large blocks into full buffers
- MEDIUM: mux-h2: implement true zero-copy send of large HTX DATA blocks
- MINOR: mux-h2: force reads to be HTX-aligned in HTX mode
- MINOR: cli: change 'show proc' output of old processes
- BUG/MEDIUM: mux-h1: Fix the zero-copy on output for chunked messages
- BUG: dns: Prevent stack-exhaustion via recursion loop in dns_read_name
- BUG: dns: Prevent out-of-bounds read in dns_read_name()
- BUG: dns: Prevent out-of-bounds read in dns_validate_dns_response()
- BUG: dns: Fix out-of-bounds read via signedness error in dns_validate_dns_response()
- BUG: dns: Fix off-by-one write in dns_validate_dns_response()
- REGTEST: the cache regtest requires haproxy 1.9
- MEDIUM: cli: store CLI level in the appctx
- MEDIUM: cli: show and change CLI permissions
- CLEANUP: cli: use dedicated define instead of appctx ones
- MEDIUM: cli: handle CLI level from the master CLI
- BUG/MEDIUM: cli: handle correctly prefix and payload
- BUILD: Makefile: Implements the help target
- REGTESTS: adjust the http-rules regtest to support window updates
- BUG/MEDIUM: connections: Remove CS_FL_EOS | CS_FL_REOS on retry.
- BUG/MEDIUM: stream_interface: Don't report read0 if we were not connected.
- BUG/MEDIUM: connection: Just make sure we closed the fd on connection failure.
- MEDIUM: mux: Add an optional "reset" method.
- BUG/MEDIUM: mux-h1: Fix loop if server closes its connection with unparsed data
- MINOR: mux-h1: Add helper functions to wake a stream from recv or send
- BUG/MEDIUM: mux-h1: Wake the stream for send once the connection is established
- BUG/MEDIUM: connections: Don't attempt to reuse an unusable connection.
- MEDIUM: htx: Try to take a connection over if it has no owner.
- REGTEST: Reg testing improvements.
- REGTEST: Add a first test for health-checks.
- REGTEST: Reg test for "check" health-check option.
- REGTEST: level 1 health-check test 2.
- REGTEST: Add miscellaneous reg tests for health-checks.
- REGTEST: add a few HTTP messaging tests
- MINOR: lb: make the leastconn algorithm more accurate
- REGTEST: fix missing space in checks/s00001
- REGTEST: http-messaging: add "option http-buffer-request" for H2 tests
- BUG/MEDIUM: cache: fix random crash on filter parser's error path
- MINOR: connection: realign empty buffers in muxes, not transport layers
- MINOR: mux_h1/h2: simplify the zero-copy Rx alignment
- MINOR: backend: count the number of connect and reuse per server and per backend
- BUG/MINOR: stats: fix inversion of failed header rewrites and other statuses
- MINOR: tools: increase the number of ITOA strings to 16
- MINOR: cache: report the number of cache lookups and cache hits
- MEDIUM: tasks: check the global task mask instead of the thread number
- MINOR: mworker: set all_threads_mask and pid_bit to 1
- BUG/MINOR: proto_htx: Fix htx_res_set_status to also set the reason
- BUG/MINOR: stats: Parse post data for HTX streams
- MINOR: payload/htx: Adapt smp_fetch_len to be HTX aware
- MINOR: http_fecth: Implement body_len and body_size sample fetches for the HTX
- MAJOR: lua: Forbid calls to Channel functions for LUA scripts in HTTP proxies
- MEDIUM: lua/htx: Adapt functions of the HTTP to be compatible with HTX
- MINOR: lua/htx: Adapt the functions get_in_length and is_full to be HTX aware
- MAJOR: lua/htx: Adapt HTTP applets to support HTX messages
- MINOR: lua: Remove useless check on the messages state in HTTP functions
- BUG/MEDIUM: htx: When performing zero-copy, start from the right offset.
- BUG/MINOR: mworker: don't use unitialized mworker_proc struct
- MINOR: mworker/cli: indicate in the master prompt when a reload failed
- MINOR: cli: implements 'reload' on master CLI
- BUG/MEDIUM: log: Don't call sample_fetch_as_type if we don't have a stream.
- BUG/MEDIUM: mux-h1: make sure we always have at least one HTX block to send
- BUG/MAJOR: backend: only update server's counters when the server exists
- MINOR: tools: preset the port of fd-based "sockets" to zero
- BUG/MINOR: log: fix logging to both FD and IP
- REGTEST: Add a reg test for HTTP cookies.
- BUILD: ssl: Fix compilation without deprecated OpenSSL 1.1 APIs
- BUILD: thread: properly report multi-thread support
- BUG/MINOR: logs: leave startup-logs global and not per-thread
- BUG/MEDIUM: threads: don't close the thread waker pipe if not init
- BUG/MAJOR: compression/cache: Make it really works with these both filters
- BUG/MEDIUM: h2: Don't forget to destroy the h2s after deferred shut.
- MEDIUM: proxy: Set http-reuse safe as default.
- MEDIUM: servers: Add a command to limit the number of idling connections.
- MEDIUM: servers: Replace idle-timeout with pool-purge-delay.
- MEDIUM: mux: Destroy the stream before trying to add the conn to the idle list.
- MEDIUM: mux: provide the session to the init() and attach() method.
- MEDIUM: sessions: Don't keep an infinite number of idling connections.
- MEDIUM: servers: Be more agressive when adding H2 connection to idle lists.
- MEDIUM: mux_h2: Always set CS_FL_NOT_FIRST for new conn_streams.
- BUG/MEDIUM: htx/cache: use the correct class of error codes on abort
- BUG/MINOR: cache: also consider CF_SHUTR to abort delivery
- MINOR: pools: Cast to volatile int * instead of int *.
- MINOR: debug: make the ABORT_NOW macro use a volatile int
- BUG/MEDIUM: h2: Don't destroy the h2s if it still has a cs attached.
- BUG/MEDIUM: mux-h1: don't try to process an empty input buffer
- DOC: clarify the agent-check status line syntax
- BUG/MAJOR: hpack: fix length check for short names encoding
- DOC: split the README into README + INSTALL
Willy Tarreau [Sun, 16 Dec 2018 21:27:15 +0000 (22:27 +0100)]
DOC: split the README into README + INSTALL
The README was barely usable after all the additions having accumulated
over the years. This patch introduces a new INSTALL file explaining how
to build and install haproxy with various levels of details. The README
is now mostly an index to the list of useful documentations.
Willy Tarreau [Sun, 16 Dec 2018 08:38:30 +0000 (09:38 +0100)]
BUG/MAJOR: hpack: fix length check for short names encoding
Commit 19ed92b ("MINOR: hpack: optimize header encoding for short names")
introduced an error in the space computation for short names, as it removed
the length encoding from the count without replacing with 1 (the minimum
byte). This results in the last byte of the area being occasionally
overwritten, which is immediately detected with -DDEBUG_MEMORY_POOLS as
the canary at the end gets overwritten.
Willy Tarreau [Sun, 16 Dec 2018 07:42:56 +0000 (08:42 +0100)]
DOC: clarify the agent-check status line syntax
Nick Ramirez reported that the wording is confusing and lets one think
that the CR or LF are both optional, which is not the case (either is
optional). Let's reformulate this.
Willy Tarreau [Sun, 16 Dec 2018 07:29:56 +0000 (08:29 +0100)]
BUG/MEDIUM: mux-h1: don't try to process an empty input buffer
h1_process_input() may occasionally be called with an empty input
buffer, and the code behind cannot deal with that, let's check the
condition at the beginning.