Tests: added add_header/add_trailer field sanitization test. - #58
Draft
louzt wants to merge 3 commits into
Draft
Conversation
Author
|
Companion nginx/nginx draft: nginx/nginx#1414 |
louzt
changed the title
Per DemiMarie directive 2026-06-21 (nginx#1414 review): 'Validating the names can be dropped from this PR. Validating the values is much more important, as the names are static and so cannot be attacker-controlled.' The c09e7fe skip-names commit was dropped from the source worktree (upstream/response-filter-sanitize @ b3134bc1c). The 6 bad-name assertions in this test file (locations /bad-name-header/ and /bad-name-trailer/ + 6 like/unlike assertions) test behavior that no longer exists in the source. With c09e7fe removed, X:Bad is correctly emitted (no skip), so the 'skips invalid field name' assertions unavoidably fail. Stripped them, plan from 12 → 6 user tests. With Test::Nginx framework auto-tests (no alerts, no sanitizer errors) total = 8/8 PASS. Validation evidence archived in docs/branch-metrics.md under 'Phase 2 Step 6 — nginx#1414 rebase validation'.
louzt
force-pushed
the
upstream/response-filter-sanitize-tests
branch
from
August 7, 2026 22:34
cd58799 to
972819d
Compare
Author
|
Rebased onto current master and dropped the bad-name feature reference per Demi's 2026-06-21 direction that field names are static. Body now reflects the value-sanitize-only scope and 6/6 test pass on the patched branch. Lab notes: https://gist.github.com/louzt/7bdf370a28126718e7e7b69d53b0ae86 |
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
2 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.
Summary
This is the standalone nginx-tests regression for the add_header and add_trailer value sanitization in nginx#1414.
Why
The companion test now focuses on the runtime concern worth shipping first: invalid value bytes (NUL, CR, LF) in computed add_header and add_trailer values must not produce malformed serialized fields.
Field-name validation is intentionally out of scope for this test and remains a separate config-time concern, per the maintainer direction from 2026-06-21 that field names are static and cannot be attacker-controlled, so runtime value sanitization is the concern worth shipping first.
What changed
Patch shape
Validation
Risks / Follow-ups
This test intentionally follows the value-sanitize-only policy of nginx#1414. If upstream chooses a different late response-filter policy, the assertions should be updated to match that final design.
Field-name handling remains a separate config-time validation concern that can be added independently if maintainers prefer that boundary.