Feat mimetype exclude negation tokens - #102
Conversation
A token prefixed with `!` negates a match against the built-in MIME exclusion list, allowing individual entries to be re-enabled for compression. Within the same list, when multiple tokens match the same MIME type, the last one listed wins, positive or negative (gitignore-style precedence). The built-in list is only consulted as a fallback when the user-configured list produces no definitive match.
Cover the default built-in exclusion, overriding it with a negation token, and the last-matching-token-wins precedence when a positive and a negative token in the same list both match, in either order.
Explain the `!` token for re-enabling individual entries from the built-in exclusion list, and the last-match-wins precedence rule (gitignore-style: later tokens in the same list override earlier ones, whether positive or negative).
📝 WalkthroughWalkthroughBrotli MIME exclusion matching now supports negated tokens, exact and wildcard matches, last-match precedence, and fallback to built-in exclusions. CGI tests cover default exclusions and override behavior. The README documents the new configuration rules. ChangesMIME exclusion matching
Estimated code review effort: 3 (Moderate) | ~20 minutes Mergeability Score: ⚪ Minimal · up to The remaining concerns are limited to optional coverage for negated wildcard and nonmatching MIME-type cases; no actionable merge-blocking risk remains. Sequence Diagram(s)sequenceDiagram
participant BrotliOutputHandler
participant ConfiguredMimeMatcher
participant BuiltInExclusionList
participant HTTPResponse
BrotliOutputHandler->>ConfiguredMimeMatcher: Evaluate configured MIME tokens
ConfiguredMimeMatcher-->>BrotliOutputHandler: Return match status
alt No configured token matches
BrotliOutputHandler->>BuiltInExclusionList: Check built-in exclusions
BuiltInExclusionList-->>BrotliOutputHandler: Return exclusion status
end
BrotliOutputHandler->>HTTPResponse: Compress or exclude response
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/ob_exclude_006.phpt (1)
10-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a negated wildcard test.
This test covers exact negation only. Add a CGI PHPT case with
brotli.output_compression_exclude_types="!image/*"andContent-Type: image/png. Expect Brotli output.This verifies the documented negated wildcard path independently from the exact-token cases.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/ob_exclude_006.phpt` around lines 10 - 24, Add a separate CGI PHPT test based on tests/ob_exclude_006.phpt that sets brotli.output_compression_exclude_types to "!image/*" while retaining Content-Type image/png and HTTP_ACCEPT_ENCODING=br. Expect Brotli-compressed output using the existing files/ob_hi.br fixture and the same Content-Encoding and Vary headers.tests/ob_exclude_005.phpt (1)
10-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a configured no-match fallback test.
This test covers an empty configured list. It does not cover the new fallback path for a nonempty list with no matching token.
Add a CGI PHPT case with
brotli.output_compression_exclude_types="application/json"andContent-Type: image/png. Expect uncompressed output. This verifies that the built-in exclusion still applies after configured matching returns no match.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/ob_exclude_005.phpt` around lines 10 - 20, Add a separate CGI PHPT test covering a nonempty exclusion list with no matching token: configure brotli.output_compression_exclude_types as application/json, send Content-Type image/png with Brotli accepted, and assert uncompressed output. Keep the test focused on verifying the built-in exclusion fallback after configured matching finds no match.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/ob_exclude_005.phpt`:
- Around line 10-20: Add a separate CGI PHPT test covering a nonempty exclusion
list with no matching token: configure brotli.output_compression_exclude_types
as application/json, send Content-Type image/png with Brotli accepted, and
assert uncompressed output. Keep the test focused on verifying the built-in
exclusion fallback after configured matching finds no match.
In `@tests/ob_exclude_006.phpt`:
- Around line 10-24: Add a separate CGI PHPT test based on
tests/ob_exclude_006.phpt that sets brotli.output_compression_exclude_types to
"!image/*" while retaining Content-Type image/png and HTTP_ACCEPT_ENCODING=br.
Expect Brotli-compressed output using the existing files/ob_hi.br fixture and
the same Content-Encoding and Vary headers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 616cd7bb-f5be-40e1-8988-413ec02258cf
📒 Files selected for processing (6)
README.mdbrotli.ctests/ob_exclude_005.phpttests/ob_exclude_006.phpttests/ob_exclude_007.phpttests/ob_exclude_008.phpt
Summary by CodeRabbit
New Features
!tokens, and last-match precedence.Bug Fixes
Documentation