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: