89 Commits

Author SHA1 Message Date
Olivier Houchard
6db1699f77 BUG/MEDIUM: streams: Try to L7 retry before aborting the connection.
In htx_wait_for_response, in case of error, attempt a L7 retry before
aborting the connection if the TX_NOT_FIRST flag is set.
If we don't do that, then we wouldn't attempt L7 retries after the first
request, or if we use HTTP/2, as with HTTP/2 that flag is always set.
2019-05-17 15:49:21 +02:00
Olivier Houchard
ad26d8d820 BUG/MEDIUM: streams: Make sur SI_FL_L7_RETRY is set before attempting a retry.
In a few cases, we'd just check if the backend is configured to do retries,
and not if it's still allowed on the stream_interface.
The SI_FL_L7_RETRY flag could have been removed because we failed to allocate
a buffer, or because the request was too big to fit in a single buffer,
so make sure it's there before attempting a retry.
2019-05-10 17:48:59 +02:00
Dragan Dosen
2674303912 MEDIUM: regex: modify regex_comp() to atomically allocate/free the my_regex struct
Now we atomically allocate the my_regex struct within function
regex_comp() and compile the regex or free both in case of failure. The
pointer to the allocated my_regex struct is returned directly. The
my_regex* argument to regex_comp() is removed.

Function regex_free() was modified so that it systematically frees the
my_regex entry. The function does nothing when called with a NULL as
argument (like free()). It will avoid existing risk of not properly
freeing the initialized area.

Other structures are also updated in order to be compatible (the ones
related to Lua and action rules).
2019-05-07 06:58:15 +02:00
Willy Tarreau
223995e8ca BUG/MINOR: stream: also increment the retry stats counter on L7 retries
It happens that the retries stats use their own counter and are not
derived from the stream interface, so we need to update it as well
when performing an L7 retry.

No backport is needed.
2019-05-04 10:40:00 +02:00
Olivier Houchard
e3249a98e2 MEDIUM: streams: Add a new keyword for retry-on, "junk-response"
Add a way to retry requests if we got a junk response from the server, ie
an incomplete response, or something that is not valid HTTP.
To do so, one can use the new "junk-response" keyword for retry-on.
2019-05-04 10:20:24 +02:00
Olivier Houchard
865d8392bb MEDIUM: streams: Add a way to replay failed 0rtt requests.
Add a new keyword for retry-on, 0rtt-rejected. If set, we will try to
replay requests for which we sent early data that got rejected by the
server.
If that option is set, we will attempt to use 0rtt if "allow-0rtt" is set
on the server line even if the client didn't send early data.
2019-05-04 10:20:24 +02:00
Olivier Houchard
a254a37ad7 MEDIUM: streams: Add the ability to retry a request on L7 failure.
When running in HTX mode, if we sent the request, but failed to get the
answer, either because the server just closed its socket, we hit a server
timeout, or we get a 404, 408, 425, 500, 501, 502, 503 or 504 error,
attempt to retry the request, exactly as if we just failed to connect to
the server.

To do so, add a new backend keyword, "retry-on".

It accepts a list of keywords, which can be "none" (never retry),
"conn-failure" (we failed to connect, or to do the SSL handshake),
"empty-response" (the server closed the connection without answering),
"response-timeout" (we timed out while waiting for the server response),
or "404", "408", "425", "500", "501", "502", "503" and "504".

The default is "conn-failure".
2019-05-04 10:19:56 +02:00
Christopher Faulet
1907ccc2f7 BUG/MINOR: http: Call stream_inc_be_http_req_ctr() only one time per request
The function stream_inc_be_http_req_ctr() is called at the beginning of the
analysers AN_REQ_HTTP_PROCESS_FE/BE. It as an effect only on the backend. But we
must be careful to call it only once. If the processing of HTTP rules is
interrupted in the middle, when the analyser is resumed, we must not call it
again. Otherwise, the tracked counters of the backend are incremented several
times.

This bug was reported in github. See issue #74.

This fix should be backported as far as 1.6.
2019-04-29 16:01:47 +02:00
Christopher Faulet
e84289e585 BUG/MEDIUM: thread/http: Add missing locks in set-map and add-acl HTTP rules
Locks are missing in the rules "http-request set-map" and "http-response
add-acl" when an acl or map update is performed. Pattern elements must be
locked.

This patch must be backported to 1.9 and 1.8. For the 1.8, the HTX part must be
ignored.
2019-04-19 15:53:23 +02:00
Willy Tarreau
6f7a02a381 BUILD: htx: fix a used uninitialized warning on is_cookie2
gcc-3.4 reports this which actually looks like a valid warning when
looking at the code, it's unsure why others didn't notice it :

src/proto_htx.c: In function `htx_manage_server_side_cookies':
src/proto_htx.c:4266: warning: 'is_cookie2' might be used uninitialized in this function
2019-04-15 21:55:48 +02:00
Christopher Faulet
0ef372a390 MAJOR: muxes/htx: Handle inplicit upgrades from h1 to h2
The upgrade is performed when an H2 preface is detected when the first request
on a connection is parsed. The CS is destroyed by setting EOS flag on it. A
special flag is added on the HTX message to warn the HTX analyzers the stream
will be closed because of an upgrade. This way, no error and no log are
emitted. When the mux h1 is released, we create a mux h2, without any CS and
passing the buffer with the unparsed H2 preface.
2019-04-12 22:06:53 +02:00
Christopher Faulet
c62c2b9d92 BUG/MEDIUM: htx: Fix the process of HTTP CONNECT with h2 connections
In HTX, the HTTP tunneling does not work if h1 and h2 are mixed (an h1 client
sending requests to an h2 server or this opposite) because the h1 multiplexer
always adds an EOM before switching it to tunnel mode. The h2 multiplexer
interprets it as an end of stream, closing the stream as for any other
transaction.

To make it works again, we need to swith to the tunnel mode without emitting any
EOM blocks. Because of that, HTX analyzers have been updated to switch the
transaction to tunnel mode before end of the message (because there is no end of
message...).

To be consistent, the protocol switching is also handled the same way even
though the 101 responses are not supported in h2.

This patch must be backported to 1.9.
2019-04-12 22:06:53 +02:00
Christopher Faulet
03b9d8ba4a MINOR: proto_htx: Don't adjust transaction mode anymore in HTX analyzers
Because the option http-tunnel is now ignored in HTX, there is no longer any
need to adjust the transaction mode in HTX analyzers. A channel can still be
switch to the tunnel mode for legitimate cases (HTTP CONNECT or switching
protocols). So the function htx_adjust_conn_mode() is now useless.

This patch must be backported to 1.9. It is not strictly speaking required but
it will ease futur backports.
2019-04-12 22:06:53 +02:00
Christopher Faulet
6c9bbb2265 MEDIUM: htx: Deprecate the option 'http-tunnel' and ignore it in HTX
The option http-tunnel disables any HTTP processing past the first
transaction. In HTX, it works for full h1 transactions. As for the legacy HTTP,
it is a workaround, but it works. But it is impossible to make it works with an
h2 connection. In such case, it has no effect, the stream is closed at the end
of the transaction. So to avoid any inconsistancies between h1 and h2
connections, this option is now always ignored when the HTX is enabled. It is
also a good opportinity to deprecate an old and ugly option. A warning is
emitted during HAProxy startup to encourage users to remove this option.

Note that in legacy HTTP, this option only works with full h1 transactions
too. If an h2 connection is established on a frontend with this option enabled,
it will have no effect at all. But we keep it for the legacy HTTP for
compatibility purpose. It will be removed with the legacy HTTP.

So to be short, if you have to really (REALLY) use it, it will only work for
legacy HTTP frontends with H1 clients.

The documentation has been updated accordingly.

This patch must be backported to 1.9. It is not strictly speaking required but
it will ease futur backports.
2019-04-12 22:06:53 +02:00
Christopher Faulet
aed68d4390 BUG/MINOR: proto_htx: Reset to_forward value when a message is set to DONE
Because we try to forward infinitly message body, when its state is set to DONE,
we must be sure to reset to_foward value of the corresponding
channel. Otherwise, some errors can be errornously triggered.

No need to backport this patch.
2019-04-01 15:43:40 +02:00
Christopher Faulet
66af0b2b99 MEDIUM: proto_htx: Reintroduce the infinite forwarding on data
This commit was reverted because of bugs. Now it should be ok. Difference with
the commit f52170d2f ("MEDIUM: proto_htx: Switch to infinite forwarding if there
is no data filte") is that when the infinite forwarding is enabled, the message
is switched to the state HTTP_MSG_DONE if the flag CF_EOI is set.
2019-03-25 06:55:23 +01:00
Christopher Faulet
769d0e98b8 BUG/MEDIUM: http/htx: Fix handling of the option abortonclose
Because the flag CF_SHUTR is no more set to mark the end of the message by the
H2 multiplexer, we can rely on it again to detect aborts. there is no more need
to make a check on the flag SI_FL_CLEAN_ABRT when the option abortonclose is
enabled. So, this option should work as before for h2 clients.

This patch must be backported to 1.9 with the previous EOI patches.
2019-03-25 06:55:13 +01:00
Willy Tarreau
6e8d6a9163 Revert "MEDIUM: proto_htx: Switch to infinite forwarding if there is no data filter"
This reverts commit f52170d2f47efbace729bc88349eb189968df568.

This commit was merged too early, some areas are not ready and
transfers from H1 to H2 often stall. Christopher suggested to wait
for the other parts to be ready before reintroducing it.
2019-03-21 18:28:31 +01:00
Willy Tarreau
0f22299435 CLEANUP: cache: don't export http_cache_applet anymore
This one can become static since it's not used by http/htx anymore.
2019-03-19 09:58:35 +01:00
Christopher Faulet
2571bc6410 MINOR: http/applets: Handle all applets intercepting HTTP requests the same way
In addition to stats and cache applets, there are also HTTP applet services
declared in an http-request rule. All these applets are now handled the same
way. Among other things, the header Expect is handled at the same place for all
these applets.
2019-03-19 09:54:20 +01:00
Christopher Faulet
bcf242a1d5 MINOR: stats/cache: Handle the header Expect when applets are registered
First of all, it is a way to handle 100-Continue for the cache without
duplicating code. Then, for the stats, it is no longer necessary to wait for the
request body.
2019-03-19 09:53:14 +01:00
Christopher Faulet
4a28a536a3 MINOR: proto_htx: Add function to handle the header "Expect: 100-continue"
The function htx_handle_expect_hdr() is now responsible to search the header
"Expect" and send the corresponding response if necessary.
2019-03-19 09:51:38 +01:00
Christopher Faulet
f52170d2f4 MEDIUM: proto_htx: Switch to infinite forwarding if there is no data filter
Because in HTX the parsing is done by the multiplexers, there is no reason to
limit the amount of data fast-forwarded. Of course, it is only true when there
is no data filter registered on the corresponding channel. So now, we enable the
infinite forwarding when possible. However, the HTTP message state remains
HTTP_MSG_DATA. Then, when infinite forwarding is enabled, if the flag CF_SHUTR
is set, the state is switched to HTTP_MSG_DONE.
2019-03-19 09:48:05 +01:00
Christopher Faulet
93e02d8b73 MINOR: proto-http/proto-htx: Make error handling clearer during data forwarding
It is just a cleanup. Error handling is grouped at the end HTTP data analysers.

This patch must be backported to 1.9 because it is used by another patch to fix
a bug.
2019-03-18 15:50:23 +01:00
Willy Tarreau
d1fd6f5f64 BUG/MINOR: http/counters: fix missing increment of fe->srv_aborts
When a server aborts a transfer, we used to increment the backend's
counter but not the frontend's during the forwarding phase. This fixes
it. It might be backported to all supported versions (possibly removing
the htx part) though it is of very low importance.
2019-03-18 15:50:23 +01:00
Christopher Faulet
5d45e381b4 BUG/MINOR: stats: Be more strict on what is a valid request to the stats applet
First of all, only GET, HEAD and POST methods are now allowed. Others will be
rejected with the status code STAT_STATUS_IVAL (invalid request). Then, for the
legacy HTTP, only POST requests with a content-length are allowed. Now, chunked
encoded requests are also considered as invalid because the chunk formatting
will interfere with the parsing of POST parameters. In HTX, It is not a problem
because data are unchunked.

This patch must be backported to 1.9. For prior versions too, but HTX part must
be removed. The patch introducing the status code STAT_STATUS_IVAL must also be
backported.
2019-03-15 14:35:11 +01:00
Olivier Houchard
a798bf56e2 MEDIUM: http: Use the new _HA_ATOMIC_* macros.
Use the new _HA_ATOMIC_* macros and add barriers where needed.
2019-03-11 17:02:38 +01:00
Willy Tarreau
4236f035fe MINOR: htx: unconditionally handle parsing errors in requests or responses
The htx request and response processing functions currently only check
for HTX_FL_PARSING_ERROR on incomplete messages because that's how mux_h1
delivers these. However with H2 we have to detect some parsing errors in
the format of certain pseudo-headers (e.g. :path), so we do have a complete
message but we want to report an error.

Let's move the parse error check earlier so that it always triggers when
the flag is present. It was also moved for htx_wait_for_request_body()
since we definitely want to be able to abort processing such an invalid
request even if it appears complete, but it was not changed in the forward
functions so as not to truncate contents before the position of the first
error.
2019-03-05 10:56:34 +01:00
Christopher Faulet
02e771a9e0 BUG/MEDIUM: proto_htx: Fix functions applying regex filters on HTX messages
The HTX functions htx_apply_filter_to_req_headers() and
htx_apply_filter_to_resp_headers() contain 2 bugs. The first one is about the
matching on each header. The chunk 'hdr' used to format a full header line was
never reset. The second bug appears when we try to replace or remove a
header. The variable ctx was not fully initialized, leading to sefaults.

This patch must be backported to 1.9.
2019-02-26 15:45:02 +01:00
Christopher Faulet
834eee7928 BUG/MINOR: proto-htx: Consider a XFER_LEN message as chunked by default
An HTX message with a known body length is now considered by default as
chunked. It means the header "content-length" must be found to consider it as a
non-chunked message. Before, it was the reverse, the message was considered with
a content length by default. But it is a bug for HTTP/2 messages. There is no
chunked transfer encoding in HTTP/2 but internally messages without content
length are considered as chunked. It eases HTTP/1 <-> HTTP/2 conversions.

This patch must be backported to 1.9.
2019-02-18 16:25:06 +01:00
Christopher Faulet
6cdaf2ad9a BUG/MEDIUM: proto_htx: Fix data size update if end of the cookie is removed
When client-side or server-side cookies are parsed, if the end of the cookie
line is removed, the HTX message must be updated. The length of the HTX block is
decreased and the data size of the HTX message is modified accordingly. The
update of the HTX block was ok but the update of the HTX message was wrong,
leading to undefined behaviours during the data forwarding. One of possible
effect was a freeze of the connection and no data forward.

This patch must be backported in 1.9.
2019-02-13 09:56:54 +01:00
Christopher Faulet
dcd8c5eed4 BUG/MINOR: proto-htx: Return an error if all headers cannot be received at once
When an HTX stream is waiting for a request or a response, it reports an error
(400 for the request or 502 for the response) if a parsing error is reported by
the mux (HTX_FL_PARSING_ERROR). The mux-h1 uses this error, among other things,
when the headers are too big to be analyzed at once. But the mux-h2 doesn't. So
the stream must also report an error if the multiplexer is unable to emit all
headers at once. The multiplexers must always emit all the headers at once
otherwise it is an error.

There are 2 ways to detect this error:

  * The end-of-headers marker was not received yet _AND_ the HTX message is not
    empty.

  * The end-of-headers marker was not received yet _AND_ the multiplexer have
    some data to emit but it is waiting for more space in the channel's buffer.

Note the mux-h2 is buggy for now when HTX is enabled. It does not respect the
reserve. So there is no way to hit this bug.

This patch must be backported to 1.9.
2019-01-23 11:27:34 +01:00
Christopher Faulet
ed7a066b45 BUG/MEDIUM: stats: Get the right scope pointer depending on HTX is used or not
For HTX streams, the scope pointer is relative to the URI in the start-line. But
for streams using the legacy HTTP representation, the scope pointer is relative
to the beginning of output data in the channel's buffer. So we must be careful
to use the right one depending on the HTX is used or not.

Because the start-line is used to get de scope pointer, it is important to keep
it after the parsing of post paramters. So now, instead of removing blocks when
read in the function stats_process_http_post(), we just move on next, leaving it
in the HTX message.

Thanks to Pieter (PiBa-NL) to report this bug.

This patch must be backported to 1.9.
2019-01-16 17:27:49 +01:00
Willy Tarreau
089eaa0ba7 BUG/MINOR: backend: don't use url_param_name as a hint for BE_LB_ALGO_PH
At a few places in the code we used to rely on this variable to guess
what LB algo was in place. This is wrong because if the defaults section
presets "balance url_param foo" and a backend uses "balance roundrobin",
these locations will still see this url_param_name set and consider it.
The harm is limited, as this only causes the beginning of the request
body to be buffered. And in general this is a bad practice which prevents
us from cleaning the lbprm stuff. Let's explicitly check the LB algo
instead.

This may be backported to all currently maintained versions.
2019-01-14 19:33:17 +01:00
Christopher Faulet
202c6ce1a2 BUG/MINOR: proto_htx: Use HTX versions to truncate or erase a buffer
channel_truncate() is not aware of the underlying format of the messages. So if
there are some outgoing data in the channel when called, it does some unexpected
operations on the channel's buffer. So the HTX version, channel_htx_truncate(),
must be used. The same is true for channel_erase(). It resets the buffer but not
the HTX message. So channel_htx_erase() must be used instead. This patch is
flagged as a bug, but as far as we know, it was never hitted.

This patch should be backported to 1.9. If so, following patch must be
backported too:

  * MINOR: channel/htx: Add the HTX version of channel_truncate/erase
2019-01-08 12:06:55 +01:00
Christopher Faulet
d01ce4003d BUG/MEDIUM: proto-htx: Set SI_FL_NOHALF on server side when request is done
In the function htx_end_request, the flag SI_FL_NOHALF must be set on the server
side once the request is in the state HTTP_MSG_DONE. But the response state was
checked before and the flag was only set when the response was also in the state
HTTP_MSG_DONE. Of course, it is not desirable.

This patch must be backported to 1.9.
2019-01-03 18:45:00 +01:00
Jérôme Magnin
86cef23266 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.
2018-12-28 15:48:12 +01:00
Willy Tarreau
14bfe9af12 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.
2018-12-19 15:25:43 +01:00
Christopher Faulet
b6aadbd19e 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.
2018-12-19 13:45:53 +01:00
Christopher Faulet
87a2c0d3f4 BUG/MINOR: proto_htx: Fix htx_res_set_status to also set the reason
Becaue the check on the return value of the function http_replace_res_status was
done upside down, no reason was never set.
2018-12-14 16:03:29 +01:00
Willy Tarreau
b96b77ed6e REORG: htx: merge types+proto into common/htx.h
All the HTX definition is self-contained and doesn't really depend on
anything external since it's a mostly protocol. In addition, some
external similar files (like h2) also placed in common used to rely
on it, making it a bit awkward.

This patch moves the two htx.h files into a single self-contained one.
The historical dependency on sample.h could be also removed since it
used to be there only for http_meth_t which is now in http.h.
2018-12-11 17:15:04 +01:00
Christopher Faulet
54a8d5a4a0 MEDIUM: cache/htx: Add the HTX support into the cache
The cache is now able to store and resend HTX messages. When an HTX message is
stored in the cache, the headers are prefixed with their block's info (an
uint32_t), containing its type and its length. Data, on their side, are stored
without any prefix. Only the value is copied in the cache. 2 fields have been
added in the structure cache_entry, hdrs_len and data_len, to known the size, in
the cache, of the headers part and the data part. If the message is chunked, the
trailers are also copied, the same way as data. When the HTX message is
recreated in the cache applet, the trailers size is known removing the headers
length and the data lenght from the total object length.
2018-12-11 17:09:31 +01:00
Willy Tarreau
1a18b54142 REORG: connection: centralize the conn_set_{tos,mark,quickack} functions
There were a number of ugly setsockopt() calls spread all over
proto_http.c, proto_htx.c and hlua.c just to manipulate the front
connection's TOS, mark or TCP quick-ack. These ones entirely relied
on the connection, its existence, its control layer's presence, and
its addresses. Worse, inet_set_tos() was placed in proto_http.c,
exported and used from the two other ones, surrounded in #ifdefs.

This patch moves this code to connection.h and makes the other ones
rely on it without ifdefs.
2018-12-11 16:41:51 +01:00
Willy Tarreau
2e754bff23 MINOR: htx: switch to case sensitive search of lower case header names
Now that we know that htx only contains lower case header names, there
is no need anymore for looking them up in a case-insensitive manner.

Note that http_find_header() still does it because header names to
compare against may come from everywhere there.
2018-12-07 13:25:59 +01:00
Christopher Faulet
b2aedea142 MEDIUM: channel/htx: Add functions for forward HTX data
To ease the fast forwarding and the infinte forwarding on HTX proxies, 2
functions have been added to let the channel be almost aware of the way data are
stored in its buffer. By calling these functions instead of legacy ones, we are
sure to forward the right amount of data.
2018-12-05 17:29:30 +01:00
Christopher Faulet
27ba2dc6d6 MEDIUM: htx: Rework conversion from a buffer to an htx structure
Now, the function htx_from_buf() will set the buffer's length to its size
automatically. In return, the caller should call htx_to_buf() at the end to be
sure to leave the buffer hosting the HTX message in the right state. When the
caller can use the function htxbuf() to get the HTX message without any update
on the underlying buffer.
2018-12-05 17:10:16 +01:00
Christopher Faulet
f3d480517f BUG/MINOR: proto_htx: Truncate the request when an error is detected
When HTTP_MSGF_ERROR is set on a channel (the request or the response), the
request must be truncated, not the response.
2018-12-04 16:43:30 +01:00
Willy Tarreau
b54c40ac0b BUILD: threads: fix minor build warnings when threads are disabled
These potential null-deref warnings are emitted on gcc 7 and above
when threads are disabled due to the use of objt_server() after an
existing validity test. Let's switch to __objt_server() since we
know the pointer is valid, it will not confuse the compiler.

Some of these may be backported to 1.8.
2018-12-02 19:28:41 +01:00
Joseph Herlant
e9d5c727c1 CLEANUP: Fix a typo in the proto_htx subsystem
Fixes a typo in the code comments of the proto_htx subsystem.
2018-12-02 18:38:48 +01:00
Joseph Herlant
c42c0e9969 CLEANUP: fix typos in the htx subsystem
Fix typos detected in the code comments of the htx subsystem.
2018-12-02 18:37:50 +01:00