diff --git a/ngx_http_zstd_common.h b/ngx_http_zstd_common.h index bbc8375..4a94612 100644 --- a/ngx_http_zstd_common.h +++ b/ngx_http_zstd_common.h @@ -312,15 +312,17 @@ ngx_http_zstd_accept_encoding(ngx_str_t *ae) /* - * ngx_http_zstd_ok() + * ngx_http_zstd_accepts() * - * Returns NGX_OK if the request is a main request whose client advertises - * acceptable zstd support (Accept-Encoding accepts "zstd" with q > 0, via - * an explicit token or the "*" wildcard). - * Sets r->gzip_tested / r->gzip_ok as side effects for Vary handling. + * Side-effect-free acceptance predicate: NGX_OK iff this is a main request + * whose client advertises acceptable zstd support (Accept-Encoding accepts + * "zstd" with q > 0, via an explicit token or the "*" wildcard). Does NOT + * touch r->gzip_tested / r->gzip_ok — callers that only need the decision + * (e.g. the static module, which must not suppress a gzip_static fallback + * before it even knows whether a .zst file exists) use this. */ static ngx_int_t -ngx_http_zstd_ok(ngx_http_request_t *r) +ngx_http_zstd_accepts(ngx_http_request_t *r) { ngx_table_elt_t *ae; @@ -338,7 +340,30 @@ ngx_http_zstd_ok(ngx_http_request_t *r) * "shorter than 'zstd'" fast-reject is no longer valid; an empty value * is still a decline (the walk below returns NGX_DECLINED). */ - if (ngx_http_zstd_accept_encoding(&ae->value) != NGX_OK) { + return ngx_http_zstd_accept_encoding(&ae->value); +} + + +/* + * ngx_http_zstd_ok() + * + * As ngx_http_zstd_accepts(), but additionally latches r->gzip_tested / + * r->gzip_ok = 0 on a positive result, so a later gzip filter/handler + * declines and does not double-compress a response we are about to encode as + * zstd. Only the on-the-fly filter module uses this: it calls ngx_http_zstd_ok() + * at the point it commits to compressing (Content-Encoding: zstd is set + * immediately after), so latching gzip off here is always followed by an + * actual zstd encoding — the latch never strands a response with neither + * coding. The static module must NOT use this (see ngx_http_zstd_accepts()). + * + * ngx_inline: only the filter TU calls this; the static TU includes the header + * but uses ngx_http_zstd_accepts() instead, so a plain `static` definition + * trips -Werror=unused-function there. An inline definition is exempt. + */ +static ngx_inline ngx_int_t +ngx_http_zstd_ok(ngx_http_request_t *r) +{ + if (ngx_http_zstd_accepts(r) != NGX_OK) { return NGX_DECLINED; } diff --git a/static/ngx_http_zstd_static_module.c b/static/ngx_http_zstd_static_module.c index 6b8560c..898ad1a 100644 --- a/static/ngx_http_zstd_static_module.c +++ b/static/ngx_http_zstd_static_module.c @@ -120,7 +120,15 @@ ngx_http_zstd_static_handler(ngx_http_request_t *r) } if (zscf->enable == NGX_HTTP_ZSTD_STATIC_ON) { - rc = ngx_http_zstd_ok(r); + /* + * Side-effect-free predicate, NOT ngx_http_zstd_ok(): the latter + * latches r->gzip_ok = 0, which would suppress a gzip_static / gzip + * fallback for THIS request even when we go on to decline below + * (e.g. the .zst file is absent). We only decide here; when we + * actually serve the .zst the response carries Content-Encoding: zstd, + * which makes the gzip filter decline on its own. + */ + rc = ngx_http_zstd_accepts(r); } else { rc = NGX_OK; diff --git a/t/01-static.t b/t/01-static.t index ba6bafa..04f7a72 100644 --- a/t/01-static.t +++ b/t/01-static.t @@ -600,3 +600,79 @@ Content-Encoding: zstd --- error_code: 200 --- no_error_log [error] + + + +=== TEST 27: zstd_static "on" declining does not suppress the gzip fallback +# Regression for the gzip-fallback latch. ngx_http_zstd_ok() latches +# r->gzip_tested=1 / r->gzip_ok=0 as a side effect; the static handler +# used to call it (before the .zst existence check), so when the .zst was +# absent it declined but left gzip permanently marked "not ok" for the +# request. A later gzip filter/handler then short-circuited on the cached +# decision and served identity instead of gzip. The fix routes the static +# decision through the side-effect-free ngx_http_zstd_accepts(). +# +# Reproduces with the always-present gzip *filter* (no gzip_static needed): +# request a plain file that has NO sibling .zst. zstd_static declines; the +# core static handler serves it; the gzip filter must still compress it. +# Pre-fix: Content-Encoding is absent (gzip suppressed). Post-fix: gzip. +--- config + location /gz/ { + zstd_static on; + gzip on; + gzip_min_length 1; + gzip_types text/plain; + root html; + } +--- user_files +>>> gz/plain.txt +gzip fallback body long enough to exceed gzip_min_length and actually compress padding padding padding padding +--- request +GET /gz/plain.txt +--- more_headers +Accept-Encoding: gzip, zstd +--- response_headers +Content-Encoding: gzip +--- error_code: 200 +--- no_error_log +[error] + + + +=== TEST 28: zstd_static coexists with gzip_static; .gz still served +# Interop guard, sibling of TEST 27 but for the gzip_static *module*. +# NOTE: unlike the gzip *filter* (TEST 27), gzip_static is a CONTENT_PHASE +# handler. It is a built-in module, so its handler is pushed onto the +# content-phase array before the dynamically-loaded zstd_static handler and +# runs FIRST -- it serves the .gz before zstd_static ever runs, so the +# ngx_http_zstd_ok() latch could not suppress it even on the pre-fix code. +# (Verified: this test passes on both pre-fix and the fixed tree; the real +# latch regression is covered by TEST 27 via the post-content gzip filter.) +# Kept as a coexistence contract: with both directives on and a .gz but no +# .zst present, zstd_static declines and gzip_static serves the .gz -- the +# two static handlers must not fight over the request. +# +# The request has NO sibling .zst (so zstd_static declines) but DOES have a +# real gzip-compressed plain.txt.gz (so gzip_static must serve it). +--- config + location /gzs/ { + zstd_static on; + gzip_static on; + root html; + } +--- user_files eval +my $body = "gzip_static fallback body long enough to matter " x 4; +my $gz; +require IO::Compress::Gzip; +IO::Compress::Gzip::gzip(\$body => \$gz) + or die "gzip failed: $IO::Compress::Gzip::GzipError"; +">>> gzs/plain.txt\n$body>>> gzs/plain.txt.gz\n$gz"; +--- request +GET /gzs/plain.txt +--- more_headers +Accept-Encoding: gzip, zstd +--- response_headers +Content-Encoding: gzip +--- error_code: 200 +--- no_error_log +[error]