BUG/MEDIUM: mux-h1: Set outgoing message to DONE when payload length is reached

When a message is sent, we can switch it state to MSG_DONE if all the
announced payload was processed. This way, if the EOM flag is not set on the
message when the last expected data block is processed, the message can
still be set to MSG_DONE state.

This bug is related to the previous ones. There is a design issue with the
HTX since the 2.4. When the EOM HTX block was replaced by a flag, I tried
hard to be sure the flag is always set with the last HTX block on a
message. It works pretty well for all messages received from a client or a
server. But for internal messages, it is not so simple. Because applets
cannot always properly handle the end of messages. So, there are some cases
where the EOM flag is set on an empty message.

As a workaround, for chunked messages, we can add an EOT HTX block. It does
the trick. But for messages with a content-length, there is no empty DATA
block. Thus, the only way to be sure the end of the message was reached in
this case is to detect it in the H1 multiplexr.

We already count the amount of data processed when the payload length is
announced. Thus, we must only switch the message in DONE state when last
bytes of the payload are received. Or when the EOM flag is received of
course.

This patch must be backported as far as 2.4.
This commit is contained in:
Christopher Faulet 2022-04-07 10:29:38 +02:00
parent be69cbdafe
commit 2eb5243e7f

View File

@ -1975,6 +1975,8 @@ static size_t h1_process_mux(struct h1c *h1c, struct buffer *buf, size_t count)
goto error;
}
h1m->curr_len -= count;
if (!h1m->curr_len)
last_data = 1;
}
if (chn_htx->flags & HTX_FL_EOM) {
TRACE_DEVEL("last message block", H1_EV_TX_DATA|H1_EV_TX_BODY, h1c->conn, h1s);
@ -2282,8 +2284,9 @@ static size_t h1_process_mux(struct h1c *h1c, struct buffer *buf, size_t count)
TRACE_STATE("1xx response xferred", H1_EV_TX_DATA|H1_EV_TX_HDRS, h1c->conn, h1s);
}
else {
/* EOM flag is set and it is the last block */
if (htx_is_unique_blk(chn_htx, blk) && (chn_htx->flags & HTX_FL_EOM)) {
/* EOM flag is set or empty payload (C-L to 0) and it is the last block */
if (htx_is_unique_blk(chn_htx, blk) &&
((chn_htx->flags & HTX_FL_EOM) || ((h1m->flags & H1_MF_CLEN) && !h1m->curr_len))) {
if (h1m->flags & H1_MF_CHNK) {
if (!chunk_memcat(&tmp, "\r\n0\r\n\r\n", 7))
goto full;
@ -2381,8 +2384,11 @@ static size_t h1_process_mux(struct h1c *h1c, struct buffer *buf, size_t count)
H1_EV_TX_DATA|H1_EV_TX_BODY, h1c->conn, h1s, 0, (size_t[]){v.len});
skip_data:
if (h1m->state == H1_MSG_DATA && (h1m->flags & H1_MF_CLEN))
if (h1m->state == H1_MSG_DATA && (h1m->flags & H1_MF_CLEN)) {
h1m->curr_len -= vlen;
if (!h1m->curr_len)
last_data = 1;
}
if (last_data)
goto done;
break;
@ -2432,7 +2438,7 @@ static size_t h1_process_mux(struct h1c *h1c, struct buffer *buf, size_t count)
goto error; /* For now return an error */
done:
if (!(chn_htx->flags & HTX_FL_EOM)) {
if (!(chn_htx->flags & HTX_FL_EOM) && (!(h1m->flags & H1_MF_CLEN) || h1m->curr_len)) {
TRACE_STATE("No EOM flags in done state", H1_EV_TX_DATA|H1_EV_H1C_ERR|H1_EV_H1S_ERR, h1c->conn, h1s);
goto error; /* For now return an error */
}