Repository navigation
fix: use Charset-based comparison for Accept-Charset negotiation - #107
Merged
Merged
Conversation
The charset comparison used string equality with "iso_8859_1" (underscore), which never matched the standard "iso-8859-1" (hyphen) that clients send. Replace with java.nio.charset.Charset-based comparison to handle all aliases (latin1, iso_8859_1, iso-8859-1, etc.) and correctly apply the RFC 9110 §12.5.3 default quality of 1.0 for ISO-8859-1. Closes #104 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Pre-resolve Accept-Charset entries to canonical Charset objects once outside the lambda, avoiding repeated Charset.forName() calls per available charset (O(available × accepts) → O(available + accepts)) - Deduplicate Charset.forName(charset) call — reuse resolved Charset for both alias matching and ISO-8859-1 default check - Skip "*" wildcard in alias resolution loop to avoid unnecessary UnsupportedCharsetException - Add test for Accept-Charset wildcard "*" Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Lowercase accept keys when building the accepts map so that Accept-Charset: UTF-8 matches available "utf-8" on the fast path (RFC 9110 charset comparison is case-insensitive) - Add merge function to handle duplicate keys after lowercasing - Add comment explaining the second wildcardQ check (reached when Charset.forName throws for unrecognized available charset names) - Add case-sensitivity test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Replace Double.parseDouble() with strict RFC 9110 §12.4.2 qvalue regex — rejects scientific notation (1e-1), >3 decimal places (0.1234), and out-of-range values (2.0). Remove unused clamp(). - Use sorted set string for cache keys instead of Set.toString() to avoid nondeterministic iteration order causing cache misses. - Support escaped quotes in quoted-string parameter values per RFC 9110 §5.6.4: change "[^"]*" to "(?:[^"\\]|\\.)*" Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Wrap bestAllowedCharset body in charsetCache.computeIfAbsent() using stableCacheKey, consistent with bestAllowedContentType and bestAllowedLanguage. Avoids repeated Charset.forName() lookups for the same Accept-Charset header and available set. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
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
charset.equals("iso_8859_1")) withjava.nio.charset.Charset-based comparison inAcceptHeaderNegotiator.bestAllowedCharset()latin1,iso_8859_1,iso-8859-1) via JDK's canonical charset resolutionCloses #104
Test plan
acceptCharsetMatchesStandardName—iso-8859-1in Accept-Charset matchesiso-8859-1in available setacceptCharsetMatchesAlias—latin1in Accept-Charset matchesiso-8859-1in available setacceptCharsetDefaultsIso88591— ISO-8859-1 gets default q=1.0 when not listed, beatingutf-8;q=0.5🤖 Generated with Claude Code