Skip to content

Tests: gzip Accept-Encoding HTAB whitespace - #97

Open
louzt wants to merge 1 commit into
nginx:masterfrom
louzt:test/gzip-accept-encoding-htab
Open

Tests: gzip Accept-Encoding HTAB whitespace#97
louzt wants to merge 1 commit into
nginx:masterfrom
louzt:test/gzip-accept-encoding-htab

Conversation

@louzt

@louzt louzt commented Aug 8, 2026

Copy link
Copy Markdown

Adds gzip_accept_encoding_htab.t, a focused regression test for HTAB
handling in the Accept-Encoding request header. Per RFC 9110 §5.6.2 /
§5.6.3, the surrounding field grammar defines optional whitespace as
SP / HTAB, and Vadim's nginx#1577 already taught the multi-header parser
to accept HTAB. The gzip_accept_encoding helper in
ngx_http_core_module.c carries its own boundary check, so it must be
updated to match.

The gap is the single-character path that runs after the gzip token
has been matched: the switches in ngx_http_gzip_accept_encoding()
only carry case ' ' and exit on the bare comma that ends the list,
so HTAB falls into the default branch and the encoding is rejected.

The test covers four baseline cases that document the current
behaviour (SP, no whitespace, HTAB after comma, malformed input
without comma) and three TODO cases that document the gap and will
succeed once the companion nginx#1627 lands.

The malformed-input case (gzip\tdeflate, no comma) is held as a
baseline regression: RFC 9110 §5.6.2 requires comma separators
between elements, and whitespace alone is not enough.

Testing

Verified against a patched and an unpatched binary:

  • unpatched (current master): 9 tests overall, 3 TODO cases fail as
    expected, 4 baseline cases pass, suite reports PASS.
  • patched (companion C PR applied): 9/9 pass, all 3 TODO cases now
    succeed.
  • gzip.t regression suite: 10/10 pass on the patched binary.
# Unpatched master
1..9
ok 1 - gzip enabled with SP between tokens
ok 2 - gzip enabled with no whitespace between tokens
ok 3 - gzip enabled with HTAB after comma
ok 4 - malformed Accept-Encoding without comma is rejected
not ok 5 - gzip enabled with HTAB before comma # TODO
not ok 6 - gzip enabled with HTAB before q-value # TODO
not ok 7 - gzip enabled with HTAB after q-value before comma # TODO
ok 8 - no alerts
ok 9 - no sanitizer errors
All tests successful.

# Patched companion
1..9
ok 1 - gzip enabled with SP between tokens
ok 2 - gzip enabled with no whitespace between tokens
ok 3 - gzip enabled with HTAB after comma
ok 4 - malformed Accept-Encoding without comma is rejected
ok 5 - gzip enabled with HTAB before comma
ok 6 - gzip enabled with HTAB before q-value
ok 7 - gzip enabled with HTAB after q-value before comma
ok 8 - no alerts
ok 9 - no sanitizer errors
All tests successful.

Companion C PR

nginx#1627: HTTP: HTAB support in Accept-Encoding list parsing
(5-line surgical patch to ngx_http_core_module.c).

References

  • nginx#1577 — multi-header parser rework, merged 2026-08-05;
    Vadim explicitly carved ngx_http_gzip_accept_encoding() out
    because the helper carries its own boundary check.
  • RFC 9110 §5.6.2 — optional whitespace definition (OWS = SP / HTAB).
  • RFC 9110 §5.6.3 — Accept-Encoding field grammar (#rule).

Adds gzip_accept_encoding_htab.t, a focused regression test for HTAB
handling in the Accept-Encoding request header. Per RFC 9110 §5.6.2 /
§5.6.3, the surrounding field grammar defines optional whitespace as
SP / HTAB, and Vadim's nginx#1577 already taught the multi-header parser
to accept HTAB. The gzip_accept_encoding helper in
ngx_http_core_module.c carries its own boundary check, so it must be
updated to match.

The TODO block carries four forward-looking cases that currently fail
on master (HTAB before comma, between tokens, before q-value, after
q-value); they document the gap and will pass once the companion C
change lands. The baseline cases (SP, no whitespace, HTAB after comma)
already pass and are held as regression sentinels.
@louzt
louzt force-pushed the test/gzip-accept-encoding-htab branch from beaaabf to 735b981 Compare August 8, 2026 01:12
@louzt
louzt marked this pull request as ready for review August 8, 2026 01:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: New

Development

Successfully merging this pull request may close these issues.

2 participants