Merge 8cf0abf093fc19813c92a31181ec9efd3e9ad27a into c127eb83ab94c069c32d37530d2faecd381cd2a8

app/src/main/java/com/bitchat/android/mesh/BluetoothConnectionTracker.kt

@@ -171,18 +171,39 @@ class BluetoothConnectionTracker(
     }
     
     /**
-     * Add a subscribed device
+     * Subscribe a device to broadcast notifications.
+     *
+     * Idempotent by address: a peer may rewrite the notification descriptor as
+     * often as it likes, and each extra entry here would notify that address one
+     * more time for every broadcast the mesh produces. Returns true only when
+     * the device was not already subscribed, so callers can keep peer-connect
+     * side effects tied to an actual new subscription.
      */
-    fun addSubscribedDevice(device: BluetoothDevice) {
-        subscribedDevices.add(device)
-    }
+    fun addSubscribedDevice(device: BluetoothDevice): Boolean =
+        synchronized(connectionStateLock) {
+            if (subscribedDevices.any { it.address == device.address }) {
+                false
+            } else {
+                subscribedDevices.add(device)
+                true
+            }
+        }
     
     /**
-     * Remove a subscribed device
+     * Remove a subscribed device. Matches on address, like the connection
+     * cleanup paths, so an entry cannot outlive an unsubscribe because the
+     * framework handed us a different BluetoothDevice instance for the same
+     * address.
      */
     fun removeSubscribedDevice(device: BluetoothDevice) {
-        subscribedDevices.remove(device)
+        synchronized(connectionStateLock) {
+            subscribedDevices.removeAll { it.address == device.address }
+        }
     }
+
+    /** Whether this address currently receives broadcast notifications. */
+    fun isSubscribed(deviceAddress: String): Boolean =
+        subscribedDevices.any { it.address == deviceAddress }
     
     /**
      * Check if device is already connected

app/src/main/java/com/bitchat/android/mesh/BluetoothGattServerManager.kt

@@ -282,15 +282,28 @@ class BluetoothGattServerManager(
                     return
                 }
 
-                if (BluetoothGattDescriptor.ENABLE_NOTIFICATION_VALUE.contentEquals(value)) {
-                    connectionTracker.addSubscribedDevice(device)
-
-                    connectionScope.launch {
-                        delay(100)
-                        if (isActive) { // Check if still active
-                            delegate?.onDeviceConnected(device)
+                when (GattSubscriptionPolicy.classify(value)) {
+                    GattSubscriptionPolicy.Request.ENABLE -> {
+                        // A repeat descriptor write is not a new connection. Without
+                        // this gate a peer can re-enable notifications in a loop and
+                        // drive both the duplicate-notify fan-out and the announce
+                        // broadcast onDeviceConnected triggers, unauthenticated.
+                        if (connectionTracker.addSubscribedDevice(device)) {
+                            connectionScope.launch {
+                                delay(100)
+                                if (isActive) { // Check if still active
+                                    delegate?.onDeviceConnected(device)
+                                }
+                            }
                         }
                     }
+
+                    GattSubscriptionPolicy.Request.DISABLE ->
+                        // The client asked to stop receiving the broadcast feed;
+                        // keeping it subscribed until disconnect ignores that.
+                        connectionTracker.removeSubscribedDevice(device)
+
+                    GattSubscriptionPolicy.Request.UNRECOGNIZED -> Unit
                 }
                 
                 if (responseNeeded) {

app/src/main/java/com/bitchat/android/mesh/GattSubscriptionPolicy.kt

@@ -0,0 +1,34 @@
+package com.bitchat.android.mesh
+
+import android.bluetooth.BluetoothGattDescriptor
+
+/**
+ * Decides what a Client Characteristic Configuration Descriptor write means for
+ * our broadcast-notification subscription list.
+ *
+ * A CCCD write is the one GATT operation a peer can repeat freely on an open
+ * connection: it needs no authentication, no Noise session and no announcement,
+ * and the Bluetooth spec places no limit on how often a client may rewrite the
+ * descriptor. Every effect the server attaches to that write therefore has to be
+ * idempotent, or a peer gets to drive it at whatever rate it likes.
+ */
+internal object GattSubscriptionPolicy {
+
+    enum class Request {
+        /** Client asked to receive broadcast notifications. */
+        ENABLE,
+
+        /** Client asked to stop receiving them. Honouring this is spec-required. */
+        DISABLE,
+
+        /** Any other descriptor value, including indications we do not use. */
+        UNRECOGNIZED
+    }
+
+    fun classify(value: ByteArray?): Request = when {
+        value == null -> Request.UNRECOGNIZED
+        BluetoothGattDescriptor.ENABLE_NOTIFICATION_VALUE.contentEquals(value) -> Request.ENABLE
+        BluetoothGattDescriptor.DISABLE_NOTIFICATION_VALUE.contentEquals(value) -> Request.DISABLE
+        else -> Request.UNRECOGNIZED
+    }
+}

app/src/test/kotlin/com/bitchat/android/mesh/GattBroadcastSubscriptionTest.kt

@@ -0,0 +1,129 @@
+package com.bitchat.android.mesh
+
+import android.bluetooth.BluetoothDevice
+import android.bluetooth.BluetoothGattDescriptor
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.SupervisorJob
+import kotlinx.coroutines.cancel
+import org.junit.After
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.mockito.kotlin.mock
+import org.mockito.kotlin.whenever
+import org.robolectric.RobolectricTestRunner
+
+/**
+ * A CCCD write is the one GATT operation an unauthenticated peer can repeat at
+ * will on an open connection. Everything the server hangs off it — the broadcast
+ * fan-out list and the peer-connect side effects — therefore has to be
+ * idempotent, and an explicit unsubscribe has to be honoured.
+ */
+@RunWith(RobolectricTestRunner::class)
+class GattBroadcastSubscriptionTest {
+    private val scope = CoroutineScope(Dispatchers.Unconfined + SupervisorJob())
+    private val tracker = BluetoothConnectionTracker(scope, mock())
+
+    @After
+    fun tearDown() {
+        scope.cancel()
+    }
+
+    private fun device(address: String): BluetoothDevice = mock<BluetoothDevice>().also {
+        whenever(it.address).thenReturn(address)
+    }
+
+    @Test
+    fun `enabling notifications is recognized`() {
+        assertEquals(
+            GattSubscriptionPolicy.Request.ENABLE,
+            GattSubscriptionPolicy.classify(BluetoothGattDescriptor.ENABLE_NOTIFICATION_VALUE)
+        )
+    }
+
+    @Test
+    fun `disabling notifications is recognized instead of being ignored`() {
+        assertEquals(
+            GattSubscriptionPolicy.Request.DISABLE,
+            GattSubscriptionPolicy.classify(BluetoothGattDescriptor.DISABLE_NOTIFICATION_VALUE)
+        )
+    }
+
+    @Test
+    fun `any other descriptor value changes nothing`() {
+        assertEquals(
+            GattSubscriptionPolicy.Request.UNRECOGNIZED,
+            GattSubscriptionPolicy.classify(BluetoothGattDescriptor.ENABLE_INDICATION_VALUE)
+        )
+        assertEquals(GattSubscriptionPolicy.Request.UNRECOGNIZED, GattSubscriptionPolicy.classify(null))
+        assertEquals(GattSubscriptionPolicy.Request.UNRECOGNIZED, GattSubscriptionPolicy.classify(ByteArray(0)))
+    }
+
+    @Test
+    fun `repeated enables subscribe an address exactly once`() {
+        val peer = device("AA:BB:CC:DD:EE:01")
+
+        assertTrue("first enable must be a new subscription", tracker.addSubscribedDevice(peer))
+        repeat(64) {
+            assertFalse("a repeat enable is not a new subscription", tracker.addSubscribedDevice(peer))
+        }
+
+        assertEquals(1, tracker.getSubscribedDevices().count { it.address == peer.address })
+    }
+
+    @Test
+    fun `a second framework instance for the same address does not add a second entry`() {
+        // The stack hands out distinct BluetoothDevice objects for one address,
+        // so identity comparison would let the fan-out list grow anyway.
+        assertTrue(tracker.addSubscribedDevice(device("AA:BB:CC:DD:EE:02")))
+        assertFalse(tracker.addSubscribedDevice(device("AA:BB:CC:DD:EE:02")))
+
+        assertEquals(1, tracker.getSubscribedDevices().size)
+    }
+
+    @Test
+    fun `unsubscribing removes the address whichever instance carries it`() {
+        assertTrue(tracker.addSubscribedDevice(device("AA:BB:CC:DD:EE:03")))
+
+        tracker.removeSubscribedDevice(device("AA:BB:CC:DD:EE:03"))
+
+        assertTrue(tracker.getSubscribedDevices().isEmpty())
+        assertFalse(tracker.isSubscribed("AA:BB:CC:DD:EE:03"))
+    }
+
+    @Test
+    fun `unsubscribing one peer leaves the others receiving broadcasts`() {
+        val first = device("AA:BB:CC:DD:EE:04")
+        val second = device("AA:BB:CC:DD:EE:05")
+        tracker.addSubscribedDevice(first)
+        tracker.addSubscribedDevice(second)
+
+        tracker.removeSubscribedDevice(first)
+
+        assertEquals(listOf(second.address), tracker.getSubscribedDevices().map { it.address })
+    }
+
+    @Test
+    fun `resubscribing after an unsubscribe counts as new again`() {
+        val peer = device("AA:BB:CC:DD:EE:06")
+
+        assertTrue(tracker.addSubscribedDevice(peer))
+        tracker.removeSubscribedDevice(peer)
+
+        assertTrue(tracker.addSubscribedDevice(peer))
+        assertEquals(1, tracker.getSubscribedDevices().size)
+    }
+
+    @Test
+    fun `disconnect cleanup still drops the subscription`() {
+        val peer = device("AA:BB:CC:DD:EE:07")
+        tracker.addSubscribedDevice(peer)
+
+        tracker.cleanupDeviceConnection(peer.address)
+
+        assertFalse(tracker.isSubscribed(peer.address))
+    }
+}

docs/device_manager.md

@@ -55,6 +55,14 @@ Timers:
 - On connection setup complete (descriptor enable) and also after initial connect, start monitoring via `onConnectionEstablished(addr)`.
 - On packet write, call `deviceMonitor.onAnyPacketReceived(addr)`.
 - On disconnect, call `deviceMonitor.onDeviceDisconnected(addr, status)`.
+- Broadcast-notification subscription is keyed by device address and is
+  idempotent. A CCCD write needs no authentication and the Bluetooth spec puts
+  no limit on how often a client may repeat it, so the server must treat a
+  repeat enable as a no-op: exactly one entry per address in the notification
+  fan-out list, and peer-connect side effects (including the announce broadcast
+  `onDeviceConnected` triggers) run only on the transition into the subscribed
+  state. A CCCD write of `DISABLE_NOTIFICATION_VALUE` unsubscribes the address;
+  it is not deferred to disconnect.
 
 4) ANNOUNCE Binding
 - File: `BluetoothMeshService.kt` (in the ANNOUNCE handler where we first map device → peer)