-
Notifications
You must be signed in to change notification settings - Fork 688
fix: combine duplicate Vary headers into single header #8848
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
PR Code Suggestions ✨Explore these optional code suggestions:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Summary
This PR fixes the duplicate Vary header issue by modifying the update_head function in the compression middleware. Previously, the middleware simply appended accept-encoding to the Vary header, resulting in multiple Vary headers when CORS middleware had already set one.
Key changes:
- Collects all existing Vary header values and parses comma-separated values
- Performs case-insensitive deduplication to avoid duplicate
accept-encodingentries - Removes all existing Vary headers and inserts a single combined header
- Adds three comprehensive test cases covering CORS headers, deduplication, and combining multiple values
Impact:
- Resolves Suricata alerts triggered by duplicate headers
- Ensures consistent caching behavior across reverse proxies
- Maintains RFC 9110 compliance while following best practices
Confidence Score: 5/5
- This PR is safe to merge with minimal risk
- The implementation is sound with proper parsing, case-insensitive deduplication, and error handling. Three new comprehensive tests validate the fix for CORS headers, deduplication, and combining multiple values. The change is localized to the
update_headfunction with no breaking changes to the API. - No files require special attention
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| src/handler/http/router/middlewares/encoding/encoder.rs | 5/5 | Fixes duplicate Vary headers by combining all values into single header with case-insensitive deduplication; includes comprehensive tests |
Sequence Diagram
sequenceDiagram
participant Client
participant CompressMiddleware
participant CORSMiddleware
participant Encoder
participant ResponseHead
Client->>CompressMiddleware: HTTP Request with Accept-Encoding
CompressMiddleware->>CORSMiddleware: Forward Request
CORSMiddleware->>ResponseHead: Set Vary: Origin, Access-Control-Request-Method, Access-Control-Request-Headers
CORSMiddleware-->>CompressMiddleware: Response with CORS headers
CompressMiddleware->>Encoder: Encode response body
Encoder->>Encoder: update_head(encoding, head)
Note over Encoder: Collect existing Vary headers
Encoder->>ResponseHead: Get all Vary header values
ResponseHead-->>Encoder: ["Origin, Access-Control-Request-Method, Access-Control-Request-Headers"]
Note over Encoder: Split by comma, trim, deduplicate
Encoder->>Encoder: Check if "accept-encoding" exists (case-insensitive)
Encoder->>Encoder: Add "accept-encoding" to list
Encoder->>ResponseHead: Remove all Vary headers
Encoder->>ResponseHead: Insert combined Vary: Origin, Access-Control-Request-Method, Access-Control-Request-Headers, accept-encoding
Encoder-->>CompressMiddleware: Encoded response with single Vary header
CompressMiddleware-->>Client: Compressed response
1 file reviewed, no comments
|
| Status | Total | Passed | Failed | Skipped | Flaky | Pass Rate | Duration |
|---|---|---|---|---|---|---|---|
| All tests passed | 364 | 340 | 0 | 19 | 5 | 93% | 4m 40s |
The compression middleware was appending a separate Vary: accept-encoding header instead of combining it with existing CORS Vary headers. This caused Suricata alerts and caching inconsistencies. Now all Vary values are collected and combined into a single header with case-insensitive deduplication. Fixes #8847
477f446 to
81da0b1
Compare
| // Remove all existing Vary headers and insert a single combined one | ||
| head.headers_mut().remove(header::VARY); | ||
| let combined = vary_values.join(", "); | ||
| if let Ok(header_value) = HeaderValue::from_str(&combined) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add a log here in case from_str fails?
User description
The compression middleware was appending a separate Vary: accept-encoding header instead of combining it with existing CORS Vary headers. This caused Suricata alerts and caching inconsistencies.
Now all Vary values are collected and combined into a single header with case-insensitive deduplication.
Fixes #8847
PR Type
Bug fix, Tests
Description
Combine duplicate Vary headers into one
Deduplicate values case-insensitively
Preserve existing CORS Vary values
Add tests for header consolidation
Diagram Walkthrough
File Walkthrough
encoder.rs
Consolidate and deduplicate Vary header handlingsrc/handler/http/router/middlewares/encoding/encoder.rs