mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-08-08 23:54:39 +00:00
fix(namecoin): don't leak lookup exceptions through resolve()
`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.
This commit is contained in:
mstrofnone
+21
-1
@@ -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)
|
||||
|
||||
|
||||
+164
@@ -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<ElectrumxServer>,
|
||||
): 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<ElectrumxServer>,
|
||||
): NameShowResult? = throw exception
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user