BUG/MEDIUM: mux-h1: don't lose the end of the previous chunk in h1_make_chunk()

h1_make_chunk() is used during zero-copy forwarding to emit the chunk size
line of the next chunk, preceded by the CRLF ending the previous chunk when
the message is in H1_MSG_CHUNK_CRLF state. Both are written into a temporary
view of the output buffer which is only committed on success. But the state
was switched to H1_MSG_CHUNK_SIZE as soon as the CRLF was written into this
view. So if the chunk size line itself did not fit, the function returned on
the "full" path without committing anything, while the state indicated that
the CRLF was already emitted.  On the next attempt, the chunk size was
emitted directly after the previous chunk's data, without the CRLF,
producing an invalid chunked message.

Now the state is only changed once both the CRLF and the chunk size were
successfully written.

This was introduced by commit 6dff013fa ("MINOR: mux-h1: Add function to add
size of a chunk to an outgoind message") in 2.9. It must be backported as
far as 3.0.

diff --git a/src/mux_h1.c b/src/mux_h1.c
index c38d9ad19cca04446d293f9dcb2b1c67f1538733..63f3637960293c49a3a30e024e967666933bcece 100644 (file)
--- a/src/mux_h1.c
+++ b/src/mux_h1.c
@@ -3620,11 +3620,11 @@ static size_t h1_make_chunk(struct h1s *h1s, struct h1m * h1m, size_t len)
                b_slow_realign(&h1c->obuf, trash.area, b_data(&h1c->obuf));
        outbuf = b_make(b_tail(&h1c->obuf), b_contig_space(&h1c->obuf), 0, 0);
 
-       if (h1m->state == H1_MSG_CHUNK_CRLF) {
-               if (!chunk_memcat(&outbuf, "\r\n", 2))
-                       goto full;
-               h1m->state = H1_MSG_CHUNK_SIZE;
-       }
+       /* Note: <outbuf> is only committed on success, so the message state
+        * must not change before.
+        */
+       if (h1m->state == H1_MSG_CHUNK_CRLF && !chunk_memcat(&outbuf, "\r\n", 2))
+               goto full;
        if (!h1_append_chunk_size(&outbuf, len))
                goto full;