BUG/MINOR: filters: fix silent compression disable with legacy "filter compression...

The legacy "filter compression" declaration stored a pointer on the proxy
compression options in the filter configuration at parsing time. When the
"compression" directives were placed after the filter declaration, the
options were not allocated yet, so the filter kept a NULL pointer and the
compression was silently disabled at runtime.

The compression options are now allocated when the filter is declared, the
"compression" directives parsed later reuse the same structure, in both
declaration orders.

This bug was introduced by the commit d759f474 ("MINOR: flt-http-comp: Use
compression options as filter config"), which started to store the pointer
on the proxy compression options in the filter configuration and to check
it at runtime, while it was previously always NULL and the options were
resolved at runtime through the frontend and backend proxies. There is no
need to backport this fix, it is a 3.5-specific issue.

diff --git a/reg-tests/compression/filter-before-options.vtc b/reg-tests/compression/filter-before-options.vtc
new file mode 100644 (file)
index 0000000..7e5230f
--- /dev/null
+++ b/reg-tests/compression/filter-before-options.vtc
@@ -0,0 +1,54 @@
+varnishtest "Legacy 'filter compression' declared before the compression options"
+
+# Regression test for the legacy "filter compression" declaration placed
+# before the "compression" options. The filter used to capture a NULL
+# pointer on the proxy compression options, parsed later by the "compression"
+# directives, silently disabling the compression. The options are now
+# allocated when the filter is declared.
+
+feature ignore_unknown_macro
+
+server s1 {
+    rxreq
+    txresp -hdr "Content-Type: text/plain" -bodylen 500
+} -repeat 2 -start
+
+haproxy h1 -conf {
+    defaults
+        mode http
+        timeout connect "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout client "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout server "${HAPROXY_TEST_TIMEOUT-5s}"
+
+    # the filter is declared before the options
+    listen li1
+        bind "fd@${li1}"
+        filter compression
+        compression algo gzip
+        compression type text/plain
+        server s1 ${s1_addr}:${s1_port}
+
+    # the filter is declared after the options
+    listen li2
+        bind "fd@${li2}"
+        compression algo gzip
+        compression type text/plain
+        filter compression
+        server s1 ${s1_addr}:${s1_port}
+} -start
+
+client c1 -connect ${h1_li1_sock} {
+    txreq -url /a -hdr "Accept-Encoding: gzip"
+    rxresp
+    expect resp.status == 200
+    expect resp.http.content-encoding == gzip
+    gunzip
+} -run
+
+client c2 -connect ${h1_li2_sock} {
+    txreq -url /b -hdr "Accept-Encoding: gzip"
+    rxresp
+    expect resp.status == 200
+    expect resp.http.content-encoding == gzip
+    gunzip
+} -run

diff --git a/src/flt_http_comp.c b/src/flt_http_comp.c
index 8b79e380a8d2e8c86e88db08de5b0fee37b08585..c911b759e21845be09e4d69a262cc69419cb7479 100644 (file)
--- a/src/flt_http_comp.c
+++ b/src/flt_http_comp.c
@@ -1073,6 +1073,7 @@ parse_http_comp_flt(char **args, int *cur_arg, struct proxy *px,
 {
        struct flt_conf *fc, *back;
        struct flt_conf *fconf_res;
+       struct comp *comp;
 
        list_for_each_entry_safe(fc, back, &px->filter_configs, list) {
                if (fc->id == http_comp_req_flt_id || fc->id == http_comp_res_flt_id) {
@@ -1081,9 +1082,20 @@ parse_http_comp_flt(char **args, int *cur_arg, struct proxy *px,
                }
        }
 
+       /* Allocate the compression options now, they will be filled by the
+        * "compression" directives, which may be parsed after this filter
+        * declaration. The filter config keeps a pointer on them, so they
+        * must not be reallocated later.
+        */
+       comp = proxy_get_comp(px, 0);
+       if (!comp) {
+               memprintf(err, "'%s' : out of memory", args[*cur_arg]);
+               return -1;
+       }
+
        fconf->id   = http_comp_req_flt_id;
        fconf->name = "comp-req";
-       fconf->conf = px->comp;
+       fconf->conf = comp;
        fconf->ops  = &comp_req_ops;
 
        /* FILTER API prepared a single filter_conf struct as it is meant to
@@ -1099,7 +1111,7 @@ parse_http_comp_flt(char **args, int *cur_arg, struct proxy *px,
        }
        fconf_res->id = http_comp_res_flt_id;
        fconf_res->name = "comp-res";
-       fconf_res->conf = px->comp;
+       fconf_res->conf = comp;
        fconf_res->ops = &comp_res_ops;
 
        /* manually add the fconf_res to the list because filter API doesn't