refactor: centralize shared character-class predicates#46
Merged
Conversation
256c6b6 to
0f71fdd
Compare
…s and scheme
Five grammar / character-class predicates were each re-derived as 3-5 nearly
identical private copies scattered across host/, parser/, serialize/, percent/,
scheme/ and idna/:
- the RFC 3986 `unreserved` class and its "-._~" literal
- the "%HH" percent-triplet in-bounds hex-pair test and the octet decode
- ASCII-only lowercasing (deliberately locale-invariant, unlike the stdlib
lowercaseChar())
- the scheme-continuation character class
- the all-ASCII string test and the 0x80 boundary constant
Promote each to its canonical home: most to text/CharClasses, beside the
existing isAsciiHexDigit / hexDigitToInt atoms, and the scheme-tail predicate
to the scheme package next to schemeColonIndex. Every copy was confirmed
identical before replacing, so the security-sensitive `index + 2 < length`
bound on the percent-triplet check now has a single definition.
While here, converge the URL parser's scheme-buffer lowercasing from the
Unicode-aware stdlib lowercaseChar() to the shared ASCII form; both call sites
are already guarded to ASCII characters, so the output is unchanged and the
inconsistency is removed.
Pure refactor: no behavior change, all tests and conformance suites pass
unmodified, public API unchanged.
5f30ed1 to
4037494
Compare
The centralized percentByteAt dropped the misuse guard the previous PercentCodec.tripletByte carried. Restore it as a require on the two decoded nibbles: hexDigitToInt already runs, so validating its results is free, and a caller that reaches percentByteAt without the hasPercentHexPairAt check now fails loudly instead of composing a garbage octet from a -1 nibble. No behavior change on verified input.
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.
What
Five grammar / character-class predicates were each re-derived as 3–5 nearly identical private copies across
host/,parser/,serialize/,percent/,scheme/andidna/:unreservedclass and its"-._~"literal (×3)%HHpercent-triplet in-bounds hex-pair test (×5) and the octet decode (×2)lowercaseChar()(×3, plus a divergent 4th)0x80boundary constantChange
Promote each to its canonical home — most to
text/CharClassesbeside the existingisAsciiHexDigit/hexDigitToIntatoms, and the scheme-tail predicate to theschemepackage next toschemeColonIndex. Newinternalhelpers:Char.isUnreserved(),Char.asciiLowercased(),CharSequence.isAllAscii(),hasPercentHexPairAt(),percentByteAt(), andisSchemeContinuationChar().Every copy was confirmed identical before replacing, so the security-sensitive
index + 2 < lengthbound on the percent-triplet check now has a single definition.The URL parser's scheme-buffer lowercasing is also converged from the Unicode-aware stdlib
lowercaseChar()to the shared ASCII form — both call sites are already guarded to ASCII, so output is unchanged and the last inconsistency is removed.Safety
Pure refactor — no behavior change. All tests and conformance suites pass unmodified; public API unchanged (
apiCheckpasses with noapiDump). Verified on the JVM gate and a Kotlin/NativemacosArm64compile.CharClassesgains a@file:Suppress("TooManyFunctions")— expected for the shared char-class utility file.Stacked on #45.