From 46f84b61f091b267663100a57db7bc147a46801b Mon Sep 17 00:00:00 2001 From: heyaim <223061694+heyaim@users.noreply.github.com> Date: Mon, 7 Sep 2026 20:56:21 -0500 Subject: [PATCH] Validate relay directories with iOS's rules #914 aligned the file both platforms read and how rows are keyed and ordered; the acceptance rules still differed. iOS rejects a whole directory on one malformed or conflicting row, validates the header, caps size and rows, and screens every host. This client skipped bad rows and accepted almost any host. The bitchat repo validates the file before it lands; this port bounds what the client accepts as well. parseCsv and canonicalHost are replaced by a port of GeoRelayDirectory.validatedEntries and validatedDirectoryAddress. A rejected download keeps the current list; one that keeps less than half of the known entries is rejected. Five details follow iOS's compiled validator rather than a plain reading of the Swift, each with a test. The cache refetches at startup when its source URL changed. Cross-checked against the Swift validator on 84 fixture runs and 100,000 fuzzed inputs. Full suite, lint and build pass. --- .../bitchat/android/nostr/RelayDirectory.kt | 353 +++++++++++++++--- .../RelayDirectoryCacheInvalidationTest.kt | 82 ++++ .../android/nostr/RelayDirectoryTest.kt | 37 +- .../nostr/RelayDirectoryValidationTest.kt | 233 ++++++++++++ docs/client-rewrite-contracts.md | 9 + 5 files changed, 649 insertions(+), 65 deletions(-) create mode 100644 app/src/test/kotlin/com/bitchat/android/nostr/RelayDirectoryCacheInvalidationTest.kt create mode 100644 app/src/test/kotlin/com/bitchat/android/nostr/RelayDirectoryValidationTest.kt diff --git a/app/src/main/java/com/bitchat/android/nostr/RelayDirectory.kt b/app/src/main/java/com/bitchat/android/nostr/RelayDirectory.kt index fd7d5d2a..938b39bc 100644 --- a/app/src/main/java/com/bitchat/android/nostr/RelayDirectory.kt +++ b/app/src/main/java/com/bitchat/android/nostr/RelayDirectory.kt @@ -3,12 +3,10 @@ package com.bitchat.android.nostr import android.app.Application import android.content.SharedPreferences import android.util.Log -import java.io.BufferedReader import java.io.File import java.io.FileInputStream import java.io.FileOutputStream import java.io.InputStream -import java.io.InputStreamReader import java.security.MessageDigest import java.util.concurrent.TimeUnit import kotlin.math.* @@ -39,8 +37,20 @@ object RelayDirectory { private const val DOWNLOADED_FILE = "nostr_relays_latest.csv" private const val PREFS_NAME = "relay_directory_prefs" private const val KEY_LAST_UPDATE_MS = "last_update_ms" + private const val KEY_SOURCE_URL = "source_url" private val ONE_DAY_MS = TimeUnit.DAYS.toMillis(1) + // GeoRelayDirectoryValidationPolicy.live, ported verbatim. The directory is an + // unsigned third-party file (docs/security-review-jul-27.md M9); iOS bounds what + // it will accept from it and rejects the rest, and both platforms must bound it + // the same way or a file one side accepts and the other rejects splits their + // relay selections at every geohash at once. + internal const val MAX_DIRECTORY_BYTES = 512 * 1024 + internal const val MAX_DIRECTORY_ROWS = 5_000 + internal const val MAX_DIRECTORY_ENTRIES = 5_000 + internal const val MIN_REMOTE_ENTRIES = 50 + internal const val MIN_RETAINED_FRACTION = 0.5 + private val ioScope = CoroutineScope(SupervisorJob() + Dispatchers.IO) private val httpClient: OkHttpClient get() = com.bitchat.android.net.OkHttpProvider.httpClient() @@ -63,6 +73,7 @@ object RelayDirectory { if (initialized) return try { val downloaded = getDownloadedFile(application) + invalidateCacheIfSourceChanged(getPrefs(application), downloaded) val loadedFromDownloaded = if (downloaded.exists() && downloaded.canRead()) { loadFromFile(downloaded, sourceLabel = "downloaded") } else { @@ -129,17 +140,26 @@ object RelayDirectory { return R * c } - private fun normalizeRelayUrl(raw: String): String { - val trimmed = raw.trim() - if (trimmed.isEmpty()) return trimmed - return if ("://" in trimmed) trimmed else "wss://$trimmed" - } - // ===== Implementation details ===== private fun getPrefs(application: Application): SharedPreferences = application.getSharedPreferences(PREFS_NAME, Application.MODE_PRIVATE) + /** + * An install upgraded across the source move still holds a cache fetched from + * the old URL. Drop it and clear the update stamp, and the staleness check + * refetches, rather than keep selecting from a file the current source no + * longer matches. Returns whether a cache was dropped. + */ + internal fun invalidateCacheIfSourceChanged(prefs: SharedPreferences, downloaded: File): Boolean { + if (!downloaded.exists()) return false + if (prefs.getString(KEY_SOURCE_URL, null) == ASSET_FILE_URL) return false + downloaded.delete() + prefs.edit().remove(KEY_LAST_UPDATE_MS).apply() + Log.i(TAG, "Dropped cached relay list fetched from a previous source URL") + return true + } + private fun getDownloadedFile(application: Application): File = File(application.filesDir, DOWNLOADED_FILE) @@ -174,9 +194,17 @@ object RelayDirectory { return } - val parsed = parseCsv(FileInputStream(tmpFile)) - if (parsed.isEmpty()) { - Log.w(TAG, "Downloaded relay CSV parsed to 0 entries; ignoring") + if (tmpFile.length() > MAX_DIRECTORY_BYTES) { + Log.w(TAG, "Downloaded relay CSV exceeds $MAX_DIRECTORY_BYTES bytes; keeping current list") + tmpFile.delete() + return + } + // The current directory is the baseline: a rejected download keeps it, + // in memory and on disk, the way iOS keeps its previous copy. + val baseline = synchronized(relaysLock) { relays.toSet() } + val parsed = validatedEntries(tmpFile.readBytes(), MIN_REMOTE_ENTRIES, baseline) + if (parsed == null) { + Log.w(TAG, "Downloaded relay CSV failed validation; keeping current list") tmpFile.delete() return } @@ -197,7 +225,10 @@ object RelayDirectory { relays.addAll(parsed) } - getPrefs(application).edit().putLong(KEY_LAST_UPDATE_MS, System.currentTimeMillis()).apply() + getPrefs(application).edit() + .putLong(KEY_LAST_UPDATE_MS, System.currentTimeMillis()) + .putString(KEY_SOURCE_URL, ASSET_FILE_URL) + .apply() Log.i(TAG, "✅ Using downloaded relay list (${dest.absolutePath}), entries=$entries, sha256=$hash, updatedAtMs=${getPrefs(application).getLong(KEY_LAST_UPDATE_MS, 0L)}") } catch (e: Exception) { @@ -214,9 +245,24 @@ object RelayDirectory { return false } val body = resp.body ?: return false + if (body.contentLength() > MAX_DIRECTORY_BYTES) { + Log.w(TAG, "Relay CSV content length exceeds $MAX_DIRECTORY_BYTES bytes; aborting") + return false + } FileOutputStream(dest).use { out -> body.byteStream().use { input -> - input.copyTo(out) + val buf = ByteArray(8192) + var total = 0L + while (true) { + val read = input.read(buf) + if (read <= 0) break + total += read + if (total > MAX_DIRECTORY_BYTES.toLong()) { + Log.w(TAG, "Relay CSV download exceeded $MAX_DIRECTORY_BYTES bytes; aborting") + return false + } + out.write(buf, 0, read) + } } } true @@ -229,9 +275,9 @@ object RelayDirectory { private fun loadFromFile(file: File, sourceLabel: String): Boolean { return try { - val list = parseCsv(FileInputStream(file)) - if (list.isEmpty()) { - Log.w(TAG, "${sourceLabel} relay CSV has 0 entries; ignoring") + val list = validatedEntries(file.readBytes(), minimumEntries = 1) + if (list == null) { + Log.w(TAG, "${sourceLabel} relay CSV failed validation; ignoring it") false } else { synchronized(relaysLock) { @@ -250,7 +296,11 @@ object RelayDirectory { private fun loadFromAssets(application: Application) { val list = try { - parseCsv(application.assets.open(ASSET_FILE)) + val bytes = application.assets.open(ASSET_FILE).use { it.readBytes() } + validatedEntries(bytes, minimumEntries = 1) ?: run { + Log.e(TAG, "Bundled asset $ASSET_FILE failed validation") + emptyList() + } } catch (e: Exception) { Log.e(TAG, "Failed to open asset $ASSET_FILE: ${e.message}") emptyList() @@ -270,49 +320,248 @@ object RelayDirectory { Log.i(TAG, "📦 Loaded ${list.size} relay entries from assets/$ASSET_FILE, sha256=$hash") } - internal fun parseCsv(input: InputStream): List { - val result = mutableListOf() - // The directory lists some relays twice, once bare and once with an explicit - // :443, which is the same server over wss. Without this check both copies can - // land in a nearest-N selection, and one of its slots connects nowhere new. - val seenEndpoints = HashSet() - BufferedReader(InputStreamReader(input)).use { reader -> - var line: String? - while (true) { - line = reader.readLine() - if (line == null) break - val trimmed = line!!.trim() - if (trimmed.isEmpty()) continue - if (trimmed.lowercase().startsWith("relay url")) continue - val parts = trimmed.split(",") - if (parts.size < 3) continue - val raw = normalizeRelayUrl(parts[0].trim()) - val lat = parts[1].trim().toDoubleOrNull() - val lon = parts[2].trim().toDoubleOrNull() - if (raw.isEmpty() || lat == null || lon == null) continue - val canonical = canonicalHost(raw) - if (canonical.isEmpty() || !seenEndpoints.add(canonical)) continue - result.add(RelayInfo(url = "wss://$canonical", latitude = lat, longitude = lon)) + /** + * GeoRelayDirectory.validatedEntries, ported in full. One malformed or + * conflicting row rejects the complete dataset, and the caller keeps whatever + * directory it already has: a partial parse would leave this client selecting + * from a different row set than iOS, the failure #914 exists to close. Returns + * null when the data is rejected. + * + * baselineEntries carries the current directory when validating a download. A + * new file that keeps less than half of the known entries is rejected even when + * well formed, matching iOS: a hijacked or truncated upstream cannot swap the + * whole relay population in one fetch. + */ + internal fun validatedEntries( + data: ByteArray, + minimumEntries: Int, + baselineEntries: Set? = null + ): List? { + if (data.isEmpty() || data.size > MAX_DIRECTORY_BYTES) return null + var text = decodeUtf8Strict(data) ?: return null + // Foundation's UTF-8 decode strips one leading BOM before iOS's own BOM + // check runs, so a single BOM passes on iOS and only a doubled one is + // rejected. Mirror that exactly (verified against the real Swift code). + if (text.startsWith('\uFEFF')) text = text.substring(1) + if (text.startsWith('\uFEFF')) return null + + val lines = text + .split('\u000A', '\u000B', '\u000C', '\u000D', '\u0085', '\u2028', '\u2029') + .map { it.trim() } + .filter { it.isNotEmpty() } + val header = lines.firstOrNull() ?: return null + if (lines.size - 1 > MAX_DIRECTORY_ROWS) return null + + val headerParts = header.split(",").map { it.trim().lowercase() } + val supportedHeaders = listOf( + listOf("relay url", "latitude", "longitude"), + listOf("relay url", "lat", "lon") + ) + if (headerParts !in supportedHeaders) return null + + val entriesByHost = LinkedHashMap() + for (line in lines.drop(1)) { + val parts = line.split(",").map { it.trim() } + if (parts.size != 3) return null + val host = validatedDirectoryAddress(parts[0]) ?: return null + val latitude = parseCoordinate(parts[1]) ?: return null + if (latitude !in -90.0..90.0) return null + val longitude = parseCoordinate(parts[2]) ?: return null + if (longitude !in -180.0..180.0) return null + + val entry = RelayInfo(url = "wss://$host", latitude = latitude, longitude = longitude) + val existing = entriesByHost[host] + // One endpoint cannot truthfully occupy two coordinates. Matching iOS, + // row order does not get to choose which location clients trust. The + // comparison is IEEE equality, not equals(): Swift's == calls -0.0 and + // 0.0 the same coordinate, and the last equal row's value is kept, raw + // bits included, the way Swift dictionary assignment keeps it. + if (existing != null && !sameEntry(existing, entry)) { + return null } + entriesByHost[host] = entry + if (entriesByHost.size > MAX_DIRECTORY_ENTRIES) return null } - return result + + val parsed = entriesByHost.values.toList() + if (parsed.size < minimumEntries) return null + + if (baselineEntries != null) { + val required = ceil(baselineEntries.size * MIN_RETAINED_FRACTION).toInt() + val overlap = parsed.count { p -> baselineEntries.any { b -> sameEntry(p, b) } } + if (overlap < required) return null + } + + return parsed.sortedWith(compareBy({ it.url }, { it.latitude }, { it.longitude })) } /** - * The host string iOS builds for the same row (GeoRelayDirectory's - * validatedDirectoryAddress): host lowercased, an explicit port kept unless it is - * 443, the wss default. A relay on :8443 stays distinct from one on :443. Dedup + * GeoRelayDirectory.validatedDirectoryAddress, ported in full: the host + * key both platforms build for a row, or null when the address is one the + * directory must not carry (non-ASCII, credentials, paths, queries, local and + * internal names, malformed labels, out-of-range ports). An explicit port stays + * in the key unless it is 443, the wss default, which keeps a relay on :8443 + * distinct and collapses the directory's bare and :443 duplicate rows. Dedup * and tie ordering both key on this, which is what keeps the two platforms' * selections aligned row for row. */ - internal fun canonicalHost(url: String): String { - val hostPort = url.substringAfter("://").substringBefore("/") - val idx = hostPort.lastIndexOf(':') - val hasPort = idx > 0 && idx < hostPort.length - 1 && - hostPort.substring(idx + 1).all { it.isDigit() } - val host = (if (hasPort) hostPort.substring(0, idx) else hostPort).lowercase() - val port = if (hasPort) hostPort.substring(idx + 1).toInt() else 443 - return if (port == 443) host else "$host:$port" + internal fun validatedDirectoryAddress(rawValue: String): String? { + val value = rawValue.trim() + if (value.isEmpty()) return null + if (!value.all { it.code in 0x20..0x7E }) return null + + val encoded = if ("://" in value) value else "wss://$value" + // URLComponents percent-decodes before iOS's checks run, so re%6Cay.example + // is relay.example to iOS; java.net.URI does not decode. Decode the same + // way first, and reject invalid escapes the way URLComponents rejects them. + val candidate = percentDecodedOrNull(encoded) ?: return null + val uri = try { java.net.URI(candidate) } catch (_: Exception) { return null } + val scheme = uri.scheme?.lowercase() ?: return null + if (scheme != "wss" && scheme != "https") return null + if (uri.userInfo != null) return null + if (uri.query != null) return null + if (uri.fragment != null) return null + val path = uri.path ?: "" + if (path.isNotEmpty() && path != "/") return null + // java.net.URI follows RFC 2396, which requires the final host label to + // start with a letter, and returns no host for names like b.08obllot + // that iOS's RFC 3986 parser accepts. When URI refuses only for that + // reason, a plain hostname[:port] authority is taken as the host and + // the screens below judge it; fuzzing found this as the dominant + // divergence class (Android rejecting what iOS accepts). + val rawHost = uri.host + ?: plainAuthorityHost(candidate) + ?: return null + + val host = rawHost.lowercase() + if (host.isEmpty() || host.length > 253) return null + if (!host.all { it.code <= 0x7F }) return null + if (host.endsWith(".")) return null + if (host == "localhost" || host.endsWith(".localhost") || + host.endsWith(".local") || host.endsWith(".internal")) return null + + val labels = host.split(".") + if (labels.size < 2) return null + // URLComponents IDNA-decodes xn-- labels: valid punycode decodes to + // non-ASCII and fails iOS's screen, invalid punycode fails its parse, and + // both reject the address (verified against the real validator). java.net + // URI passes the label through, so the label form itself is refused here. + if (labels.any { it.startsWith("xn--") }) return null + if (labels.all { label -> label.all { it.isDigit() } }) return null + if (!labels.all { label -> + label.length in 1..63 && label.first() != '-' && label.last() != '-' && + label.all { it in 'a'..'z' || it in '0'..'9' || it == '-' } + } + ) return null + + val port = if (uri.host != null) uri.port else plainAuthorityPort(candidate) + if (port != -1) { + if (port !in 1..65535) return null + if (port != 443) return "$host:$port" + } + return host + } + + + // The authority of the candidate when it is nothing but hostname[:port] in + // the host charset. Anything with userinfo, brackets, escapes, or other + // structure stays with java.net.URI's verdict. + private fun plainAuthority(candidate: String): Pair? { + val afterScheme = candidate.substringAfter("://", "") + if (afterScheme.isEmpty()) return null + val authority = afterScheme.takeWhile { it != '/' && it != '?' && it != '#' } + if (authority.length != afterScheme.length) { + val rest = afterScheme.substring(authority.length) + if (rest != "/") return null + } + val colon = authority.lastIndexOf(':') + val hostPart: String + val port: Int + if (colon >= 0) { + val portPart = authority.substring(colon + 1) + // RFC 3986 allows an empty port ("host:"), and Foundation treats it + // as no port at all. + if (portPart.isNotEmpty() && !portPart.all { it in '0'..'9' }) return null + hostPart = authority.substring(0, colon) + port = if (portPart.isEmpty()) -1 else portPart.toIntOrNull() ?: return null + } else { + hostPart = authority + port = -1 + } + if (hostPart.isEmpty()) return null + if (!hostPart.all { it in 'a'..'z' || it in 'A'..'Z' || it in '0'..'9' || it == '.' || it == '-' }) return null + return hostPart to port + } + + private fun plainAuthorityHost(candidate: String): String? = plainAuthority(candidate)?.first + + private fun plainAuthorityPort(candidate: String): Int = plainAuthority(candidate)?.second ?: -1 + + // IEEE equality on the coordinates: -0.0 equals 0.0 here, as it does in the + // Swift Entry's ==, where equals() would call them different. + private fun sameEntry(a: RelayInfo, b: RelayInfo): Boolean = + a.url == b.url && a.latitude == b.latitude && a.longitude == b.longitude + + // Strict %XX decoding with UTF-8 byte semantics and no '+' handling. Returns + // null on an invalid or truncated escape, matching URLComponents. + private fun percentDecodedOrNull(value: String): String? { + if ('%' !in value) return value + val bytes = java.io.ByteArrayOutputStream(value.length) + var i = 0 + while (i < value.length) { + val c = value[i] + if (c == '%') { + if (i + 2 >= value.length) return null + val hi = Character.digit(value[i + 1], 16) + val lo = Character.digit(value[i + 2], 16) + if (hi < 0 || lo < 0) return null + val decoded = ((hi shl 4) or lo).toChar() + // URLComponents decodes AFTER structural parsing, so a decoded + // ':' stays inside the host and fails iOS's label screen; decoding + // it here first would instead create a port. Only escapes that + // decode to host-legal characters may pass (verified against the + // real validator: %6C and %2E accept, %3A and the rest reject). + if (!(decoded in 'A'..'Z' || decoded in 'a'..'z' || + decoded in '0'..'9' || decoded == '.' || decoded == '-')) return null + bytes.write(decoded.code) + i += 3 + } else { + bytes.write(c.code) + i += 1 + } + } + return decodeUtf8Strict(bytes.toByteArray()) + } + + // iOS String(data:encoding:.utf8) fails on invalid UTF-8 where Kotlin's + // String(bytes) substitutes replacement characters. Decode strictly so both + // platforms reject the same bytes. + private fun decodeUtf8Strict(data: ByteArray): String? = try { + Charsets.UTF_8.newDecoder() + .onMalformedInput(java.nio.charset.CodingErrorAction.REPORT) + .onUnmappableCharacter(java.nio.charset.CodingErrorAction.REPORT) + .decode(java.nio.ByteBuffer.wrap(data)) + .toString() + } catch (_: Exception) { + null + } + + // Two Swift-vs-Java parsing differences, both verified against the real + // validator: Double.parseDouble takes trailing f/F/d/D suffixes that Swift + // rejects (and in hex those characters are digits, not suffixes), and Swift + // accepts hex like "0x10" without the binary exponent Java requires. + // Infinity and NaN spellings parse on both and fail the finite check. + private fun parseCoordinate(raw: String): Double? { + if (raw.isEmpty()) return null + val body = raw.removePrefix("+").removePrefix("-") + val isHex = body.startsWith("0x") || body.startsWith("0X") + if (!isHex) { + val last = raw.last() + if (last == 'f' || last == 'F' || last == 'd' || last == 'D') return null + } + val candidate = if (isHex && !raw.contains('p') && !raw.contains('P')) raw + "p0" else raw + val value = candidate.toDoubleOrNull() ?: return null + return if (value.isFinite()) value else null } private fun fileSha256Hex(file: File): String = try { diff --git a/app/src/test/kotlin/com/bitchat/android/nostr/RelayDirectoryCacheInvalidationTest.kt b/app/src/test/kotlin/com/bitchat/android/nostr/RelayDirectoryCacheInvalidationTest.kt new file mode 100644 index 00000000..be6f541b --- /dev/null +++ b/app/src/test/kotlin/com/bitchat/android/nostr/RelayDirectoryCacheInvalidationTest.kt @@ -0,0 +1,82 @@ +package com.bitchat.android.nostr + +import android.app.Application +import android.content.Context +import android.os.Build +import androidx.test.core.app.ApplicationProvider +import java.io.File +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +/** + * A cached directory is only as current as the URL it was fetched from. An install + * upgraded across the source move still holds a cache from the old URL; these pin + * that it is dropped and refetched instead of selecting from stale data. + */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [Build.VERSION_CODES.P], manifest = Config.NONE) +class RelayDirectoryCacheInvalidationTest { + + private val application: Application = ApplicationProvider.getApplicationContext() + + private fun prefs() = application.getSharedPreferences("relay_directory_prefs", Context.MODE_PRIVATE) + + private fun cacheFile(content: String = "stub"): File = + File(application.filesDir, "test_relay_cache.csv").apply { writeText(content) } + + @Test + fun `a cache with no recorded source url is dropped`() { + val cache = cacheFile() + prefs().edit().clear().putLong("last_update_ms", 123L).commit() + + val dropped = RelayDirectory.invalidateCacheIfSourceChanged(prefs(), cache) + + assertTrue(dropped) + assertFalse(cache.exists()) + assertFalse(prefs().contains("last_update_ms")) + } + + @Test + fun `a cache recorded from a different source url is dropped`() { + val cache = cacheFile() + prefs().edit().clear() + .putString("source_url", "https://old.host.example/nostr_relays.csv") + .putLong("last_update_ms", 123L) + .commit() + + val dropped = RelayDirectory.invalidateCacheIfSourceChanged(prefs(), cache) + + assertTrue(dropped) + assertFalse(cache.exists()) + } + + @Test + fun `a cache recorded from the current source url is kept`() { + val cache = cacheFile() + prefs().edit().clear() + .putString("source_url", RelayDirectory.ASSET_FILE_URL) + .putLong("last_update_ms", 123L) + .commit() + + val dropped = RelayDirectory.invalidateCacheIfSourceChanged(prefs(), cache) + + assertFalse(dropped) + assertTrue(cache.exists()) + assertTrue(prefs().contains("last_update_ms")) + } + + @Test + fun `a missing cache changes nothing`() { + val cache = File(application.filesDir, "absent.csv") + prefs().edit().clear().putLong("last_update_ms", 123L).commit() + + val dropped = RelayDirectory.invalidateCacheIfSourceChanged(prefs(), cache) + + assertFalse(dropped) + assertTrue(prefs().contains("last_update_ms")) + } +} diff --git a/app/src/test/kotlin/com/bitchat/android/nostr/RelayDirectoryTest.kt b/app/src/test/kotlin/com/bitchat/android/nostr/RelayDirectoryTest.kt index d5541d91..e14d8a61 100644 --- a/app/src/test/kotlin/com/bitchat/android/nostr/RelayDirectoryTest.kt +++ b/app/src/test/kotlin/com/bitchat/android/nostr/RelayDirectoryTest.kt @@ -1,7 +1,7 @@ package com.bitchat.android.nostr -import java.io.ByteArrayInputStream import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Test @@ -17,11 +17,17 @@ import org.junit.Test * 3. Distance ties order by that key, the way iOS orders them. Rows are geocoded to * city centroids, so whole tie groups sit at one coordinate and tie order decides * most selections. + * + * Parsing goes through the validator ported from GeoRelayDirectory.validatedEntries; + * RelayDirectoryValidationTest pins its acceptance rules. These tests parse files the + * validator accepts. */ class RelayDirectoryTest { private fun parse(csv: String) = - RelayDirectory.parseCsv(ByteArrayInputStream(csv.toByteArray())) + requireNotNull(RelayDirectory.validatedEntries(csv.toByteArray(), minimumEntries = 1)) { + "fixture unexpectedly rejected" + } private val header = "Relay URL,Latitude,Longitude\n" @@ -34,11 +40,13 @@ class RelayDirectoryTest { } @Test - fun `canonical host matches the key ios builds`() { - assertEquals("relay.example.com", RelayDirectory.canonicalHost("wss://relay.example.com")) - assertEquals("relay.example.com", RelayDirectory.canonicalHost("wss://relay.example.com:443")) - assertEquals("relay.example.com:8443", RelayDirectory.canonicalHost("wss://relay.example.com:8443")) - assertEquals("relay.example.com", RelayDirectory.canonicalHost("wss://Relay.Example.Com/")) + fun `validated addresses match the key ios builds`() { + assertEquals("relay.example.com", RelayDirectory.validatedDirectoryAddress("wss://relay.example.com")) + assertEquals("relay.example.com", RelayDirectory.validatedDirectoryAddress("wss://relay.example.com:443")) + assertEquals("relay.example.com:8443", RelayDirectory.validatedDirectoryAddress("wss://relay.example.com:8443")) + assertEquals("relay.example.com", RelayDirectory.validatedDirectoryAddress("wss://Relay.Example.Com/")) + assertEquals("relay.example.com", RelayDirectory.validatedDirectoryAddress("relay.example.com")) + assertEquals("relay.example.com", RelayDirectory.validatedDirectoryAddress("https://relay.example.com")) } @Test @@ -67,14 +75,17 @@ class RelayDirectoryTest { } @Test - fun `first row wins when an endpoint is listed twice`() { - val entries = parse( - header + + fun `an endpoint listed at two different coordinates rejects the file`() { + // One endpoint cannot truthfully occupy two coordinates. iOS rejects the + // whole file rather than letting row order choose which location clients + // trust; the earlier first-row-wins behavior is gone with it. + val rejected = RelayDirectory.validatedEntries( + (header + "relay.example.com,10.0,20.0\n" + - "relay.example.com:443,50.0,60.0\n" + "relay.example.com:443,50.0,60.0\n").toByteArray(), + minimumEntries = 1 ) - assertEquals(1, entries.size) - assertEquals(10.0, entries[0].latitude, 0.0) + assertNull(rejected) } @Test diff --git a/app/src/test/kotlin/com/bitchat/android/nostr/RelayDirectoryValidationTest.kt b/app/src/test/kotlin/com/bitchat/android/nostr/RelayDirectoryValidationTest.kt new file mode 100644 index 00000000..52ea0c30 --- /dev/null +++ b/app/src/test/kotlin/com/bitchat/android/nostr/RelayDirectoryValidationTest.kt @@ -0,0 +1,233 @@ +package com.bitchat.android.nostr + +import java.io.File +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Test + +/** + * Pins the directory validation ported from GeoRelayDirectory.validatedEntries and + * validatedDirectoryAddress. The rules are part of the cross-client contract: a file + * one platform accepts and the other rejects splits the two relay selections at every + * geohash at once, which is a larger divergence than the two-file split #914 closed. + * Every rejection here is a whole-file rejection; the caller keeps its previous copy. + */ +class RelayDirectoryValidationTest { + + private val header = "Relay URL,Latitude,Longitude\n" + + private fun validate(csv: String, minimumEntries: Int = 1, baseline: Set? = null) = + RelayDirectory.validatedEntries(csv.toByteArray(), minimumEntries, baseline) + + // MARK: file-level rules + + @Test + fun `an empty file is rejected`() { + assertNull(RelayDirectory.validatedEntries(ByteArray(0), minimumEntries = 1)) + } + + @Test + fun `a file with only a header is rejected`() { + assertNull(validate(header)) + } + + @Test + fun `both header forms ios accepts parse here`() { + assertNotNull(validate("Relay URL,Latitude,Longitude\nrelay-a.example,1.0,2.0\n")) + assertNotNull(validate("relay url,lat,lon\nrelay-a.example,1.0,2.0\n")) + } + + @Test + fun `a header ios rejects rejects the file here`() { + assertNull(validate("Relay URL,Lat,Long\nrelay-a.example,1.0,2.0\n")) + assertNull(validate("url,latitude,longitude\nrelay-a.example,1.0,2.0\n")) + } + + @Test + fun `a single byte order mark is stripped the way ios strips it`() { + // Foundation's UTF-8 decode removes one leading BOM before iOS's BOM + // check runs; only a doubled BOM reaches the check. Verified against the + // real Swift code over these exact bytes. + assertNotNull(validate("" + header + "relay-a.example,1.0,2.0\n")) + assertNull(validate("" + header + "relay-a.example,1.0,2.0\n")) + } + + @Test + fun `percent escapes decode into the host key the way ios decodes them`() { + assertEquals("relay.example", RelayDirectory.validatedDirectoryAddress("re%6Cay.example")) + assertEquals("a.b.example", RelayDirectory.validatedDirectoryAddress("a%2Eb.example")) + assertNull("invalid escape", RelayDirectory.validatedDirectoryAddress("wss://h%GGx.example")) + assertNull("truncated escape", RelayDirectory.validatedDirectoryAddress("wss://hx.example%2")) + assertNull("escape decoding to a query", RelayDirectory.validatedDirectoryAddress("wss://h%3Fx.example")) + assertNull("escape decoding to non-ascii", RelayDirectory.validatedDirectoryAddress("re%C3%A9seau.example")) + // iOS decodes after structural parsing, so a decoded colon stays in the + // host and fails the label screen; decoded first it would become a port. + assertNull("escape decoding to a colon", RelayDirectory.validatedDirectoryAddress("relay-a.example%3A8443")) + } + + @Test + fun `hex coordinates parse the way swift parses them`() { + val plain = validate(header + "relay-a.example,0x10,2.0\n") + assertNotNull(plain) + assertEquals(16.0, plain!![0].latitude, 0.0) + val hexDigitTail = validate(header + "relay-a.example,0x1d,2.0\n") + assertNotNull("d is a hex digit, not a suffix", hexDigitTail) + assertEquals(29.0, hexDigitTail!![0].latitude, 0.0) + } + + @Test + fun `signed zero is one coordinate and the last row's bits are kept`() { + val entries = validate(header + "z.example,0.0,1.0\n" + "z.example,-0.0,1.0\n") + assertNotNull("swift's == treats -0.0 and 0.0 as the same coordinate", entries) + assertEquals(1, entries!!.size) + assertEquals((-0.0).toRawBits(), entries[0].latitude.toRawBits()) + } + + @Test + fun `every punycode label is rejected`() { + // iOS IDNA-decodes xn-- labels: valid punycode becomes non-ASCII and is + // rejected, and most invalid forms fail its parse, both pinned in the + // battery. Foundation lets a few exotic invalid forms through + // literally; this port rejects every xn-- label instead, stricter in + // the safe direction, measured by the fuzz round. + assertNull(RelayDirectory.validatedDirectoryAddress("xn--bcher-kva.example")) + assertNull(RelayDirectory.validatedDirectoryAddress("xn--x.example")) + } + + @Test + fun `hosts uri refuses but ios accepts are salvaged by the fallback`() { + // java.net.URI follows RFC 2396 and returns no host when the final + // label starts with a digit, or when a trailing colon carries no port; + // iOS's RFC 3986 parser accepts both. The plain-authority fallback + // covers exactly these shapes. + assertEquals("b.08relay", RelayDirectory.validatedDirectoryAddress("https://b.08relay")) + assertEquals("relay.example.1", RelayDirectory.validatedDirectoryAddress("relay.example.1:")) + assertEquals("d.7ex:8443", RelayDirectory.validatedDirectoryAddress("wss://d.7ex:8443")) + } + + @Test + fun `invalid utf8 rejects the file`() { + assertNull(RelayDirectory.validatedEntries(byteArrayOf(0xFF.toByte(), 0xFE.toByte(), 0x41), minimumEntries = 1)) + } + + @Test + fun `a file over the byte cap is rejected`() { + val oversized = ByteArray(RelayDirectory.MAX_DIRECTORY_BYTES + 1) { 'a'.code.toByte() } + assertNull(RelayDirectory.validatedEntries(oversized, minimumEntries = 1)) + } + + @Test + fun `more rows than the row cap rejects the file`() { + val rows = buildString { + append(header) + repeat(RelayDirectory.MAX_DIRECTORY_ROWS + 1) { append("relay-a.example,1.0,2.0\n") } + } + assertNull(validate(rows)) + } + + // MARK: row-level rules, each rejecting the whole file + + @Test + fun `one malformed row rejects the whole file`() { + val csv = header + + "relay-a.example,1.0,2.0\n" + + "relay-b.example,2.0\n" + + "relay-c.example,3.0,4.0\n" + assertNull(validate(csv)) + } + + @Test + fun `an out of range coordinate rejects the whole file`() { + assertNull(validate(header + "relay-a.example,91.0,2.0\n")) + assertNull(validate(header + "relay-a.example,1.0,181.0\n")) + assertNull(validate(header + "relay-a.example,abc,2.0\n")) + } + + @Test + fun `a coordinate spelling only java parses rejects the file`() { + // Double.parseDouble takes "1.5f"; Swift's Double(String) does not. Both + // platforms must refuse the row, or one keeps a file the other drops. + assertNull(validate(header + "relay-a.example,1.5f,2.0\n")) + assertNull(validate(header + "relay-a.example,1.0,2.0d\n")) + } + + @Test + fun `a row with a rejected host rejects the whole file`() { + val csv = header + + "relay-a.example,1.0,2.0\n" + + "localhost,1.0,2.0\n" + assertNull(validate(csv)) + } + + @Test + fun `hosts ios rejects are rejected here`() { + val rejected = mapOf( + "wss://relay.example.com/path" to "path beyond /", + "wss://user@relay.example.com" to "userinfo", + "wss://relay.example.com?x=1" to "query", + "wss://relay.example.com#frag" to "fragment", + "ws://relay.example.com" to "scheme other than wss or https", + "localhost" to "localhost", + "node.local" to ".local", + "svc.internal" to ".internal", + "a.localhost" to ".localhost", + "singlelabel" to "single label", + "192.0.2.7" to "all-numeric labels", + "réseau.example" to "non-ascii", + "-bad.example" to "label starting with hyphen", + "bad-.example" to "label ending with hyphen", + "${"a".repeat(64)}.example" to "label over 63 chars", + "${(1..4).joinToString(".") { "a".repeat(63) }}.ex" to "host over 253 chars", + "relay.example.com:0" to "port below 1", + "relay.example.com:70000" to "port above 65535", + "relay.example." to "trailing dot" + ) + for ((raw, reason) in rejected) { + assertNull(reason, RelayDirectory.validatedDirectoryAddress(raw)) + } + } + + // MARK: floors and the hijack guard + + @Test + fun `a remote file below the entry floor is rejected`() { + val csv = header + "relay-a.example,1.0,2.0\n" + assertNull(validate(csv, minimumEntries = RelayDirectory.MIN_REMOTE_ENTRIES)) + assertNotNull(validate(csv, minimumEntries = 1)) + } + + @Test + fun `a download keeping less than half of the known entries is rejected`() { + val baseline = setOf( + RelayDirectory.RelayInfo("wss://base-a.example", 1.0, 1.0), + RelayDirectory.RelayInfo("wss://base-b.example", 2.0, 2.0), + RelayDirectory.RelayInfo("wss://base-c.example", 3.0, 3.0), + RelayDirectory.RelayInfo("wss://base-d.example", 4.0, 4.0) + ) + val keepsTwo = header + + "base-a.example,1.0,1.0\n" + + "base-b.example,2.0,2.0\n" + + "fresh-a.example,5.0,5.0\n" + assertNotNull(validate(keepsTwo, baseline = baseline)) + + val keepsOne = header + + "base-a.example,1.0,1.0\n" + + "fresh-a.example,5.0,5.0\n" + + "fresh-b.example,6.0,6.0\n" + assertNull(validate(keepsOne, baseline = baseline)) + } + + // MARK: the shipped asset + + @Test + fun `the bundled asset passes validation with the expected entry count`() { + val asset = listOf( + File("src/main/assets/nostr_relays.csv"), + File("app/src/main/assets/nostr_relays.csv") + ).firstOrNull { it.isFile } ?: error("bundled relay asset not found") + val entries = RelayDirectory.validatedEntries(asset.readBytes(), minimumEntries = 1) + assertNotNull("the shipped snapshot must pass its own gate", entries) + assertEquals(326, entries!!.size) + } +} diff --git a/docs/client-rewrite-contracts.md b/docs/client-rewrite-contracts.md index f37809f0..6fc1b5a3 100644 --- a/docs/client-rewrite-contracts.md +++ b/docs/client-rewrite-contracts.md @@ -45,6 +45,15 @@ relay sets for the same geohash and silently fail to exchange messages. `RelayDirectoryTest` covers the Android side; iOS implements the same rules in `GeoRelayDirectory`. +Directory acceptance is part of the same contract. Both platforms validate a +directory file with the same rules (exact header, per-row host and coordinate +checks, size, row, and entry caps, and a minimum overlap with the previous +entries for downloads) and reject a violating file whole, keeping the previous +copy. A file one platform accepts and the other rejects splits the two relay +selections at every geohash at once. `RelayDirectoryValidationTest` covers the +Android side; iOS implements the same rules in +`GeoRelayDirectory.validatedEntries`. + ## Rewrite acceptance gate From a configured Android development environment, run: