]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
6 years agoBUG/MEDIUM: mux-h1: make HTX chunking consistent with H2
Willy Tarreau [Thu, 3 Jan 2019 16:39:54 +0000 (17:39 +0100)] 
BUG/MEDIUM: mux-h1: make HTX chunking consistent with H2

When transfering from H1 to H1, chunking is always indicated by the
presence of the Transfer-encoding header field. But when a message
comes from H2 there is no such header and only HTX_SL_F_XFER_LEN
ought to be relied on. This one will also result in H1_MF_XFER_LEN
to be set, just like transfer-encoding, so let's always rely on
this latter flag to detect the need for chunking (when CLEN is not
here) and automatically add the transfer-encoding header if it was
not present, as reported by H1_MF_CHNK.

This must be backported to 1.9.

6 years agoBUG/MEDIUM: mux-h1: use per-direction flags to indicate transitions
Willy Tarreau [Thu, 3 Jan 2019 16:46:56 +0000 (17:46 +0100)] 
BUG/MEDIUM: mux-h1: use per-direction flags to indicate transitions

The H1 mux needs to store some information regarding the states that
were met (EOD, trailers, etc) for each direction but currently uses
only one set of flags. This results in failures when both the request
and the response use chunked-encoding because some elements are believed
to have been met already and a trailing 0 CRLF or just a CRLF may be
missing at the end.

The solution here consists in splitting these flags per direction, one
set for input processing and another set for output processing. Only
two flags were affected so this is not a big deal.

This needs to be backported to 1.9.

6 years agoBUG/MINOR: mux-h2: only update rxbuf's length for H1 headers
Willy Tarreau [Thu, 3 Jan 2019 09:26:23 +0000 (10:26 +0100)] 
BUG/MINOR: mux-h2: only update rxbuf's length for H1 headers

In h2c_decode_headers() we update the buffer's length according to the
amount of data produced (outlen). But in case of HTX this outlen value
is not a quantity, just an indicator of success, resulting in the buffer
being added one extra byte and temporarily showing .data > .size, which
is wrong. Fortunately this is overridden when leaving the function by
htx_to_buf() so the impact only exists in step-by-step debugging, but
it definitely needs to be fixed.

This must be backported to 1.9.

6 years agoBUG/MINOR: mux-h2: mark end-of-stream after processing response HEADERS, not before
Willy Tarreau [Thu, 3 Jan 2019 08:32:20 +0000 (09:32 +0100)] 
BUG/MINOR: mux-h2: mark end-of-stream after processing response HEADERS, not before

When dealing with a server's H2 response, we used to set the
end-of-stream flag on the conn_stream and the stream before parsing
the response, which is incorrect since we can fail to process this
response by lack of room, buffer or anything. The extend of this problem
is still limited to a few rare cases, but with trailers it will cause a
systematic failure.

This fix must be backported to 1.9.

6 years agoBUG/MINOR: mux-h2: don't check the CS count in h2c_bck_handle_headers()
Willy Tarreau [Thu, 3 Jan 2019 08:20:05 +0000 (09:20 +0100)] 
BUG/MINOR: mux-h2: don't check the CS count in h2c_bck_handle_headers()

This function handles response HEADERS frames, it is not responsible
for creating new streams thus it must not check if we've reached the
stream count limit, otherwise it could lead to some undesired pauses
which bring no benefit.

This must be backported to 1.9.

6 years agoBUG/MINOR: mux-h2: set the stream-full flag when leaving h2c_decode_headers()
Willy Tarreau [Thu, 3 Jan 2019 07:52:09 +0000 (08:52 +0100)] 
BUG/MINOR: mux-h2: set the stream-full flag when leaving h2c_decode_headers()

If we exit this function because some data are pending in the rxbuf, we
currently don't indicate any blocking flag, which will prevent the operation
from being attempted again. Let's set H2_CF_DEM_SFULL in this case to indicate
there's not enough room in the stream buffer so that the operation may be
attempted again once we make room. It seems that this issue cannot be
triggered right now but it definitely will with trailers.

This fix should be backported to 1.9 for completeness.

6 years agoBUG/MEDIUM: mux-h2: always restart reading if data are available
Willy Tarreau [Thu, 3 Jan 2019 07:27:41 +0000 (08:27 +0100)] 
BUG/MEDIUM: mux-h2: always restart reading if data are available

h2c_restart_reading() is used at various place to resume processing of
demux data, but this one refrains from doing so if the mux is already
subscribed for receiving. It just happens that even if some incoming
frame processing is interrupted, the mux is always subscribed for
receiving, so this condition alone is not enough, it must be combined
with the fact that the demux buffer is empty, otherwise some resume
events are lost. This typically happens when we refrain from processing
some incoming data due to missing room in the stream's rxbuf, and want
to resume in h2c_rcv_buf(). It will become even more visible with trailers
since these ones want to have an empty rxbuf before proceeding.

This must be backported to 1.9.

6 years agoCLEANUP: mux-h2: fix end-of-stream flag name when processing headers
Willy Tarreau [Thu, 3 Jan 2019 07:10:14 +0000 (08:10 +0100)] 
CLEANUP: mux-h2: fix end-of-stream flag name when processing headers

In h2c_decode_headers() we mistakenly check for H2_F_DATA_END_STREAM
while we should check for H2_F_HEADERS_END_STREAM. Both have the same
value (1) but better stick to the correct flag.

6 years agoMINOR: lb: allow redispatch when using consistent hash
Willy Tarreau [Wed, 2 Jan 2019 13:48:31 +0000 (14:48 +0100)] 
MINOR: lb: allow redispatch when using consistent hash

Redispatch traditionally only worked for cookie based persistence.

Adding redispatch support for consistent hash based persistence - also
update docs.

Reported by Oskar Stenman on discourse:
https://discourse.haproxy.org/t/balance-uri-consistent-hashing-redispatch-3-not-redispatching/3344

Should be backported to 1.8.

Cc: Lukas Tribus <lukas@ltri.eu>
6 years agoBUG/MAJOR: htx: Return the good block address after a defrag
Christopher Faulet [Wed, 2 Jan 2019 10:23:44 +0000 (11:23 +0100)] 
BUG/MAJOR: htx: Return the good block address after a defrag

When an HTX structure is defragmented, it is possible to retrieve the new block
corresponding to an old one. This is useful to do a defrag during a loop on
blocks, to be sure to continue looping on the good block. But, instead of
returning the address of the new block in the HTX structure, the one in the
temporary structure used to do the defrag was returned, leading to unexpected
behaviours.

This patch must be backported to 1.9.

6 years agoBUG/MEDIUM: cache: Be sure to end the forwarding when XFER length is unknown
Christopher Faulet [Wed, 2 Jan 2019 13:10:01 +0000 (14:10 +0100)] 
BUG/MEDIUM: cache: Be sure to end the forwarding when XFER length is unknown

This bug exists in the HTX code and in the legacy one. When the body length is
unknown, the applet hangs. For the legacy code, it hangs because the end of the
cached object is not correctly handled and the applet is never recalled. For the
HTX code, only the begining of the response (the 1st buffer) is sent then the
applet hangs. To work in HTX, The fast forwarding must be correctly handled.

This patch must be backported to 1.9.

[cf: the patch adding the function channel_add_input must be backported with
this one. It does not exist in 1.8 because only responses with a C-L are cached.]

6 years agoMINOR: stats/htx: Call channel_add_input instead of updating channel state by hand
Christopher Faulet [Wed, 2 Jan 2019 13:34:39 +0000 (14:34 +0100)] 
MINOR: stats/htx: Call channel_add_input instead of updating channel state by hand

This way we are sure the channel state is always correctly upadated, especially
the amount of data directly forwarded. For the stats applet, it is not a bug
because the fast forwarding is never used (the response is chunked and the HTX
extra field is always set to 0).

This patch must be backported to 1.9.

6 years agoMINOR: channel: Add the function channel_add_input
Christopher Faulet [Wed, 2 Jan 2019 13:24:35 +0000 (14:24 +0100)] 
MINOR: channel: Add the function channel_add_input

This function must be called when new incoming data are pushed in the channel's
buffer. It updates the channel state and take care of the fast forwarding by
consuming right amount of data and decrementing "->to_forward" accordingly when
necessary. In fact, this patch just moves a part of ci_putblk in a dedicated
function.

This patch must be backported to 1.9.

6 years agoBUG/MEDIUM: log: don't mark log FDs as non-blocking on terminals
Willy Tarreau [Wed, 2 Jan 2019 19:09:33 +0000 (20:09 +0100)] 
BUG/MEDIUM: log: don't mark log FDs as non-blocking on terminals

With the new ability to log to a terminal, it's convenient to be able
to use "log stdout" in a config file, except that it now results in
setting the terminal to non-blocking mode, breaking every utility
relying on stdin afterwards. Since the only reason for logging to a
terminal is to debug, do not set the FD to non-blocking mode when it's
a terminal.

This fix must be backported to 1.9.

6 years agoMINOR: payload: add sample fetch for TLS ALPN
Alex Zorin [Sun, 30 Dec 2018 02:56:28 +0000 (13:56 +1100)] 
MINOR: payload: add sample fetch for TLS ALPN

Application-Layer Protocol Negotiation (ALPN, RFC7301) is a TLS
extension which allows a client to present a preference for which
protocols it wishes to connect to, when a single port supports multiple
multiple application protocols.
It allows a transparent proxy to take a decision based on the beginning
of an SSL/TLS stream without deciphering it.

The new fetch "req.ssl_alpn" extracts the ALPN protocol names that may
be present in the ClientHello message.

6 years agoMEDIUM: sessions: Keep track of which connections are idle.
Olivier Houchard [Fri, 28 Dec 2018 17:50:57 +0000 (18:50 +0100)] 
MEDIUM: sessions: Keep track of which connections are idle.

Instead of keeping track of the number of connections we're responsible for,
keep track of the number of connections we're responsible for that we are
currently considering idling (ie that we are not using, they may be in use
by other sessions), that way we can actually reuse connections when we have
more connections than the max configured.

6 years agoMEDIUM: servers: Be smarter when switching connections.
Olivier Houchard [Fri, 28 Dec 2018 15:20:25 +0000 (16:20 +0100)] 
MEDIUM: servers: Be smarter when switching connections.

When connecting to a server, and reusing a connection, always attempt to give
the owner of the previous session one of its own connections, so that one
session won't be responsible for too many connections.

This should be backported to 1.9.

6 years agoBUG/MEDIUM: servers: Flag the stream_interface on handshake error.
Olivier Houchard [Thu, 27 Dec 2018 17:59:46 +0000 (18:59 +0100)] 
BUG/MEDIUM: servers: Flag the stream_interface on handshake error.

When creating a new outgoing connection, if we're using ALPN and waiting
for the handshake completion to choose the mux, and for some reason the
handshake failed, add the SI_FL_ERR flag to the stream_interface, so that
process_streams() knows the connection failed, and can attempt to retry,
instead of just hanging.

This should be backported to 1.9.

6 years agoBUG/MAJOR: sessions: Use an unlimited number of servers for the conn list.
Olivier Houchard [Thu, 27 Dec 2018 16:20:54 +0000 (17:20 +0100)] 
BUG/MAJOR: sessions: Use an unlimited number of servers for the conn list.

When a session adds a connection to its connection list, we used to remove
connections for an another server if there were not enough room for our
server. This can't work, because those lists are now the list of connections
we're responsible for, not just the idle connections.
To fix this, allow for an unlimited number of servers, instead of using
an array, we're now using a linked list.

6 years agoBUG/MAJOR: servers: Correctly use LIST_ELEM().
Olivier Houchard [Thu, 27 Dec 2018 14:35:22 +0000 (15:35 +0100)] 
BUG/MAJOR: servers: Correctly use LIST_ELEM().

To access the first element of the list, correctly use LIST_ELEM(), or we
end up getting the head of the list, instead of getting the first connection.

This should be backported to 1.9.

6 years agoBUG/MAJOR: servers: Use the list api correctly to avoid crashes.
Olivier Houchard [Thu, 27 Dec 2018 14:29:53 +0000 (15:29 +0100)] 
BUG/MAJOR: servers: Use the list api correctly to avoid crashes.

In connect_server(), if we looked for an usable connection and failed to
find one, srv_conn won't be NULL at the end of list_for_each_entry(), but
will point to the head of a list, which is not a pointer to a struct
connection, so explicitely set it to NULL.

This should be backported to 1.9.

6 years agoBUG/MEDIUM: servers: Fail if we fail to allocate a conn_stream.
Olivier Houchard [Fri, 28 Dec 2018 13:45:47 +0000 (14:45 +0100)] 
BUG/MEDIUM: servers: Fail if we fail to allocate a conn_stream.

If, for some reason we failed to allocate a conn_stream when reusing an
existing connection, set srv_conn to NULL, so that we fail later, instead
of pretending all is right. This ends up giving a stream_interface with
no endpoint, and so the stream will never end.

This should be backported to 1.9.

6 years agoBUG/MEDIUM: mux_h2: Don't add to the idle list if we're full.
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.

This should be backported to 1.9.

6 years agoBUG/MINOR: htx: send the proper authenticate header when using http-request auth
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.

To be backported to 1.9.

6 years agoBUG/MEDIUM: tasks: Decrement tasks_run_queue in tasklet_free().
Olivier Houchard [Mon, 24 Dec 2018 13:03:10 +0000 (14:03 +0100)] 
BUG/MEDIUM: tasks: Decrement tasks_run_queue in tasklet_free().

If the tasklet is in the list, don't forget to decrement tasks_run_queue
in tasklet_free().

This should be backported to 1.9.

6 years agoBUG/MEDIUM: servers: Don't try to reuse connection if we switched server.
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.

This should be backported to 1.9.

6 years agoCLEANUP: mux-h2: remove misleading comments about CONTINUATION
Willy Tarreau [Mon, 24 Dec 2018 10:40:26 +0000 (11:40 +0100)] 
CLEANUP: mux-h2: remove misleading comments about CONTINUATION

These ones were left-over from copy-pastes that are unrelated to
CONTINUATION frames.

6 years agoMEDIUM: mux-h2: handle decoding of CONTINUATION frames
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.

6 years agoMINOR: mux-h2: make h2_peek_frame_hdr() support an offset
Willy Tarreau [Sat, 22 Dec 2018 17:11:41 +0000 (18:11 +0100)] 
MINOR: mux-h2: make h2_peek_frame_hdr() support an offset

This function will be used to parse multiple subsequent frames so it
needs to support an offset.

6 years agoMINOR: buffers: add a new b_move() function
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.

6 years agoMINOR: mux-h2: fail stream creation more cleanly using RST_STREAM
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

6 years agoMINOR: 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.

6 years agoMINOR: mux-h2: make h2c_send_rst_stream() use the dummy stream's error code
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.

6 years agoMEDIUM: mux-h2: decode HEADERS frames before allocating the stream
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.

6 years agoMINOR: mux-h2: remove useless check for empty frame length in h2s_decode_headers()
Willy Tarreau [Mon, 24 Dec 2018 09:21:36 +0000 (10:21 +0100)] 
MINOR: mux-h2: remove useless check for empty frame length in h2s_decode_headers()

This test for an empty frame was already performed in the callers, there
is no need for checking it again.

6 years agoMINOR: h2: add a bit-based frame type representation
Willy Tarreau [Fri, 21 Dec 2018 13:56:57 +0000 (14:56 +0100)] 
MINOR: h2: add a bit-based frame type representation

This will ease checks among sets of frames.

6 years agoMEDIUM: mux-h2: remove padlen during headers phase
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.

6 years agoBUG/MEDIUM: mux-h2: mark that we have too many CS once we have more than the max
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

6 years agoBUG/MINOR: mux-h2: make empty HEADERS frame return a connection error
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.

6 years agoREGTESTS: remove the expected window updates from H2 handshakes
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.

6 years agoMINOR: mux-h2: only increase the connection window with the first update
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.

6 years ago[RELEASE] Released version 2.0-dev0 v2.0-dev0
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.

6 years agoMEDIUM: 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.

6 years agoMINOR: ssl: Add ssl_sock_set_alpn().
Olivier Houchard [Fri, 21 Dec 2018 18:45:40 +0000 (19:45 +0100)] 
MINOR: ssl: Add ssl_sock_set_alpn().

Add a new function, ssl_sock_set_alpn(), to be able to change the ALPN
for a connection, instead of relying of the one defined in the SSL_CTX.

6 years agoBUG/MEDIUM: server: Also copy "check-sni" for server templates.
Olivier Houchard [Fri, 21 Dec 2018 18:42:01 +0000 (19:42 +0100)] 
BUG/MEDIUM: server: Also copy "check-sni" for server templates.

When using server templates, if "check-sni" is used, make sure it shows up
in all the created servers.

This should be backported to 1.8 and 1.9.

6 years agoREGTEST: A basic test for "http-buffer-request"
Frédéric Lécaille [Fri, 21 Dec 2018 10:56:26 +0000 (11:56 +0100)] 
REGTEST: A basic test for "http-buffer-request"

6 years agoBUG/MEDIUM: mux-h2: don't needlessly wake up the demux on short frames
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.

Must be backported to 1.9.

6 years agoREGTEST: Add a basic test for the compression
Christopher Faulet [Fri, 21 Dec 2018 14:24:57 +0000 (15:24 +0100)] 
REGTEST: Add a basic test for the compression

6 years agoREGTEST: script: Add support of alternatives in requited options list
Christopher Faulet [Fri, 21 Dec 2018 14:18:02 +0000 (15:18 +0100)] 
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.

6 years agoMEDIUM: mux_h1: Implement h1_show_fd.
Olivier Houchard [Fri, 21 Dec 2018 14:20:29 +0000 (15:20 +0100)] 
MEDIUM: mux_h1: Implement h1_show_fd.

In the mux_h1, implement h1_show_fd, to give a bit more information about
each H1 stream, when using "show fd".

6 years agoBUG/MINOR: compression/htx: Don't add the last block of data if it is empty
Christopher Faulet [Fri, 21 Dec 2018 14:10:25 +0000 (15:10 +0100)] 
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.

6 years agoBUG/MINOR: compression/htx: Don't compress responses with unknown body length
Christopher Faulet [Fri, 21 Dec 2018 13:53:54 +0000 (14:53 +0100)] 
BUG/MINOR: compression/htx: Don't compress responses with unknown body length

In HTX, if the body length of a response cannot be determined, we must not try
to compress it.

6 years agoBUG/MEDIUM: dns: overflowed dns name start position causing invalid dns error
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.

This should be backported as far as 1.7.

6 years agoBUG/MEDIUM: dns: Don't prevent reading the last byte of the payload in dns_validate_r...
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.

this must be backported in 1.8.

6 years agoBUG/MEDIUM: mux-h2: Don't forget to quit the send list on error reports
Willy Tarreau [Thu, 20 Dec 2018 14:35:57 +0000 (15:35 +0100)] 
BUG/MEDIUM: mux-h2: Don't forget to quit the send list on error reports

Similar to last fix, we need to quit the send list when reporting an
error via the send side.

This should be backported to 1.9.

6 years agoBUG/MEDIUM: h2: Don't forget to quit the sending_list if SUB_CALL_UNSUBSCRIBE.
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.

This should be backported to 1.9.

6 years agoREGTEST: Exclude freebsd target for some reg tests.
Frédéric Lécaille [Thu, 20 Dec 2018 08:55:45 +0000 (09:55 +0100)] 
REGTEST: Exclude freebsd target for some reg tests.

This patch excludes freebsd target for health-checks reg tests
which rely on TCP_DEFER_ACCEPT usage. This is specific to Linux.

6 years agoREGTEST: Missing enclosing quotes for ${tmpdir} macro.
Frédéric Lécaille [Thu, 20 Dec 2018 08:55:44 +0000 (09:55 +0100)] 
REGTEST: Missing enclosing quotes for ${tmpdir} macro.

6 years agoREGTEST: Be less Linux specific with a syslog regex.
Frédéric Lécaille [Thu, 20 Dec 2018 08:55:43 +0000 (09:55 +0100)] 
REGTEST: Be less Linux specific with a syslog regex.

This patch makes at least this test pass on FreeBSD systems.
The regex to be matched with syslog message was too much Linux specific.

6 years agoREGTEST: Fix a typo about barrier type.
Frédéric Lécaille [Thu, 20 Dec 2018 08:55:42 +0000 (09:55 +0100)] 
REGTEST: Fix a typo about barrier type.

Fix a typo, furthermore there is no reason to use a barrier with "sock" as type
which are more varnish cache specific: used to synchronize the VCL.

6 years agoREGTEST: Make reg-tests target support argument.
Frédéric Lécaille [Wed, 19 Dec 2018 15:27:47 +0000 (16:27 +0100)] 
REGTEST: Make reg-tests target support argument.

With this patch we can provide a list of argument to reg-tests target.
Useful to run reg tests for a list of VTC files like that:

    $ VARNISHTEST_PROGRAM=<...> make reg-tests reg-tests/checks/*.vtc

6 years agoREGTEST: Add option to use HTX prefixed by the macro 'no-htx'
Christopher Faulet [Wed, 19 Dec 2018 10:49:39 +0000 (11:49 +0100)] 
REGTEST: Add option to use HTX prefixed by the macro 'no-htx'

So some tests have been removed.

6 years agoREGTEST: script: Print only errors in the results report
Christopher Faulet [Wed, 19 Dec 2018 10:28:49 +0000 (11:28 +0100)] 
REGTEST: script: Print only errors in the results report

6 years agoREGTEST: script: Add the option --use-htx to enable the HTX in regtests
Christopher Faulet [Wed, 19 Dec 2018 10:22:04 +0000 (11:22 +0100)] 
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.

6 years agoREGTEST: script: Add the option --keep-logs to keep all log directories
Christopher Faulet [Wed, 19 Dec 2018 09:25:07 +0000 (10:25 +0100)] 
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).

6 years agoREGTEST: script: Add the option --debug to show logs on standard ouput
Christopher Faulet [Wed, 19 Dec 2018 09:22:01 +0000 (10:22 +0100)] 
REGTEST: script: Add the option --debug to show logs on standard ouput

It implies the option --v. It is handy when you run the same test during a debug
session.

6 years agoREGTEST: script: Add the option --clean to remove previous log direcotries
Christopher Faulet [Wed, 19 Dec 2018 09:18:40 +0000 (10:18 +0100)] 
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.

6 years agoREGTEST: script: Evaluate the varnishtest command to allow quoted parameters
Christopher Faulet [Tue, 18 Dec 2018 21:47:23 +0000 (22:47 +0100)] 
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:

  > ./scripts/run-regtests.sh --varnishtestparams "-Dmacro='some value' -n 10"

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.

6 years agoREGTEST: script: Process script arguments before everything else
Christopher Faulet [Tue, 18 Dec 2018 21:41:20 +0000 (22:41 +0100)] 
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.

6 years agoREGTEST: Require the option LUA to run lua tests
Christopher Faulet [Wed, 19 Dec 2018 10:50:17 +0000 (11:50 +0100)] 
REGTEST: Require the option LUA to run lua tests

6 years agoBUG/MAJOR: connections: Close the connection before freeing it.
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.

This should be backported to 1.9.

6 years ago[RELEASE] Released version 1.9.0 v1.9.0
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

6 years agoDOC: mention in the readme that 1.9 is a stable version now
Willy Tarreau [Wed, 19 Dec 2018 18:11:18 +0000 (19:11 +0100)] 
DOC: mention in the readme that 1.9 is a stable version now

It's going to happen soon :-)

6 years agoCLEANUP: remove my name and address from the copyright banner
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.

6 years agoSCRIPTS: add the slack channel URL to the announce script
Willy Tarreau [Wed, 19 Dec 2018 17:59:51 +0000 (18:59 +0100)] 
SCRIPTS: add the slack channel URL to the announce script

It's just to provide the URL in the usual URLs when releasing.

6 years agoMINOR: cli/show_fd: report that a connection is back or not
Willy Tarreau [Wed, 19 Dec 2018 17:40:58 +0000 (18:40 +0100)] 
MINOR: cli/show_fd: report that a connection is back or not

It simply adds a "back={0,1}" depending on the connection's side, it's
useful to debug.

6 years agoBUG/MEDIUM: h2: Don't wait for flow control if the connection had a shutr.
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.

No backport is needed.

6 years agoBUG/MINOR: mux-h2: make sure we check the conn_stream in early data
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.

No backport is needed.

6 years agoCLEANUP: mux-h2: make use of cs_set_error()
Willy Tarreau [Wed, 19 Dec 2018 17:00:29 +0000 (18:00 +0100)] 
CLEANUP: mux-h2: make use of cs_set_error()

It's cleaner than open-coding the conditions and error bits.

6 years agoMINOR: connection: add cs_set_error() to set the error bits
Willy Tarreau [Wed, 19 Dec 2018 16:59:30 +0000 (17:59 +0100)] 
MINOR: connection: add cs_set_error() to set the error bits

Depending on the CS_FL_EOS status, we either set CS_FL_ERR_PENDING
or CS_FL_ERROR at various places. Let's have a generic function to
do this.

6 years agoBUG/MEDIUM: mux-h2: make use of h2s_alert() to report aborts
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.

No backport is needed.

6 years agoMINOR: mux-h2: add a new function h2s_alert() to call the data layer
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().

6 years agoCLEANUP: mux-h2: implement h2s_notify_{send,recv} to report events to subscribers
Willy Tarreau [Wed, 19 Dec 2018 17:08:52 +0000 (18:08 +0100)] 
CLEANUP: mux-h2: implement h2s_notify_{send,recv} to report events to subscribers

Till now we had to open-code all the manipulation of the wait_event,
let's use standarized functions for this and reduce the risk of bugs.

6 years agoBUG/MEDIUM: h2: Make sure we don't set CS_FL_ERROR if there's still data.
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.

6 years agoMEDIUM: stream-int: always consider all CS errors on the send side
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.

6 years agoBUG/MEDIUM: mux-h2: pass CS_FL_ERR_PENDING to h2_wake_some_streams()
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.

No backport is needed.

6 years agoBUG/MEDIUM: H2: Make sure htx is set even on empty frames.
Olivier Houchard [Wed, 19 Dec 2018 14:53:53 +0000 (15:53 +0100)] 
BUG/MEDIUM: H2: Make sure htx is set even on empty frames.

When transfering data, make sure htx is set even on empty frames, or we
will never add a HTX_BLK_EOM block.

6 years agoBUG/MEDIUM: h1: Don't shutw/shutr the connection if we have keepalive.
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.

6 years agoCLEANUP: stream-int: consistently call the si/stream_int functions
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.

6 years agoMINOR: connection: remove an unwelcome dependency on struct stream
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.

6 years agoCLEANUP: connection: rename conn->mux_ctx to conn->ctx
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.

6 years agoCLEANUP: connection: rename subscription events values and event field
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.

6 years agoMINOR: compression: Remove the thread_local variable buf_output
Christopher Faulet [Mon, 17 Dec 2018 12:17:53 +0000 (13:17 +0100)] 
MINOR: compression: Remove the thread_local variable buf_output

By doing a c_rew() at the right place, we can avoid to use this variable. This
slightly simplifly the compression for the legacy HTTP.

6 years agoBUG/MINOR: lua: Return an error if a legacy HTTP applet doesn't send anything
Christopher Faulet [Tue, 18 Dec 2018 20:20:57 +0000 (21:20 +0100)] 
BUG/MINOR: lua: Return an error if a legacy HTTP applet doesn't send anything

In legacy mode, if an HTTP applet does not send any response, an error 500 is
returned.

6 years agoBUG/MEDIUM: lua/htx: Handle EOM in receive/get_line calls in HTTP applets
Christopher Faulet [Tue, 18 Dec 2018 15:43:35 +0000 (16:43 +0100)] 
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.

6 years agoBUG/MINOR: proto_htx: Call the HTX version of the function managing client cookies
Christopher Faulet [Tue, 18 Dec 2018 15:41:31 +0000 (16:41 +0100)] 
BUG/MINOR: proto_htx: Call the HTX version of the function managing client cookies

Because of a typo, the legacy version was called instead of the HTX one.

6 years agoBUG/MEDIUM: stream: Forward the right amount of data before infinite forwarding
Christopher Faulet [Tue, 18 Dec 2018 20:57:24 +0000 (21:57 +0100)] 
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).

6 years agoMEDIUM: h2: properly check and deduplicate the content-length header in 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.

6 years agoBUG/MEDIUM: stream-int: also wake the stream up on end of transfer
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>
6 years agoBUG/MINOR: mux-h1: report the correct frontend in error captures
Willy Tarreau [Tue, 18 Dec 2018 17:10:38 +0000 (18:10 +0100)] 
BUG/MINOR: mux-h1: report the correct frontend in error captures

The error captures provided in HTX by the H1 mux would always report the
backend as the "other end". We need to assign the backend only on requests.

No backport is needed.