From 1f18e85863fd846b58e3f46d78fdee01e4eeab28 Mon Sep 17 00:00:00 2001 From: Vitor Pamplona Date: Mon, 21 Sep 2026 17:15:54 -0400 Subject: [PATCH] fix: read the query of an opaque uri on JVM and Android `amethyst+walletconnect:dlnwc?value=...` came back as "Amethyst received a URI to open but that uri was invalid", while the `//` spelling of the very same link worked. isWalletConnectRoute() accepts three spellings; only two survived the parse behind it. A uri whose scheme is followed by anything but `/` is *opaque* to java.net.URI: it reports no query at all and folds `dlnwc?value=...` into one scheme-specific part. The linux actual of UriParser -- a hand-rolled parser -- looks for `?` wherever it sits, so the actuals disagreed and the JVM one was the one losing data. Nip47DeepLink.parseCallbackValue rides the same call, so NWC callbacks were exposed to it too. The query is now recovered from the scheme-specific part when the uri is opaque. The fragment needs no such rescue: java.net.URI does parse `#...` off an opaque uri, and a test pins that. UriParserTest states the contract for every actual. The existing UriToRouteTest only covered the bare `dlnwc?value=` form -- the spelling that already worked -- which is how this went unnoticed; it now covers all three, and fails on the one-colon form without this change. Co-Authored-By: Claude Opus 5 (1M context) --- .../amethyst/ui/UriToRouteTest.kt | 18 ++++ .../quartz/utils/UriParserTest.kt | 97 +++++++++++++++++++ .../quartz/utils/UriParser.jvmAndroid.kt | 22 ++++- 3 files changed, 136 insertions(+), 1 deletion(-) create mode 100644 quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/utils/UriParserTest.kt diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/UriToRouteTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/UriToRouteTest.kt index 67425d5405..95b20c73c2 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/UriToRouteTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/UriToRouteTest.kt @@ -93,6 +93,24 @@ class UriToRouteTest { ) } + @Test + fun walletConnectDeepLinksRouteInEverySpellingIsWalletConnectRouteAccepts() { + // isWalletConnectRoute() accepts three spellings, so all three have to survive the parse + // behind it. They did not: `java.net.URI` calls a scheme followed by anything but `/` an + // *opaque* uri and reports it as having no query at all, so the one-colon form lost its + // `value=` and came back to the user as "that uri was invalid" -- while the `//` spelling + // of the very same link worked. Only the bare form was covered here before. + val encoded = URLEncoder.encode(NWC_URI, Charsets.UTF_8.name()) + + listOf( + "dlnwc?value=$encoded", + "amethyst+walletconnect:dlnwc?value=$encoded", + "amethyst+walletconnect://dlnwc?value=$encoded", + ).forEach { deepLink -> + assertEquals(deepLink, Route.WalletAddNwc(NWC_URI), uriToRoute(deepLink, account)) + } + } + @Test fun theLauncherShortcutOpensTheScannerDirectly() { // res/xml/shortcuts.xml fires `amethyst:scanqr`. If this stops resolving, long-pressing the diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/utils/UriParserTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/utils/UriParserTest.kt new file mode 100644 index 0000000000..a76776ad7f --- /dev/null +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/utils/UriParserTest.kt @@ -0,0 +1,97 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.quartz.utils + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNull + +/** + * The contract every actual of [UriParser] has to keep. + * + * The interesting case is the *opaque* URI -- a scheme followed by anything but `/`. The JVM and + * Android actual delegates to `java.net.URI`, which declares such a URI to have no query at all + * and folds `dlnwc?value=...` into one scheme-specific part, while the hand-rolled actuals look + * for `?` wherever it sits. That disagreement is what made a wallet-connect deep link report + * "that uri was invalid" on Android while the `//` spelling of the same link worked. + */ +class UriParserTest { + private val nwc = "nostr+walletconnect://abc?relay=wss://r.example&secret=s1" + + @Test + fun readsTheQueryOfAnOpaqueUri() { + val parser = UriParser("amethyst+walletconnect:dlnwc?value=${encode(nwc)}") + + assertEquals(nwc, parser.getQueryParameter("value")?.firstOrNull()) + } + + /** The spelling that already worked, so the rescue above cannot have broken it. */ + @Test + fun readsTheQueryOfAHierarchicalUri() { + val parser = UriParser("amethyst+walletconnect://dlnwc?value=${encode(nwc)}") + + assertEquals(nwc, parser.getQueryParameter("value")?.firstOrNull()) + } + + /** No scheme at all, which is how the callback arrives from some wallets. */ + @Test + fun readsTheQueryOfASchemelessUri() { + val parser = UriParser("dlnwc?value=${encode(nwc)}") + + assertEquals(nwc, parser.getQueryParameter("value")?.firstOrNull()) + } + + @Test + fun opaqueUriWithoutAQueryHasNoParameters() { + val parser = UriParser("cashu:sometoken") + + assertNull(parser.getQueryParameter("value")) + assertEquals(emptySet(), parser.queryParameterNames()) + } + + @Test + fun namesEveryParameterOfAnOpaqueUri() { + val parser = UriParser("bunker:pubkeyhex?relay=wss%3A%2F%2Fr.example&secret=s1") + + assertEquals(setOf("relay", "secret"), parser.queryParameterNames()) + assertEquals("wss://r.example", parser.getQueryParameter("relay")?.firstOrNull()) + assertEquals("s1", parser.getQueryParameter("secret")?.firstOrNull()) + } + + /** A fragment is parsed off an opaque URI by every actual already; keep it that way. */ + @Test + fun keepsTheFragmentOfAnOpaqueUri() { + val parser = UriParser("scheme:thing?a=1#b=2") + + assertEquals("1", parser.getQueryParameter("a")?.firstOrNull()) + assertEquals(mapOf("b" to "2"), parser.fragments()) + } + + private fun encode(value: String) = + value + .replace("%", "%25") + .replace("+", "%2B") + .replace(":", "%3A") + .replace("/", "%2F") + .replace("?", "%3F") + .replace("&", "%26") + .replace("=", "%3D") +} diff --git a/quartz/src/jvmAndroid/kotlin/com/vitorpamplona/quartz/utils/UriParser.jvmAndroid.kt b/quartz/src/jvmAndroid/kotlin/com/vitorpamplona/quartz/utils/UriParser.jvmAndroid.kt index 72057834ef..a961dbb53f 100644 --- a/quartz/src/jvmAndroid/kotlin/com/vitorpamplona/quartz/utils/UriParser.jvmAndroid.kt +++ b/quartz/src/jvmAndroid/kotlin/com/vitorpamplona/quartz/utils/UriParser.jvmAndroid.kt @@ -28,8 +28,28 @@ actual class UriParser actual constructor( ) { private val myUri = URI.create(uri) + /** + * The query string, including the one [java.net.URI] refuses to find. + * + * A URI whose scheme is followed by anything other than `/` is *opaque* to `java.net.URI`: + * `amethyst+walletconnect:dlnwc?value=...` has no query at all as far as it is concerned, and + * the whole `dlnwc?value=...` is one scheme-specific part. Every other actual of this class + * looks for `?` wherever it sits, so on JVM and Android alone the parameters of an opaque URI + * silently vanished -- which is how a perfectly valid wallet-connect deep link came back as + * "that uri was invalid", while the `//` spelling of the very same link worked. + * + * The fragment needs no such rescue: `java.net.URI` does parse `#...` off an opaque URI. + */ + private val rawQuery: String? = + myUri.rawQuery + ?: myUri + .takeIf { it.isOpaque } + ?.rawSchemeSpecificPart + ?.substringAfter('?', "") + ?.ifBlank { null } + private val queryParameters: Map> by lazy { - myUri.rawQuery?.ifBlank { null }?.let { query -> + rawQuery?.ifBlank { null }?.let { query -> val params = mutableMapOf>() query.split('&').forEach { paramValue ->