Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 32 additions & 7 deletions ngx_http_zstd_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
}

Expand Down
10 changes: 9 additions & 1 deletion static/ngx_http_zstd_static_module.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
76 changes: 76 additions & 0 deletions t/01-static.t
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Loading