From: Christopher Faulet Date: Wed, 16 Sep 2026 09:11:28 +0000 (+0200) Subject: MAJOR: filters: Revert the filter-sequence directive X-Git-Tag: v3.5-dev7~13 X-Git-Url: https://git.kaiwu.me/?a=commitdiff_plain;h=4bd2c91960e14d3aaaffa8e947f8260103b77d4b;p=haproxy.git MAJOR: filters: Revert the filter-sequence directive The filter-sequence directive was introduced to be able to define a different evaluation order of filters for the request and the response. The first usage should be to mix the compression and the decompression, when a specific order must be defined. However, there were several bugs. And working on fixing those bugs, it appeared that the feature was underdesigned. To fix the bugs, the syntax of the directive must be changed. And even this way, it will be difficult to use it properly. In fact, we really underestimate the changes to perform. So we decided to revert the directive for now, especially to remove it from the 3.4. It is not really mandatory on this version. Because of all current bugs, it was probably not used yet on production. On 3.5, the feature will be fully and deeply redesigned because it is mandatory. This patch reverts the following commits: * efe1099c3 ("BUG/MEDIUM: filters: check the filter name before comparing it") * 3eebd5f67 ("REGTESTS: add a test for "filter-sequence" directive") * 8d28c0e37 ("MEDIUM: filters: add "filter-sequence" directive") It must be backported t 3.4. --- diff --git a/doc/configuration.txt b/doc/configuration.txt index b5615ba49..5fe5e4bfb 100644 --- a/doc/configuration.txt +++ b/doc/configuration.txt @@ -6145,7 +6145,6 @@ external-check path X - X X force-persist - - X X force-be-switch - X X - filter - X X X -filter-sequence - X X X fullconn X - X X guid - X X X hash-balance-factor X - X X @@ -7427,7 +7426,8 @@ decompression algo-res ... implicitly enable decompression filter ("decomp-req" or "decomp-res" whether the decompression option targets request, response) automatically. However to prevent misconfiguration, it is recommended to explicitly declare filters - when multiple filters are involved on the proxy. See also "filter-sequence". + when multiple filters are involved on the proxy, because filters are + evaluated in their declaration order. May be used in the following contexts: http, tcp @@ -7533,8 +7533,8 @@ decompression mode-res { none | auto | always } associated decompression filter is automatically enabled. For requests, the filter is called "decomp-req", and for responses, the filter is "decomp-res". In order to prevent misconfigurations, particularly when combining multiple - filters, it is highly recommended to explicitly declare the filters sequence - via the "filter-sequence" directive in the proxy. + filters, it is highly recommended to explicitly declare the filters, because + they are evaluated in their declaration order. Mode "auto", which is only available for HTTP, means haproxy will try to find a compatible algorithms in algo-req or algo-res list to be able to decompress @@ -7567,8 +7567,7 @@ decompression mode-res { none | auto | always } a hostile agent could trivially amplify its traffic 1000x by sending it compressed. - See also "decompression algo-req", "decompression algo-res", and - "filter-sequence". + See also "decompression algo-req", "decompression algo-res", and "filter". default-server [name ] [from ] [param*] @@ -8183,41 +8182,8 @@ filter [param*] server srv1 192.168.0.1:80 - See also : section 9., "filter-sequence" + See also : section 9. -filter-sequence { request | response } - - Specifies in which order filters declared on the proxy should be - executed. - - May be used in the following contexts: tcp, http - - May be used in sections: defaults | frontend | listen | backend - no | yes | yes | yes - - Comma-separated list of filter names () to specify in which - order filters declared on the proxy should be executed, for request - or response path, respectively. - - When filter-sequence is not specified for a given path (ie: request vs - response), the order in which filters are declared on the proxy is used. - - If filter-sequence omits some filters that were declared on the proxy, - they will not be executed. This is an effective way of temporarily - disabling a filter without removing it from the configuration. - - Example: - global - lua-load my-filter.lua # defines custom "lua.my-filter" - frontend myfront - filter comp-req - filter comp-res - filter lua.my-filter - - filter-sequence request lua.my-filter,comp-req - filter-sequence response lua.my-filter,comp-res - - See also : "filter" fullconn Specify at what backend load the servers will reach their maxconn diff --git a/include/haproxy/filters-t.h b/include/haproxy/filters-t.h index 19e20fc3d..ff1d82be3 100644 --- a/include/haproxy/filters-t.h +++ b/include/haproxy/filters-t.h @@ -215,12 +215,6 @@ struct flt_conf { unsigned int flags; /* FLT_CFG_FL_* */ }; -struct filter_sequence_elt { - char *flt_name; /* filter name (set during parsing) */ - struct flt_conf *flt_conf; /* associated filter conf (set after parsing) */ - struct list list; /* list element */ -}; - /* * Structure reprensenting a filter instance attached to a stream * diff --git a/include/haproxy/proxy-t.h b/include/haproxy/proxy-t.h index aca81a192..64a555f69 100644 --- a/include/haproxy/proxy-t.h +++ b/include/haproxy/proxy-t.h @@ -517,12 +517,6 @@ struct proxy { * name is used */ struct list filter_configs; /* list of the filters that are declared on this proxy */ - struct { /* sequence in which declared filters on the proxy should be executed - * (list of filter_sequence_elt) - */ - struct list req; /* during request handling */ - struct list res; /* during response handling */ - } filter_sequence; struct guid_node guid; /* GUID global tree node */ struct mt_list watcher_list; /* list of elems which currently references this proxy instance (currently only used with backends) */ diff --git a/reg-tests/filters/dummy_filters.lua b/reg-tests/filters/dummy_filters.lua deleted file mode 100644 index af0e5942d..000000000 --- a/reg-tests/filters/dummy_filters.lua +++ /dev/null @@ -1,46 +0,0 @@ -FilterA = {} -FilterA.id = "A filter" -FilterA.flags = filter.FLT_CFG_FL_HTX -FilterA.__index = FilterA - -function FilterA:new() - local filter = {} - setmetatable(filter, FilterA) - return filter -end - -function FilterA:start_analyze(txn, chn) - if chn:is_resp() then - core.Info("FilterA.resp") - else - core.Info("FilterA.req") - end -end - -core.register_filter("FilterA", FilterA, function(flt, args) - return flt -end) - -FilterB = {} -FilterB.id = "A filter" -FilterB.flags = filter.FLT_CFG_FL_HTX -FilterB.__index = FilterB - -function FilterB:new() - local filter = {} - setmetatable(filter, FilterB) - return filter -end - -function FilterB:start_analyze(txn, chn) - if chn:is_resp() then - core.Info("FilterB.resp") - else - core.Info("FilterB.req") - end - -end - -core.register_filter("FilterB", FilterB, function(flt,args) - return flt -end) diff --git a/reg-tests/filters/filter_sequence.vtc b/reg-tests/filters/filter_sequence.vtc deleted file mode 100644 index e4fe05954..000000000 --- a/reg-tests/filters/filter_sequence.vtc +++ /dev/null @@ -1,86 +0,0 @@ -varnishtest "Test filter-sequence feature" - -feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(3.4-dev0)'" -feature ignore_unknown_macro -feature cmd "$HAPROXY_PROGRAM -cc 'feature(LUA)'" - -barrier b1 cond 2 -cyclic - -server s1 { - rxreq - txresp -} -repeat 2 -start - -syslog Slg1 -level info { - recv - expect ~ ".* haproxy ${h1_pid} .* FilterA.req" - recv - expect ~ ".* haproxy ${h1_pid} .* FilterB.req" - recv - expect ~ ".* haproxy ${h1_pid} .* FilterA.resp" - recv - expect ~ ".* haproxy ${h1_pid} .* FilterB.resp" - - barrier b1 sync - - recv - expect ~ ".* haproxy ${h1_pid} .* FilterB.req" - recv - expect ~ ".* haproxy ${h1_pid} .* FilterA.req" - recv - expect ~ ".* haproxy ${h1_pid} .* FilterB.resp" -} -start - -haproxy h1 -conf { - global - tune.lua.bool-sample-conversion normal - lua-load ${testdir}/dummy_filters.lua - log udp@${Slg1_addr}:${Slg1_port} format rfc5424 local0 - - defaults - timeout connect "${HAPROXY_TEST_TIMEOUT-5s}" - timeout client "${HAPROXY_TEST_TIMEOUT-5s}" - timeout server "${HAPROXY_TEST_TIMEOUT-5s}" - - listen fe1 - mode http - filter lua.FilterA - filter lua.FilterB - - bind "fd@${fe1}" - - # test normal sequence (inherited from filter ordering) - - server app1 ${s1_addr}:${s1_port} - - listen fe2 - mode http - filter lua.FilterA - filter lua.FilterB - - # test forced filter-sequence on request - filter-sequence request lua.FilterB,lua.FilterA - # ignore filterA on response - filter-sequence response lua.FilterB - - bind "fd@${fe2}" - - server app1 ${s1_addr}:${s1_port} -} -start - -client c0 -connect ${h1_fe1_sock} { - txreq -url "/" - rxresp - expect resp.status == 200 -} -start -wait - -# Wait matching log messages -barrier b1 sync - -client c1 -connect ${h1_fe2_sock} { - txreq -url "/" - rxresp - expect resp.status == 200 -} -start -wait - -syslog Slg1 -wait diff --git a/src/filters.c b/src/filters.c index a972c7218..d985de3fa 100644 --- a/src/filters.c +++ b/src/filters.c @@ -250,8 +250,6 @@ parse_filter(char **args, int section_type, struct proxy *curpx, cur_arg = 1; kw = flt_find_kw(args[cur_arg]); if (kw) { - /* default name is keyword name, unless overridden by parse func */ - fconf->name = kw->kw; if (!kw->parse) { memprintf(err, "parsing [%s:%d] : '%s' : " "'%s' option is not implemented in this version (check build options).", @@ -298,136 +296,6 @@ parse_filter(char **args, int section_type, struct proxy *curpx, } -/* - * Parses the "filter-sequence" keyword - */ -static int -parse_filter_sequence(char **args, int section_type, struct proxy *curpx, - const struct proxy *defpx, const char *file, int line, char **err) -{ - /* filter-sequence cannot be defined on a default proxy */ - if (curpx == defpx) { - memprintf(err, "parsing [%s:%d] : %s is not allowed in a 'default' section.", - file, line, args[0]); - return -1; - } - if (strcmp(args[0], "filter-sequence") == 0) { - struct list *list; - char *str; - size_t cur_sep; - - if (!*args[1]) { - memprintf(err, - "parsing [%s:%d] : missing argument for '%s' in %s '%s'.", - file, line, args[0], proxy_type_str(curpx), curpx->id); - goto error; - } - - if (strcmp(args[1], "request") == 0) - list = &curpx->filter_sequence.req; - else if (strcmp(args[1], "response") == 0) - list = &curpx->filter_sequence.res; - else { - memprintf(err, - "parsing [%s:%d] : expected either 'request' or 'response' for '%s' in %s '%s'.", - file, line, args[0], proxy_type_str(curpx), curpx->id); - goto error; - } - - if (!*args[2]) { - memprintf(err, - "parsing [%s:%d] : missing filter list for '%s' in %s '%s'.", - file, line, args[0], proxy_type_str(curpx), curpx->id); - goto error; - } - - str = args[2]; - while (str[0]) { - struct filter_sequence_elt *elt; - - elt = calloc(1, sizeof(*elt)); - if (!elt) { - memprintf(err, "'%s %s' : out of memory", args[0], args[1]); - goto error; - } - - cur_sep = strcspn(str, ","); - - elt->flt_name = my_strndup(str, cur_sep); - if (!elt->flt_name) { - ha_free(&elt); - goto error; - } - - LIST_APPEND(list, &elt->list); - - if (str[cur_sep]) - str += cur_sep + 1; - else - str += cur_sep; - } - } - - return 0; - - error: - return -1; -} - -static int compile_filter_sequence_elt(struct proxy *px, struct filter_sequence_elt *elt, char **errmsg) -{ - struct flt_conf *fconf; - int ret = ERR_NONE; - - list_for_each_entry(fconf, &px->filter_configs, list) { - if (fconf->name && strcmp(elt->flt_name, fconf->name) == 0) { - elt->flt_conf = fconf; - break; - } - } - if (!elt->flt_conf) { - memprintf(errmsg, "invalid filter name: '%s' is not defined on the proxy", elt->flt_name); - ret = ERR_FATAL; - } - - return ret; -} - -/* after config is checked, time to resolve filter-sequence (both request and response) - * used on the proxy in order to associate filter names with valid flt_conf entries - * this will help decrease filter lookup time during runtime (filter ids are compared - * using their address, not string content) - */ -static int postcheck_filter_sequence(struct proxy *px) -{ - struct filter_sequence_elt *elt; - char *errmsg = NULL; - int ret = ERR_NONE; - - list_for_each_entry(elt, &px->filter_sequence.req, list) { - ret = compile_filter_sequence_elt(px, elt, &errmsg); - if (ret & ERR_CODE) { - memprintf(&errmsg, "error while postparsing request filter-sequence '%s' : %s", elt->flt_name, errmsg); - goto error; - } - } - list_for_each_entry(elt, &px->filter_sequence.res, list) { - ret = compile_filter_sequence_elt(px, elt, &errmsg); - if (ret & ERR_CODE) { - memprintf(&errmsg, "error while postparsing response filter-sequence '%s' : %s", elt->flt_name, errmsg); - goto error; - } - } - - return ret; - - error: - ha_alert("%s: %s\n", px->id, errmsg); - ha_free(&errmsg); - return ret; -} -REGISTER_POST_PROXY_CHECK(postcheck_filter_sequence); - /* * Calls 'init' callback for all filters attached to a proxy. This happens after * the configuration parsing. Filters can finish to fill their config. Returns @@ -533,7 +401,6 @@ void flt_deinit(struct proxy *proxy) { struct flt_conf *fconf, *back; - struct filter_sequence_elt *fsequence, *fsequenceb; list_for_each_entry_safe(fconf, back, &proxy->filter_configs, list) { if (fconf->ops->deinit) @@ -541,16 +408,6 @@ flt_deinit(struct proxy *proxy) LIST_DELETE(&fconf->list); free(fconf); } - list_for_each_entry_safe(fsequence, fsequenceb, &proxy->filter_sequence.req, list) { - LIST_DEL_INIT(&fsequence->list); - ha_free(&fsequence->flt_name); - ha_free(&fsequence); - } - list_for_each_entry_safe(fsequence, fsequenceb, &proxy->filter_sequence.res, list) { - LIST_DEL_INIT(&fsequence->list); - ha_free(&fsequence->flt_name); - ha_free(&fsequence); - } } /* @@ -581,7 +438,7 @@ flt_deinit_all_per_thread() /* Attaches a filter to a stream. Returns -1 if an error occurs, 0 otherwise. */ static int -flt_stream_add_filter(struct stream *s, struct proxy *px, struct flt_conf *fconf, unsigned int flags) +flt_stream_add_filter(struct stream *s, struct flt_conf *fconf, unsigned int flags) { struct filter *f; @@ -604,38 +461,18 @@ flt_stream_add_filter(struct stream *s, struct proxy *px, struct flt_conf *fconf } LIST_APPEND(&strm_flt(s)->filters, &f->list); - LIST_INIT(&f->req_list); - LIST_INIT(&f->res_list); - /* use filter config ordering unless filter-sequence says otherwise */ - if (LIST_ISEMPTY(&px->filter_sequence.req)) - LIST_APPEND(&s->req.flt.filters, &f->req_list); - if (LIST_ISEMPTY(&px->filter_sequence.res)) - LIST_APPEND(&s->res.flt.filters, &f->res_list); + /* for now f->req_list == f->res_list to preserve + * historical behavior, but the ordering will change + * in the future + */ + LIST_APPEND(&s->req.flt.filters, &f->req_list); + LIST_APPEND(&s->res.flt.filters, &f->res_list); strm_flt(s)->flags |= STRM_FLT_FL_HAS_FILTERS; return 0; } -static void flt_stream_organize_filters(struct stream *s, struct proxy *px) -{ - struct filter_sequence_elt *fsequence; - struct filter *filter; - - list_for_each_entry(fsequence, &px->filter_sequence.req, list) { - list_for_each_entry(filter, &strm_flt(s)->filters, list) { - if (filter->config == fsequence->flt_conf && !LIST_INLIST(&filter->req_list)) - LIST_APPEND(&s->req.flt.filters, &filter->req_list); - } - } - list_for_each_entry(fsequence, &px->filter_sequence.res, list) { - list_for_each_entry(filter, &strm_flt(s)->filters, list) { - if (filter->config == fsequence->flt_conf && !LIST_INLIST(&filter->res_list)) - LIST_APPEND(&s->res.flt.filters, &filter->res_list); - } - } -} - /* * Called when a stream is created. It attaches all frontend filters to the * stream. Returns -1 if an error occurs, 0 otherwise. @@ -652,10 +489,9 @@ flt_stream_init(struct stream *s) memset(&s->res.flt, 0, sizeof(s->res.flt)); LIST_INIT(&s->res.flt.filters); list_for_each_entry(fconf, &strm_fe(s)->filter_configs, list) { - if (flt_stream_add_filter(s, strm_fe(s), fconf, 0) < 0) + if (flt_stream_add_filter(s, fconf, 0) < 0) return -1; } - flt_stream_organize_filters(s, strm_fe(s)); return 0; } @@ -769,12 +605,10 @@ flt_set_stream_backend(struct stream *s, struct proxy *be) goto end; list_for_each_entry(fconf, &be->filter_configs, list) { - if (flt_stream_add_filter(s, be, fconf, FLT_FL_IS_BACKEND_FILTER) < 0) + if (flt_stream_add_filter(s, fconf, FLT_FL_IS_BACKEND_FILTER) < 0) return -1; } - flt_stream_organize_filters(s, be); - end: list_for_each_entry(filter, &strm_flt(s)->filters, list) { if (FLT_OPS(filter)->stream_set_backend) { @@ -1411,7 +1245,6 @@ handle_analyzer_result(struct stream *s, struct channel *chn, * not enabled. */ static struct cfg_kw_list cfg_kws = {ILH, { { CFG_LISTEN, "filter", parse_filter }, - { CFG_LISTEN, "filter-sequence", parse_filter_sequence }, { 0, NULL, NULL }, } }; diff --git a/src/proxy.c b/src/proxy.c index b9efedaff..d50da98fa 100644 --- a/src/proxy.c +++ b/src/proxy.c @@ -1627,8 +1627,6 @@ void init_new_proxy(struct proxy *p) LIST_INIT(&p->conf.lf_checks); LIST_INIT(&p->filter_configs); LIST_INIT(&p->tcpcheck.preset_vars); - LIST_INIT(&p->filter_sequence.req); - LIST_INIT(&p->filter_sequence.res); MT_LIST_INIT(&p->lbprm.lb_free_list);