MEDIUM: compression: don't compress when no data

This patch makes changes in the http_response_forward_body state
machine. It checks if the compress algorithm had consumed data before
swapping the temporary and the input buffer. So it prevents null sized
zlib chunks.

diff --git a/include/proto/compression.h b/include/proto/compression.h
index fa4c7b743679beff6de2c249d069a20116852e7e..cfab62a07511a7e50d3833ab26786e9204b37cd1 100644 (file)
--- a/include/proto/compression.h
+++ b/include/proto/compression.h
@@ -34,7 +34,7 @@ int http_compression_buffer_add_data(struct session *s, struct buffer *in, struc
 int http_compression_buffer_end(struct session *s, struct buffer **in, struct buffer **out, int end);
 
 int identity_init(struct comp_ctx *comp_ctx, int level);
-int identity_add_data(struct comp_ctx *comp_ctx, const char *in_data, int in_len, char *out_data, int out_len);
+int identity_add_data(struct comp_ctx *comp_ctx, const char *in_data, int in_len, struct buffer *out);
 int identity_flush(struct comp_ctx *comp_ctx, struct buffer *out, int flag);
 int identity_reset(struct comp_ctx *comp_ctx);
 int identity_end(struct comp_ctx *comp_ctx);
@@ -43,17 +43,12 @@ int identity_end(struct comp_ctx *comp_ctx);
 #ifdef USE_ZLIB
 
 int deflate_init(struct comp_ctx *comp_ctx, int level);
-int deflate_add_data(struct comp_ctx *comp_ctx, const char *in_data, int in_len, char *out_data, int out_len);
+int deflate_add_data(struct comp_ctx *comp_ctx, const char *in_data, int in_len, struct buffer *out);
 int deflate_flush(struct comp_ctx *comp_ctx, struct buffer *out, int flag);
 int deflate_reset(struct comp_ctx *comp_ctx);
 int deflate_end(struct comp_ctx *comp_ctx);
 
 int gzip_init(struct comp_ctx *comp_ctx, int level);
-int gzip_add_data(struct comp_ctx *comp_ctx, const char *in_data, int in_len, char *out_data, int out_len);
-int gzip_flush(struct comp_ctx *comp_ctx, struct buffer *out, int flag);
-int gzip_reset(struct comp_ctx *comp_ctx);
-int gzip_end(struct comp_ctx *comp_ctx);
-
 #endif /* USE_ZLIB */
 
 #endif /* _PROTO_COMP_H */

diff --git a/include/types/compression.h b/include/types/compression.h
index da356ad0a562eb1e6b9ae9c2f40ef4ba689d5285..cf56e457b3b6a542d24c223cef318a6aca67d03b 100644 (file)
--- a/include/types/compression.h
+++ b/include/types/compression.h
@@ -51,7 +51,7 @@ struct comp_algo {
        char *name;
        int name_len;
        int (*init)(struct comp_ctx *comp_ctx, int level);
-       int (*add_data)(struct comp_ctx *comp_ctx, const char *in_data, int in_len, char *out_data, int out_len);
+       int (*add_data)(struct comp_ctx *comp_ctx, const char *in_data, int in_len, struct buffer *out);
        int (*flush)(struct comp_ctx *comp_ctx, struct buffer *out, int flag);
        int (*reset)(struct comp_ctx *comp_ctx);
        int (*end)(struct comp_ctx *comp_ctx);

diff --git a/src/compression.c b/src/compression.c
index abceddfc9b8ac08a06ff816242738df8b06f1079..72c9996b931acd0d21405c395d09e732692ca359 100644 (file)
--- a/src/compression.c
+++ b/src/compression.c
@@ -164,6 +164,7 @@ int http_compression_buffer_init(struct session *s, struct buffer *in, struct bu
 int http_compression_buffer_add_data(struct session *s, struct buffer *in, struct buffer *out)
 {
        struct http_msg *msg = &s->txn.rsp;
+       int consumed_data = 0;
        int data_process_len;
        int left;
        int ret;
@@ -186,28 +187,23 @@ int http_compression_buffer_add_data(struct session *s, struct buffer *in, struc
 
        left = data_process_len - bi_contig_data(in);
        if (left <= 0) {
-               ret = s->comp_algo->add_data(&s->comp_ctx, bi_ptr(in),
-                                            data_process_len, bi_end(out),
-                                            out->size - buffer_len(out));
+               consumed_data += ret = s->comp_algo->add_data(&s->comp_ctx, bi_ptr(in), data_process_len, out);
                if (ret < 0)
                        return -1;
-               out->i += ret;
 
        } else {
-               ret = s->comp_algo->add_data(&s->comp_ctx, bi_ptr(in), bi_contig_data(in), bi_end(out), out->size - buffer_len(out));
+               consumed_data += ret = s->comp_algo->add_data(&s->comp_ctx, bi_ptr(in), bi_contig_data(in), out);
                if (ret < 0)
                        return -1;
-               out->i += ret;
-               ret = s->comp_algo->add_data(&s->comp_ctx, in->data, left, bi_end(out), out->size - buffer_len(out));
+               consumed_data += ret = s->comp_algo->add_data(&s->comp_ctx, in->data, left, out);
                if (ret < 0)
                        return -1;
-               out->i += ret;
        }
 
        b_adv(in, data_process_len);
        msg->chunk_len -= data_process_len;
 
-       return 0;
+       return consumed_data;
 }
 
 /*
@@ -309,15 +305,20 @@ int identity_init(struct comp_ctx *comp_ctx, int level)
 
 /*
  * Process data
- *   Return size of processed data or -1 on error
+ *   Return size of consumed data or -1 on error
  */
-int identity_add_data(struct comp_ctx *comp_ctx, const char *in_data, int in_len, char *out_data, int out_len)
+int identity_add_data(struct comp_ctx *comp_ctx, const char *in_data, int in_len, struct buffer *out)
 {
+       char *out_data = bi_end(out);
+       int out_len = out->size - buffer_len(out);
+
        if (out_len < in_len)
                return -1;
 
        memcpy(out_data, in_data, in_len);
 
+       out->i += in_len;
+
        return in_len;
 }
 
@@ -462,10 +463,13 @@ int deflate_init(struct comp_ctx *comp_ctx, int level)
        return 0;
 }
 
-int deflate_add_data(struct comp_ctx *comp_ctx, const char *in_data, int in_len, char *out_data, int out_len)
+/* Return the size of consumed data or -1 */
+int deflate_add_data(struct comp_ctx *comp_ctx, const char *in_data, int in_len, struct buffer *out)
 {
-       z_stream *strm = &comp_ctx->strm;
        int ret;
+       z_stream *strm = &comp_ctx->strm;
+       char *out_data = bi_end(out);
+       int out_len = out->size - buffer_len(out);
 
        if (in_len <= 0)
                return 0;
@@ -484,8 +488,9 @@ int deflate_add_data(struct comp_ctx *comp_ctx, const char *in_data, int in_len,
                return -1;
 
        /* deflate update the available data out */
+       out->i += out_len - strm->avail_out;
 
-       return out_len - strm->avail_out;
+       return in_len - strm->avail_in;
 }
 
 int deflate_flush(struct comp_ctx *comp_ctx, struct buffer *out, int flag)

diff --git a/src/proto_http.c b/src/proto_http.c
index 848e745c0d537698615ffdfac380fa76b3797c59..e410b6dddc2f36882d4141cf236be68e2337461a 100644 (file)
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -5546,6 +5546,7 @@ int http_response_forward_body(struct session *s, struct channel *res, int an_bi
        unsigned int bytes;
        static struct buffer *tmpbuf = NULL;
        int compressing = 0;
+       int consumed_data = 0;
 
        if (unlikely(msg->msg_state < HTTP_MSG_BODY))
                return 0;
@@ -5606,18 +5607,28 @@ int http_response_forward_body(struct session *s, struct channel *res, int an_bi
                }
 
                if (msg->msg_state == HTTP_MSG_DATA) {
+                       int ret;
+
                        /* must still forward */
-                       if (compressing)
-                               http_compression_buffer_add_data(s, res->buf, tmpbuf);
+                       if (compressing) {
+                               consumed_data += ret = http_compression_buffer_add_data(s, res->buf, tmpbuf);
+                               if (ret < 0)
+                                       goto aborted_xfer;
+                       }
 
                        if (res->to_forward || msg->chunk_len)
                                goto missing_data;
 
                        /* nothing left to forward */
-                       if (msg->flags & HTTP_MSGF_TE_CHNK)
+                       if (msg->flags & HTTP_MSGF_TE_CHNK) {
                                msg->msg_state = HTTP_MSG_CHUNK_CRLF;
-                       else
+                       } else {
                                msg->msg_state = HTTP_MSG_DONE;
+                               if (compressing && consumed_data) {
+                                       http_compression_buffer_end(s, &res->buf, &tmpbuf, 1);
+                                       compressing = 0;
+                               }
+                       }
                }
                else if (msg->msg_state == HTTP_MSG_CHUNK_SIZE) {
                        /* read the chunk size and assign it to ->chunk_len, then
@@ -5633,14 +5644,21 @@ int http_response_forward_body(struct session *s, struct channel *res, int an_bi
                                        http_capture_bad_message(&s->be->invalid_rep, s, msg, HTTP_MSG_CHUNK_SIZE, s->fe);
                                goto return_bad_res;
                        }
-                       /* skipping data if we are in compression mode */
-                       if (compressing && msg->chunk_len > 0) {
-                               b_adv(res->buf, msg->next);
-                               msg->next = 0;
-                               msg->sov = 0;
-                               msg->sol = 0;
+                       if (compressing) {
+                               if (likely(msg->chunk_len > 0)) {
+                                       /* skipping data if we are in compression mode */
+                                       b_adv(res->buf, msg->next);
+                                       msg->next = 0;
+                                       msg->sov = 0;
+                                       msg->sol = 0;
+                               } else {
+                                       if (consumed_data) {
+                                               http_compression_buffer_end(s, &res->buf, &tmpbuf, 1);
+                                               compressing = 0;
+                                       }
+                               }
                        }
-                       /* otherwise we're in HTTP_MSG_DATA or HTTP_MSG_TRAILERS state */
+               /* otherwise we're in HTTP_MSG_DATA or HTTP_MSG_TRAILERS state */
                }
                else if (msg->msg_state == HTTP_MSG_CHUNK_CRLF) {
                        /* we want the CRLF after the data */
@@ -5672,20 +5690,10 @@ int http_response_forward_body(struct session *s, struct channel *res, int an_bi
                                        http_capture_bad_message(&s->be->invalid_rep, s, msg, HTTP_MSG_TRAILERS, s->fe);
                                goto return_bad_res;
                        }
-                       if (compressing) {
-                               http_compression_buffer_end(s, &res->buf, &tmpbuf, 1);
-                               compressing = 0;
-                       }
-                       /* we're in HTTP_MSG_DONE now */
+                                       /* we're in HTTP_MSG_DONE now */
                }
                else {
                        int old_state = msg->msg_state;
-
-                       if (compressing) {
-                               http_compression_buffer_end(s, &res->buf, &tmpbuf, 1);
-                               compressing = 0;
-                       }
-
                        /* other states, DONE...TUNNEL */
                        /* for keep-alive we don't want to forward closes on DONE */
                        if ((txn->flags & TX_CON_WANT_MSK) == TX_CON_WANT_KAL ||
@@ -5714,7 +5722,7 @@ int http_response_forward_body(struct session *s, struct channel *res, int an_bi
        }
 
  missing_data:
-       if (compressing) {
+       if (compressing && consumed_data) {
                http_compression_buffer_end(s, &res->buf, &tmpbuf, 0);
                compressing = 0;
        }