Skip to content

fix: sort response headers for deterministic order#1646

Open
kitsuyui wants to merge 1 commit into
mainfrom
fix/audit-response-header-order-nondeterministic-001
Open

fix: sort response headers for deterministic order#1646
kitsuyui wants to merge 1 commit into
mainfrom
fix/audit-response-header-order-nondeterministic-001

Conversation

@kitsuyui

Copy link
Copy Markdown
Owner

Summary

Response headers forwarded by the proxy were enumerated in a non-deterministic
order. Playwright's response.headers() returns a plain object whose key
ordering depends on the browser engine and the upstream server's transmission
order. As a result, identical requests could produce different header orderings
across browser types or runs.

This change adds a sortHeaders utility that sorts header keys
lexicographically and applies it in renderToResponse after appendVaryHeader,
so the proxy always emits headers in a stable alphabetical order regardless of
the upstream or browser source.

Changes

  • src/lib/headers.ts: add and export sortHeaders(headers: Headers): Headers
  • src/lib/headers.spec.ts: unit tests for sortHeaders (empty input,
    lexicographic order, value preservation, input immutability)
  • src/server/index.ts: wrap appendVaryHeader(...) result with sortHeaders
    in renderToResponse

Verification

  • bun run lint: clean
  • bun run typecheck: clean
  • bun run vitest run src/lib/headers.spec.ts: 15/15 passed
  • collateral check: no violations

Trade-offs

Sorting adds one O(n log n) pass over the header set per response, where n is
the number of forwarded headers (typically < 20). The overhead is negligible
compared to the browser render time.

Add `sortHeaders` to `src/lib/headers.ts` and apply it in
`renderToResponse` after `appendVaryHeader`. This eliminates
non-determinism introduced by Playwright's `response.headers()`
enumeration order, which varies by browser and upstream server.

Adds unit tests for `sortHeaders` covering: empty input, lexicographic
ordering, value preservation, and input immutability.
@github-actions

Copy link
Copy Markdown

gh-counter

PR gate

Removed Added +/-
TODO/expect-error 0 0 0

Repo dashboard

main (de5f702) #1646 (b9cc2fe) +/-
TODO/expect-error 4 4 0

Reported by gh-counter

@github-actions

Copy link
Copy Markdown

Code Metrics Report

main (de5f702) #1646 (b9cc2fe) +/-
Coverage 86.9% 87.2% +0.2%
Code to Test Ratio 1:0.5 1:0.5 +0.0
Test Execution Time 6s 7s +1s
Details
  |                     | main (de5f702) | #1646 (b9cc2fe) |  +/-  |
  |---------------------|----------------|-----------------|-------|
+ | Coverage            |          86.9% |           87.2% | +0.2% |
  |   Files             |             11 |              11 |     0 |
  |   Lines             |            207 |             211 |    +4 |
+ |   Covered           |            180 |             184 |    +4 |
+ | Code to Test Ratio  |          1:0.5 |           1:0.5 |  +0.0 |
  |   Code              |           1648 |            1694 |   +46 |
+ |   Test              |            961 |             994 |   +33 |
- | Test Execution Time |             6s |              7s |   +1s |

Code coverage of files in pull request scope (86.0% → 86.6%)

Files Coverage +/- Status
src/lib/headers.ts 100.0% 0.0% modified
src/server/index.ts 76.9% 0.0% modified

Reported by octocov

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant