refactor: centralize UTF-16 code-point primitives#45
Merged
Conversation
The surrogate/code-point helpers (toCodePoint, appendCodePoint, codePointsOf, charCount, codePointAt, isSurrogatePairAt) and their backing surrogate constants were copy-pasted near-verbatim across the four idna engine files (Idna, IdnaValidity, Normalizer, Punycode) and again in query/ and percent/. Kotlin's common stdlib has no code-point iteration (Character.toChars / codePointAt are JVM-only), so the shim is necessary — but duplicating the surrogate bit-math 4-5 times is a needless drift risk in logic that is easy to get subtly wrong. Extract a single internal text/CodePoints.kt and route every call site through it. IdnaValidity keeps its boxing-free IntArray scan (now calling the shared toCodePoint); Idna.NON_ASCII_MIN and Punycode.INITIAL_N stay separate (same value, different meaning). Pure refactor: output is byte-identical and the IDNA and NFC conformance suites pass unmodified.
Uri.relativize verified its candidate by resolving it through the structured Resolver.resolve overload, which threw IllegalStateException internally when the recomposed target failed to parse. That broke relativize's documented contract that it never throws: a candidate suffix such as "/.//h:zz" is a valid rootless reference, but dot-segment removal collapses it to "//h:zz", which re-reads as an authority with a non-numeric port and fails the strict parser. Make the structured Resolver.resolve overload return ParseResult<ParsedComponents> rather than raising an internal error, and have relativize fold a resolution failure (or a non-matching round-trip) to null. The candidate is still resolved from its already-parsed components, so no redundant string re-parse is added.
256c6b6 to
0f71fdd
Compare
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.
This branch bundles two independent changes on top of #44: an internal code-point refactor and a correctness fix to
Uri.relativize.1. Centralize UTF-16 code-point primitives
What
The UTF-16 code-point / surrogate primitives —
toCodePoint,appendCodePoint,codePointsOf,charCount,codePointAt,isSurrogatePairAtand their backing surrogate constants — were copy-pasted near-verbatim across the four idna engine files (Idna,IdnaValidity,Normalizer,Punycode) and again inquery/QueryParametersBuilderandpercent/PercentCodec.Kotlin's common stdlib has no code-point iteration (
Character.toChars/codePointAtare JVM-only), so the shim is genuinely needed — but duplicating the surrogate bit-math 4–5 times is an avoidable drift risk in logic that is easy to get subtly wrong.Change
Extract a single
internaltext/CodePoints.ktand route every call site through it.IdnaValiditykeeps its boxing-freeIntArraycode-point scan (it's on a hot path), now delegating to the sharedtoCodePoint.Idna.NON_ASCII_MINandPunycode.INITIAL_Nare left separate — same value (0x80), different meaning.internal, the restprivate.Net: ~254 lines of near-verbatim duplication collapsed into one 97-line source of truth. Pure refactor — output is byte-identical for every input.
2. Keep
Uri.relativizetotal when the resolved candidate is invalidUri.relativizeis documented to never throw, but it could raiseIllegalStateException. It verifies its candidate reference by resolving it through the structuredResolver.resolveoverload, which raised an internal error when the recomposed target failed to parse. A candidate suffix such as/.//h:zzis a valid rootless reference, but dot-segment removal collapses it to//h:zz, which re-reads as an authority with a non-numeric port and is rejected by the strict parser — so arelativizeon two valid URIs (s:/a/bagainsts:/a//.//h:zz) threw instead of returningnull.The structured
Resolver.resolveoverload now returnsParseResult<ParsedComponents>rather than raising, andrelativizefolds a resolution failure (or a non-matching round-trip) tonull— the same "no relative form exists" outcome it already returns for its other unreachable cases. The candidate is still resolved from its already-parsed components, so the round-trip stays free of a redundant string re-parse. A regression test pins the/.//h:zzcase.The overload is
internal, so there is no public API change.Safety
Verified on the JVM gate (
jvmTest+ ktlint + detekt + apiCheck): green, with the IDNA (UTS-46) and NFC conformance suites passing unmodified. Every new or changed symbol isinternal, so the public API is untouched (apiCheckpasses with noapiDump). The code-point refactor is behaviour-preserving with byte-identical output; therelativizechange touches only the error-handling path (a resolution failure now yieldsnullinstead of throwing).Stacked on #44.