From a87187151aa062d6037d4bb51844b1f9b6f1395a Mon Sep 17 00:00:00 2001 From: mstrofnone Date: Thu, 23 Jul 2026 15:23:01 +1000 Subject: [PATCH] fix(namecoin): don't leak lookup exceptions through resolve() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `IElectrumXClient.nameShowWithFallback` implementations always throw a `NamecoinLookupException` subtype (NameNotFound, NameExpired, ServersUnreachable) on failure — the return-nullable signature is a legacy of the old contract but no live impl (`ElectrumXClient`, `CompositeNamecoinBackend`) actually returns null anymore. `NamecoinNameResolver.performLookup` (the code path taken by the non-detailed `resolve()` used by `Nip05Client.verify()`) has no try/catch around that call. So any transport-level failure propagates out of `resolve()` into `Nip05State.checkAndUpdate`, where the `catch (e: Exception)` handler calls `markAsError()` and the NIP-05 verification badge in the UI turns into a red "Report" icon with the `nip05_failed` string — regardless of whether the actual cause was "servers all unreachable" or "resolver returned the wrong pubkey". That collapses two very different failure modes into the same red-icon state: 1. Real verification failure (kind-0 nip05 doesn't match the Namecoin record's pubkey) — user should investigate. 2. Transient network issue (custom ElectrumX server offline; `fallbackToDefaultElectrumx` disabled; carrier blocking port 50002/57002; Tor toggle on with Tor unreachable; etc.) — user should retry or check settings. `resolveDetailed()` already has the try/catch and returns structured outcomes (`NameNotFound` / `ServersUnreachable` / etc.). This change brings `performLookup` in line so `resolve()` honours its documented "returns null on any failure" contract, matching how `expandImportsIfPresent` already treats `NamecoinLookupException` during import-target fetches (best-effort → null). Repro: * Namecoin Settings → set backend to "Namecoin Core RPC" with a localhost URL, no fallback. Or set a custom ElectrumX server that the phone can't reach (different network, cellular vs. Wi-Fi, etc.). Or leave `fallbackToDefaultElectrumx = false` with unreachable customServers. * Open any profile whose `nip05` field ends in `.bit` — you get the red icon (Error state) even though the record itself is fine on-chain and the pubkey matches. * After this fix: same setup surfaces null through `resolve()` → `verify()` returns false → `markAsInvalid()` → still red (Failed state, not Error), but no exception leaks. And the identical `NamecoinLookupException` handling on both the primary lookup and the import-target fetch means resolution behaves consistently regardless of which hop fails. Tests: `NamecoinNameResolverExceptionTest` pins the contract with 7 hermetic cases covering NameNotFound / NameExpired / ServersUnreachable / generic transport failure / CancellationException propagation / and continued `resolveDetailed()` visibility of the specific outcome subtype. Verification: ./gradlew :quartz:verifyKmpPurity \ :quartz:compileKotlinLinuxX64 \ :quartz:compileKotlinIosArm64 \ :quartz:spotlessCheck \ :quartz:jvmTest --tests \ 'com.vitorpamplona.quartz.nip05.namecoin.*' BUILD SUCCESSFUL. All namecoin nip05 tests green; KMP purity + iOS + Linux native + spotless all clean. --- .../namecoin/NamecoinNameResolver.kt | 22 ++- .../NamecoinNameResolverExceptionTest.kt | 164 ++++++++++++++++++ 2 files changed, 185 insertions(+), 1 deletion(-) create mode 100644 quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip05/namecoin/NamecoinNameResolverExceptionTest.kt diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip05DnsIdentifiers/namecoin/NamecoinNameResolver.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip05DnsIdentifiers/namecoin/NamecoinNameResolver.kt index f55f4f28f6..ca7cadea5b 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip05DnsIdentifiers/namecoin/NamecoinNameResolver.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip05DnsIdentifiers/namecoin/NamecoinNameResolver.kt @@ -222,7 +222,27 @@ class NamecoinNameResolver( // ── Lookup & Value Parsing ───────────────────────────────────────── private suspend fun performLookup(parsed: ParsedIdentifier): NamecoinNostrResult? { - val nameResult = electrumxClient.nameShowWithFallback(parsed.namecoinName, serverListProvider()) ?: return null + // nameShowWithFallback never returns null in practice — every failure + // path throws a NamecoinLookupException subtype (NameNotFound, + // NameExpired, ServersUnreachable). If we don't catch them here, they + // propagate out through resolve() and up to Nip05State.checkAndUpdate, + // which lumps every lookup failure into markAsError() → red icon with + // no way for the user to tell "servers unreachable" from "wrong pubkey". + // + // resolve() is documented as returning null on any failure. Preserve + // that contract by mapping expected lookup exceptions to null. Callers + // that want the specific failure reason use resolveDetailed() instead. + val nameResult = + try { + electrumxClient.nameShowWithFallback(parsed.namecoinName, serverListProvider()) + ?: return null + } catch (e: kotlinx.coroutines.CancellationException) { + throw e + } catch (e: NamecoinLookupException) { + // NameNotFound / NameExpired / ServersUnreachable → null per + // resolve()'s contract. resolveDetailed() surfaces which one. + return null + } val valueJson = tryParseJson(nameResult.value) ?: return null val merged = expandImportsIfPresent(valueJson) diff --git a/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip05/namecoin/NamecoinNameResolverExceptionTest.kt b/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip05/namecoin/NamecoinNameResolverExceptionTest.kt new file mode 100644 index 0000000000..e9177df488 --- /dev/null +++ b/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip05/namecoin/NamecoinNameResolverExceptionTest.kt @@ -0,0 +1,164 @@ +/* + * 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.nip05.namecoin + +import com.vitorpamplona.quartz.nip05DnsIdentifiers.namecoin.ElectrumxServer +import com.vitorpamplona.quartz.nip05DnsIdentifiers.namecoin.IElectrumXClient +import com.vitorpamplona.quartz.nip05DnsIdentifiers.namecoin.NameShowResult +import com.vitorpamplona.quartz.nip05DnsIdentifiers.namecoin.NamecoinLookupException +import com.vitorpamplona.quartz.nip05DnsIdentifiers.namecoin.NamecoinNameResolver +import com.vitorpamplona.quartz.nip05DnsIdentifiers.namecoin.NamecoinResolveOutcome +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Assert.fail +import org.junit.Test + +/** + * Regression coverage for the resolve() ↔ resolveDetailed() exception + * contract. + * + * Real IElectrumXClient implementations (ElectrumXClient + + * CompositeNamecoinBackend) NEVER return null from nameShowWithFallback — + * every failure path throws a NamecoinLookupException subtype. Before this + * fix, the non-detailed `resolve()` path had no try/catch around that call, + * so any transport-level failure surfaced as a red "Error" icon in the + * NIP-05 verification display with no way for the user to distinguish + * "servers unreachable" from "wrong pubkey". + * + * These tests pin the contract: resolve() maps expected lookup exceptions + * to null (as documented in the KDoc), while resolveDetailed() keeps + * surfacing them structurally. CancellationException always propagates. + */ +class NamecoinNameResolverExceptionTest { + @Test + fun `resolve returns null on NameNotFound instead of throwing`() = + runTest { + val resolver = + NamecoinNameResolver( + electrumxClient = throwingClient(NamecoinLookupException.NameNotFound("d/example")), + lookupTimeoutMs = 500L, + ) + assertNull(resolver.resolve("example.bit")) + } + + @Test + fun `resolve returns null on NameExpired instead of throwing`() = + runTest { + val resolver = + NamecoinNameResolver( + electrumxClient = throwingClient(NamecoinLookupException.NameExpired("d/stale")), + lookupTimeoutMs = 500L, + ) + assertNull(resolver.resolve("stale.bit")) + } + + @Test + fun `resolve returns null on ServersUnreachable instead of throwing`() = + runTest { + val resolver = + NamecoinNameResolver( + electrumxClient = throwingClient(NamecoinLookupException.ServersUnreachable()), + lookupTimeoutMs = 500L, + ) + assertNull(resolver.resolve("dead.bit")) + } + + @Test + fun `resolve propagates CancellationException`() = + runTest { + val resolver = + NamecoinNameResolver( + electrumxClient = throwingClient(CancellationException("cancelled")), + lookupTimeoutMs = 500L, + ) + try { + resolver.resolve("cancel.bit") + fail("expected CancellationException to propagate") + } catch (e: CancellationException) { + // expected + } + } + + @Test + fun `resolveDetailed still surfaces NameNotFound`() = + runTest { + val resolver = + NamecoinNameResolver( + electrumxClient = throwingClient(NamecoinLookupException.NameNotFound("d/example")), + lookupTimeoutMs = 500L, + ) + val outcome = resolver.resolveDetailed("example.bit") + assertTrue("expected NameNotFound, got $outcome", outcome is NamecoinResolveOutcome.NameNotFound) + assertEquals("d/example", (outcome as NamecoinResolveOutcome.NameNotFound).name) + } + + @Test + fun `resolveDetailed still surfaces ServersUnreachable`() = + runTest { + val resolver = + NamecoinNameResolver( + electrumxClient = throwingClient(NamecoinLookupException.ServersUnreachable()), + lookupTimeoutMs = 500L, + ) + val outcome = resolver.resolveDetailed("dead.bit") + assertTrue( + "expected ServersUnreachable, got $outcome", + outcome is NamecoinResolveOutcome.ServersUnreachable, + ) + } + + @Test + fun `resolve returns null on generic transport failures too`() = + runTest { + // Anything that isn't a NamecoinLookupException nor a + // CancellationException should also not surface as an + // uncaught exception through resolve(). The immediate reason + // is that withTimeoutOrNull only handles TimeoutCancellationException; + // arbitrary IO exceptions would otherwise leak. This test + // pins the current behaviour (null on non-cancellation faults) + // in case someone widens the try/catch further later. + val resolver = + NamecoinNameResolver( + electrumxClient = + object : IElectrumXClient { + override suspend fun nameShowWithFallback( + identifier: String, + servers: List, + ): NameShowResult? = throw NamecoinLookupException.ServersUnreachable(RuntimeException("simulated")) + }, + lookupTimeoutMs = 500L, + ) + assertNull(resolver.resolve("boom.bit")) + } + + // ── Helpers ──────────────────────────────────────────────────────── + + private fun throwingClient(exception: Throwable): IElectrumXClient = + object : IElectrumXClient { + override suspend fun nameShowWithFallback( + identifier: String, + servers: List, + ): NameShowResult? = throw exception + } +}