MEDIUM: flt_http_comp: split "compression" filter in 2 distinct filters

Existing "compression" filter is a multi-purpose filter that will try
to compress both requests and responses according to "compression"
settings, such as "compression direction".

One of the pre-requisite work identified to implement decompression
filter is that we needed a way to manually define the sequence of
enabled filters to chain them in the proper order to make
compression and decompression chains work as expected in regard
to the intended use-case.

Due to the current nature of the "compression" filter this was not
possible, because the filter has a combined action as it will try
to compress both requests and responses, and as we are about to
implement "filter-sequence" directive, we will not be able to
change the order of execution of the compression filter between
requests and responses.

A possible solution we identified to solve this issue is to split the
existing "compression" filter into 2 distinct filters, one which is
request-oriented, "comp-req", and another one which is response-oriented
"comp-res". This is what we are doing in this commit. Compression logic
in itself is unchanged, "comp-req" will only aim to compress the request
while "comp-res" will try to compress the response. Both filters will
still be invoked on request and responses hooks, but they only do their
part of the job.

From now on, to compress both requests and responses, both filters have
to be enabled on the proxy. To preserve original behavior, the "compression"
filter is still supported, what it does is that it instantiates both
"comp-req" and "comp-res" filters implicitly, as the compression filter is
now effectively split into 2 separate filters under the hood.

When using "comp-res" and "comp-req" filters explicitly, the use of the
"compression direction" setting is not relevant anymore. Indeed, the
compression direction is assumed as soon as one or both filters are
enabled. Thus "compression direction" is kept as a legacy option in
order to configure the "compression" generic filter.

Documentation was updated.

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 5311f49198db1924805d60c7702c3365efbd4fff..105a7178ff7fe9045e8c9e2fbab626cb3f5ee607 100644 (file)
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -6935,12 +6935,16 @@ compression offload
 
   See also : "compression type", "compression algo", "compression direction"
 
-compression direction <direction>
+compression direction <direction> (deprecated)
   Makes haproxy able to compress both requests and responses.
   Valid values are "request", to compress only requests, "response", to
   compress only responses, or "both", when you want to compress both.
   The default value is "response".
 
+  This directive is only relevant when legacy "filter compression" was
+  enabled, as with explicit comp-req and comp-res filters compression
+  direction is redundant.
+
   May be used in the following contexts: http
 
   See also : "compression type", "compression algo", "compression offload"
@@ -29904,7 +29908,21 @@ a server by adding some latencies in the processing.
 9.2. HTTP compression
 ---------------------
 
-filter compression
+filter comp-req
+
+Enables filter that explicitly tries to compress HTTP requests according to
+"compression" settings. Implicitly sets "compression direction request".
+
+filter comp-res
+
+Enables filter that explicitly tries to compress HTTP responses according to
+"compression" settings. Implicitly sets "compression direction response"
+
+filter compression (deprecated)
+
+Alias for backward compatibility purposes that is functionnally equivalent to
+enabling both "comp-req" and "comp-res" filter. "compression" keyword must be
+used to configure appropriate behavior:
 
 The HTTP compression has been moved in a filter in HAProxy 1.7. "compression"
 keyword must still be used to enable and configure the HTTP compression. And

diff --git a/include/haproxy/filters.h b/include/haproxy/filters.h
index d2c5dd4075a2f9356a5a256c09e38dc0358362af..c59fed3c89f26442d643cab1e05742d44d84f534 100644 (file)
--- a/include/haproxy/filters.h
+++ b/include/haproxy/filters.h
@@ -28,7 +28,9 @@
 #include <haproxy/stream-t.h>
 
 extern const char *trace_flt_id;
-extern const char *http_comp_flt_id;
+extern const char *http_comp_req_flt_id;
+extern const char *http_comp_res_flt_id;
+
 extern const char *cache_store_flt_id;
 extern const char *spoe_filter_id;
 extern const char *fcgi_flt_id;

diff --git a/src/cache.c b/src/cache.c
index fd3d7c5d7c1d5cff35a007e638e237d5dfcd71ba..dfb9f6bf5f29dddd0e1f61db7668cd7244a1bcd5 100644 (file)
--- a/src/cache.c
+++ b/src/cache.c
@@ -626,7 +626,7 @@ cache_store_check(struct proxy *px, struct flt_conf *fconf)
                                return 1;
                        }
                }
-               else if (f->id == http_comp_flt_id)
+               else if (f->id == http_comp_req_flt_id || f->id == http_comp_res_flt_id)
                        comp = 1;
                else if (f->id == fcgi_flt_id)
                        continue;

diff --git a/src/fcgi-app.c b/src/fcgi-app.c
index 58ad007284c8823335ece217c8cff9f900114aee..dfa9af5a61313d4a6caa51d65ad4b1df01f4fabd 100644 (file)
--- a/src/fcgi-app.c
+++ b/src/fcgi-app.c
@@ -221,7 +221,8 @@ static int fcgi_flt_check(struct proxy *px, struct flt_conf *fconf)
        }
 
        list_for_each_entry(f, &px->filter_configs, list) {
-               if (f->id == http_comp_flt_id || f->id == cache_store_flt_id)
+               if (f->id == http_comp_req_flt_id || f->id == http_comp_res_flt_id ||
+                   f->id == cache_store_flt_id)
                        continue;
                else if ((f->id == fconf->id) && f->conf != fcgi_conf) {
                        ha_alert("proxy '%s' : only one fcgi-app supported per backend.\n",

diff --git a/src/flt_http_comp.c b/src/flt_http_comp.c
index 6a744f401bfcc0f8f99d6ebd95fc8dff6d57c4d0..98733138b8118306cc46a62173b81e015d443caf 100644 (file)
--- a/src/flt_http_comp.c
+++ b/src/flt_http_comp.c
@@ -27,9 +27,11 @@
 
 #define COMP_STATE_PROCESSING 0x01
 
-const char *http_comp_flt_id = "compression filter";
+const char *http_comp_req_flt_id = "comp-req filter";
+const char *http_comp_res_flt_id = "comp-res filter";
 
-struct flt_ops comp_ops;
+struct flt_ops comp_req_ops;
+struct flt_ops comp_res_ops;
 
 struct comp_state {
        /*
@@ -198,33 +200,59 @@ fail:
 }
 
 static int
-comp_http_headers(struct stream *s, struct filter *filter, struct http_msg *msg)
+comp_req_http_headers(struct stream *s, struct filter *filter, struct http_msg *msg)
 {
        struct comp_state *st = filter->ctx;
        int comp_flags = 0;
 
        if (!strm_fe(s)->comp && !s->be->comp)
                goto end;
+
        if (strm_fe(s)->comp)
                comp_flags |= strm_fe(s)->comp->flags;
        if (s->be->comp)
                comp_flags |= s->be->comp->flags;
 
+       if (!(comp_flags & COMP_FL_DIR_REQ))
+               goto end;
+
        if (!(msg->chn->flags & CF_ISRESP)) {
-               if (comp_flags & COMP_FL_DIR_REQ) {
-                           comp_prepare_compress_request(st, s, msg);
-                           if (st->comp_algo[COMP_DIR_REQ]) {
-                                   if (!set_compression_header(st, s, msg))
-                                           goto end;
-                                   register_data_filter(s, msg->chn, filter);
-                                   st->flags |= COMP_STATE_PROCESSING;
-                           }
+               comp_prepare_compress_request(st, s, msg);
+               if (st->comp_algo[COMP_DIR_REQ]) {
+                       if (!set_compression_header(st, s, msg))
+                               goto end;
+                       register_data_filter(s, msg->chn, filter);
+                       st->flags |= COMP_STATE_PROCESSING;
                }
-               if (comp_flags & COMP_FL_DIR_RES)
-                       select_compression_request_header(st, s, msg);
-       } else if (comp_flags & COMP_FL_DIR_RES) {
+       }
+
+  end:
+       return 1;
+}
+
+static int
+comp_res_http_headers(struct stream *s, struct filter *filter, struct http_msg *msg)
+{
+       struct comp_state *st = filter->ctx;
+       int comp_flags = 0;
+
+       if (!strm_fe(s)->comp && !s->be->comp)
+               goto end;
+
+       if (strm_fe(s)->comp)
+               comp_flags |= strm_fe(s)->comp->flags;
+       if (s->be->comp)
+               comp_flags |= s->be->comp->flags;
+
+       if (!(comp_flags & COMP_FL_DIR_RES))
+               goto end;
+
+
+       if (!(msg->chn->flags & CF_ISRESP))
+               select_compression_request_header(st, s, msg);
+       else {
                /* Response headers have already been checked in
-                * comp_http_post_analyze callback. */
+                * comp_res_http_post_analyze callback. */
                if (st->comp_algo[COMP_DIR_RES]) {
                        if (!set_compression_header(st, s, msg))
                                goto end;
@@ -238,8 +266,8 @@ comp_http_headers(struct stream *s, struct filter *filter, struct http_msg *msg)
 }
 
 static int
-comp_http_post_analyze(struct stream *s, struct filter *filter,
-                      struct channel *chn, unsigned an_bit)
+comp_res_http_post_analyze(struct stream *s, struct filter *filter,
+                           struct channel *chn, unsigned an_bit)
 {
        struct http_txn   *txn = s->txn;
        struct http_msg   *msg = &txn->rsp;
@@ -259,19 +287,13 @@ comp_http_post_analyze(struct stream *s, struct filter *filter,
 
 static int
 comp_http_payload(struct stream *s, struct filter *filter, struct http_msg *msg,
-                 unsigned int offset, unsigned int len)
+                 unsigned int offset, unsigned int len, int dir)
 {
        struct comp_state *st = filter->ctx;
        struct htx *htx = htxbuf(&msg->chn->buf);
        struct htx_ret htxret = htx_find_offset(htx, offset);
        struct htx_blk *blk, *next;
        int ret, consumed = 0, to_forward = 0, last = 0;
-       int dir;
-
-       if (msg->chn->flags & CF_ISRESP)
-               dir = COMP_DIR_RES;
-       else
-               dir = COMP_DIR_REQ;
 
        blk = htxret.blk;
        offset = htxret.ret;
@@ -384,10 +406,29 @@ comp_http_payload(struct stream *s, struct filter *filter, struct http_msg *msg,
        return -1;
 }
 
+static int
+comp_req_http_payload(struct stream *s, struct filter *filter, struct http_msg *msg,
+                      unsigned int offset, unsigned int len)
+{
+       if (msg->chn->flags & CF_ISRESP)
+               return 0;
+
+       return comp_http_payload(s, filter, msg, offset, len, COMP_DIR_REQ);
+}
+
+static int
+comp_res_http_payload(struct stream *s, struct filter *filter, struct http_msg *msg,
+                      unsigned int offset, unsigned int len)
+{
+       if (!(msg->chn->flags & CF_ISRESP))
+               return 0;
+
+       return comp_http_payload(s, filter, msg, offset, len, COMP_DIR_RES);
+}
 
 static int
-comp_http_end(struct stream *s, struct filter *filter,
-             struct http_msg *msg)
+comp_res_http_end(struct stream *s, struct filter *filter,
+                 struct http_msg *msg)
 {
        struct comp_state *st = filter->ctx;
 
@@ -769,17 +810,28 @@ htx_compression_buffer_end(struct comp_state *st, struct buffer *out, int end, i
 
 
 /***********************************************************************/
-struct flt_ops comp_ops = {
+
+struct flt_ops comp_req_ops = {
        .init              = comp_flt_init,
 
        .attach = comp_strm_init,
        .detach = comp_strm_deinit,
 
-       .channel_post_analyze  = comp_http_post_analyze,
+       .http_headers          = comp_req_http_headers,
+       .http_payload          = comp_req_http_payload,
+};
+
+struct flt_ops comp_res_ops = {
+       .init              = comp_flt_init,
 
-       .http_headers          = comp_http_headers,
-       .http_payload          = comp_http_payload,
-       .http_end              = comp_http_end,
+       .attach = comp_strm_init,
+       .detach = comp_strm_deinit,
+
+       .channel_post_analyze  = comp_res_http_post_analyze,
+
+       .http_headers          = comp_res_http_headers,
+       .http_payload          = comp_res_http_payload,
+       .http_end              = comp_res_http_end,
 };
 
 /* returns compression options from <proxy> proxy or allocates them if
@@ -985,27 +1037,109 @@ parse_http_comp_flt(char **args, int *cur_arg, struct proxy *px,
                     struct flt_conf *fconf, char **err, void *private)
 {
        struct flt_conf *fc, *back;
+       struct flt_conf *fconf_res;
 
        list_for_each_entry_safe(fc, back, &px->filter_configs, list) {
-               if (fc->id == http_comp_flt_id) {
+               if (fc->id == http_comp_req_flt_id || fc->id == http_comp_res_flt_id) {
                        memprintf(err, "%s: Proxy supports only one compression filter\n", px->id);
                        return -1;
                }
        }
 
-       fconf->id   = http_comp_flt_id;
+       fconf->id   = http_comp_req_flt_id;
        fconf->conf = NULL;
-       fconf->ops  = &comp_ops;
+       fconf->ops  = &comp_req_ops;
+
+       /* FILTER API prepared a single filter_conf struct as it is meant to
+        * initialize exactly one fconf per keyword, but with the "compression"
+        * filter, for retro-compatibility we want to emulate the historical
+        * behavior which is to compress both requests and responses, so to
+        * emulate that we manually initialize the comp-res filter as well
+        */
+       fconf_res = calloc(1, sizeof(*fconf_res));
+       if (!fconf_res) {
+               memprintf(err, "'%s' : out of memory", args[0]);
+               return -1;
+       }
+       fconf_res->id = http_comp_res_flt_id;
+       fconf_res->conf = NULL;
+       fconf_res->ops = &comp_res_ops;
+
+       /* manually add the fconf_res to the list because filter API doesn't
+        * know about it
+        */
+       LIST_APPEND(&px->filter_configs, &fconf_res->list);
+
+
        (*cur_arg)++;
 
        return 0;
 }
 
+static int
+parse_http_comp_req_flt(char **args, int *cur_arg, struct proxy *px,
+                        struct flt_conf *fconf, char **err, void *private)
+{
+       struct flt_conf *fc, *back;
+       struct comp *comp;
+
+       list_for_each_entry_safe(fc, back, &px->filter_configs, list) {
+               if (fc->id == http_comp_req_flt_id) {
+                       memprintf(err, "%s: Proxy supports only one comp-req filter\n", px->id);
+                       return -1;
+               }
+       }
+
+       comp = proxy_get_comp(px, 0);
+       if (comp == NULL) {
+               memprintf(err, "memory failure\n");
+               return -1;
+       }
+       comp->flags |= COMP_FL_DIR_REQ;
+
+       fconf->id   = http_comp_req_flt_id;
+       fconf->conf = NULL;
+       fconf->ops  = &comp_req_ops;
+       (*cur_arg)++;
+
+       return 0;
+}
+
+static int
+parse_http_comp_res_flt(char **args, int *cur_arg, struct proxy *px,
+                        struct flt_conf *fconf, char **err, void *private)
+{
+       struct flt_conf *fc, *back;
+       struct comp *comp;
+
+       list_for_each_entry_safe(fc, back, &px->filter_configs, list) {
+               if (fc->id == http_comp_res_flt_id) {
+                       memprintf(err, "%s: Proxy supports only one comp-res filter\n", px->id);
+                       return -1;
+               }
+       }
+
+       comp = proxy_get_comp(px, 0);
+       if (comp == NULL) {
+               memprintf(err, "memory failure\n");
+               return -1;
+       }
+       comp->flags |= COMP_FL_DIR_RES;
+
+       fconf->id   = http_comp_res_flt_id;
+       fconf->conf = NULL;
+       fconf->ops  = &comp_res_ops;
+       (*cur_arg)++;
+
+       return 0;
+}
 
 int
 check_implicit_http_comp_flt(struct proxy *proxy)
 {
        struct flt_conf *fconf;
+       struct flt_conf *fconf_req = NULL;
+       struct flt_conf *fconf_res = NULL;
        int explicit = 0;
        int comp = 0;
        int err = 0;
@@ -1014,7 +1148,7 @@ check_implicit_http_comp_flt(struct proxy *proxy)
                goto end;
        if (!LIST_ISEMPTY(&proxy->filter_configs)) {
                list_for_each_entry(fconf, &proxy->filter_configs, list) {
-                       if (fconf->id == http_comp_flt_id)
+                       if (fconf->id == http_comp_req_flt_id || fconf->id == http_comp_res_flt_id)
                                comp = 1;
                        else if (fconf->id == cache_store_flt_id) {
                                if (comp) {
@@ -1042,17 +1176,25 @@ check_implicit_http_comp_flt(struct proxy *proxy)
 
        /* Implicit declaration of the compression filter is always the last
         * one */
-       fconf = calloc(1, sizeof(*fconf));
-       if (!fconf) {
+       fconf_req = calloc(1, sizeof(*fconf));
+       fconf_res = calloc(1, sizeof(*fconf));
+       if (!fconf_req || !fconf_res) {
                ha_alert("config: %s '%s': out of memory\n",
                         proxy_type_str(proxy), proxy->id);
+               ha_free(&fconf_req);
+               ha_free(&fconf_res);
                err++;
                goto end;
        }
-       fconf->id   = http_comp_flt_id;
-       fconf->conf = NULL;
-       fconf->ops  = &comp_ops;
-       LIST_APPEND(&proxy->filter_configs, &fconf->list);
+       fconf_req->id   = http_comp_req_flt_id;
+       fconf_req->conf = NULL;
+       fconf_req->ops  = &comp_req_ops;
+       LIST_APPEND(&proxy->filter_configs, &fconf_req->list);
+
+       fconf_res->id   = http_comp_res_flt_id;
+       fconf_res->conf = NULL;
+       fconf_res->ops  = &comp_res_ops;
+       LIST_APPEND(&proxy->filter_configs, &fconf_res->list);
  end:
        return err;
 }
@@ -1087,7 +1229,7 @@ smp_fetch_res_comp_algo(const struct arg *args, struct sample *smp,
                return 0;
 
        list_for_each_entry(filter, &strm_flt(smp->strm)->filters, list) {
-               if (FLT_ID(filter) != http_comp_flt_id)
+               if (FLT_ID(filter) != http_comp_res_flt_id)
                        continue;
 
                if (!(st = filter->ctx))
@@ -1114,6 +1256,8 @@ INITCALL1(STG_REGISTER, cfg_register_keywords, &cfg_kws);
 /* Declare the filter parser for "compression" keyword */
 static struct flt_kw_list filter_kws = { "COMP", { }, {
                { "compression", parse_http_comp_flt, NULL },
+               { "comp-req", parse_http_comp_req_flt, NULL },
+               { "comp-res", parse_http_comp_res_flt, NULL },
                { NULL, NULL, NULL },
        }
 };