fix: add decoded userinfo/fragment accessors and fix % round-tripping in kuri-bind

kuri-bind/src/jvmMain/kotlin/org/dexpace/kuri/bind/internal/BuilderSink.kt

@@ -8,6 +8,13 @@ import org.dexpace.kuri.Uri
 import org.dexpace.kuri.Url
 import org.dexpace.kuri.percent.Percent
 
+/**
+ * Escapes a literal `%` to `%25` so it survives every downstream encode set unambiguously (none of
+ * which reserve `%` — see [Percent.Component]) as the literal character the caller supplied, rather
+ * than being read back as (or colliding with) a percent-encoded escape.
+ */
+private fun escapeLiteralPercent(text: String): String = text.replace("%", "%25")
+
 /**
  * The narrow set of builder operations the binder needs, abstracting the two profiles.
  *
@@ -22,8 +29,8 @@ internal interface BuilderSink {
     /**
      * Sets the userinfo component from decoded [username] and optional decoded [password].
      *
-     * Each implementation encodes the parts appropriately for its profile:
-     * [UrlBuilderSink] calls split setters; [UriBuilderSink] encodes and joins around a literal `:`.
+     * Both [UrlBuilderSink] and [UriBuilderSink] delegate to their underlying builder's split
+     * `username`/`password` setters, which each encode their own part appropriately for the profile.
      */
     fun userInfo(
         username: String,
@@ -52,9 +59,13 @@ internal interface BuilderSink {
      * Percent-encodes [decoded] under the FRAGMENT set, then stores it. Both profiles' fragment
      * setters take an already-encoded string, so the encode is shared here and each sink only supplies
      * the raw store via [setEncodedFragment].
+     *
+     * A literal `%` in [decoded] is escaped to `%25` first: the FRAGMENT set doesn't reserve `%`
+     * itself, so an un-escaped `%` would pass through unambiguously as data on the way in but read
+     * back as (or collide with) a percent-encoded escape on the way out.
      */
     fun fragmentDecoded(decoded: String) {
-        setEncodedFragment(Percent.encode(decoded, Percent.Component.FRAGMENT))
+        setEncodedFragment(Percent.encode(escapeLiteralPercent(decoded), Percent.Component.FRAGMENT))
     }
 
     /** Stores an already-encoded fragment verbatim on the underlying builder. */
@@ -75,8 +86,8 @@ internal class UrlBuilderSink(
     /**
      * Fully replaces the userinfo slot from decoded [username] and [password] (an empty or absent
      * password clears the slot), so an object's userinfo never leaks a base builder's existing
-     * password. The URL builder encodes each part under the userinfo set; the `:` separator is
-     * managed by the builder.
+     * password. [Url.Builder.username]/[Url.Builder.password] each percent-encode under the userinfo
+     * set and escape a literal `%` themselves, so no manual escape is needed here.
      */
     override fun userInfo(
         username: String,
@@ -101,11 +112,16 @@ internal class UrlBuilderSink(
         builder.addPathSegment(decoded)
     }
 
+    /**
+     * A literal `%` in [name] or [value] is escaped to `%25` first: the query encode sets don't
+     * reserve `%` itself, so an un-escaped `%` would read back as (or collide with) a
+     * percent-encoded escape.
+     */
     override fun addQueryParameter(
         name: String,
         value: String?,
     ) {
-        builder.addQueryParameter(name, value)
+        builder.addQueryParameter(escapeLiteralPercent(name), value?.let { escapeLiteralPercent(it) })
     }
 
     override fun setEncodedFragment(encoded: String) {
@@ -114,12 +130,9 @@ internal class UrlBuilderSink(
 }
 
 /**
- * Projects decoded contributions onto a [Uri.Builder]: encodes username and password separately
- * under the USER_INFO set, then joins them around a literal `:` before passing the verbatim
- * userinfo string to the builder. Fragment is handled the same way as [UrlBuilderSink].
- *
- * [Uri.Builder.userInfo] stores the value verbatim (already-encoded), so encoding must happen
- * here before the call.
+ * Projects decoded contributions onto a [Uri.Builder]: split username/password setters, and a
+ * percent-encoded fragment stored via the raw-fragment setter. Fragment is handled the same way
+ * as [UrlBuilderSink].
  */
 internal class UriBuilderSink(
     private val builder: Uri.Builder,
@@ -129,25 +142,22 @@ internal class UriBuilderSink(
     }
 
     /**
-     * Percent-encodes [username] and [password] under the USER_INFO set, then joins them with a
-     * literal `:` separator. The joined string is stored verbatim by [Uri.Builder.userInfo].
-     *
-     * The `:` itself is NOT encoded because it acts as the structural delimiter between the two
-     * userinfo sub-components, not as data.
+     * Fully replaces the userinfo slot from decoded [username] and [password] (an absent password
+     * clears the slot), so an object's userinfo never leaks a base builder's existing password.
+     * [Uri.Builder.username]/[Uri.Builder.password] each percent-encode under the USER_INFO set and
+     * escape a literal `%` themselves, join with `:`, and switch the builder to split userinfo mode
+     * (discarding any verbatim [Uri.Builder.userInfo] value the base builder carried), so no manual
+     * encode/join is needed here.
      */
     override fun userInfo(
         username: String,
         password: String?,
     ) {
         require(username.isNotEmpty() || password != null) { "at least one of username or password must be present" }
-        val encodedUser = Percent.encode(username, Percent.Component.USER_INFO)
-        val joined =
-            if (password == null) {
-                encodedUser
-            } else {
-                "$encodedUser:${Percent.encode(password, Percent.Component.USER_INFO)}"
-            }
-        builder.userInfo(joined)
+        builder.username(username)
+        // Always set the password (empty clears it) so the object's userinfo FULLY replaces a base
+        // builder's slot — otherwise a username-only object would leak a base URI's existing password.
+        builder.password(password ?: "")
     }
 
     override fun hostText(value: String) {
@@ -162,11 +172,16 @@ internal class UriBuilderSink(
         builder.addPathSegment(decoded)
     }
 
+    /**
+     * A literal `%` in [name] or [value] is escaped to `%25` first: the query encode sets don't
+     * reserve `%` itself, so an un-escaped `%` would read back as (or collide with) a
+     * percent-encoded escape.
+     */
     override fun addQueryParameter(
         name: String,
         value: String?,
     ) {
-        builder.addQueryParameter(name, value)
+        builder.addQueryParameter(escapeLiteralPercent(name), value?.let { escapeLiteralPercent(it) })
     }
 
     override fun setEncodedFragment(encoded: String) {

kuri-bind/src/jvmTest/kotlin/org/dexpace/kuri/bind/IntegrationTest.kt

@@ -4,6 +4,7 @@
  */
 package org.dexpace.kuri.bind
 
+import org.dexpace.kuri.percent.Percent
 import java.math.BigDecimal
 import java.time.Instant
 import java.util.UUID
@@ -492,14 +493,42 @@ private class DotSegment(
     @Path val b: String = "..",
 )
 
-// Pins the inherited `%`-under-encoding of the core query set.
+// A literal `%` in a decoded query value: the core query set doesn't reserve `%` itself, so the
+// binder boundary escapes it to `%25` before that value reaches the core encode call (#76).
 @Url
 private class PercentValue(
     @Scheme val scheme: String = "https",
     @Host val host: String = "h",
     @Query("q") val q: String = "a%2Bb",
 )
 
+// As [PercentValue], but on the `@Uri` profile: no `@Uri` fixture elsewhere in this file exercises
+// `@Query`, so this proves the same query-value escape end-to-end for that profile too.
+@Uri
+private class UriPercentValue(
+    @Scheme val scheme: String = "https",
+    @Host val host: String = "h",
+    @Query("q") val q: String = "a%2Bb",
+)
+
+// As [UrlCreds], but with a literal `%` in each credential: proves the builder-level escape
+// (BuilderSinkTest's direct-Sink coverage) also holds through the full annotation-binding path.
+@Url
+private class UrlCredsPercent(
+    @Scheme val scheme: String = "https",
+    @Host val host: String = "h",
+    @Username val user: String = "50%",
+    @Password val pass: String = "60%",
+)
+
+// As [UriCreds], but with a literal `%` in each half of the userinfo token.
+@Uri
+private class UriCredsPercent(
+    @Scheme val scheme: String = "https",
+    @Host val host: String = "h",
+    @UserInfo val creds: String = "50%:60%",
+)
+
 // A delimited `@Query` list: its elements join into one comma-separated parameter value instead of
 // fanning out, and `QueryParameters.split` (core, `feat/query-value-split`) is its read-side inverse.
 @Url
@@ -934,10 +963,43 @@ class IntegrationTest {
     }
 
     @Test
-    fun `pins the inherited percent under-encoding of the query set`() {
+    fun `escapes a literal percent sign in a query value so it round-trips through decode`() {
         // The core query set leaves `%` literal (WHATWG "already-encoded" convention) rather than
-        // re-encoding it to `%25` — pinned to document the contract, not a binder-level choice.
-        assertEquals("q=a%2Bb", KuriBind.toUrl(PercentValue()).query)
+        // re-encoding it to `%25`, so the binder boundary escapes it first: "a%2Bb" reaches the wire
+        // as "a%252Bb", which decodes back to the original literal "a%2Bb" rather than misreading the
+        // embedded "%2B" as an escaped '+'.
+        val url = KuriBind.toUrl(PercentValue())
+
+        assertEquals("q=a%252Bb", url.query)
+        assertEquals("a%2Bb", url.queryParameters.get("q"))
+    }
+
+    @Test
+    fun `escapes a literal percent sign in a uri profile query value so it round-trips through decode`() {
+        // As above, but through the `@Uri` profile end-to-end path — no other `@Uri` fixture in this
+        // file exercises `@Query` at all.
+        val uri = KuriBind.toUri(UriPercentValue())
+
+        assertEquals("q=a%252Bb", uri.query)
+        assertEquals("a%2Bb", uri.queryParameters().get("q"))
+    }
+
+    @Test
+    fun `pairs a literal percent sign in sibling username and password into split url userinfo`() {
+        val url = KuriBind.toUrl(UrlCredsPercent())
+
+        assertEquals("50%", url.decodedUsername)
+        assertEquals("60%", url.decodedPassword)
+    }
+
+    @Test
+    fun `encodes a literal percent sign in a whole userinfo token on the uri profile`() {
+        val uri = KuriBind.toUri(UriCredsPercent())
+        val userInfo = requireNotNull(uri.userInfo)
+        val (user, pass) = userInfo.split(':', limit = 2)
+
+        assertEquals("50%", Percent.decode(user))
+        assertEquals("60%", Percent.decode(pass))
     }
 
     @Test

kuri-bind/src/jvmTest/kotlin/org/dexpace/kuri/bind/internal/BuilderSinkTest.kt

@@ -6,6 +6,7 @@ package org.dexpace.kuri.bind.internal
 
 import org.dexpace.kuri.Uri
 import org.dexpace.kuri.Url
+import org.dexpace.kuri.percent.Percent
 import kotlin.test.Test
 import kotlin.test.assertEquals
 
@@ -97,4 +98,137 @@ class BuilderSinkTest {
         val uri = b.build()
         assertEquals("https://h?k=v", uri.toString())
     }
+
+    // A literal '%' in a decoded value must survive as data, not be misread as a percent-escape
+    // introducer, once it reaches the wire — the query/fragment/userinfo encode sets don't reserve
+    // '%' themselves (issue #76).
+
+    @Test
+    fun `url sink escapes a literal percent in a query value so it reads back literally`() {
+        val b = Url.Builder().scheme("https").host("h")
+        val sink = UrlBuilderSink(b)
+        sink.addQueryParameter("q", "100%3Doff")
+        val url = b.build()
+        assertEquals("100%3Doff", url.queryParameters.get("q"))
+    }
+
+    @Test
+    fun `uri sink escapes a literal percent in a query value so it reads back literally`() {
+        val b = Uri.Builder().scheme("https").host("h")
+        val sink = UriBuilderSink(b)
+        sink.addQueryParameter("q", "100%3Doff")
+        val uri = b.build()
+        assertEquals("100%3Doff", uri.queryParameters().get("q"))
+    }
+
+    @Test
+    fun `url sink escapes a literal percent in a query parameter name so it reads back literally`() {
+        val b = Url.Builder().scheme("https").host("h")
+        val sink = UrlBuilderSink(b)
+        sink.addQueryParameter("100%3Doff", "v")
+        val url = b.build()
+        assertEquals("v", url.queryParameters.get("100%3Doff"))
+    }
+
+    @Test
+    fun `uri sink escapes a literal percent in a query parameter name so it reads back literally`() {
+        val b = Uri.Builder().scheme("https").host("h")
+        val sink = UriBuilderSink(b)
+        sink.addQueryParameter("100%3Doff", "v")
+        val uri = b.build()
+        assertEquals("v", uri.queryParameters().get("100%3Doff"))
+    }
+
+    @Test
+    fun `url sink escapes a literal percent in a decoded fragment so it reads back literally`() {
+        val b = Url.Builder().scheme("https").host("h")
+        val sink = UrlBuilderSink(b)
+        sink.fragmentDecoded("100%3Doff")
+        val url = b.build()
+        assertEquals("100%3Doff", Percent.decode(requireNotNull(url.encodedFragment)))
+    }
+
+    @Test
+    fun `uri sink escapes a literal percent in a decoded fragment so it reads back literally`() {
+        val b = Uri.Builder().scheme("https").host("h")
+        val sink = UriBuilderSink(b)
+        sink.fragmentDecoded("100%3Doff")
+        val uri = b.build()
+        assertEquals("100%3Doff", Percent.decode(requireNotNull(uri.fragment)))
+    }
+
+    @Test
+    fun `url sink escapes a literal percent in a username so it reads back literally`() {
+        val b = Url.Builder().scheme("https").host("h")
+        val sink = UrlBuilderSink(b)
+        sink.userInfo("100%3Doff", null)
+        val url = b.build()
+        assertEquals("100%3Doff", Percent.decode(url.username))
+    }
+
+    @Test
+    fun `url sink escapes a literal percent in a password so it reads back literally`() {
+        val b = Url.Builder().scheme("https").host("h")
+        val sink = UrlBuilderSink(b)
+        sink.userInfo("bob", "100%3Doff")
+        val url = b.build()
+        assertEquals("100%3Doff", Percent.decode(url.password))
+    }
+
+    @Test
+    fun `uri sink escapes a literal percent in a username so it reads back literally`() {
+        val b = Uri.Builder().scheme("https").host("h")
+        val sink = UriBuilderSink(b)
+        sink.userInfo("100%3Doff", null)
+        val uri = b.build()
+        assertEquals("100%3Doff", Percent.decode(requireNotNull(uri.userInfo)))
+    }
+
+    @Test
+    fun `uri sink escapes a literal percent in a password so it reads back literally`() {
+        val b = Uri.Builder().scheme("https").host("h")
+        val sink = UriBuilderSink(b)
+        sink.userInfo("bob", "100%3Doff")
+        val uri = b.build()
+        val password = requireNotNull(uri.userInfo).substringAfter(':')
+        assertEquals("100%3Doff", Percent.decode(password))
+    }
+
+    @Test
+    fun `url sink escaping round-trips a bare percent beside a character the set reserves`() {
+        val b = Url.Builder().scheme("https").host("h")
+        val sink = UrlBuilderSink(b)
+        sink.addQueryParameter("q", "100% off")
+        val url = b.build()
+        assertEquals("100% off", url.queryParameters.get("q"))
+    }
+
+    @Test
+    fun `uri sink escaping round-trips a bare percent beside a character the set reserves`() {
+        val b = Uri.Builder().scheme("https").host("h")
+        val sink = UriBuilderSink(b)
+        sink.addQueryParameter("q", "100% off")
+        val uri = b.build()
+        assertEquals("100% off", uri.queryParameters().get("q"))
+    }
+
+    @Test
+    fun `url sink query value with no percent sign is unaffected by the escape`() {
+        val b = Url.Builder().scheme("https").host("h")
+        val sink = UrlBuilderSink(b)
+        sink.addQueryParameter("q", "hello world")
+        val url = b.build()
+        assertEquals("q=hello%20world", url.query)
+        assertEquals("hello world", url.queryParameters.get("q"))
+    }
+
+    @Test
+    fun `uri sink query value with no percent sign is unaffected by the escape`() {
+        val b = Uri.Builder().scheme("https").host("h")
+        val sink = UriBuilderSink(b)
+        sink.addQueryParameter("q", "hello world")
+        val uri = b.build()
+        assertEquals("q=hello%20world", uri.query)
+        assertEquals("hello world", uri.queryParameters().get("q"))
+    }
 }