Run zstd before brotli if both are enabled#44
Open
kamermans wants to merge 1 commit into
Open
Conversation
Contributor
13 tasks
|
@SuperSandro2000 you need this patch when u want brotli and zstd running at the same time. |
13 tasks
|
Hi there! Any news? |
|
Hi, last day i try to compile NGINX 1.29.1 with OpenSSL 3.5.2 and zstd-nginx-module, anyway sometimes NGINX respond using br compression instead zstd. I cannot understand why. When i test with command line curl like this : curl -sS --http2 -D - -o /dev/null -H 'Accept-Encoding: zstd, br, gzip' https://www.inventivashop.com | tr -d '\r' | grep -iE 'HTTP/|content-encoding|alt-svc|server' Response : HTTP/2 200 NGINX respond correctly using ALWAY zstd. When i use Chrome the 90% of test response in BR compression. I cannot understand why. |
eilandert
referenced
this pull request
in eilandert/zstd-nginx-module
May 10, 2026
Fixes issue #40 and #44 - when both zstd and brotli modules are enabled, zstd now takes priority since all browsers supporting zstd also support brotli, and zstd offers better compression and performance. For dynamic modules: ngx_module_order already handles priority (zstd before brotli) For static modules: Add explicit filter reordering: - In filter/config: Check for brotli_filter_module before gzip - In static/config: Add ngx_module_order constraint for zstd_static This ensures zstd compression is actually used for static file serving (CSS, JS, fonts, etc.) when both modules are compiled in. Related: PR #44 (kamermans), PR #31 (HanadaLee), Issue #40 (mklooss/kamermans) Test: Verify Accept-Encoding with 'zstd, br' now compresses with zstd.
hsw
added a commit
to hsw/zstd-nginx-module
that referenced
this pull request
May 19, 2026
… direction Closes the dynamic-build coverage gap for Issue tokers#40 / PR tokers#44 (MarcoMarcoaldi report). Until now Issue tokers#40 was verified only on ubuntu-24.04-brotli (static build, all modules linked into one binary) where the HTTP_FILTER_MODULES / HTTP_MODULES sed-rewrite is the mechanism. The new ubuntu-24.04-dynamic-brotli image builds ngx_brotli AND zstd-nginx-module as four dynamic modules loaded via load_module against the apt-installed nginx.org mainline binary — exercising the ngx_module_order mechanism instead. Building the variant surfaced a real bug in our ngx_module_order: static/config previously declared ngx_module_order="$ngx_module_name brotli_static gzip_static" ngx_module_order semantics: the FIRST entry comes BEFORE the rest in ngx_modules[]. Content-phase handlers are reversed at request time in ngx_http_init_phase_handlers (src/http/ngx_http.c:551) -- LIFO chain -- so a module at a HIGHER ngx_modules[] index has its handler called FIRST. The old order placed zstd_static at the LOWEST index of the three static-module siblings, so brotli_static won at request time whenever both sidecars existed. test_static_brotli_priority on the new dynamic-brotli image reproduces this deterministically: brotli served instead of zstd. Fix: invert the list so $ngx_module_name comes LAST. Constraint set is now { ngx_http_static_module, ngx_http_gzip_static_module, ngx_http_brotli_static_module } < zstd_static — zstd_static lands at the highest index → called first → wins. Matches the equivalent intent of the static-build HTTP_MODULES sed-rewrite below (which appends zstd_static AFTER brotli_static / gzip_static, yielding the same final order). Test helper updates (t/regression/test_filter_priority.py + test_static_brotli_priority.py): - _has_brotli() now also accepts dynamic builds (.so present in /etc/nginx/modules), not just `nginx -V | grep ngx_brotli`. - New _brotli_load_modules() helper emits the four load_module lines needed on dynamic builds and empty on static builds. Tests pass it into render_template() instead of hardcoded load_modules="". Dockerfile.dynamic-brotli notes: - Reuses build-module.sh dynamic mode; brotli source is cloned at build time (pin via NGX_BROTLI_REF), built as two .so alongside the two zstd .so via a single configure invocation. Module order on the configure cmdline (extras BEFORE main module) matches the static variant's pattern so the upstream-coverage.md "load_module Order must match ngx_module_order constraint" assumption holds. - Default-rendered nginx.conf load_modules brotli first, then zstd, matching the recommended deployment order. Anchored sed (^__LOAD_MODULES__$) so the multi-line replacement doesn't bleed into the template's marker-documentation comment block. Verification: - t/build.sh ubuntu-24.04-dynamic-brotli — green - docker run … pytest test_filter_priority.py test_static_brotli_priority.py on ubuntu-24.04-brotli: 7 passed (3 filter + 4 static) on ubuntu-24.04-dynamic-brotli: 7 passed (3 filter + 4 static) Before the static/config fix, the dynamic-brotli image failed test_zstd_wins_over_brotli_when_both_accepted with Content-Encoding=br. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Since Safari doesn't support zstd yet, I have to keep brotli enabled, however, this module is run after brotli, so if both are supported, brotli is used. This is a problem since all browsers that support zstd also support brotli, so zstd is never actually used.
This PR reorders the module/filter before brotli so it takes priority (since it's faster and the compression is better).
I took this patch from #40 by @mklooss (thanks!)