From 7e0814c66bca6a279ab1fac630284248a6528a85 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Tue, 7 Jul 2026 16:17:45 +0300 Subject: [PATCH] refactor: route host/authority/path serialization through single owners MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Serialization logic was reimplemented in several places instead of delegating to the canonical renderers: - parser/Resolver held a byte-for-byte copy of host/HostSerialization's serializeHost (the host `when`, the zone-id suffix, the "%25" constant) — a latent drift bug in the resolution path's re-serialization. Deleted; it now calls serializeHost. - Url.encodedPath and Url.authority re-implemented Serializer's URL-path and authority serialization, including the non-obvious leading-"/." path guard. Promoted Serializer's helpers to internal top-level functions (serializeUrlPath, serializeAuthority) and delegated Url to them; Serializer still uses them itself. - Removed the redundant internal Host.serialize() extension — a synonym for the public Host.asText() with an identical body — and routed its callers to asText(). This drops one tautological Ipv6 test that only asserted the two synonyms agree; asText() stays covered by the host validation / ipv4 / ipv6 suites and the coverage floor still holds. Two duplications are left in place on purpose. Uri.reconstructAuthority keeps its own assembly: its nullable-userInfo empty->null distinction can't be expressed by the string-only shared helper. Resolver's authority assembly is also left duplicated — consolidating it would add a parser->serialize import and form a package cycle with the existing serialize->parser edge, which four trivial lines don't justify. Pure refactor: no behavior change, public API unchanged. --- .../commonMain/kotlin/org/dexpace/kuri/Uri.kt | 5 +- .../commonMain/kotlin/org/dexpace/kuri/Url.kt | 36 +------ .../dexpace/kuri/host/HostSerialization.kt | 13 --- .../org/dexpace/kuri/parser/Resolver.kt | 23 +---- .../org/dexpace/kuri/serialize/Serializer.kt | 97 +++++++++++-------- .../kotlin/org/dexpace/kuri/host/Ipv6Test.kt | 11 +-- 6 files changed, 66 insertions(+), 119 deletions(-) diff --git a/kuri/src/commonMain/kotlin/org/dexpace/kuri/Uri.kt b/kuri/src/commonMain/kotlin/org/dexpace/kuri/Uri.kt index c60e404..794e371 100644 --- a/kuri/src/commonMain/kotlin/org/dexpace/kuri/Uri.kt +++ b/kuri/src/commonMain/kotlin/org/dexpace/kuri/Uri.kt @@ -8,7 +8,6 @@ import org.dexpace.kuri.error.ParseResult import org.dexpace.kuri.error.UriSyntaxException import org.dexpace.kuri.error.map import org.dexpace.kuri.host.Host -import org.dexpace.kuri.host.serialize import org.dexpace.kuri.parser.BuilderPath import org.dexpace.kuri.parser.ParsedComponents import org.dexpace.kuri.parser.Resolver @@ -92,7 +91,7 @@ public class Uri internal constructor( /** The serialized host text (brackets included for IPv6), or `null` when there is no authority. */ @get:JvmName("hostName") public val hostName: String? - get() = components.host?.serialize() + get() = components.host?.asText() /** * The explicit port, preserved exactly as parsed, or `null` when no port was present. @@ -517,7 +516,7 @@ public class Uri internal constructor( val authorityHost = components.host ?: return null val credentials = reconstructUserInfo()?.let { "$it@" } ?: "" val portPart = components.port?.let { ":$it" } ?: "" - return "$credentials${authorityHost.serialize()}$portPart" + return "$credentials${authorityHost.asText()}$portPart" } /** Decoded path and segments, computed once each; the value is immutable, mirroring [canonicalUri]. */ diff --git a/kuri/src/commonMain/kotlin/org/dexpace/kuri/Url.kt b/kuri/src/commonMain/kotlin/org/dexpace/kuri/Url.kt index 243d91c..e7064a3 100644 --- a/kuri/src/commonMain/kotlin/org/dexpace/kuri/Url.kt +++ b/kuri/src/commonMain/kotlin/org/dexpace/kuri/Url.kt @@ -9,7 +9,6 @@ import org.dexpace.kuri.error.UriSyntaxException import org.dexpace.kuri.error.ValidationError import org.dexpace.kuri.error.map import org.dexpace.kuri.host.Host -import org.dexpace.kuri.host.serialize import org.dexpace.kuri.parser.BuilderPath import org.dexpace.kuri.parser.ParsedComponents import org.dexpace.kuri.parser.UrlParser @@ -25,6 +24,8 @@ import org.dexpace.kuri.query.QueryState import org.dexpace.kuri.query.applyParameterEdit import org.dexpace.kuri.scheme.Scheme import org.dexpace.kuri.serialize.Serializer +import org.dexpace.kuri.serialize.serializeAuthority +import org.dexpace.kuri.serialize.serializeUrlPath import kotlin.jvm.JvmName import kotlin.jvm.JvmOverloads import kotlin.jvm.JvmStatic @@ -109,7 +110,7 @@ public class Url internal constructor( */ @get:JvmName("hostName") public val hostName: String? - get() = components.host?.serialize() + get() = components.host?.asText() /** * The explicit port, or `null` when the port was elided or equals the scheme default. @@ -144,7 +145,7 @@ public class Url internal constructor( /** The canonical encoded path string (e.g. `/a/b`, or the opaque path verbatim). */ @get:JvmName("encodedPath") public val encodedPath: String - get() = serializeEncodedPath() + get() = serializeUrlPath(components) /** * The raw encoded query without its leading `?`, or `null` when no `?` was present. @@ -179,7 +180,7 @@ public class Url internal constructor( /** The `userinfo@host:port` authority, or `null` when the URL has no authority. */ @get:JvmName("authority") public val authority: String? - get() = components.host?.let { serializeAuthority(it) } + get() = if (components.host == null) null else serializeAuthority(components) /** * The ASCII serialization of this URL's WHATWG origin (§11.6). @@ -407,33 +408,6 @@ public class Url internal constructor( /** The hash of the canonical [href], consistent with [equals]. */ override fun hashCode(): Int = href.hashCode() - /** Recomposes the encoded path, applying the no-authority leading-`/.` guard. */ - private fun serializeEncodedPath(): String = - when (val path = components.path) { - is UrlPath.Opaque -> path.path - is UrlPath.Segments -> joinSegments(path.segments) - } - - /** Joins encoded [segments] as `/`-prefixed runs, guarding a hostless `//`-opening path. */ - private fun joinSegments(segments: List): String { - val joined = segments.joinToString("") { "/$it" } - val needsGuard = components.host == null && segments.size > 1 && segments[0] == "" - return if (needsGuard) "/.$joined" else joined - } - - /** Serializes `[userinfo@]host[:port]` for a present authority. */ - private fun serializeAuthority(authorityHost: Host): String { - val credentials = if (username.isEmpty() && password.isEmpty()) "" else credentialsPrefix() - val portPart = components.port?.let { ":$it" } ?: "" - return "$credentials${authorityHost.serialize()}$portPart" - } - - /** The `user[:password]@` credentials prefix for a value that includes credentials. */ - private fun credentialsPrefix(): String { - val passwordPart = if (password.isNotEmpty()) ":$password" else "" - return "$username$passwordPart@" - } - /** The tuple origin `scheme://host[:port]`: host elides to `""`, port and userinfo are omitted. */ private fun tupleOrigin(): String = "$scheme://${hostName.orEmpty()}${port?.let { ":$it" } ?: ""}" diff --git a/kuri/src/commonMain/kotlin/org/dexpace/kuri/host/HostSerialization.kt b/kuri/src/commonMain/kotlin/org/dexpace/kuri/host/HostSerialization.kt index b41bd7d..08b49f3 100644 --- a/kuri/src/commonMain/kotlin/org/dexpace/kuri/host/HostSerialization.kt +++ b/kuri/src/commonMain/kotlin/org/dexpace/kuri/host/HostSerialization.kt @@ -34,18 +34,5 @@ internal fun serializeHost(host: Host): String = Host.Empty -> "" } -/** - * Renders this [Host] to its canonical authority text, bracketing IPv6 / IP-future literals. - * - * An internal extension alias onto [serializeHost], kept as an ergonomic receiver form for the - * `Uri`/`Url` serializers; the public renderer is the [Host.asText] member. It applies the §11.2 / - * RFC 3986 §3.2.2 bracketing rules, so the result carries brackets for [Host.Ipv6] / [Host.IpFuture] - * and the value verbatim otherwise. - * - * @return the host's canonical authority text. - * @see Host.asText - */ -internal fun Host.serialize(): String = serializeHost(this) - /** Renders an optional IPv6 zone id as the RFC 6874 `%25` suffix, or `""` when absent. */ private fun zoneSuffix(zoneId: String?): String = if (zoneId == null) "" else "$ZONE_PREFIX$zoneId" diff --git a/kuri/src/commonMain/kotlin/org/dexpace/kuri/parser/Resolver.kt b/kuri/src/commonMain/kotlin/org/dexpace/kuri/parser/Resolver.kt index 012b898..18cdcb3 100644 --- a/kuri/src/commonMain/kotlin/org/dexpace/kuri/parser/Resolver.kt +++ b/kuri/src/commonMain/kotlin/org/dexpace/kuri/parser/Resolver.kt @@ -9,9 +9,7 @@ import org.dexpace.kuri.ZONE_ID_ENABLED import org.dexpace.kuri.carriesZoneId import org.dexpace.kuri.error.ParseResult import org.dexpace.kuri.error.UriParseError -import org.dexpace.kuri.host.Host -import org.dexpace.kuri.host.Ipv4 -import org.dexpace.kuri.host.Ipv6 +import org.dexpace.kuri.host.serializeHost import org.dexpace.kuri.scheme.Scheme import org.dexpace.kuri.scheme.schemeColonIndex @@ -48,9 +46,6 @@ private const val SEGMENT_DOT: String = "." /** §5.2.4 case D terminal: the whole remaining input is the bare complete segment `..`. */ private const val SEGMENT_DOTDOT: String = ".." -/** RFC 6874 zone-id introducer used when re-serializing an IPv6 authority. */ -private const val ZONE_PREFIX: String = "%25" - /** * RFC 3986 §5 reference resolution — the SPEC §9 "resolve a relative reference against an absolute * base" operation. @@ -378,7 +373,7 @@ internal object Resolver { private fun authorityOf(c: ParsedComponents): String? { val host = c.host ?: return null val port = if (c.port != null) ":${c.port}" else "" - return userinfoPrefix(c) + hostString(host) + port + return userinfoPrefix(c) + serializeHost(host) + port } /** Builds the `userinfo@` prefix from the decoded credentials, or `""` when no userinfo is present. */ @@ -389,20 +384,6 @@ internal object Resolver { else -> "${c.username}:${c.password}@" } - /** Serializes a [Host] to its authority text, applying IPv6/IP-future bracketing (§3.5, §7.9). */ - private fun hostString(host: Host): String = - when (host) { - is Host.RegName -> host.value - is Host.Opaque -> host.value - is Host.Ipv4 -> Ipv4.serialize(host.value) - is Host.Ipv6 -> "[${Ipv6.serialize(host.pieces)}${zoneSuffix(host.zoneId)}]" - is Host.IpFuture -> "[${host.value}]" - Host.Empty -> "" - } - - /** Renders an optional IPv6 zone id as the RFC 6874 `%25` suffix, or `""` when absent. */ - private fun zoneSuffix(zoneId: String?): String = if (zoneId == null) "" else "$ZONE_PREFIX$zoneId" - /** The behaviour-free five-component view the §5 algorithm operates on (each component nullable per §5.3). */ private data class UriParts( val scheme: String?, diff --git a/kuri/src/commonMain/kotlin/org/dexpace/kuri/serialize/Serializer.kt b/kuri/src/commonMain/kotlin/org/dexpace/kuri/serialize/Serializer.kt index 6507c64..c67148c 100644 --- a/kuri/src/commonMain/kotlin/org/dexpace/kuri/serialize/Serializer.kt +++ b/kuri/src/commonMain/kotlin/org/dexpace/kuri/serialize/Serializer.kt @@ -103,48 +103,6 @@ internal object Serializer { return sb.toString() } - /** §11.2 [NORM-16] authority `[userinfo "@"] host [":" port]`; the credentials rule is shared (below). */ - private fun serializeAuthority(c: ParsedComponents): String { - val host = requireNotNull(c.host) { "authority serialization requires a host" } - val port = if (c.port != null) ":${c.port}" else "" - return credentialsPrefix(c) + serializeHost(host) + port - } - - /** - * The `userinfo@` prefix ([NORM-16] / [NORM-30]); empty for a value with no credentials. - * - * The WHATWG "includes credentials" rule and the RFC null/empty rule coincide here because - * [ParsedComponents] holds the credentials as (possibly empty) strings, never null: the prefix - * appears iff `username` or `password` is non-empty, and `:password` iff `password` is non-empty. - */ - private fun credentialsPrefix(c: ParsedComponents): String { - if (c.username.isEmpty() && c.password.isEmpty()) return "" - val password = if (c.password.isNotEmpty()) ":${c.password}" else "" - return "${c.username}$password@" - } - - /** WHATWG URL path serialization: an opaque path verbatim, else the segment list ([NORM-15] step 3). */ - private fun serializeUrlPath(c: ParsedComponents): String = - when (val path = c.path) { - is UrlPath.Opaque -> path.path - is UrlPath.Segments -> serializeUrlSegments(path.segments, noAuthority = c.host == null) - } - - /** - * Joins URL path [segments] as `"/" + segment` runs, applying the [NORM-18] `/.` guard. - * - * The guard fires only for a no-authority value whose first segment is empty and which has a - * further segment (so the serialization would open `//` and re-parse with a spurious authority). - */ - private fun serializeUrlSegments( - segments: List, - noAuthority: Boolean, - ): String { - val needsGuard = noAuthority && segments.size > 1 && segments[0] == "" - val prefix = if (needsGuard) LEADING_DOT_GUARD else "" - return prefix + segments.joinToString("") { "$SLASH$it" } - } - /** Appends `?query` then (unless excluded) `#fragment`, each only when its component is present. */ private fun appendQueryFragment( sb: StringBuilder, @@ -155,3 +113,58 @@ internal object Serializer { if (!excludeFragment && c.fragment != null) sb.append('#').append(c.fragment) } } + +/** + * §11.2 [NORM-16] authority serializer `[userinfo "@"] host [":" port]`; the single home shared by both + * profiles' URL-side authority rendering (the `Serializer` object and `Url.authority`). + * + * @param components the stored components whose authority is rendered; MUST carry a non-null host. + * @return the authority text `[userinfo@]host[:port]`. + */ +internal fun serializeAuthority(components: ParsedComponents): String { + val host = requireNotNull(components.host) { "authority serialization requires a host" } + val port = if (components.port != null) ":${components.port}" else "" + return credentialsPrefix(components) + serializeHost(host) + port +} + +/** + * The `userinfo@` prefix ([NORM-16] / [NORM-30]); empty for a value with no credentials. + * + * The WHATWG "includes credentials" rule and the RFC null/empty rule coincide here because + * [ParsedComponents] holds the credentials as (possibly empty) strings, never null: the prefix + * appears iff `username` or `password` is non-empty, and `:password` iff `password` is non-empty. + */ +private fun credentialsPrefix(components: ParsedComponents): String { + if (components.username.isEmpty() && components.password.isEmpty()) return "" + val password = if (components.password.isNotEmpty()) ":${components.password}" else "" + return "${components.username}$password@" +} + +/** + * WHATWG URL path serialization: an opaque path verbatim, else the segment list ([NORM-15] step 3). + * + * The single home shared by the `Serializer` object and `Url.encodedPath`. + * + * @param components the stored components whose path is rendered. + * @return the encoded URL path string, with the [NORM-18] `/.` guard applied where required. + */ +internal fun serializeUrlPath(components: ParsedComponents): String = + when (val path = components.path) { + is UrlPath.Opaque -> path.path + is UrlPath.Segments -> serializeUrlSegments(path.segments, noAuthority = components.host == null) + } + +/** + * Joins URL path [segments] as `"/" + segment` runs, applying the [NORM-18] `/.` guard. + * + * The guard fires only for a no-authority value whose first segment is empty and which has a + * further segment (so the serialization would open `//` and re-parse with a spurious authority). + */ +private fun serializeUrlSegments( + segments: List, + noAuthority: Boolean, +): String { + val needsGuard = noAuthority && segments.size > 1 && segments[0] == "" + val prefix = if (needsGuard) LEADING_DOT_GUARD else "" + return prefix + segments.joinToString("") { "$SLASH$it" } +} diff --git a/kuri/src/commonTest/kotlin/org/dexpace/kuri/host/Ipv6Test.kt b/kuri/src/commonTest/kotlin/org/dexpace/kuri/host/Ipv6Test.kt index a04a0fe..19bbd6d 100644 --- a/kuri/src/commonTest/kotlin/org/dexpace/kuri/host/Ipv6Test.kt +++ b/kuri/src/commonTest/kotlin/org/dexpace/kuri/host/Ipv6Test.kt @@ -191,19 +191,12 @@ class Ipv6Test { @Test fun `a zoned host serializes with the raw zone after the introducer`() { val host = Host.Ipv6(listOf(0xFE80, 0, 0, 0, 0, 0, 0, 1), zoneId = "eth0") - assertEquals("[fe80::1%25eth0]", host.serialize()) + assertEquals("[fe80::1%25eth0]", host.asText()) } @Test fun `an unzoned host serializes with no zone suffix`() { val host = Host.Ipv6(listOf(0xFE80, 0, 0, 0, 0, 0, 0, 1)) - assertEquals("[fe80::1]", host.serialize()) - } - - @Test - fun `asText matches the serialize extension for a bracketed ipv6 host`() { - val host = Host.Ipv6(listOf(0x2001, 0xDB8, 0, 0, 0, 0, 0, 1)) - assertEquals(host.serialize(), host.asText(), "the member and extension must agree") - assertEquals("[2001:db8::1]", host.asText()) + assertEquals("[fe80::1]", host.asText()) } }