mirrors/haproxy
1
0
Fork 0
mirror of https://git.haproxy.org/git/haproxy.git/ synced 2025-08-07 07:37:02 +02:00
Code Issues Projects Releases Wiki Activity

BUG/MEDIUM: mux-h1: Fix again 0-copy forwarding of chunks with an unknown size

Browse Source 
There is still an issue with zero-copy forwarding of chunks with an unknown
size. It is possible for a producer to fill the sapce reserved for the CRLF
at the end of the chunk. The root cause is that this space is not accounted
in the iobuf offset. So, from the producer point of view, the space may be
used. We can also argue the current design for iobuf is not well suited for
this case. Instead of using a pointer on the consumer's buffer, it could be
easier to use a custom buffer built on top of the consumer one, via a call
to b_make(), with the size, head and data field reflecting the avaialble
space the producer can use.

By the way, because of this bug, it is possible to trigger a BUG_ON() when
we try to write the CRLF at the end of the chunk because the buffer is
full. It is unexpected. Only the stats applet may hit this bug.

To fix the issue, instead of writting this CRLF when the current chunk is
consumed, it is written before consuming the next one. This way, all space
reserved to create the chunk formatting is always placed before forwarding
data.

No backport needed.
This commit is contained in:
Christopher Faulet 2024-02-28 08:01:06 +01:00
parent 60edfabc7b
commit 489e583ac5
1 changed files with 11 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
src/mux_h1.c
View File

@ -4455,7 +4455,7 @@ static size_t h1_nego_ff(struct stconn *sc, struct buffer *input, size_t count,
struct h1s *h1s = __sc_mux_strm(sc);
struct h1c *h1c = h1s->h1c;
struct h1m *h1m = (!(h1c->flags & H1C_F_IS_BACK) ? &h1s->res : &h1s->req);
size_t sz, prefix = 0, suffix = 0, ret = 0;
size_t sz, offset = 0, ret = 0;
TRACE_ENTER(H1_EV_STRM_SEND, h1c->conn, h1s, 0, (size_t[]){count});
@ -4494,17 +4494,15 @@ static size_t h1_nego_ff(struct stconn *sc, struct buffer *input, size_t count,
else {
/* The producer does not know the chunk size, thus this will be emitted at the
* end, in done_ff(). So splicing cannot be used (see TODO below).
* We will reserve 12 bytes to handle at most 4Go chunk with all CRLFs !
* (<8-bytes SIZE><CRLF><CHUNK-DATA><CRLF>, 10 bytes at the beginning and
* 2 bytes reserved at the end)
* We will reserve 10 bytes to handle at most 4Go chunk !
* (<8-bytes SIZE><CRLF><CHUNK-DATA>)
*/
if (count > MAX_RANGE(unsigned int))
count = MAX_RANGE(unsigned int);
prefix = 10;
suffix = 2;
offset = 10;
/* Add 2 more bytes to finish the previous chunk */
if (h1m->state == H1_MSG_CHUNK_CRLF)
prefix += 2;
offset += 2;
goto no_splicing;
}
}
@ -4540,7 +4538,7 @@ static size_t h1_nego_ff(struct stconn *sc, struct buffer *input, size_t count,
if (b_space_wraps(&h1c->obuf))
b_slow_realign(&h1c->obuf, trash.area, b_data(&h1c->obuf));
if (b_contig_space(&h1c->obuf) <= prefix + suffix) {
if (b_contig_space(&h1c->obuf) <= offset) {
h1c->flags |= H1C_F_OUT_FULL;
h1s->sd->iobuf.flags |= IOBUF_FL_FF_BLOCKED;
TRACE_STATE("output buffer full", H1_EV_STRM_SEND|H1_EV_H1S_BLK, h1c->conn, h1s);
@ -4548,12 +4546,12 @@ static size_t h1_nego_ff(struct stconn *sc, struct buffer *input, size_t count,
}
/* Cannot forward more than available room in output buffer */
sz = b_contig_space(&h1c->obuf) - prefix - suffix;
sz = b_contig_space(&h1c->obuf) - offset;
if (count > sz)
count = sz;
h1s->sd->iobuf.buf = &h1c->obuf;
h1s->sd->iobuf.offset = prefix;
h1s->sd->iobuf.offset = offset;
h1s->sd->iobuf.data = 0;
/* forward remaining input data */
@ -4607,13 +4605,10 @@ static size_t h1_done_ff(struct stconn *sc)
b_peek_ofs(&h1c->obuf, b_data(&h1c->obuf) - sd->iobuf.data + sd->iobuf.offset),
sd->iobuf.data);
h1_prepend_chunk_size(&buf, sd->iobuf.data, sd->iobuf.offset - ((h1m->state == H1_MSG_CHUNK_CRLF) ? 2 : 0));
h1_append_chunk_crlf(&buf);
b_add(&h1c->obuf, sd->iobuf.offset + 2);
if (h1m->state == H1_MSG_CHUNK_CRLF) {
if (h1m->state == H1_MSG_CHUNK_CRLF)
h1_prepend_chunk_crlf(&buf);
b_add(&h1c->obuf, 2);
}
h1m->state = H1_MSG_CHUNK_SIZE;
b_add(&h1c->obuf, sd->iobuf.offset);
h1m->state = H1_MSG_CHUNK_CRLF;
}
total = sd->iobuf.data;