diff --git a/app/src/main/java/com/bitchat/android/geohash/Geohash.kt b/app/src/main/java/com/bitchat/android/geohash/Geohash.kt index 00b2cdf39..2123dbe19 100644 --- a/app/src/main/java/com/bitchat/android/geohash/Geohash.kt +++ b/app/src/main/java/com/bitchat/android/geohash/Geohash.kt @@ -66,6 +66,13 @@ object Geohash { return geohash.toString() } + /** + * True when every character of [geohash] is a base32 geohash digit and the + * string is non-empty, i.e. when it names an actual cell. + */ + fun isValid(geohash: String): Boolean = + geohash.isNotEmpty() && geohash.lowercase().all { charToValue.containsKey(it) } + /** * Decodes a geohash string to the center latitude/longitude of its cell. * @return Pair(latitude, longitude) @@ -77,8 +84,31 @@ object Geohash { return latCenter to lonCenter } + /** + * Decodes a geohash string to the center latitude/longitude of its cell, or + * null when the string does not name a cell. + * + * Prefer this over [decodeToCenter] wherever the string can come from + * outside: an empty or malformed geohash decodes to the (0, 0) box, and + * (0, 0) is a real place in the Gulf of Guinea, so a caller that cannot + * tell the two apart silently acts on a location the user never chose. + */ + fun decodeToCenterOrNull(geohash: String): Pair? = + if (isValid(geohash)) decodeToCenter(geohash) else null + + /** + * Decodes a geohash string to a bounding box, or null when the string does + * not name a cell. See [decodeToCenterOrNull]. + */ + fun decodeToBoundsOrNull(geohash: String): Bounds? = + if (isValid(geohash)) decodeToBounds(geohash) else null + /** * Decodes a geohash string to bounding box (lat/lon min/max). + * + * An empty or malformed geohash yields the degenerate box at (0, 0); use + * [decodeToBoundsOrNull] when that has to be distinguishable from a real + * cell there. */ fun decodeToBounds(geohash: String): Bounds { if (geohash.isEmpty()) return Bounds(0.0, 0.0, 0.0, 0.0) 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 980b95efd..13bafce21 100644 --- a/app/src/main/java/com/bitchat/android/nostr/RelayDirectory.kt +++ b/app/src/main/java/com/bitchat/android/nostr/RelayDirectory.kt @@ -88,10 +88,10 @@ object RelayDirectory { fun closestRelaysForGeohash(geohash: String, nRelays: Int): List { val snapshot = synchronized(relaysLock) { relays.toList() } if (snapshot.isEmpty()) return emptyList() - val center = try { - val c = com.bitchat.android.geohash.Geohash.decodeToCenter(geohash) - c - } catch (e: Exception) { + // An empty or malformed geohash decodes to (0, 0), which would pick the + // relays closest to the Gulf of Guinea rather than declining to choose. + val center = com.bitchat.android.geohash.Geohash.decodeToCenterOrNull(geohash) + if (center == null) { Log.e(TAG, "Failed to decode geohash") return emptyList() } diff --git a/app/src/test/java/com/bitchat/android/geohash/GeohashValidityTest.kt b/app/src/test/java/com/bitchat/android/geohash/GeohashValidityTest.kt new file mode 100644 index 000000000..47140af0c --- /dev/null +++ b/app/src/test/java/com/bitchat/android/geohash/GeohashValidityTest.kt @@ -0,0 +1,61 @@ +package com.bitchat.android.geohash + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * A geohash that names no cell has to be distinguishable from one that names a + * cell at (0, 0): the origin is a real place in the Gulf of Guinea, so anything + * that picks relays or geocodes a name from a decoded centre would otherwise + * act on a location the user never chose. + */ +class GeohashValidityTest { + + @Test + fun `base32 strings are valid regardless of case`() { + assertTrue(Geohash.isValid("u4pruydqqvj")) + assertTrue(Geohash.isValid("U4PRUYDQQVJ")) + assertTrue(Geohash.isValid("9")) + } + + @Test + fun `empty and malformed strings are invalid`() { + assertFalse(Geohash.isValid("")) + // a, i, l and o are not geohash digits + assertFalse(Geohash.isValid("hello")) + assertFalse(Geohash.isValid("u4pr!")) + assertFalse(Geohash.isValid("café")) + } + + @Test + fun `nullable decoders reject what the plain ones map onto the origin`() { + assertEquals(0.0 to 0.0, Geohash.decodeToCenter("")) + assertEquals(0.0 to 0.0, Geohash.decodeToCenter("hello")) + + assertNull(Geohash.decodeToCenterOrNull("")) + assertNull(Geohash.decodeToCenterOrNull("hello")) + assertNull(Geohash.decodeToBoundsOrNull("")) + assertNull(Geohash.decodeToBoundsOrNull("hello")) + } + + @Test + fun `nullable decoders agree with the plain ones on real cells`() { + val geohash = "u4pruydqqvj" + assertEquals(Geohash.decodeToCenter(geohash), Geohash.decodeToCenterOrNull(geohash)) + assertEquals(Geohash.decodeToBounds(geohash), Geohash.decodeToBoundsOrNull(geohash)) + assertNotNull(Geohash.decodeToCenterOrNull("s000")) + } + + @Test + fun `a cell at the origin still decodes`() { + // "s000..." is the cell containing (0, 0) — a valid geohash, not a failure. + val center = Geohash.decodeToCenterOrNull("s0000") + assertNotNull(center) + assertTrue(center!!.first in -1.0..1.0) + assertTrue(center.second in -1.0..1.0) + } +}