BUG/MEDIUM: compression: handle rewrite errors when updating response headers

When an HTTP response is compressed by HAProxy, the headers are updated.
However it is possible to encounter a rewrite error because the buffer is
full. In this case, the compression is aborted. Thus, we must be sure to
leave the response in a valid state.

For now, it is an issue because the "Content-Encoding" header is added
before all other headers manipulations. So if the compression is aborted on
error, the "Content-Encoding" header may remain while the payload is not
compressed.

So now, we take care to leave with a valid response on error by reordering
the headers manipulations. It is too painful to really rollback all changes,
especially for an edge case.

This patch should be backported as far as 2.0. Note that on the 2.0, the
legacy HTTP part is also concerned.

diff --git a/src/flt_http_comp.c b/src/flt_http_comp.c
index cac7dc04be8225620e360d279616af7dd150d340..f2f04ee62f1e275b3afc89a6f22b697a7c77e78b 100644 (file)
--- a/src/flt_http_comp.c
+++ b/src/flt_http_comp.c
@@ -304,23 +304,18 @@ set_compression_response_header(struct comp_state *st, struct stream *s, struct
        struct htx_sl *sl;
        struct http_hdr_ctx ctx;
 
-       /*
-        * Add Content-Encoding header when it's not identity encoding.
-        * RFC 2616 : Identity encoding: This content-coding is used only in the
-        * Accept-Encoding header, and SHOULD NOT be used in the Content-Encoding
-        * header.
-        */
-       if (st->comp_algo->cfg_name_len != 8 || memcmp(st->comp_algo->cfg_name, "identity", 8) != 0) {
-               struct ist v = ist2(st->comp_algo->ua_name, st->comp_algo->ua_name_len);
-
-               if (!http_add_header(htx, ist("Content-Encoding"), v))
-                       goto error;
-       }
-
        sl = http_get_stline(htx);
        if (!sl)
                goto error;
 
+       /* add "Transfer-Encoding: chunked" header */
+       if (!(msg->flags & HTTP_MSGF_TE_CHNK)) {
+               if (!http_add_header(htx, ist("Transfer-Encoding"), ist("chunked")))
+                       goto error;
+               msg->flags |= HTTP_MSGF_TE_CHNK;
+               sl->flags |= (HTX_SL_F_XFER_ENC|HTX_SL_F_CHNK);
+       }
+
        /* remove Content-Length header */
        if (msg->flags & HTTP_MSGF_CNT_LEN) {
                ctx.blk = NULL;
@@ -330,14 +325,6 @@ set_compression_response_header(struct comp_state *st, struct stream *s, struct
                sl->flags &= ~HTX_SL_F_CLEN;
        }
 
-       /* add "Transfer-Encoding: chunked" header */
-       if (!(msg->flags & HTTP_MSGF_TE_CHNK)) {
-               if (!http_add_header(htx, ist("Transfer-Encoding"), ist("chunked")))
-                       goto error;
-               msg->flags |= HTTP_MSGF_TE_CHNK;
-               sl->flags |= (HTX_SL_F_XFER_ENC|HTX_SL_F_CHNK);
-       }
-
        /* convert "ETag" header to a weak ETag */
        ctx.blk = NULL;
        if (http_find_header(htx, ist("ETag"), &ctx, 1)) {
@@ -355,6 +342,19 @@ set_compression_response_header(struct comp_state *st, struct stream *s, struct
        if (!http_add_header(htx, ist("Vary"), ist("Accept-Encoding")))
                goto error;
 
+       /*
+        * Add Content-Encoding header when it's not identity encoding.
+        * RFC 2616 : Identity encoding: This content-coding is used only in the
+        * Accept-Encoding header, and SHOULD NOT be used in the Content-Encoding
+        * header.
+        */
+       if (st->comp_algo->cfg_name_len != 8 || memcmp(st->comp_algo->cfg_name, "identity", 8) != 0) {
+               struct ist v = ist2(st->comp_algo->ua_name, st->comp_algo->ua_name_len);
+
+               if (!http_add_header(htx, ist("Content-Encoding"), v))
+                       goto error;
+       }
+
        return 1;
 
   error: