fix: zstd truncation/abort under proxy_buffering off (bug B) + regression tests

.github/workflows/build-test.yml

@@ -484,6 +484,85 @@ jobs:
           python3 tools/test_terminal_frame.py --nginx-binary "$TEST_NGINX_BINARY" --port 18085
           echo "✓ Terminal-frame regression test passed"
 
+      - name: Run proxy_buffering-off truncation regression (bug B)
+        run: |
+          cd "$GITHUB_WORKSPACE"
+          # Reproduces the production trigger: zstd over an unbuffered
+          # (proxy_buffering off) chunked, no-Content-Length upstream.
+          # Decodes and byte-compares the full body across the
+          # ZSTD_CStreamInSize buffer-boundary size matrix. Fails on a
+          # module that forwards a zero-size buffer ("zero size buf in
+          # writer" -> aborted/truncated response).
+          python3 tools/test_proxy_unbuffered_truncation.py \
+            --nginx-binary "$TEST_NGINX_BINARY" \
+            --port 18086 --backend-port 18087
+          echo "✓ proxy_buffering-off truncation regression passed"
+
+      - name: Run broad compression correctness matrix
+        timeout-minutes: 12
+        run: |
+          cd "$GITHUB_WORKSPACE"
+          # ~504 cells: {proxy-buffered, proxy-unbuffered, static}
+          # x {incompressible, highly-compressible, css}
+          # x {1, CStreamInSize +/-1, 200000, 1MiB}
+          # x {zstd, gzip+zstd, gzip-only, identity} x repeat.
+          # Every cell decodes per Content-Encoding and byte-compares
+          # to origin, or asserts correct fallback. Catches the whole
+          # truncation / zero-size-buf / livelock bug class across the
+          # circumstances it has historically recurred in (a step-level
+          # timeout makes a CPU-spin livelock a hard FAIL, not a hang).
+          python3 tools/test_compression_matrix.py \
+            --nginx-binary "$TEST_NGINX_BINARY" \
+            --port 18096 --backend-port 18097
+          echo "✓ compression correctness matrix passed"
+
+      - name: Run per-request CCtx isolation regression
+        timeout-minutes: 6
+        run: |
+          cd "$GITHUB_WORKSPACE"
+          # 774b4a5 class: a shared/not-reset ZSTD_CCtx silently bleeds
+          # one request's compressor state into another (well-formed
+          # but wrong body). Fire many distinct concurrent requests
+          # through one worker over reused keepalive connections; each
+          # response must decode byte-exact to its OWN origin.
+          python3 tools/test_concurrent_cctx_isolation.py \
+            --nginx-binary "$TEST_NGINX_BINARY" \
+            --port 18098 --backend-port 18099 \
+            --concurrency 16 --rounds 6
+          echo "✓ per-request CCtx isolation regression passed"
+
+      - name: Run reload-under-load correctness regression
+        timeout-minutes: 6
+        run: |
+          cd "$GITHUB_WORKSPACE"
+          # 2c538e0 CDict-cycle class: SIGHUP reload while requests are
+          # in flight (zstd_dict_file set). Every response — old-cycle,
+          # straddling, new-cycle — must still decode byte-exact; master
+          # must survive; error log must stay clean. Complements
+          # tools/test_reload_leak.sh (which checks leaks, not response
+          # correctness during reload).
+          python3 tools/test_reload_under_load.py \
+            --nginx-binary "$TEST_NGINX_BINARY" \
+            --port 18100 --backend-port 18101 \
+            --reloads 6 --workers 4 --duration 18
+          echo "✓ reload-under-load correctness regression passed"
+
+      - name: Run zstd_long (LDM) directive regression
+        timeout-minutes: 6
+        run: |
+          cd "$GITHUB_WORKSPACE"
+          # 8edb7bb zstd_long class: the directive had ZERO coverage.
+          # A 16 MiB body whose duplicate region repeats beyond the
+          # level-6 default window must (a) decode byte-exact and
+          # (b) compress ~2:1 smaller with zstd_long on than off —
+          # proving ZSTD_c_enableLongDistanceMatching actually reached
+          # libzstd and engaged, and the directive neither corrupts nor
+          # truncates the stream (the bug-B failure mode).
+          python3 tools/test_zstd_long_ldm.py \
+            --nginx-binary "$TEST_NGINX_BINARY" \
+            --port 18102 --backend-port 18103
+          echo "✓ zstd_long (LDM) directive regression passed"
+
       - name: Test summary
         if: success()
         run: echo "✓ All Perl + Python end-to-end tests passed"
@@ -535,6 +614,21 @@ jobs:
           python3 tools/test_encoding.py --nginx-binary "$TEST_NGINX_BINARY" --port 18091 --fixture-lines 1900
           python3 tools/test_encoding.py --nginx-binary "$TEST_NGINX_BINARY" --port 18092 --concurrent-requests 2
           python3 tools/test_terminal_frame.py --nginx-binary "$TEST_NGINX_BINARY" --port 18093
+          python3 tools/test_proxy_unbuffered_truncation.py \
+            --nginx-binary "$TEST_NGINX_BINARY" --port 18094 --backend-port 18095
+          # Matrix under ASAN/UBSAN at repeat=1 (252 cells) — the
+          # truncation/zero-size-buf/livelock class has UB+lifetime
+          # history, so exercise the full circumstance matrix with
+          # sanitizers, not just the happy path.
+          python3 tools/test_compression_matrix.py \
+            --nginx-binary "$TEST_NGINX_BINARY" --port 18096 \
+            --backend-port 18097 --repeat 1
+          # CCtx isolation under ASAN/UBSAN: a shared/not-reset context
+          # is a lifetime bug — exactly what ASAN is for. Lighter
+          # concurrency/rounds since the sanitized binary is slower.
+          python3 tools/test_concurrent_cctx_isolation.py \
+            --nginx-binary "$TEST_NGINX_BINARY" --port 18098 \
+            --backend-port 18099 --concurrency 8 --rounds 3
           echo "✓ All smoke tests clean under ASAN+UBSAN"
 
       - name: zstd_dict_file reload leak check (CDict lifecycle)

filter/ngx_http_zstd_filter_module.c

@@ -573,10 +573,24 @@ ngx_http_zstd_filter_compress(ngx_http_request_t *r, ngx_http_zstd_ctx_t *ctx)
     pos_in  = ctx->buffer_in.pos;
     pos_out = ctx->buffer_out.pos;
 
-    /* Determine the compression directive based on action state */
+    /*
+     * Determine the compression directive.
+     *
+     * END always wins (terminal frame). Otherwise a pending flush
+     * (ctx->flush) must map to ZSTD_e_flush even while the action state
+     * machine is still COMPRESS: that machine only transitions
+     * COMPRESS->FLUSH *after* a call that returned rc > 0 (zstd already
+     * had output to drain). Under `proxy_buffering off` the upstream
+     * forces a flush around a chunk that zstd consumes/buffers
+     * internally with rc == 0; if the directive stayed ZSTD_e_continue
+     * there, libzstd is never told to flush and holds those bytes
+     * indefinitely. Mapping ctx->flush -> ZSTD_e_flush forces libzstd
+     * to disgorge whatever it has buffered, exactly as the stock nginx
+     * gzip/brotli body filters issue a sync flush on a pending flush.
+     */
     if (ctx->action == NGX_HTTP_ZSTD_FILTER_END) {
         directive = ZSTD_e_end;
-    } else if (ctx->action == NGX_HTTP_ZSTD_FILTER_FLUSH) {
+    } else if (ctx->action == NGX_HTTP_ZSTD_FILTER_FLUSH || ctx->flush) {
         directive = ZSTD_e_flush;
     } else {
         directive = ZSTD_e_continue;
@@ -687,9 +701,51 @@ ngx_http_zstd_filter_compress(ngx_http_request_t *r, ngx_http_zstd_ctx_t *ctx)
                    (size_t) ngx_buf_size(ctx->out_buf), rc == 0, last,
                    ctx->last, ctx->flush, (ngx_uint_t) ctx->action);
 
-    if (ngx_buf_size(ctx->out_buf) == 0 && !last && !(rc == 0 && ctx->flush)) {
-        ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
-                       "zstd emit: suppressed empty non-terminal buffer");
+    /*
+     * Empty, non-terminal output buffer handling.
+     *
+     * Two distinct empty-buffer cases reach here:
+     *
+     *  1. A content-less *completed flush*: ctx->flush is set and the
+     *     ZSTD_e_flush call returned rc == 0 with zero bytes produced.
+     *     Per the libzstd contract rc == 0 from a flush means the
+     *     encoder is fully drained — there is genuinely nothing to
+     *     send. The OLD code's `!(rc == 0 && ctx->flush)` exception
+     *     forwarded a zero-size buffer here, which nginx's
+     *     ngx_http_write_filter rejects ("zero size buf in writer")
+     *     and aborts the request — bug B, under `proxy_buffering off`
+     *     where the upstream forces such flushes.
+     *
+     *     The flush is COMPLETE, so satisfy and clear it here: drop the
+     *     NGX_HTTP_GZIP_BUFFERED bit and ctx->flush, emit nothing, and
+     *     return NGX_AGAIN. Clearing ctx->flush is what makes the body
+     *     filter's inner loop terminate: ngx_http_zstd_filter_add_data()
+     *     keeps the loop alive while ctx->flush is set (expecting this
+     *     function to consume it); a naive "suppress the empty buffer
+     *     but leave ctx->flush set" change spins the worker at 100% CPU
+     *     (a NGX_AGAIN livelock — observed). Clearing it lets the next
+     *     add_data() fall through to NGX_DECLINED (input drained) and
+     *     break the loop cleanly.
+     *
+     *  2. Any other empty, non-terminal buffer (no pending flush):
+     *     nothing to send and nothing to satisfy — just suppress and
+     *     return NGX_AGAIN as before.
+     *
+     * The genuine terminal empty buffer (last) is never suppressed; it
+     * falls through to be emitted with last_buf set below.
+     */
+    if (ngx_buf_size(ctx->out_buf) == 0 && !last) {
+        if (rc == 0 && ctx->flush) {
+            r->connection->buffered &= ~NGX_HTTP_GZIP_BUFFERED;
+            ctx->flush = 0;
+            ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
+                           "zstd emit: content-less flush completed, "
+                           "cleared (no empty buffer forwarded)");
+        } else {
+            ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
+                           "zstd emit: suppressed empty non-terminal "
+                           "buffer");
+        }
         return NGX_AGAIN;
     }
 

t/01-static.t

@@ -404,3 +404,32 @@ Content-Encoding: zstd
 !Content-Encoding
 --- no_error_log
 [alert]
+
+
+
+# COVERAGE NOTES on gaps found by the git-history CI audit:
+#
+# Gap 2 — f7e2ef3 ("clear Accept-Ranges in static module"):
+# deliberately NOT covered by a black-box test here. The static
+# handler serves a local file and never sets r->allow_ranges (in this
+# nginx, only the upstream/proxy path sets it — see
+# ngx_http_upstream.c, and ngx_http_range_filter_module.c bails unless
+# r->allow_ranges). ngx_http_clear_accept_ranges() in the static
+# module is therefore purely defensive: no request reachable through
+# zstd_static alone makes the pre-fix and fixed builds differ
+# (empirically verified — identical 200, no Accept-Ranges, no
+# Content-Range, on both a pre-f7e2ef3 and a fixed .so). A Perl test
+# asserting !Accept-Ranges would PASS on the buggy build too, i.e. be
+# blind. Per the project's fail-first discipline we do not add a test
+# that cannot fail on the unfixed code.
+#
+# Gap 3 — HTTP/2 transport axis (8281baa bug-B class):
+# the build enables --with-http_v2_module but nothing tests the h2
+# path. HTTP/2 in nginx requires TLS + ALPN/Upgrade negotiation; h2c
+# (cleartext) does not work without Upgrade in nginx config. A Python
+# test without TLS cannot easily drive the h2 path. The bug-B defect
+# (empty-buffer, flush-state-machine, c->buffered accounting) is
+# already well-covered by test_proxy_unbuffered_truncation.py
+# (HTTP/1.1) and the matrix under ASAN; the h2-specific framing path
+# would be redundant effort without adding coverage for a new code
+# path. Left for future work when/if CI adds TLS test infrastructure.