From 156176f5fe8c72df074beb6438f85d6722d0797f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 13:22:55 +0000 Subject: [PATCH] fix(quartz): stop RelayUrlNormalizer from accepting urls that can never be relays MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Validated against a 45k-entry corpus of relay-url hints exported from real events (317k tag occurrences). The normalizer was converting ~30k distinct https:// urls with paths (Mastodon/bridge actor urls from proxy tags, web pages, images) into wss:// addresses that can never answer, wasting connection attempts and relay-pool slots. - http(s) → ws(s) scheme swap now only applies to bare hosts (host[:port] plus optional trailing slash); an http url with a path, query or fragment is a web resource, not a mistyped relay. - Authority validation for all schemes: rejects empty hosts, userinfo (@), percent-encoding and commas in the host, and paths that start with // (the signature of a second pasted url, e.g. wss://https//host). - Interior whitespace and backslashes reject the whole string (multiple urls or prose in one field). - Zero-width characters (U+200B..D, U+2060, BOM) are stripped instead of corrupting the parse (wss://\u200Bnos.lol previously normalized to the scheme-less //nos.lol/). - Schemeless candidates must look like host[:port] (single colon, numeric port), rejecting addressable pointers (31990:pubkey:dtag) and bare scheme leftovers (wss:) before the expensive RFC 3986 parse. - Protocol-relative //host/ inputs normalize as wss:// instead of resolving to https://. - normalizeOrNull now double-checks the parser output still starts with ws(s):// and rejects otherwise. Corpus impact: 30,014 garbage urls (30,333 events) now rejected, 0 real relays lost (all 15,162 kept urls normalize byte-identically), 6 broken outputs fixed. fix() itself stays allocation-free on the happy path (~357ns vs ~318ns per call on the garbage-heavy corpus). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01A9sSyh1QLJD3PZ18tVPPVK --- .../relay/normalizer/RelayUrlNormalizer.kt | 147 ++++++++++++++++-- .../nip01Core/relay/RelayUrlFormatterTest.kt | 81 ++++++++++ 2 files changed, 216 insertions(+), 12 deletions(-) diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/normalizer/RelayUrlNormalizer.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/normalizer/RelayUrlNormalizer.kt index 04f8c9cc06..8949a0b82c 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/normalizer/RelayUrlNormalizer.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/normalizer/RelayUrlNormalizer.kt @@ -84,6 +84,97 @@ class RelayUrlNormalizer { private fun norm(url: String) = NormalizedRelayUrl(Rfc3986.normalize(url)) + private fun isInvisible(c: Char) = c == '\u200B' || c == '\u200C' || c == '\u200D' || c == '\u2060' || c == '\uFEFF' + + /** + * Scans the authority (host[:port]) that starts at [start] and ends at the first + * `/`, `?` or `#`. Returns the end index, or -1 when the authority is empty or + * contains characters that never appear in a real relay host (`@` userinfo, + * percent-encoding, commas). + */ + private fun authorityEnd( + url: String, + start: Int, + ): Int { + if (start >= url.length) return -1 + var i = start + if (url[i] == '[') { + // IPv6 literal: defer validation to the RFC 3986 parser + while (i < url.length && url[i] != '/' && url[i] != '?' && url[i] != '#') i++ + return i + } + while (i < url.length) { + val c = url[i] + if (c == '/' || c == '?' || c == '#') break + if (c == '@' || c == '%' || c == ',') return -1 + i++ + } + return if (i == start) -1 else i + } + + /** + * Accepts a ws/wss url whose host starts at [hostStart] if the authority is sane + * and the path does not start with `//` (the signature of a second URL or a broken + * `https//` pasted after the scheme, e.g. `wss://https//nostr.watch/relay/x`). + */ + private fun fixWs( + url: String, + hostStart: Int, + ): String? { + val end = authorityEnd(url, hostStart) + if (end < 0) return null + if (end + 1 < url.length && url[end] == '/' && url[end + 1] == '/') return null + return url + } + + /** + * Converts an http(s) url to ws(s) only when it is a bare host — nothing after + * `host[:port]` but an optional trailing `/`. An http url with a path, query or + * fragment (Mastodon actor urls from bridge `proxy` tags, web pages, images) is + * a web resource, not a relay: converting it creates a wss:// url that can never + * answer and only wastes connection attempts. + */ + private fun fixHttp( + url: String, + hostStart: Int, + newScheme: String, + ): String? { + val end = authorityEnd(url, hostStart) + if (end < 0) return null + val bareHost = end == url.length || (end == url.length - 1 && url[end] == '/') + if (!bareHost) return null + return "$newScheme${url.substring(hostStart)}" + } + + /** + * Validates a schemeless candidate: the part before the first `/` must look like + * `host` or `host:port` — letters, digits, `.`, `-`, `_`, plus at most one `:` + * followed by digits only. Rejects addressable-event pointers (`31990:hex:dtag`), + * bare scheme leftovers (`wss:`) and anything else that would otherwise be blindly + * prefixed with `wss://`. + */ + private fun isBareHostAndPath(url: String): Boolean { + if (url[0] == '[') return true // IPv6 literal: defer to the RFC 3986 parser + var i = 0 + var portStart = -1 + while (i < url.length) { + val c = url[i] + if (c == '/') break + if (c == ':') { + if (portStart >= 0) return false + portStart = i + 1 + } else if (portStart >= 0) { + if (c < '0' || c > '9') return false + } else if (!c.isLetterOrDigit() && c != '.' && c != '-' && c != '_') { + return false + } + i++ + } + if (i == 0) return false + if (portStart >= 0 && portStart == i) return false + return true + } + @OptIn(ExperimentalContracts::class) fun fix(rawUrl: String): String? { if (rawUrl.length < 4) return null @@ -109,17 +200,33 @@ class RelayUrlNormalizer { } } - val trimmed = + var trimmed = if (url[0].isWhitespace() || url[url.length - 1].isWhitespace()) { url.trim() } else { url } + // Single pass: interior whitespace means multiple urls or prose in one field, + // backslashes never appear in a real relay url; both are garbage. Invisible + // characters (zero-width spaces, BOM) are copy-paste artifacts — strip them. + var hasInvisible = false + for (c in trimmed) { + if (c == '\\') return null + if (c.isWhitespace()) return null + if (isInvisible(c)) hasInvisible = true + } + if (hasInvisible) { + trimmed = buildString(trimmed.length) { for (c in trimmed) if (!isInvisible(c)) append(c) } + if (trimmed.length < 4) return null + } + // fast for good wss:// urls if (isRelaySchemePrefix(trimmed)) { - if (isRelaySchemePrefixSecure(trimmed) || isRelaySchemePrefixInsecure(trimmed)) { - return trimmed + if (isRelaySchemePrefixSecure(trimmed)) { + return fixWs(trimmed, 6) + } else if (isRelaySchemePrefixInsecure(trimmed)) { + return fixWs(trimmed, 5) } } @@ -127,31 +234,31 @@ class RelayUrlNormalizer { if (isHttpPrefix(trimmed)) { if (isHttpSSuffix(trimmed)) { // https:// - return "wss://${trimmed.drop(8)}" + return fixHttp(trimmed, 8, "wss://") } else if (isHttpSuffix(trimmed)) { // http:// - return "ws://${trimmed.drop(7)}" + return fixHttp(trimmed, 7, "ws://") } } // fast for good ww:// urls if (trimmed.startsWith("ww://")) { - return "wss://${trimmed.drop(5)}" + return fixWs("wss://${trimmed.drop(5)}", 6) } // fast for good ww:// urls if (trimmed.startsWith("was://")) { - return "wss://${trimmed.drop(6)}" + return fixWs("wss://${trimmed.drop(6)}", 6) } // fast for good ww:// urls if (trimmed.startsWith("Wws://")) { - return "wss://${trimmed.drop(6)}" + return fixWs("wss://${trimmed.drop(6)}", 6) } // fast for good ww:// urls if (trimmed.startsWith("Wss://")) { - return "wss://${trimmed.drop(6)}" + return fixWs("wss://${trimmed.drop(6)}", 6) } if (trimmed.contains("://")) { @@ -160,10 +267,19 @@ class RelayUrlNormalizer { return null } - return if (isOnion(trimmed) || isLocalHost(trimmed)) { - "ws://$trimmed" + // protocol-relative urls (`//host/`) are just missing the scheme + val bare = if (trimmed.startsWith("//")) trimmed.drop(2) else trimmed + if (bare.length < 4) return null + + if (!isBareHostAndPath(bare)) { + Log.d("RelayUrlNormalizer") { "Rejected $url" } + return null + } + + return if (isOnion(bare) || isLocalHost(bare)) { + "ws://$bare" } else { - "wss://$trimmed" + "wss://$bare" } } @@ -186,6 +302,13 @@ class RelayUrlNormalizer { val fixed = fix(url) if (fixed != null) { val normalized = norm(fixed) + // the RFC 3986 parser can drop or replace the scheme on odd inputs; + // anything that is not ws(s):// at this point cannot be connected to. + if (!isRelayUrl(normalized.url)) { + Log.d("NormalizedRelayUrl") { "Rejected $url" } + normalizedUrls.put(url, NormalizationResult.Error) + return null + } normalizedUrls.put(url, NormalizationResult.Success(normalized)) normalized } else { diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/RelayUrlFormatterTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/RelayUrlFormatterTest.kt index 82287dcbad..fc708eddba 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/RelayUrlFormatterTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/RelayUrlFormatterTest.kt @@ -52,4 +52,85 @@ class RelayUrlFormatterTest { fun weirdRelay() { assertNull(RelayUrlNormalizer.normalizeOrNull("wss://relay%20list%20to%20discover%20the%20user's%20content")) } + + @Test + fun httpWithPathIsNotARelay() { + // Mastodon/bridge actor urls from `proxy` tags: web resources, not relays + assertNull(RelayUrlNormalizer.normalizeOrNull("https://mastodon.social/users/amanita_muscaria")) + assertNull(RelayUrlNormalizer.normalizeOrNull("https://fosstodon.org/ap/users/115532410310000993")) + assertNull(RelayUrlNormalizer.normalizeOrNull("http://example.com/relay")) + assertNull(RelayUrlNormalizer.normalizeOrNull("https://nostr.mom/?author=0")) + assertNull(RelayUrlNormalizer.normalizeOrNull("https://nostr.mom/#section")) + + // but bare hosts still convert, with or without port and trailing slash + assertEquals("wss://nostr.mom/", RelayUrlNormalizer.normalizeOrNull("https://nostr.mom")?.url) + assertEquals("wss://nostr.mom/", RelayUrlNormalizer.normalizeOrNull("https://nostr.mom/")?.url) + assertEquals("wss://nostr.mom:4443/", RelayUrlNormalizer.normalizeOrNull("https://nostr.mom:4443/")?.url) + assertEquals("ws://nostr.mom/", RelayUrlNormalizer.normalizeOrNull("http://nostr.mom")?.url) + } + + @Test + fun wsWithPathIsStillARelay() { + assertEquals("wss://relay.nostr.band/all", RelayUrlNormalizer.normalizeOrNull("wss://relay.nostr.band/all")?.url) + assertEquals( + "wss://bostr.lecturify.net/?accept=0,1", + RelayUrlNormalizer.normalizeOrNull("wss://bostr.lecturify.net/?accept=0,1")?.url, + ) + } + + @Test + fun brokenSchemeGarbage() { + assertNull(RelayUrlNormalizer.normalizeOrNull("wss:")) + assertNull(RelayUrlNormalizer.normalizeOrNull("wss://https//nostr.watch/relay/nostr.21crypto.ch")) + assertNull(RelayUrlNormalizer.normalizeOrNull("wss://https://lockbox.fiatjaf.com")) + assertNull(RelayUrlNormalizer.normalizeOrNull("ws://http//nos.lol")) + assertNull(RelayUrlNormalizer.normalizeOrNull("wss://://plebstr.com")) + } + + @Test + fun nostrUriIsNotARelay() { + assertNull(RelayUrlNormalizer.normalizeOrNull("nostr://nrelay1qqxhwumn8ghj77tpvf6jumt9e2ckgn/")) + assertNull(RelayUrlNormalizer.normalizeOrNull("nostr://npub1dwy079xmpz7mk02kvz6wan49h02635umk32aa4ufek8t8mjxv58qy2nr22/")) + assertNull(RelayUrlNormalizer.normalizeOrNull("nostr:nrelay1qq8k2cnfwejhyum99eek7cmfv9kqsm7sdm")) + } + + @Test + fun addressablePointerIsNotARelay() { + assertNull(RelayUrlNormalizer.normalizeOrNull("31990:6be38f8c63df7dbf84db7ec4a6e6fbbd8d19dca3b980efad18585c46f04b26f9:mostr")) + } + + @Test + fun authorityGarbage() { + // userinfo, percent-encoding and commas never appear in a real relay host + assertNull(RelayUrlNormalizer.normalizeOrNull("wss://catuaba@plebs.place/")) + assertNull(RelayUrlNormalizer.normalizeOrNull("wss://africa.nostr.joburg%0A/")) + assertNull(RelayUrlNormalizer.normalizeOrNull("wss://bitcoiner,social/")) + assertNull(RelayUrlNormalizer.normalizeOrNull("wss://#web3/")) + assertNull(RelayUrlNormalizer.normalizeOrNull("name@domain.com")) + } + + @Test + fun interiorWhitespaceAndBackslashes() { + assertNull(RelayUrlNormalizer.normalizeOrNull("wss://nos lol")) + assertNull(RelayUrlNormalizer.normalizeOrNull("wss://nos.lol/ wss:/nostr.land/ avatar wss:/nostr.wine/")) + assertNull(RelayUrlNormalizer.normalizeOrNull("wss://\\\\relay.damus.io/")) + } + + @Test + fun invisibleCharactersAreStripped() { + assertEquals("wss://nos.lol/", RelayUrlNormalizer.normalizeOrNull("wss://\u200Bnos.lol")?.url) + assertEquals("wss://nos.lol/", RelayUrlNormalizer.normalizeOrNull("\uFEFFwss://nos.lol")?.url) + } + + @Test + fun protocolRelativeUrls() { + assertEquals("wss://relay.most.pub/", RelayUrlNormalizer.normalizeOrNull("//relay.most.pub/")?.url) + assertEquals("wss://nos.lol/", RelayUrlNormalizer.normalizeOrNull("//nos.lol/")?.url) + } + + @Test + fun ipv6AndLanHostsStillWork() { + assertEquals("ws://[31b:6f20:c7f2:3ddf::3221]/", RelayUrlNormalizer.normalizeOrNull("ws://[31b:6f20:c7f2:3ddf::3221]/")?.url) + assertEquals("ws://geyser-relay:7777/", RelayUrlNormalizer.normalizeOrNull("ws://geyser-relay:7777/")?.url) + } }