MINOR: filters: Reject mixing legacy and filter-* directives

Do not allow filter-config, filter-enable/filter-disable or filter-sequence
together with an explicit legacy filter directive in the same proxy,
regardless of parsing order. A legacy filter must also be rejected when the
proxy inherits explicit filter-* directives from its defaults section.

Mark configurations created by the legacy filter parser so implicit legacy
filters do not select a mode. Share the explicit-instance/directive check
with proxy_ref_defaults(), ignoring FLT_INST_F_IMPLICIT instances on both
sides. Check for conflicts before invoking parsers or recording directives.

Add native VTC coverage for all four directives, both declaration orders,
inherited directives and response-only instances. Also check that each mode
permits multiple declarations, different proxies can use different modes,
and implicit cache filters can coexist with either explicit syntax.

diff --git a/include/haproxy/filters-t.h b/include/haproxy/filters-t.h
index 48e25a025dff4e46510394edb1784d0fc94baa75..ca6ec88d4c7b7aba06b1422513885a35cc7fb0bc 100644 (file)
--- a/include/haproxy/filters-t.h
+++ b/include/haproxy/filters-t.h
@@ -26,6 +26,7 @@
 
 /* Flags set on a filter config */
 #define FLT_CFG_FL_HTX    0x00000001  /* The filter can filter HTX streams */
+#define FLT_CFG_FL_LEGACY 0x00000002  /* Declared with the legacy "filter" directive */
 
 /* Flags set on a filter instance */
 #define FLT_FL_IS_BACKEND_FILTER  0x0001 /* The filter is a backend filter */

diff --git a/include/haproxy/filters.h b/include/haproxy/filters.h
index 3355b2c93549fc5d189af262ab4495c16ebaf1a7..b9253543b2c655d52a37f46f0eb22b20518ddd39 100644 (file)
--- a/include/haproxy/filters.h
+++ b/include/haproxy/filters.h
@@ -150,6 +150,7 @@ int fcgi_flt_parse_instance(char **args, struct proxy *px, struct filter_instanc
 
 void flt_deinit(struct proxy *p);
 int  flt_check(struct proxy *p);
+int  flt_has_explicit_config(const struct proxy *px);
 int  flt_copy_instances(struct proxy *px, const struct proxy *defpx);
 void flt_free_instances(struct proxy *px);
 int  flt_init_class_refs(struct proxy *px);

diff --git a/reg-tests/filters/config-mode.vtc b/reg-tests/filters/config-mode.vtc
new file mode 100644 (file)
index 0000000..72103e1
--- /dev/null
+++ b/reg-tests/filters/config-mode.vtc
@@ -0,0 +1,452 @@
+varnishtest "Legacy filter declarations and filter-* directives cannot be mixed"
+
+feature ignore_unknown_macro
+
+# The two modes can be selected independently on different proxies. Multiple
+# declarations within one mode are allowed, including instances-mode defaults.
+haproxy h1 -conf-OK {
+    global
+    .if feature(THREAD)
+        thread-groups 1
+    .endif
+
+    defaults plain
+        mode http
+        timeout connect "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout client  "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout server  "${HAPROXY_TEST_TIMEOUT-5s}"
+
+    frontend legacy_fe from plain
+        bind "fd@${legacy_fe}"
+        filter trace
+        filter trace
+        default_backend instances_be
+
+    backend instances_be from plain
+        filter-config trace id a
+        filter-config trace id b enabled
+        filter-enable trace/a
+        filter-disable trace/b
+        filter-sequence request trace/b:before(trace/a)
+
+    backend legacy_be from plain
+        filter trace
+
+    defaults instances
+        mode http
+        timeout connect "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout client  "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout server  "${HAPROXY_TEST_TIMEOUT-5s}"
+        filter-config trace id inherited
+        filter-enable trace/inherited
+
+    frontend instances_fe from instances
+        bind "fd@${instances_fe}"
+        filter-config trace id local
+        filter-enable trace/local
+        filter-sequence response trace/local:before(trace/inherited)
+        default_backend legacy_be
+}
+
+# Implicit cache filters populate both legacy configs and instances. They do
+# not select a mode: either explicit syntax can follow an implicit declaration.
+haproxy h2 -conf-OK {
+    global
+    .if feature(THREAD)
+        thread-groups 1
+    .endif
+
+    defaults
+        mode http
+        timeout connect "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout client  "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout server  "${HAPROXY_TEST_TIMEOUT-5s}"
+
+    cache store
+        total-max-size 1
+
+    backend implicit_then_legacy
+        http-request cache-use store
+        filter cache store
+
+    backend implicit_then_instances
+        http-request cache-use store
+        filter-config trace id test
+        filter-enable trace/test
+        filter-disable cache-store/store
+        filter-sequence response cache-store/store:before(trace/test)
+
+    backend implicit_then_explicit_instance
+        http-request cache-use store
+        filter-config cache-store id store enabled
+}
+
+# Reject every directive in either declaration order, in frontends, backends
+# and listen sections. Also reject legacy filters after inherited directives.
+# Listen inheritance is already forbidden and is covered in defaults-inheritance.
+# Response-only instances must be detected as well as bidirectional ones.
+
+# filter-config (bidirectional)
+# frontend, legacy-first.
+haproxy h3 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+
+    frontend px from filters
+        filter trace
+        filter-config trace id test
+}
+
+# frontend, instances-first.
+haproxy h4 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+
+    frontend px from filters
+        filter-config trace id test
+        filter trace
+}
+
+# frontend, inherited.
+haproxy h5 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+        filter-config trace id test
+
+    frontend px from filters
+        filter trace
+}
+
+# backend, legacy-first.
+haproxy h6 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+
+    backend px from filters
+        filter trace
+        filter-config trace id test
+}
+
+# backend, instances-first.
+haproxy h7 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+
+    backend px from filters
+        filter-config trace id test
+        filter trace
+}
+
+# backend, inherited.
+haproxy h8 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+        filter-config trace id test
+
+    backend px from filters
+        filter trace
+}
+
+# listen, legacy-first.
+haproxy h9 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+
+    listen px from filters
+        filter trace
+        filter-config trace id test
+}
+
+# listen, instances-first.
+haproxy h10 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+
+    listen px from filters
+        filter-config trace id test
+        filter trace
+}
+
+# filter-config (response-only)
+# frontend, legacy-first.
+haproxy h11 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+
+    frontend px from filters
+        filter trace
+        filter-config cache-store id store
+}
+
+# frontend, instances-first.
+haproxy h12 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+
+    frontend px from filters
+        filter-config cache-store id store
+        filter trace
+}
+
+# frontend, inherited.
+haproxy h13 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+        filter-config cache-store id store
+
+    frontend px from filters
+        filter trace
+}
+
+# backend, legacy-first.
+haproxy h14 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+
+    backend px from filters
+        filter trace
+        filter-config cache-store id store
+}
+
+# backend, instances-first.
+haproxy h15 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+
+    backend px from filters
+        filter-config cache-store id store
+        filter trace
+}
+
+# backend, inherited.
+haproxy h16 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+        filter-config cache-store id store
+
+    backend px from filters
+        filter trace
+}
+
+# listen, legacy-first.
+haproxy h17 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+
+    listen px from filters
+        filter trace
+        filter-config cache-store id store
+}
+
+# listen, instances-first.
+haproxy h18 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+
+    listen px from filters
+        filter-config cache-store id store
+        filter trace
+}
+
+# filter-enable
+# frontend, legacy-first.
+haproxy h19 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+
+    frontend px from filters
+        filter trace
+        filter-enable trace/test
+}
+
+# frontend, instances-first.
+haproxy h20 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+
+    frontend px from filters
+        filter-enable trace/test
+        filter trace
+}
+
+# frontend, inherited.
+haproxy h21 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+        filter-enable trace/test
+
+    frontend px from filters
+        filter trace
+}
+
+# backend, legacy-first.
+haproxy h22 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+
+    backend px from filters
+        filter trace
+        filter-enable trace/test
+}
+
+# backend, instances-first.
+haproxy h23 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+
+    backend px from filters
+        filter-enable trace/test
+        filter trace
+}
+
+# backend, inherited.
+haproxy h24 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+        filter-enable trace/test
+
+    backend px from filters
+        filter trace
+}
+
+# listen, legacy-first.
+haproxy h25 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+
+    listen px from filters
+        filter trace
+        filter-enable trace/test
+}
+
+# listen, instances-first.
+haproxy h26 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+
+    listen px from filters
+        filter-enable trace/test
+        filter trace
+}
+
+# filter-disable
+# frontend, legacy-first.
+haproxy h27 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+
+    frontend px from filters
+        filter trace
+        filter-disable trace/test
+}
+
+# frontend, instances-first.
+haproxy h28 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+
+    frontend px from filters
+        filter-disable trace/test
+        filter trace
+}
+
+# frontend, inherited.
+haproxy h29 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+        filter-disable trace/test
+
+    frontend px from filters
+        filter trace
+}
+
+# backend, legacy-first.
+haproxy h30 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+
+    backend px from filters
+        filter trace
+        filter-disable trace/test
+}
+
+# backend, instances-first.
+haproxy h31 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+
+    backend px from filters
+        filter-disable trace/test
+        filter trace
+}
+
+# backend, inherited.
+haproxy h32 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+        filter-disable trace/test
+
+    backend px from filters
+        filter trace
+}
+
+# listen, legacy-first.
+haproxy h33 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+
+    listen px from filters
+        filter trace
+        filter-disable trace/test
+}
+
+# listen, instances-first.
+haproxy h34 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+
+    listen px from filters
+        filter-disable trace/test
+        filter trace
+}
+
+# filter-sequence
+# frontend, legacy-first.
+haproxy h35 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+
+    frontend px from filters
+        filter trace
+        filter-sequence response trace/test:before(trace)
+}
+
+# frontend, instances-first.
+haproxy h36 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+
+    frontend px from filters
+        filter-sequence response trace/test:before(trace)
+        filter trace
+}
+
+# frontend, inherited.
+haproxy h37 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+        filter-sequence response trace/test:before(trace)
+
+    frontend px from filters
+        filter trace
+}
+
+# backend, legacy-first.
+haproxy h38 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+
+    backend px from filters
+        filter trace
+        filter-sequence response trace/test:before(trace)
+}
+
+# backend, instances-first.
+haproxy h39 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+
+    backend px from filters
+        filter-sequence response trace/test:before(trace)
+        filter trace
+}
+
+# backend, inherited.
+haproxy h40 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+        filter-sequence response trace/test:before(trace)
+
+    backend px from filters
+        filter trace
+}
+
+# listen, legacy-first.
+haproxy h41 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+
+    listen px from filters
+        filter trace
+        filter-sequence response trace/test:before(trace)
+}
+
+# listen, instances-first.
+haproxy h42 -conf-BAD {'filter' and 'filter-*' directives cannot be used together in the same proxy} {
+    defaults filters
+
+    listen px from filters
+        filter-sequence response trace/test:before(trace)
+        filter trace
+}

diff --git a/src/filters.c b/src/filters.c
index 080e99643b4448aa7040b940f0d9fe27dabf48f7..9e7006cfbf8f186eacdd92c0e3a576e6512b424f 100644 (file)
--- a/src/filters.c
+++ b/src/filters.c
@@ -318,6 +318,57 @@ list_filters(FILE *out)
        free(filters);
 }
 
+/* Checks for explicit filter instances or enable/sequence directives during
+ * configuration parsing, before the class references are flattened. Inherited
+ * directives count too, but implicit instances do not select a config mode.
+ */
+int flt_has_explicit_config(const struct proxy *px)
+{
+       struct filter_class_ref *ref;
+       struct filter_instance *inst;
+
+       if (!LIST_ISEMPTY(&px->conf.filter_enabled) || !LIST_ISEMPTY(&px->conf.filter_sequences))
+               return 1;
+
+       list_for_each_entry(ref, &px->conf.filter_classes_req, list) {
+               list_for_each_entry(inst, &ref->instances, req.list) {
+                       if (!(inst->flags & FLT_INST_F_IMPLICIT))
+                               return 1;
+               }
+       }
+       list_for_each_entry(ref, &px->conf.filter_classes_res, list) {
+               list_for_each_entry(inst, &ref->instances, res.list) {
+                       if (!(inst->flags & FLT_INST_F_IMPLICIT))
+                               return 1;
+               }
+       }
+       return 0;
+}
+
+/* Reject mixing an explicit legacy "filter" declaration with "filter-*"
+ * directives in the same proxy. Implicit filters may coexist with either mode.
+ */
+static int flt_check_config_mode(struct proxy *px, int legacy, char **err)
+{
+       struct flt_conf *fconf;
+
+       if (legacy) {
+               if (flt_has_explicit_config(px))
+                       goto error;
+       }
+       else {
+               list_for_each_entry(fconf, &px->filter_configs, list) {
+                       if (fconf->flags & FLT_CFG_FL_LEGACY)
+                               goto error;
+               }
+       }
+       return 0;
+
+  error:
+       memprintf(err, "'filter' and 'filter-*' directives cannot be used together in the same proxy (including inherited directives).");
+       return -1;
+}
+
 /*
  * Parses the "filter" keyword. All keywords must be handled by filters
  * themselves
@@ -334,6 +385,9 @@ parse_filter(char **args, int section_type, struct proxy *curpx,
                          file, line, args[0]);
                return -1;
        }
+       if (flt_check_config_mode(curpx, 1, err) < 0)
+               return -1;
+
        if (strcmp(args[0], "filter") == 0) {
                struct flt_kw *kw;
                int cur_arg;
@@ -388,6 +442,7 @@ parse_filter(char **args, int section_type, struct proxy *curpx,
                        goto error;
                }
 
+               fconf->flags |= FLT_CFG_FL_LEGACY;
                LIST_APPEND(&curpx->filter_configs, &fconf->list);
        }
        return 0;
@@ -1928,6 +1983,9 @@ static int parse_filter_config(char **args, int section_type, struct proxy *curp
                return -1;
        }
 
+       if (flt_check_config_mode(curpx, 0, err) < 0)
+               return -1;
+
        inst = NULL;
        if (!*args[cur_arg]) {
                memprintf(err,
@@ -2137,6 +2195,9 @@ static int parse_filter_enable(char **args, int section_type, struct proxy *curp
                return -1;
        }
 
+       if (flt_check_config_mode(curpx, 0, err) < 0)
+               return -1;
+
        for (cur_arg = 1; *args[cur_arg]; cur_arg++) {
                if (parse_filter_enable_entry(args[cur_arg], enable, curpx, file, line, err) < 0)
                        return -1;
@@ -2215,6 +2276,9 @@ static int parse_filter_sequence(char **args, int section_type, struct proxy *cu
                return -1;
        }
 
+       if (flt_check_config_mode(curpx, 0, err) < 0)
+               return -1;
+
        if (!*args[1]) {
                memprintf(err, "missing argument for '%s' in %s '%s'.", args[0], proxy_type_str(curpx), curpx->id);
                goto error;

diff --git a/src/haload.c b/src/haload.c
index 024afc56c6f0477a741ce8a2c25b49caf3d4fcc0..72c3fe19177e06780bf6f4374161b3039dfe0ef9 100644 (file)
--- a/src/haload.c
+++ b/src/haload.c
@@ -2429,6 +2429,7 @@ static int hld_cfg_finalize(void)
                fconf->id = fcgi_flt_id;
                fconf->conf = fcgi_conf;
                fconf->ops = &fcgi_flt_ops;
+               fconf->flags |= FLT_CFG_FL_LEGACY;
                LIST_APPEND(&hld_proxy.filter_configs, &fconf->list);
        }
 

diff --git a/src/proxy.c b/src/proxy.c
index ca7ef8c5ec3990545b88d065697e9a762190dfd2..6a94c2a62ca20cbc236203589e426da5aac2347f 100644 (file)
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -3165,48 +3165,31 @@ int proxy_ref_defaults(struct proxy *px, struct proxy *defpx, char **errmsg)
 
        /* Explicit filter instances and enable/sequence directives may only be
         * inherited by proxies of one type, never by listen or defaults sections.
-        * Class references are always present, so check their instances rather
-        * than the refs lists. Implicit instances do not restrict inheritance.
+        * Implicit instances do not restrict inheritance.
         */
-       {
-               struct filter_class_ref *ref;
-               struct filter_instance *inst;
-               int has_filters = (!LIST_ISEMPTY(&defpx->conf.filter_enabled) ||
-                                  !LIST_ISEMPTY(&defpx->conf.filter_sequences));
-
-               list_for_each_entry(ref, &defpx->conf.filter_classes_req, list) {
-                       list_for_each_entry(inst, &ref->instances, req.list)
-                               has_filters |= !(inst->flags & FLT_INST_F_IMPLICIT);
-               }
-               list_for_each_entry(ref, &defpx->conf.filter_classes_res, list) {
-                       list_for_each_entry(inst, &ref->instances, res.list)
-                               has_filters |= !(inst->flags & FLT_INST_F_IMPLICIT);
-               }
-
-               if (has_filters) {
-                       if (px->cap & PR_CAP_DEF) {
-                               memprintf(errmsg, "a defaults section cannot inherit from a defaults section defining"
-                                        " filter directives (defaults section at %s:%d)",
-                                        defpx->conf.file, defpx->conf.line);
-                               err_code |= ERR_ALERT | ERR_ABORT;
-                               goto out;
-                       }
-                       else if ((px->cap & PR_CAP_LISTEN) == PR_CAP_LISTEN) {
-                               memprintf(errmsg, "a listen section cannot inherit from a defaults section defining"
-                                        " filter directives (defaults section at %s:%d)",
-                                        defpx->conf.file, defpx->conf.line);
-                               err_code |= ERR_ALERT | ERR_ABORT;
-                               goto out;
-                       }
-                       else if ((defcap == PR_CAP_BE || defcap == PR_CAP_FE) && (px->cap & PR_CAP_LISTEN) != defcap) {
-                               memprintf(errmsg, "frontends and backends cannot inherit from the same defaults section"
-                                        " if it defines filter directives (defaults section at %s:%d)",
-                                        defpx->conf.file, defpx->conf.line);
-                               err_code |= ERR_ALERT | ERR_ABORT;
-                               goto out;
-                       }
-                       defpx->cap = (defpx->cap & ~PR_CAP_LISTEN) | (px->cap & PR_CAP_LISTEN);
+       if (flt_has_explicit_config(defpx)) {
+               if (px->cap & PR_CAP_DEF) {
+                       memprintf(errmsg, "a defaults section cannot inherit from a defaults section defining"
+                                " filter directives (defaults section at %s:%d)",
+                                defpx->conf.file, defpx->conf.line);
+                       err_code |= ERR_ALERT | ERR_ABORT;
+                       goto out;
                }
+               else if ((px->cap & PR_CAP_LISTEN) == PR_CAP_LISTEN) {
+                       memprintf(errmsg, "a listen section cannot inherit from a defaults section defining"
+                                " filter directives (defaults section at %s:%d)",
+                                defpx->conf.file, defpx->conf.line);
+                       err_code |= ERR_ALERT | ERR_ABORT;
+                       goto out;
+               }
+               else if ((defcap == PR_CAP_BE || defcap == PR_CAP_FE) && (px->cap & PR_CAP_LISTEN) != defcap) {
+                       memprintf(errmsg, "frontends and backends cannot inherit from the same defaults section"
+                                " if it defines filter directives (defaults section at %s:%d)",
+                                defpx->conf.file, defpx->conf.line);
+                       err_code |= ERR_ALERT | ERR_ABORT;
+                       goto out;
+               }
+               defpx->cap = (defpx->cap & ~PR_CAP_LISTEN) | (px->cap & PR_CAP_LISTEN);
        }
 
        /* The proxy inherits the filter instances of its defaults section. It