]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MEDIUM: mux-h1: Handle abort with an incomplete message during parsing
authorChristopher Faulet <cfaulet@haproxy.com>
Mon, 10 Oct 2022 16:05:25 +0000 (18:05 +0200)
committerChristopher Faulet <cfaulet@haproxy.com>
Wed, 12 Oct 2022 15:10:44 +0000 (17:10 +0200)
In h1_process_demux(), aborts for incomplete messages were not properly
handled. It was not an issue because the abort was detected later in
h1_process(). But it will be an issue to perform the aborts refoctoring.

First, when a read0 was detected, the SE_FL_EOI flag was set for messages in
DONE or TUNNEL state or for messages without known length (so responses in
close mode). The last statement is not accurate. The message must also be in
DATA state. Otherwise, SE_FL_EOI flag may be set on incomplete message.

Then, an error was reported, via SE_FL_ERROR flag, only when an incomplete
message was detected on the payload parsing. It must also be reported if
headers are incomplete. Here again, the error is detected later for now. But
it could be an issue later.

There is no reason to backport this patch.

src/mux_h1.c

index 45173b7dfdd887a30188ae7a55697933e194e01a..2dcbf14a7aa093e1d987a38b20b3e98d8686d435 100644 (file)
@@ -1881,12 +1881,12 @@ static size_t h1_process_demux(struct h1c *h1c, struct buffer *buf, size_t count
                se_fl_clr(h1s->sd, SE_FL_RCV_MORE | SE_FL_WANT_ROOM);
                if (h1s->flags & H1S_F_REOS) {
                        se_fl_set(h1s->sd, SE_FL_EOS);
-                       if (h1m->state >= H1_MSG_DONE || !(h1m->flags & H1_MF_XFER_LEN)) {
+                       if (h1m->state >= H1_MSG_DONE || (h1m->state > H1_MSG_LAST_LF && !(h1m->flags & H1_MF_XFER_LEN))) {
                                /* DONE or TUNNEL or SHUTR without XFER_LEN, set
                                 * EOI on the stream connector */
                                se_fl_set(h1s->sd, SE_FL_EOI);
                        }
-                       else if (h1m->state > H1_MSG_LAST_LF && h1m->state < H1_MSG_DONE) {
+                       else if (h1m->state < H1_MSG_DONE) {
                                se_fl_set(h1s->sd, SE_FL_ERROR);
                                TRACE_ERROR("message aborted, set error on SC", H1_EV_RX_DATA|H1_EV_H1S_ERR, h1c->conn, h1s);
                        }
@@ -2942,16 +2942,16 @@ static int h1_process(struct h1c * h1c)
                        h1c->flags &= ~(H1C_F_ST_IDLE|H1C_F_WAIT_NEXT_REQ);
                        TRACE_ERROR("internal error detected", H1_EV_H1C_WAKE|H1_EV_H1C_ERR);
                }
-               else if (h1s->flags & H1S_F_PARSING_ERROR) {
-                       h1_handle_parsing_error(h1c);
-                       h1c->flags = (h1c->flags & ~(H1C_F_ST_IDLE|H1C_F_WAIT_NEXT_REQ)) | H1C_F_ST_ERROR;
-                       TRACE_ERROR("parsing error detected", H1_EV_H1C_WAKE|H1_EV_H1C_ERR);
-               }
                else if (h1s->flags & H1S_F_NOT_IMPL_ERROR) {
                        h1_handle_not_impl_err(h1c);
                        h1c->flags = (h1c->flags & ~(H1C_F_ST_IDLE|H1C_F_WAIT_NEXT_REQ)) | H1C_F_ST_ERROR;
                        TRACE_ERROR("not-implemented error detected", H1_EV_H1C_WAKE|H1_EV_H1C_ERR);
                }
+               else if (h1s->flags & H1S_F_PARSING_ERROR || se_fl_test(h1s->sd, SE_FL_ERROR)) {
+                       h1_handle_parsing_error(h1c);
+                       h1c->flags = (h1c->flags & ~(H1C_F_ST_IDLE|H1C_F_WAIT_NEXT_REQ)) | H1C_F_ST_ERROR;
+                       TRACE_ERROR("parsing error detected", H1_EV_H1C_WAKE|H1_EV_H1C_ERR);
+               }
        }
        h1_send(h1c);