diff --git a/app/src/main/java/org/prolibertate/games/net/LanTransport.kt b/app/src/main/java/org/prolibertate/games/net/LanTransport.kt index 941ba87..cf939ce 100644 --- a/app/src/main/java/org/prolibertate/games/net/LanTransport.kt +++ b/app/src/main/java/org/prolibertate/games/net/LanTransport.kt @@ -201,11 +201,11 @@ class LanTransport(private val context: Context) : Transport { override suspend fun join(host: DiscoveredHost, scope: CoroutineScope): Connection = withContext(Dispatchers.IO) { val socket = Socket() - // Before connecting, and this is the whole fix for hotspot play: a + // Before connecting, and this is the fix for hotspot play: a // hotspot has no internet behind it, so Android leaves mobile data // as the default network and an unbound socket goes out over // cellular looking for a 192.168 address. - bindToWifi(context, socket) + bindForTarget(context, socket, host.address) socket.connect(InetSocketAddress(host.address, host.port), CONNECT_TIMEOUT_MS) socket.tcpNoDelay = true StreamConnection( diff --git a/app/src/main/java/org/prolibertate/games/net/LocalNetwork.kt b/app/src/main/java/org/prolibertate/games/net/LocalNetwork.kt index cb8c9f7..29ce3c0 100644 --- a/app/src/main/java/org/prolibertate/games/net/LocalNetwork.kt +++ b/app/src/main/java/org/prolibertate/games/net/LocalNetwork.kt @@ -6,6 +6,8 @@ import android.net.Network import android.net.NetworkCapabilities import android.net.wifi.WifiManager import java.net.Inet4Address +import java.net.InetAddress +import java.net.InetSocketAddress import java.net.NetworkInterface import java.net.Socket @@ -40,14 +42,42 @@ fun wifiNetwork(context: Context): Network? { } /** - * Sends this socket out over the Wi-Fi rather than wherever the system would - * otherwise send it. Silent if there is no Wi-Fi network to bind to: a phone - * running the hotspot itself has no Wi-Fi *client* network, and reaches its own - * guests over the tethering interface's own route without any help. + * Points this socket at the network [target] is actually on, before it + * connects. + * + * Two different problems wearing the same hat, depending on which end of the + * hotspot this phone is. + * + * A phone that has *joined* a hotspot has a Wi-Fi network, and naming it is + * enough. A phone that is *sharing* one does not: its guests live on the + * tethering interface, and ConnectivityManager does not offer that as a + * [Network] anything can bind to — so the socket falls back to the default + * network, which is mobile data, and a connection to a guest goes out over + * cellular and dies. Binding this end to an address on the guest's own subnet + * picks the route instead, which is the only lever available. */ -fun bindToWifi(context: Context, socket: Socket) { - val network = wifiNetwork(context) ?: return - runCatching { network.bindSocket(socket) } +fun bindForTarget(context: Context, socket: Socket, target: String) { + val network = wifiNetwork(context) + if (network != null) { + runCatching { network.bindSocket(socket) } + return + } + val source = localIpv4Addresses().firstOrNull { sameSubnet(it, target) } ?: return + runCatching { socket.bind(InetSocketAddress(InetAddress.getByName(source), 0)) } +} + +/** + * Whether two IPv4 addresses are on the same /24. + * + * A guess, but a safe one here: every Android hotspot hands out a /24, and the + * only thing this decides is which of our own addresses to send from. Getting + * it wrong costs a bind that would not have helped anyway. + */ +fun sameSubnet(a: String, b: String): Boolean { + val left = a.split('.') + val right = b.split('.') + if (left.size != 4 || right.size != 4) return false + return left.subList(0, 3) == right.subList(0, 3) } /** diff --git a/app/src/test/java/org/prolibertate/games/net/ManualAddressTest.kt b/app/src/test/java/org/prolibertate/games/net/ManualAddressTest.kt index f8369da..755eab7 100644 --- a/app/src/test/java/org/prolibertate/games/net/ManualAddressTest.kt +++ b/app/src/test/java/org/prolibertate/games/net/ManualAddressTest.kt @@ -1,7 +1,9 @@ package org.prolibertate.games.net import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue import org.junit.Test /** @@ -103,4 +105,32 @@ class ManualAddressTest { val shown = HostEndpoint(listOf("192.168.43.1"), 47654).primary!! assertEquals(ManualAddress("192.168.43.1", 47654), parseManualAddress(shown)) } + + // -- Choosing which of our own addresses to send from -------------------- + + /** + * The phone sharing the hotspot has no Wi-Fi network to bind to, so it + * picks a source address on the guest's subnet instead. Getting this wrong + * sends the connection out over mobile data. + */ + @Test + fun `addresses on one hotspot subnet match`() { + assertTrue(sameSubnet("192.168.43.1", "192.168.43.117")) + assertTrue(sameSubnet("192.168.1.20", "192.168.1.20")) + } + + @Test + fun `addresses on different subnets do not`() { + assertFalse(sameSubnet("192.168.43.1", "192.168.1.5")) + assertFalse(sameSubnet("192.168.43.1", "10.0.0.5")) + } + + /** Anything that is not a dotted quad is not a match, rather than a crash. */ + @Test + fun `nonsense is not a match`() { + assertFalse(sameSubnet("", "192.168.43.1")) + assertFalse(sameSubnet("192.168.43", "192.168.43.1")) + assertFalse(sameSubnet("pixel-9.local", "192.168.43.1")) + assertFalse(sameSubnet("192.168.43.1.7", "192.168.43.1")) + } }