The len-to-bytes conversion can be slightly simplified and optimized
by hardcoding a tree lookup. Just doing this increases by 1% the
request rate on H2. It could be made almost branch-free by using
fls() but it looks overkill for most situations since most headers
are very short.
In hpack_encode_header() there is a length check to verify that a literal
header name fits in the buffer, but there it an off-by-one in this length
check, which forgets the byte required to mark the encoding type (literal
without indexing). It should be harmless though as it cannot be triggered
since response headers passing through haproxy are limited by the reserve,
which is not the case of the output buffer.
This fix should be backported to 1.8.
Otherwise, after such replaces, the HTX message appears to wrap but the head
block address is not necessarily the first one. So adding new blocks will
override data of old ones.
If a server sends part of headers and then close its connection, the mux H1
reamins blocked in an infinite loop trying to read more data to finish the
parsing of the message. The flag CS_FL_REOS is set on the conn_stream. But
because there are some data in the input buffer, CS_FL_EOS is never set.
To fix the bug, in h1_process_input, when CS_FL_REOS is set on the conn_stream,
we also set CS_FL_EOS if the input buffer is empty OR if the channel's buffer is
empty.
When a request is fully processed, no more data are parsed until the response is
totally processed and a new transaction starts. But during this time, the mux is
trying to read more data and subscribes to read. If requests are pipelined, we
start to receive the next requests which will stay in the input buffer, leading
to a loop consuming all the CPU. This loop ends when the transaction ends. To
avoid this loop, the flag H1C_F_IN_BUSY has been added. It is set when the
request is fully parsed and unset when the transaction ends. Once set on H1C, it
blocks the reads. So the mux never tries to receive more data in this state.
Condition to process the connection mode on outgoing messages whithout
'Connection' header was wrong. It relied on the wrong H1M
state. H1_MSG_HDR_L2_LWS is only a possible state for messages with at least one
header. Now, to fix the bug, we just check the H1M state is not
H1_MSG_LAST_LF. So, we have the warranty the EOH was not processed yet.
Jerome reported that outgoing H2 failed for methods different from GET
or POST. It turns out that the HPACK encoding is performed by hand in
the outgoing headers encoding function and that the data length was not
incremented to cover the literal method value, resulting in a corrupted
HEADERS frame.
Admittedly this code should move to the generic HPACK code.
No backport is needed.
Make it obvious in the description of the sni directive that it can
not be used for health checks, and refer to the appropriate directive.
This can be backported to 1.8 as check-sni appeared in 1.8.
Make it more obvious that check-sni requires an argument, and that
it can only be a string. Also refer to sni for proxied traffic.
This can be backported to 1.8 as check-sni appeared in 1.8.
fix http-rules/h00000.vtc / http-rules/h00000.vtc as both 'bodylen' and
'body' are specified, these settings conflict with each other as they
both generate/present the body to send.
In connect_server(), don't attempt to reuse the conn_stream associated to
the stream_interface, if we already attempted a connection with it.
Using that conn_stream is only there for the cases where a connection and
a conn_stream was created ahead, mostly by http_proxy or by the LUA code.
If we already attempted to connect, that means we fail, and so we should
create a new connection.
No backport needed.
Released version 1.9-dev10 with the following main changes :
- MINOR: htx: Rename functions htx_*_to_str() to be H1 specific
- BUG/MINOR: htx: Force HTTP/1.1 on H1 formatting when version is 1.1 or above
- BUG/MINOR: fix ssl_fc_alpn and actually add ssl_bc_alpn
- BUG/MEDIUM: mworker: stop proxies which have no listener in the master
- BUG/MEDIUM: h1: Destroy a connection after detach if it has no owner.
- BUG/MEDIUM: h2: Don't forget to wake the tasklet after shutr/shutw.
- BUG/MINOR: flt_trace/compression: Use the right flag to add the HTX support
- BUG/MEDIUM: stream_interface: Make REALLY sure we read all the data.
- MEDIUM: mux-h1: Revamp the way subscriptions are handled.
- BUG/MEDIUM: mux-h1: Always set CS_FL_RCV_MORE when data are received in h1_recv()
- MINOR: mux-h1: Make sure to return 1 in h1_recv() when needed
- BUG/MEDIUM: mux-h1: Release the mux H1 in h1_process() if there is no h1s
- BUG/MINOR: proto_htx: Truncate the request when an error is detected
- BUG/MEDIUM: h2: When sending in HTX, make sure the caller knows we sent all.
- BUG/MEDIUM: mux-h2: properly update the window size in HTX mode
- BUG/MEDIUM: mux-h2: make sure to always report HTX EOM when consumed by headers
- BUG/MEDIUM: mux-h2: stop sending HTX once the mux is blocked
- BUG/MEDIUM: mux-h2: don't send more HTX data than requested
- MINOR: mux-h2: stop on non-DATA and non-EOM HTX blocks
- BUG/MEDIUM: h1: Correctly report used data with no len.
- MEDIUM: h1: Realign the ibuf before calling rcv_buf if needed.
- BUG/MEDIUM: mux_pt: Always set CS_FL_RCV_MORE.
- MINOR: htx: make htx_from_buf() adjust the size only on new buffers
- MINOR: htx: add buf_room_for_htx_data() to help optimize buffer transfers
- MEDIUM: mux-h1: make use of buf_room_for_htx_data() instead of b_room()
- MEDIUM: mux-h1: attempt to zero-copy Rx DATA transfers
- MEDIUM: mux-h1: avoid a double copy on the Tx path whenever possible
- BUG/MEDIUM: stream-int: don't mark as blocked an empty buffer on Rx
- BUG/MINOR: mux-h1: Check h1m flags to set the server conn_mode on request path
- MEDIUM: htx: Rework conversion from a buffer to an htx structure
- MEDIUM: channel/htx: Add functions for forward HTX data
- MINOR: mux-h1: Don't adjust anymore the amount of data sent in h1_snd_buf()
- CLEANUP: htx: Fix indentation here and there in HTX files
- MINOR: mux-h1: Allow partial data consumption during outgoing data processing
- BUG/MEDIUM: mux-h2: use the correct offset for the HTX start line
- BUG/MEDIUM: mux-h2: stop sending using HTX on errors
- MINOR: mux-h1: Drain obuf if the output is closed after sending data
- BUG/MEDIUM: mworker: stop every tasks in the master
- BUG/MEDIUM: htx: Set the right start-line offset after a defrag
- BUG/MEDIUM: stream: Don't dereference s->txn when it is not there yet.
- BUG/MEDIUM: connections: Reuse an already attached conn_stream.
- MINOR: stream-int: add a new blocking condition on the remote connection
- BUG/MEDIUM: stream-int: don't attempt to receive if the connection is not established
- BUG/MEDIUM: lua: block on remote connection establishment
- BUG/MEDIUM: mworker: fix several typos in mworker_cleantasks()
- SCRIPTS/REGTEST: merge grep+sed into sed in run-regtests
- BUG/MEDIUM: connections: Split CS_FL_RCV_MORE into 2 flags.
- BUG/MEDIUM: h1: Don't free the connection if it's an outgoing connection.
- BUG/MEDIUM: h1: Set CS_FL_REOS if we had a read0.
- BUG/MEDIUM: mux-h1: Be sure to have a conn_stream to set CS_FL_REOS in h1_recv
- REGTEST: Move LUA reg test 4 to level 1.
- MINOR: ist: add functions to copy/uppercase/lowercase into a buffer or string
- MEDIUM: ist: always turn header names to lower case
- MINOR: h2: don't turn HTX header names to lower case anymore
- MEDIUM: ist: use local conversion arrays to case conversion
- MINOR: htx: switch to case sensitive search of lower case header names
- MINOR: mux-h1: Set CS_FL_EOS when read0 is detected and no data are pending
- BUG/MINOR: stream-int: Process read0 even if no data was received in si_cs_recv
- REGTEST: fix the Lua test file name in test lua/h00002 :-)
- REGTEST: add a basic test for HTTP rules manipulating headers
- BUG/MEDIUM: sample: Don't treat SMP_T_METH as SMP_T_STR.
- MINOR: sample: add bc_http_major
- BUG/MEDIUM: htx: fix typo in htx_replace_stline() making it fail all the time
- REGTEST: make the HTTP rules test compatible with HTTP/2 as well
- BUG/MEDIUM: h2: Don't try to chunk data when using HTX.
- MINOR: compiler: add a new macro ALREADY_CHECKED()
- BUILD: h2: mark the start line already checked to avoid warnings
- BUG/MINOR: mux-h1: Remove the connection header when it is useless
When the connection mode can be deduced from the HTTP version, we remove the
redundant connection header. So "keep-alive" connection header is removed from
HTTP/1.1 messages and "close" connection header is remove from HTTP/1.0
messages.
Gcc 7 warns about a potential null pointer deref that cannot happen
since the start line block is guaranteed to be present in the functions
where it's dereferenced. Let's mark it as already checked.
This macro may be used to block constant propagation that lets the compiler
detect a possible NULL dereference on a variable resulting from an explicit
assignment in an impossible check. Sometimes a function is called which does
safety checks and returns NULL if safe conditions are not met. The place
where it's called cannot hit this condition and dereferencing the pointer
without first checking it will make the compiler emit a warning about a
"potential null pointer dereference" which is hard to work around. This
macro "washes" the pointer and prevents the compiler from emitting tests
branching to undefined instructions. It may only be used when the developer
is absolutely certain that the conditions are guaranteed and that the
pointer passed in argument cannot be NULL by design.
A typical use case is a top-level function doing this :
if (frame->type == HEADERS)
parse_frame(frame);
Then parse_frame() does this :
void parse_frame(struct frame *frame)
{
const char *frame_hdr;
frame_hdr = frame_hdr_start(frame);
if (*frame_hdr == FRAME_HDR_BEGIN)
process_frame(frame);
}
and :
const char *frame_hdr_start(const struct frame *frame)
{
if (frame->type == HEADERS)
return frame->data;
else
return NULL;
}
Above parse_frame() is only called for frame->type == HEADERS so it will
never get a NULL in return from frame_hdr_start(). Thus it's always safe
to dereference *frame_hdr since the check was already performed above.
It's then safe to address it this way instead of inventing dummy error
code paths that may create real bugs :
void parse_frame(struct frame *frame)
{
const char *frame_hdr;
frame_hdr = frame_hdr_start(frame);
ALREADY_CHECKED(frame_hdr);
if (*frame_hdr == FRAME_HDR_BEGIN)
process_frame(frame);
}
When we're using HTX, we don't have to generate chunk header/trailers, and
that ultimately leads to a crash when we try to access a buffer that
contains just chunk trailers.
This should not be backported.
The HTTP rules test now runs an H1 and an H2 client. Since the H2 one
requires the "proto" directive on the bind line, a new file has been
created requiring version 1.9 and the previous one was marked as usable
below 1.9 so that it's skipped by default but still usable when testing
backports.
A typo in the block type check makes this function fail all the time,
which has impact on anything rewriting a start line (set-uri, set-path
etc).
No backport needed.
This adds the sample fetch bc_http_major. It returns the backend connection's HTTP
version encoding, which may be 1 for HTTP/0.9 to HTTP/1.1 or 2 for HTTP/2.0. It is
based on the on-wire encoding, and not the version present in the request header.
In smp_dup(), don't consider a SMP_T_METH with an unknown method the same as
SMP_T_STR. The string and string length aren't stored at the same place.
This should be backported to 1.8.
There is always a risk of breaking HTTP processing when performing certain
code changes. This test modifies a request's start line, uses variables,
adds and modifies headers, interleaves them with the start-line changes,
and makes use of different header formats involving duplicated names,
duplicated values, empty fields and spaces around values. These operations
are performed both in the frontend and in the backend, for both the request
and the response. A CRC is computed on the concatenation of all the values,
and the concatenations are sent as individual header fields as well to help
debugging when the test fails.
The test reliably works since 1.6, implying that the HTTP processing did
not change. It currently fails on HTX.
The flag CS_FL_EOS can be set while no data was received. So the flas
CS_FL_RCV_MORE is not set. In this case, the read0 was never processed by the
stream interface. To be sure to process it, the test on CS_FL_RCV_MORE has been
moved after the one on CS_FL_EOS.
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.
Calling tolower/toupper for each character is slow, a lookup into a
256-byte table is cheaper, especially for common characters used in
header field names which all fit into a cache line. Let's create these
two variables marked weak so that they're included only once.
Since HTX stores header names in lower case already, we don't need to
do it again anymore. This increased H2 performance by 2.7% on quick
tests, now making H2 overr HTX about 5.5% faster than H2 over H1.
HTTP/2 and above require header names to be lower cased, while HTTP/1
doesn't care. By making lower case the standard way to store header
names in HTX, we can significantly simplify all operations applying to
header names retrieved from HTX (including, but not limited to, lookups
and lower case checks which are not needed anymore).
As a side effect of replacing memcpy() with ist2bin_lc(), a small increase
of the request rate performance of about 0.5-1% was noticed on keep-alive
traffic, very likely due to memcpy() being overkill for tiny strings.
This trivial patch was marked medium because it may have a visible end-user
impact (e.g. non-HTTP compliant agent, etc).
The ist functions were missing functions to copy an IST into a target
buffer, making some code have to resort to memcpy(), which tends to be
overkill for small strings, that the compiler cannot guess. In addition
sometimes there is a need to turn a string to lower or upper case so it
had to be overwritten after the operation.
This patch adds 6 functions to copy an ist to a buffer, as binary or as a
string (i.e. a zero is or is not appended), and optionally to apply a
lower case or upper case transformation on the fly.
A number of tests were performed to optimize the processing for small
strings. The loops are marked unlikely to dissuade the compilers from
over-optimizing them and switching to SIMD instructions. The lower case
or upper case transformations used to rely on external functions for
each character and to crappify the code due to clobbered registers,
which is not acceptable when we know that only a certain class of chars
has to be transformed, so the test was open-coded.
In the commit 6a2d33481 ("BUG/MEDIUM: h1: Set CS_FL_REOS if we had a read0."),
We set the flag CS_FL_REOS on the conn_stream when a read0 is detected. But we
must be sure to have a conn_stream first.
In h1_recv(), if we get a read0, let the conn_stream know by setting the
CS_FL_REOS flag, or it may never be aware we did hit EOS.
This should not be backported.
In h1_process(), don't release the connection if it is an outgoing connection
and we don't have an h1s associated, if it is so it is probably just in
a pool.
CS_FL_RCV_MORE is used in two cases, to let the conn_stream
know there may be more data available, and to let it know that
it needs more room. We can't easily differentiate between the
two, and that may leads to hangs, so split it into two flags,
CS_FL_RCV_MORE, that means there may be more data, and
CS_FL_WANT_ROOM, that means we need more room.
This should not be backported.
Some commands chain grep and sed while sed already does grep by
default, let's simply use sed. In addition to being more intuitive,
it saves up to 150ms per run on the 13 tests covered by level 4.
Commit 27f3fa5 ("BUG/MEDIUM: mworker: stop every tasks in the master")
used MAX_THREADS as a mask instead of MAX_THREADS_MASK to clean the
global run queue, and used rq_next (global variable) instead of next_rq.
Renamed next_rq as tmp_rq and next_wq as tmp_wq to avoid confusion.
No backport needed.
We used to wait for the other side to be connected, but the blocking
flags were inaccurate. It used to work fine almost by accident before
the stream interface changes. Now we use the new RXBLK_CONN flag to
explicitly subscribe to this event.
Thanks to Adis for reporting the issue, PiBaNL for the test case,
and Olivier for the diagnostic.
No backport is needed.
If we try to receive before the connection is established, we lose the
send event and are not woken up anymore once the connection is established.
This was diagnosed by Olivier.
No backport is needed.
There are some situations where we need to wait for the other side to
be connected. None of the current blocking flags support this. It used
to work more or less by accident using the old flags. Let's add a new
flag to mention we're blocking on this, it's removed by si_chk_rcv()
when a connection is established. It should be enough for now.
In connect_server(), if we already have a conn_stream, reuse it
instead of trying to create a new one. http_proxy and LUA both
manually create a conn_stream and a connection, and we want
to use it.
The offset was always wrong after an HTX defragmentation because the wrong
address was used and because the update could occcur several time on the same
defragmentation.
The master is not supposed to run (at the moment) any task before the
polling loop, the created tasks should be run only in the workers but in
the master they should be disabled or removed.
No backport needed.
It avoids to subscribe to send events because some may remain in the output
buffer. If the output is closed or if an error occurred, there is no way to send
these data anyway, so it is safe to drain them.
