From: Christopher Faulet Date: Thu, 12 Nov 2026 10:17:57 +0000 (+0100) Subject: BUG/MEDIUM: filters: order the implicit filters around the explicit ones X-Git-Url: https://git.kaiwu.me/%7B@url%7D?a=commitdiff_plain;h=4930d112a2a20120f053af0cdc106d13b3c07bc3;p=haproxy.git BUG/MEDIUM: filters: order the implicit filters around the explicit ones In the legacy filter mode, flt_reorder_implicit_conf() reordered the implicitly declared core filters but pinned the explicitly declared core filters in their declaration position, treating them as segment boundaries. An implicit filter bucketed after an explicit core filter ended up in a wrong position: with an explicit "filter compression" declaration followed by cache rules, the implicit cache-store filter was placed after the compression filters, and the configuration checks reported bogus errors: proxy 'li': unable to enable the compression filter on response before any cache filter. proxy 'li': unable to enable the compression filter before the cache 'c'. while this configuration is valid: the implicit cache filter must be placed at its canonical position, before the explicit compression filters. Explicitly declared core filters are now placed in the "any other filter" class, like any explicitly declared filter: their manual placement among the explicit filters is the user's responsibility, and only the implicitly declared core filters are moved to their canonical position. The placement policy is implemented in flt_order_class(), which returns the "any other filter" class for an explicit core filter. The order checks reporting conflicts between the compression, decompression and cache filters are updated accordingly: an order error is only reported when the filters involved are explicitly declared. A filter that is automatically ordered never conflicts with an explicitly declared one, since the reordering places them correctly relative to each other. No backport needed. --- diff --git a/src/cache.c b/src/cache.c index 812848974..834bb3720 100644 --- a/src/cache.c +++ b/src/cache.c @@ -708,14 +708,22 @@ static int cache_store_check_order(struct flt_conf *f, void *data) if (f->id == http_comp_req_flt_id) return 0; if (f->id == http_comp_res_flt_id) { - ctx->comp_seen = 1; + /* Only report an explicitly declared compression filter + * placed before the cache: the implicit ones are + * automatically ordered after it. + */ + if (f->flags & FLT_CFG_FL_EXPLICIT) + ctx->comp_seen = 1; return 0; } if (f->id == decomp_req_flt_id) return 0; else if (f->id == decomp_res_flt_id) { - /* The decomp-res filter must be evaluated after the cache. */ - if (ctx->cache_seen) { + /* Only report an explicitly declared decomp-res filter placed + * after an explicitly declared cache: the implicit filters + * are automatically ordered. + */ + if ((f->flags & FLT_CFG_FL_EXPLICIT) && (ctx->fconf->flags & FLT_CFG_FL_EXPLICIT) && ctx->cache_seen) { ha_alert("config: %s '%s': unable to enable the decomp-res filter after " "the cache '%s'.\n", proxy_type_str(ctx->px), ctx->px->id, ctx->cache->id); return 1; diff --git a/src/filters.c b/src/filters.c index a48a822c3..11188feb6 100644 --- a/src/filters.c +++ b/src/filters.c @@ -716,8 +716,12 @@ static int flt_is_core_flt(const struct flt_conf *fconf) * response: fcgi > decomp-res > cache-store > any other filter > * decomp-req > http-comp-res * - * Explicitly declared core filters are never moved and act as segment - * boundaries (see flt_reorder_implicit_conf()). + * Explicitly declared core filters are placed in the "any other filter" + * class, like any explicitly declared filter: their manual placement among + * the explicit filters is the user's responsibility, and a conflicting + * order between explicitly declared filters is reported by their checks. + * Only the implicitly declared core filters are moved to their canonical + * position. */ static int flt_order_class(const struct flt_conf *fconf, unsigned int side) { @@ -730,6 +734,17 @@ static int flt_order_class(const struct flt_conf *fconf, unsigned int side) int is_fcgi = (fconf->id == fcgi_flt_id); #endif + if ((fconf->flags & FLT_CFG_FL_EXPLICIT) && flt_is_core_flt(fconf)) { + switch (side) { + case FLT_SIDE_REQ: + return 1; /* any other filter */ + case FLT_SIDE_RES: + return 3; /* any other filter */ + default: + return 2; /* any other filter */ + } + } + switch (side) { case FLT_SIDE_REQ: if (is_decomp_req) @@ -805,10 +820,12 @@ static void flt_flush_order_segs(struct list *out, struct list seg[], int nb_seg * px->filters_res_configs) are used to fill the stream's request and response * filter lists. * - * Explicitly declared core filters are never moved, they keep their declaration - * order. They delimit segments of filters inside which the implicitly declared - * core filters are sorted, so that an implicit filter never crosses an - * explicitly declared one. + * Explicitly declared core filters are placed in the "any other filter" + * class, like any explicitly declared filter: their manual placement + * between the explicit filters is the user's responsibility, and the + * conflicting orders between explicitly declared filters are reported by + * their checks, which walk the lists before the reordering. Only the + * implicitly declared core filters are moved to their canonical position. */ static void flt_reorder_implicit_conf(struct proxy *px) { @@ -821,47 +838,24 @@ static void flt_reorder_implicit_conf(struct proxy *px) LIST_INIT(&seg[s][i]); } - /* First, the global order, with the 'list' member. */ - LIST_INIT(&out); - list_for_each_entry_safe(fconf, back, &px->filter_configs, list) { - if ((fconf->flags & FLT_CFG_FL_EXPLICIT) && flt_is_core_flt(fconf)) { - /* Explicitly declared core filter: flush the current - * segment before it and keep the filter in place. - */ - flt_flush_order_segs(&out, seg[0], 6); - LIST_DEL_INIT(&fconf->list); - LIST_APPEND(&out, &fconf->list); - continue; - } - LIST_DEL_INIT(&fconf->list); - LIST_APPEND(&seg[0][flt_order_class(fconf, FLT_SIDE_GLOBAL)], &fconf->list); - } - flt_flush_order_segs(&out, seg[0], 6); - LIST_SPLICE(&px->filter_configs, &out); - - /* Then the request and response orders, from the final global order, - * with the 'by_req' and 'by_res' members. Both lists are rebuilt from - * scratch: reset their heads, all the links were initialized by - * flt_conf_add()/flt_conf_insert() or above. + /* Rebuild the three orders in one pass over the declaration order: + * bucket the 'list', 'by_req' and 'by_res' members of every filter + * config by its class, then flush the buckets in class order. Within + * a class, the configs keep their declaration order. */ + LIST_INIT(&out); LIST_INIT(&px->filters_req_configs); LIST_INIT(&px->filters_res_configs); list_for_each_entry_safe(fconf, back, &px->filter_configs, list) { - if ((fconf->flags & FLT_CFG_FL_EXPLICIT) && flt_is_core_flt(fconf)) { - /* Explicitly declared core filter: flush the current - * segments before it and keep the filter in place. - */ - flt_flush_order_segs(&px->filters_req_configs, seg[1], 6); - flt_flush_order_segs(&px->filters_res_configs, seg[2], 6); - LIST_APPEND(&px->filters_req_configs, &fconf->by_req); - LIST_APPEND(&px->filters_res_configs, &fconf->by_res); - continue; - } + LIST_DEL_INIT(&fconf->list); + LIST_APPEND(&seg[0][flt_order_class(fconf, FLT_SIDE_GLOBAL)], &fconf->list); LIST_APPEND(&seg[1][flt_order_class(fconf, FLT_SIDE_REQ)], &fconf->by_req); LIST_APPEND(&seg[2][flt_order_class(fconf, FLT_SIDE_RES)], &fconf->by_res); } + flt_flush_order_segs(&out, seg[0], 6); flt_flush_order_segs(&px->filters_req_configs, seg[1], 6); flt_flush_order_segs(&px->filters_res_configs, seg[2], 6); + LIST_SPLICE(&px->filter_configs, &out); } /* diff --git a/src/flt_decomp.c b/src/flt_decomp.c index 8c1823439..56de068d4 100644 --- a/src/flt_decomp.c +++ b/src/flt_decomp.c @@ -1343,6 +1343,7 @@ struct decomp_check_ctx { int decomp_req_seen; int decomp_res_seen; int cache_seen; + unsigned int cache_flags; }; /* Checks the filter against the decomp filter being checked. It is used @@ -1361,15 +1362,21 @@ static int decomp_check_order(struct flt_conf *f, void *data) } else if (flt_use_legacy_filter(ctx->px)) { if ((ctx->px->cap & PR_CAP_BE) && f->id == decomp_res_flt_id) { - if (ctx->cache_seen) { + /* Only report an explicitly declared cache filter + * placed before decomp-res: the implicit one is + * automatically ordered before it. + */ + if (ctx->cache_seen && (ctx->cache_flags & FLT_CFG_FL_EXPLICIT)) { ha_alert("config: %s '%s': cache filter declared before a decompression filter on the response is invalid.\n", proxy_type_str(ctx->px), ctx->px->id); return 1; } ctx->decomp_res_seen = 1; } - if (f->id == cache_store_flt_id) + if (f->id == cache_store_flt_id) { ctx->cache_seen = 1; + ctx->cache_flags = f->flags; + } } return 0; } diff --git a/src/flt_http_comp.c b/src/flt_http_comp.c index 9649dc5af..b0cfd77ed 100644 --- a/src/flt_http_comp.c +++ b/src/flt_http_comp.c @@ -1312,7 +1312,7 @@ static int http_comp_check_order(struct flt_conf *f, void *data) return 0; } if (flt_use_legacy_filter(ctx->px) && f->id == cache_store_flt_id && - ctx->comp_res_seen) { + (f->flags & FLT_CFG_FL_EXPLICIT) && ctx->comp_res_seen) { ha_alert("config: %s '%s': unable to enable the compression filter on response " "before any cache filter.\n", proxy_type_str(ctx->px), ctx->px->id);