From 27d93c3f942853710ac92b5fdf491dd259f5a90e Mon Sep 17 00:00:00 2001 From: Christopher Faulet Date: Sat, 15 Dec 2018 22:32:02 +0100 Subject: [PATCH] BUG/MAJOR: compression/cache: Make it really works with these both filters Caching the response with the compression enabled was totally broken. To fix the problem, the compression must be done after caching the response. Otherwise it needs to change the cache to store compressed and uncompressed objects for the same ressource. So, because it is not possible for now, it is forbidden to declare the compression filter before the cache one. To ease the configuration, both can be implicitly declared (without "filter" keyword). The compression will automatically be inserted after the cache. Then, to make it works this way, the compression filter has been slighly modified. Now, the response headers are updated after http-response rules evaluations, instead of before. So, if the response contains a "Content-length" header, it will be kept with the response stored in the cache. So this cached response will be able to be served to clients not supporting the compression at all. --- doc/configuration.txt | 24 +++--- src/cache.c | 61 ++++++-------- src/flt_http_comp.c | 180 +++++++++++++++++++++++++++++------------- 3 files changed, 159 insertions(+), 106 deletions(-) diff --git a/doc/configuration.txt b/doc/configuration.txt index d132223313..9ff6755b33 100644 --- a/doc/configuration.txt +++ b/doc/configuration.txt @@ -17716,12 +17716,14 @@ filter compression 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 -when no other filter is used, it is enough. But it is mandatory to explicitly -use a filter line to enable the HTTP compression when two or more filters are -used for the same listener/frontend/backend. This is important to know the -filters evaluation order. +when no other filter is used, it is enough. When used with the cache enabled, +it is also enough. In this case, the compression is always done after the +response is stored in the cache. But it is mandatory to explicitly use a filter +line to enable the HTTP compression when at least one filter other than the +cache is used for the same listener/frontend/backend. This is important to know +the filters evaluation order. -See also : "compression" +See also : "compression" and section 9.4 about the cache filter. 9.3. Stream Processing Offload Engine (SPOE) @@ -17767,12 +17769,14 @@ filter cache The cache uses a filter to store cacheable responses. The HTTP rules "cache-store" and "cache-use" must be used to define how and when to use a cache. By default the correpsonding filter is implicitly defined. And when no -other filter than cache is used, it is enough. But it is mandatory to -explicitly use a filter line to use a cache when two or more filters are used -for the same listener/frontend/backend. This is important to know the filters -evaluation order. +other filters than cache or compression are used, it is enough. In such case, +the compression filter is always evaluated after the cache filter. But it is +mandatory to explicitly use a filter line to use a cache when at least one +filter other than the compression is used for the same +listener/frontend/backend. This is important to know the filters evaluation +order. -See also : section 10 about cache. +See also : section 9.2 about the compression filter and section 10 about cache. 10. Cache --------- diff --git a/src/cache.c b/src/cache.c index 199b865635..0d67840adc 100644 --- a/src/cache.c +++ b/src/cache.c @@ -43,9 +43,7 @@ * messages (legacy implementation) */ #define CACHE_F_HTX 0x00000002 /* The cache is used to store HTX messages */ -#define CACHE_FLT_F_IGNORE_CNT_ENC 0x00000001 /* Ignore 'Content-Encoding' header when response is cached - * if compression is already started */ -#define CACHE_FLT_F_IMPLICIT_DECL 0x00000002 /* The cache filtre was implicitly declared (ie without +#define CACHE_FLT_F_IMPLICIT_DECL 0x00000001 /* The cache filtre was implicitly declared (ie without * the filter keyword) */ const char *cache_store_flt_id = "cache store filter"; @@ -161,7 +159,7 @@ cache_store_check(struct proxy *px, struct flt_conf *fconf) struct cache_flt_conf *cconf = fconf->conf; struct flt_conf *f; struct cache *cache; - int ignore = 0; + int comp = 0; /* resolve the cache name to a ptr in the filter config */ list_for_each_entry(cache, &caches, list) { @@ -186,36 +184,36 @@ cache_store_check(struct proxy *px, struct flt_conf *fconf) * points on the cache filter configuration. */ /* Check all filters for proxy to know if the compression is - * enabled and if it is before or after the cache. When the compression - * is before the cache, nothing special is done. The response is stored - * compressed in the cache. When the compression is after the cache, the - * 'Content-encoding' header must be ignored because the response will - * be stored uncompressed. The compression will be done on the cached - * response too. Also check if the cache filter must be explicitly - * declaired or not. */ + * enabled and if it is after the cache. When the compression is before + * the cache, an error is returned. Also check if the cache filter must + * be explicitly declaired or not. */ list_for_each_entry(f, &px->filter_configs, list) { if (f == fconf) { - ignore = 1; - continue; + /* The compression filter must be evaluated after the cache. */ + if (comp) { + ha_alert("config: %s '%s': unable to enable the compression filter before " + "the cache '%s'.\n", proxy_type_str(px), px->id, cache->id); + return 1; + } } - - if ((f->id != fconf->id) && (cconf->flags & CACHE_FLT_F_IMPLICIT_DECL)) { - ha_alert("config: %s '%s': require an explicit filter declaration " - "to use the cache '%s'.\n", proxy_type_str(px), px->id, cache->id); - return 1; - } - - if (f->id == http_comp_flt_id) { + else if (f->id == http_comp_flt_id) { if (!(px->options2 & PR_O2_USE_HTX)) { ha_alert("config: %s '%s' : compression and cache filters cannot be " "both enabled on non HTX proxy.\n", proxy_type_str(px), px->id); return 1; } - if (ignore) - cconf->flags |= CACHE_FLT_F_IGNORE_CNT_ENC; - break; + comp = 1; } + else if ((f->id != fconf->id) && (cconf->flags & CACHE_FLT_F_IMPLICIT_DECL)) { + /* Implicit declaration is only allowed with the + * compression. For other filters, an implicit + * declaration is required. */ + ha_alert("config: %s '%s': require an explicit filter declaration " + "to use the cache '%s'.\n", proxy_type_str(px), px->id, cache->id); + return 1; + } + } return 0; } @@ -675,21 +673,6 @@ enum act_return http_action_store_cache(struct act_rule *rule, struct proxy *px, enum htx_blk_type type = htx_get_blk_type(blk); uint32_t sz = htx_get_blksz(blk); - /* Check if we need to skip 'Content-encoding' header or not */ - if ((msg->flags & HTTP_MSGF_COMPRESSING) && /* Compression in progress */ - (cconf->flags & CACHE_FLT_F_IGNORE_CNT_ENC) && /* Compression before the cache */ - (type == HTX_BLK_HDR)) { - struct ist n = htx_get_blk_name(htx, blk); - struct ist v = htx_get_blk_value(htx, blk); - - if (isteq(n, ist("content-encoding"))) - continue; - if (!(msg->flags & HTTP_MSGF_TE_CHNK) && - isteq(n, ist("transfer-encoding")) && - isteqi(v, ist("chunked"))) - continue; - } - chunk_memcat(&trash, (char *)&blk->info, sizeof(blk->info)); if (type == HTX_BLK_EOH) break; diff --git a/src/flt_http_comp.c b/src/flt_http_comp.c index 74a8ae72a0..017080961e 100644 --- a/src/flt_http_comp.c +++ b/src/flt_http_comp.c @@ -59,6 +59,9 @@ static int select_compression_request_header(struct comp_state *st, static int select_compression_response_header(struct comp_state *st, struct stream *s, struct http_msg *msg); +static int set_compression_response_header(struct comp_state *st, + struct stream *s, + struct http_msg *msg); static int htx_compression_buffer_init(struct htx *htx, struct buffer *out); static int htx_compression_buffer_add_data(struct comp_state *st, const char *data, size_t len, @@ -161,6 +164,8 @@ comp_http_headers(struct stream *s, struct filter *filter, struct http_msg *msg) /* Response headers have already been checked in * comp_http_post_analyze callback. */ if (st->comp_algo) { + if (!set_compression_response_header(st, s, msg)) + goto end; register_data_filter(s, msg->chn, filter); if (!IS_HTX_STRM(s)) st->hdrs_len = s->txn->rsp.sov; @@ -456,6 +461,102 @@ comp_http_end(struct stream *s, struct filter *filter, return 1; } /***********************************************************************/ +static int +http_set_comp_reshdr(struct comp_state *st, struct stream *s, struct http_msg *msg) +{ + struct http_txn *txn = s->txn; + + /* + * Add Content-Encoding header when it's not identity encoding. + * RFC 2616 : Identity encoding: This content-coding is used only in the + * Accept-Encoding header, and SHOULD NOT be used in the Content-Encoding + * header. + */ + if (st->comp_algo->cfg_name_len != 8 || memcmp(st->comp_algo->cfg_name, "identity", 8) != 0) { + trash.data = 18; + memcpy(trash.area, "Content-Encoding: ", trash.data); + memcpy(trash.area + trash.data, st->comp_algo->ua_name, + st->comp_algo->ua_name_len); + trash.data += st->comp_algo->ua_name_len; + trash.area[trash.data] = '\0'; + if (http_header_add_tail2(msg, &txn->hdr_idx, trash.area, trash.data) < 0) + goto error; + } + + /* remove Content-Length header */ + if (msg->flags & HTTP_MSGF_CNT_LEN) { + struct hdr_ctx ctx; + + ctx.idx = 0; + while (http_find_header2("Content-Length", 14, ci_head(&s->res), &txn->hdr_idx, &ctx)) + http_remove_header2(msg, &txn->hdr_idx, &ctx); + } + + /* add Transfer-Encoding header */ + if (!(msg->flags & HTTP_MSGF_TE_CHNK)) { + if (http_header_add_tail2(msg, &txn->hdr_idx, "Transfer-Encoding: chunked", 26) < 0) + goto error; + } + + + return 1; + + error: + st->comp_algo->end(&st->comp_ctx); + st->comp_algo = NULL; + return 0; +} + +static int +htx_set_comp_reshdr(struct comp_state *st, struct stream *s, struct http_msg *msg) +{ + struct htx *htx = htxbuf(&msg->chn->buf); + + /* + * Add Content-Encoding header when it's not identity encoding. + * RFC 2616 : Identity encoding: This content-coding is used only in the + * Accept-Encoding header, and SHOULD NOT be used in the Content-Encoding + * header. + */ + if (st->comp_algo->cfg_name_len != 8 || memcmp(st->comp_algo->cfg_name, "identity", 8) != 0) { + struct ist v = ist2(st->comp_algo->ua_name, st->comp_algo->ua_name_len); + + if (!http_add_header(htx, ist("Content-Encoding"), v)) + goto error; + } + + /* remove Content-Length header */ + if (msg->flags & HTTP_MSGF_CNT_LEN) { + struct http_hdr_ctx ctx; + + ctx.blk = NULL; + while (http_find_header(htx, ist("Content-Length"), &ctx, 1)) + http_remove_header(htx, &ctx); + } + + /* add "Transfer-Encoding: chunked" header */ + if (!(msg->flags & HTTP_MSGF_TE_CHNK)) { + if (!http_add_header(htx, ist("Transfer-Encoding"), ist("chunked"))) + goto error; + } + + return 1; + + error: + st->comp_algo->end(&st->comp_ctx); + st->comp_algo = NULL; + return 0; +} + +static int +set_compression_response_header(struct comp_state *st, struct stream *s, struct http_msg *msg) +{ + if (IS_HTX_STRM(s)) + return htx_set_comp_reshdr(st, s, msg); + else + return http_set_comp_reshdr(st, s, msg); +} + /* * Selects a compression algorithm depending on the client request. */ @@ -780,32 +881,6 @@ http_select_comp_reshdr(struct comp_state *st, struct stream *s, struct http_msg /* initialize compression */ if (st->comp_algo->init(&st->comp_ctx, global.tune.comp_maxlevel) < 0) goto fail; - - /* remove Content-Length header */ - ctx.idx = 0; - if ((msg->flags & HTTP_MSGF_CNT_LEN) && http_find_header2("Content-Length", 14, ci_head(c), &txn->hdr_idx, &ctx)) - http_remove_header2(msg, &txn->hdr_idx, &ctx); - - /* add Transfer-Encoding header */ - if (!(msg->flags & HTTP_MSGF_TE_CHNK)) - http_header_add_tail2(&txn->rsp, &txn->hdr_idx, "Transfer-Encoding: chunked", 26); - - /* - * Add Content-Encoding header when it's not identity encoding. - * RFC 2616 : Identity encoding: This content-coding is used only in the - * Accept-Encoding header, and SHOULD NOT be used in the Content-Encoding - * header. - */ - if (st->comp_algo->cfg_name_len != 8 || memcmp(st->comp_algo->cfg_name, "identity", 8) != 0) { - trash.data = 18; - memcpy(trash.area, "Content-Encoding: ", trash.data); - memcpy(trash.area + trash.data, st->comp_algo->ua_name, - st->comp_algo->ua_name_len); - trash.data += st->comp_algo->ua_name_len; - trash.area[trash.data] = '\0'; - http_header_add_tail2(&txn->rsp, &txn->hdr_idx, trash.area, - trash.data); - } msg->flags |= HTTP_MSGF_COMPRESSING; return 1; @@ -898,34 +973,6 @@ htx_select_comp_reshdr(struct comp_state *st, struct stream *s, struct http_msg /* initialize compression */ if (st->comp_algo->init(&st->comp_ctx, global.tune.comp_maxlevel) < 0) goto fail; - - /* - * Add Content-Encoding header when it's not identity encoding. - * RFC 2616 : Identity encoding: This content-coding is used only in the - * Accept-Encoding header, and SHOULD NOT be used in the Content-Encoding - * header. - */ - if (st->comp_algo->cfg_name_len != 8 || memcmp(st->comp_algo->cfg_name, "identity", 8) != 0) { - struct ist v = ist2(st->comp_algo->ua_name, st->comp_algo->ua_name_len); - - if (!http_add_header(htx, ist("Content-Encoding"), v)) - goto deinit_comp_ctx; - } - - /* remove Content-Length header */ - if (msg->flags & HTTP_MSGF_CNT_LEN) { - ctx.blk = NULL; - - while (http_find_header(htx, ist("Content-Length"), &ctx, 1)) - http_remove_header(htx, &ctx); - } - - /* add "Transfer-Encoding: chunked" header */ - if (!(msg->flags & HTTP_MSGF_TE_CHNK)) { - if (!http_add_header(htx, ist("Transfer-Encoding"), ist("chunked"))) - goto deinit_comp_ctx; - } - msg->flags |= HTTP_MSGF_COMPRESSING; return 1; @@ -1304,6 +1351,8 @@ int check_implicit_http_comp_flt(struct proxy *proxy) { struct flt_conf *fconf; + int explicit = 0; + int comp = 0; int err = 0; if (proxy->comp == NULL) @@ -1311,14 +1360,31 @@ check_implicit_http_comp_flt(struct proxy *proxy) if (!LIST_ISEMPTY(&proxy->filter_configs)) { list_for_each_entry(fconf, &proxy->filter_configs, list) { if (fconf->id == http_comp_flt_id) - goto end; + comp = 1; + else if (fconf->id == cache_store_flt_id) { + if (comp) { + ha_alert("config: %s '%s': unable to enable the compression filter " + "before any cache filter.\n", + proxy_type_str(proxy), proxy->id); + err++; + goto end; + } + } + else + explicit = 1; } - ha_alert("config: %s '%s': require an explicit filter declaration to use HTTP compression\n", - proxy_type_str(proxy), proxy->id); + } + if (comp) + goto end; + else if (explicit) { + ha_alert("config: %s '%s': require an explicit filter declaration to use " + "HTTP compression\n", proxy_type_str(proxy), proxy->id); err++; goto end; } + /* Implicit declaration of the compression filter is always the last + * one */ fconf = calloc(1, sizeof(*fconf)); if (!fconf) { ha_alert("config: %s '%s': out of memory\n",