Skip to content

refactor: centralize UTF-16 code-point primitives#45

Merged
OmarAlJarrah merged 2 commits into
mainfrom
refactor/centralize-codepoint-primitives
Jul 8, 2026
Merged

refactor: centralize UTF-16 code-point primitives#45
OmarAlJarrah merged 2 commits into
mainfrom
refactor/centralize-codepoint-primitives

Conversation

@OmarAlJarrah

@OmarAlJarrah OmarAlJarrah commented Jul 7, 2026

Copy link
Copy Markdown
Member

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, 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/QueryParametersBuilder and percent/PercentCodec.

Kotlin's common stdlib has no code-point iteration (Character.toChars / codePointAt are 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 internal text/CodePoints.kt and route every call site through it.

  • IdnaValidity keeps its boxing-free IntArray code-point scan (it's on a hot path), now delegating to the shared toCodePoint.
  • Idna.NON_ASCII_MIN and Punycode.INITIAL_N are left separate — same value (0x80), different meaning.
  • Constant visibility is narrowest-that-compiles: only the two constants consumers import are internal, the rest private.

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.relativize total when the resolved candidate is invalid

Uri.relativize is documented to never throw, but it could raise IllegalStateException. It verifies its candidate reference by resolving it through the structured Resolver.resolve overload, which raised an internal error when the recomposed target failed to parse. 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 is rejected by the strict parser — so a relativize on two valid URIs (s:/a/b against s:/a//.//h:zz) threw instead of returning null.

The structured Resolver.resolve overload now returns ParseResult<ParsedComponents> rather than raising, and relativize folds a resolution failure (or a non-matching round-trip) to null — 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:zz case.

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 is internal, so the public API is untouched (apiCheck passes with no apiDump). The code-point refactor is behaviour-preserving with byte-identical output; the relativize change touches only the error-handling path (a resolution failure now yields null instead of throwing).

Stacked on #44.

@OmarAlJarrah OmarAlJarrah changed the base branch from refactor/relativize-avoid-redundant-parse to main July 8, 2026 09:44
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.
@OmarAlJarrah OmarAlJarrah force-pushed the refactor/centralize-codepoint-primitives branch from 256c6b6 to 0f71fdd Compare July 8, 2026 10:03
@OmarAlJarrah OmarAlJarrah merged commit a106618 into main Jul 8, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant