From 9de168d0d7666e9e9414b7ecdb840bcc74a5fa0c Mon Sep 17 00:00:00 2001 From: Gunjan Jaswal Date: Sat, 25 Jul 2026 22:32:30 +0530 Subject: [PATCH 1/2] fix(mesh): reassemble BLE prepared/long writes on the GATT server iOS sends packets larger than the negotiated ATT MTU using the BLE prepared ("reliable"/long) write procedure: the payload arrives as several onCharacteristicWriteRequest callbacks with preparedWrite=true and increasing offsets, finalized by onExecuteWrite. The server only handled the offset=0 chunk and tried to parse each chunk as a whole packet, so anything over ~157 chars from iOS was silently dropped. Buffer prepared-write chunks per device by offset and reassemble the full payload in a new onExecuteWrite override before parsing. The non-prepared write path is unchanged. The reassembly is factored into a small, thread-safe GattPreparedWriteBuffer with a per-device size cap so a broken or malicious peer cannot exhaust memory, and buffers are cleared on execute, cancel, and disconnect. Fixes #90 --- .../mesh/BluetoothGattServerManager.kt | 78 ++++++++-- .../android/mesh/GattPreparedWriteBuffer.kt | 102 ++++++++++++ .../mesh/GattPreparedWriteBufferTest.kt | 147 ++++++++++++++++++ 3 files changed, 317 insertions(+), 10 deletions(-) create mode 100644 app/src/main/java/com/bitchat/android/mesh/GattPreparedWriteBuffer.kt create mode 100644 app/src/test/java/com/bitchat/android/mesh/GattPreparedWriteBufferTest.kt diff --git a/app/src/main/java/com/bitchat/android/mesh/BluetoothGattServerManager.kt b/app/src/main/java/com/bitchat/android/mesh/BluetoothGattServerManager.kt index 0c7aabc3..9c3c7672 100644 --- a/app/src/main/java/com/bitchat/android/mesh/BluetoothGattServerManager.kt +++ b/app/src/main/java/com/bitchat/android/mesh/BluetoothGattServerManager.kt @@ -50,6 +50,11 @@ class BluetoothGattServerManager( // State management private var isActive = false + // Reassembles BLE prepared ("reliable"/long) writes. iOS uses these to send + // packets larger than the negotiated ATT MTU: the payload arrives as several + // preparedWrite chunks and is finalized by onExecuteWrite. + private val preparedWriteBuffer = GattPreparedWriteBuffer() + private fun isBleTransportEnabled(): Boolean { return try { com.bitchat.android.ui.debug.DebugSettingsManager.getInstance().bleEnabled.value @@ -159,6 +164,24 @@ class BluetoothGattServerManager( * Get characteristic instance */ fun getCharacteristic(): BluetoothGattCharacteristic? = characteristic + + /** + * Parse a fully-received payload and hand it to the delegate. Shared by the + * non-prepared write path and the reassembled prepared-write path so both + * behave identically once a complete payload is in hand. + */ + private fun handleReceivedPacket(device: BluetoothDevice, value: ByteArray) { + Log.i(TAG, "Server: Received packet from ${device.address}, size: ${value.size} bytes") + val packet = BitchatPacket.fromBinaryData(value) + if (packet != null) { + val peerID = packet.senderID.take(8).toByteArray().joinToString("") { "%02x".format(it) } + Log.d(TAG, "Server: Parsed packet type ${packet.type} from $peerID") + delegate?.onPacketReceived(packet, peerID, device) + } else { + Log.w(TAG, "Server: Failed to parse packet from ${device.address}, size: ${value.size} bytes") + Log.w(TAG, "Server: Packet data: ${value.joinToString(" ") { "%02x".format(it) }}") + } + } /** * Setup GATT server with proper sequencing @@ -199,6 +222,8 @@ class BluetoothGattServerManager( BluetoothProfile.STATE_DISCONNECTED -> { Log.i(TAG, "Server: Device disconnected ${device.address}") connectionTracker.cleanupDeviceConnection(device.address) + // Drop any in-flight prepared-write buffer for this device to avoid leaks + preparedWriteBuffer.cancel(device.address) // Notify delegate about device disconnection so higher layers can update direct flags delegate?.onDeviceDisconnected(device) } @@ -235,22 +260,55 @@ class BluetoothGattServerManager( } if (characteristic.uuid == AppConstants.Mesh.Gatt.CHARACTERISTIC_UUID) { - Log.i(TAG, "Server: Received packet from ${device.address}, size: ${value.size} bytes") - val packet = BitchatPacket.fromBinaryData(value) - if (packet != null) { - val peerID = packet.senderID.take(8).toByteArray().joinToString("") { "%02x".format(it) } - Log.d(TAG, "Server: Parsed packet type ${packet.type} from $peerID") - delegate?.onPacketReceived(packet, peerID, device) - } else { - Log.w(TAG, "Server: Failed to parse packet from ${device.address}, size: ${value.size} bytes") - Log.w(TAG, "Server: Packet data: ${value.joinToString(" ") { "%02x".format(it) }}") + if (preparedWrite) { + // BLE reliable/long write (used by iOS for packets larger than the + // negotiated ATT MTU): buffer this chunk by offset and defer parsing + // until onExecuteWrite reassembles the full payload. + val accepted = preparedWriteBuffer.append(device.address, offset, value) + if (accepted) { + Log.d(TAG, "Server: Buffered prepared-write chunk from ${device.address} (offset=$offset, size=${value.size})") + } else { + Log.w(TAG, "Server: Rejected prepared-write chunk from ${device.address} (offset=$offset, size=${value.size}); buffer limit exceeded") + } + // Prepared-write protocol requires echoing the offset and value back + if (responseNeeded) { + gattServer?.sendResponse(device, requestId, BluetoothGatt.GATT_SUCCESS, offset, value) + } + return } - + + handleReceivedPacket(device, value) + if (responseNeeded) { gattServer?.sendResponse(device, requestId, BluetoothGatt.GATT_SUCCESS, 0, null) } } } + + override fun onExecuteWrite(device: BluetoothDevice, requestId: Int, execute: Boolean) { + // Guard against callbacks after service shutdown + if (!isActive) { + Log.d(TAG, "Server: Ignoring execute write after shutdown") + preparedWriteBuffer.cancel(device.address) + return + } + + if (execute) { + val assembled = preparedWriteBuffer.execute(device.address) + if (assembled != null) { + Log.i(TAG, "Server: Reassembled prepared write from ${device.address}, size: ${assembled.size} bytes") + handleReceivedPacket(device, assembled) + } else { + Log.w(TAG, "Server: Execute write from ${device.address} with no or oversized buffered data; dropped") + } + } else { + // Client cancelled the long write; discard the buffered chunks + preparedWriteBuffer.cancel(device.address) + } + + // An execute-write always expects a response + gattServer?.sendResponse(device, requestId, BluetoothGatt.GATT_SUCCESS, 0, null) + } override fun onDescriptorWriteRequest( device: BluetoothDevice, diff --git a/app/src/main/java/com/bitchat/android/mesh/GattPreparedWriteBuffer.kt b/app/src/main/java/com/bitchat/android/mesh/GattPreparedWriteBuffer.kt new file mode 100644 index 00000000..9dccb614 --- /dev/null +++ b/app/src/main/java/com/bitchat/android/mesh/GattPreparedWriteBuffer.kt @@ -0,0 +1,102 @@ +package com.bitchat.android.mesh + +import java.util.concurrent.ConcurrentHashMap + +/** + * Reassembles BLE prepared ("reliable" / long) writes on the GATT server side. + * + * When a peer (notably iOS) needs to send a payload larger than the negotiated + * ATT MTU, it uses the ATT prepared-write procedure: the payload arrives as a + * series of `onCharacteristicWriteRequest` callbacks with `preparedWrite == true` + * and increasing `offset`s, and is finalized by a single `onExecuteWrite`. Each + * individual chunk is only a slice of the packet, so parsing a chunk on its own + * fails. This buffer collects the chunks per device and returns the concatenated + * payload once the write is executed. + * + * All operations are thread-safe: these GATT callbacks can arrive on binder + * threads and several devices may be writing concurrently. Each device is keyed + * by a stable string (its BLE address) so device buffers are isolated. + * + * A per-device size cap protects against a broken or malicious peer streaming an + * unbounded prepared write to exhaust memory; once exceeded the buffer is dropped + * and [execute] returns null. + */ +class GattPreparedWriteBuffer( + private val maxPayloadSize: Int = DEFAULT_MAX_PAYLOAD_SIZE +) { + companion object { + /** + * Generous upper bound for a single reassembled payload. Real bitchat + * packets are far smaller (well under the low-KB range), so this only + * ever trips for abusive peers. + */ + const val DEFAULT_MAX_PAYLOAD_SIZE = 512 * 1024 // 512 KiB + } + + private class DeviceBuffer { + var data: ByteArray = ByteArray(0) + var length: Int = 0 + var overflowed: Boolean = false + } + + private val buffers = ConcurrentHashMap() + + /** + * Buffer a single prepared-write chunk for [deviceKey], placing [value] at + * [offset] within that device's growing payload. + * + * Chunks are written at their byte offset, so reassembly is correct even if + * chunks are delivered out of order. Returns true if the chunk was accepted, + * or false if it was rejected: a negative offset, or a write that would push + * the payload past [maxPayloadSize]. On overflow the device's buffer is + * marked so that [execute] yields null and the oversized payload is dropped. + */ + fun append(deviceKey: String, offset: Int, value: ByteArray): Boolean { + if (offset < 0) return false + val buf = buffers.getOrPut(deviceKey) { DeviceBuffer() } + synchronized(buf) { + if (buf.overflowed) return false + val end = offset.toLong() + value.size.toLong() + if (end > maxPayloadSize.toLong()) { + // Drop what we have and remember the overflow until the write is + // finalized/cancelled, so we never allocate beyond the cap. + buf.overflowed = true + buf.data = ByteArray(0) + buf.length = 0 + return false + } + val endInt = end.toInt() + if (endInt > buf.data.size) { + val newCapacity = maxOf(endInt, buf.data.size * 2).coerceAtMost(maxPayloadSize) + buf.data = buf.data.copyOf(maxOf(newCapacity, endInt)) + } + System.arraycopy(value, 0, buf.data, offset, value.size) + if (endInt > buf.length) buf.length = endInt + return true + } + } + + /** + * Finalize the prepared write for [deviceKey] and return the reassembled + * payload, or null if nothing was buffered or the buffer overflowed. The + * device's buffer is always removed. + */ + fun execute(deviceKey: String): ByteArray? { + val buf = buffers.remove(deviceKey) ?: return null + synchronized(buf) { + if (buf.overflowed || buf.length == 0) return null + return buf.data.copyOf(buf.length) + } + } + + /** + * Discard any buffered chunks for [deviceKey]. Used when a prepared write is + * cancelled or when the device disconnects, to avoid leaking buffers. + */ + fun cancel(deviceKey: String) { + buffers.remove(deviceKey) + } + + /** Number of devices currently holding buffered chunks (diagnostics/tests). */ + fun activeDeviceCount(): Int = buffers.size +} diff --git a/app/src/test/java/com/bitchat/android/mesh/GattPreparedWriteBufferTest.kt b/app/src/test/java/com/bitchat/android/mesh/GattPreparedWriteBufferTest.kt new file mode 100644 index 00000000..25dfb3c3 --- /dev/null +++ b/app/src/test/java/com/bitchat/android/mesh/GattPreparedWriteBufferTest.kt @@ -0,0 +1,147 @@ +package com.bitchat.android.mesh + +import org.junit.Assert.assertArrayEquals +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test + +/** + * Unit tests for [GattPreparedWriteBuffer], the pure reassembly logic behind the + * fix for issue #90 (iOS -> Android messages over ~157 chars silently dropped + * because BLE long/prepared writes were never reassembled on the GATT server). + */ +class GattPreparedWriteBufferTest { + + private lateinit var buffer: GattPreparedWriteBuffer + + @Before + fun setup() { + buffer = GattPreparedWriteBuffer() + } + + private fun bytes(vararg v: Int) = ByteArray(v.size) { v[it].toByte() } + + @Test + fun `single chunk is returned verbatim on execute`() { + val device = "AA:BB:CC:DD:EE:01" + val payload = bytes(1, 2, 3, 4, 5) + assertTrue(buffer.append(device, 0, payload)) + assertArrayEquals(payload, buffer.execute(device)) + } + + @Test + fun `multiple in-order chunks reassemble to the concatenation`() { + val device = "AA:BB:CC:DD:EE:02" + val c0 = bytes(10, 11, 12) + val c1 = bytes(20, 21, 22) + val c2 = bytes(30, 31) + assertTrue(buffer.append(device, 0, c0)) + assertTrue(buffer.append(device, 3, c1)) + assertTrue(buffer.append(device, 6, c2)) + + val expected = c0 + c1 + c2 + assertArrayEquals(expected, buffer.execute(device)) + } + + /** + * The key regression guard: chunks are placed by offset, so an implementation + * that simply appends in arrival order (ignoring offset) would produce the + * wrong payload. Feeding chunks out of arrival order must still reassemble + * correctly. + */ + @Test + fun `chunks delivered out of order reassemble by offset not arrival order`() { + val device = "AA:BB:CC:DD:EE:03" + val first = bytes(1, 2, 3, 4) // belongs at offset 0 + val second = bytes(5, 6, 7, 8) // belongs at offset 4 + + // Deliver the later offset first + assertTrue(buffer.append(device, 4, second)) + assertTrue(buffer.append(device, 0, first)) + + val expected = bytes(1, 2, 3, 4, 5, 6, 7, 8) + assertArrayEquals(expected, buffer.execute(device)) + } + + @Test + fun `reassembles a payload larger than a typical ATT MTU`() { + val device = "AA:BB:CC:DD:EE:04" + val total = 600 // > 157 chars / > single-MTU write from the issue + val full = ByteArray(total) { (it % 256).toByte() } + val chunkSize = 180 // mimic MTU-sized fragments + var offset = 0 + while (offset < total) { + val end = minOf(offset + chunkSize, total) + val slice = full.copyOfRange(offset, end) + assertTrue(buffer.append(device, offset, slice)) + offset = end + } + assertArrayEquals(full, buffer.execute(device)) + } + + @Test + fun `devices are isolated from one another`() { + val a = "AA:BB:CC:DD:EE:0A" + val b = "AA:BB:CC:DD:EE:0B" + val payloadA = bytes(1, 1, 1) + val payloadB = bytes(9, 9, 9, 9) + + buffer.append(a, 0, payloadA) + buffer.append(b, 0, payloadB) + + assertArrayEquals(payloadA, buffer.execute(a)) + // Draining device A must not affect device B's buffer + assertArrayEquals(payloadB, buffer.execute(b)) + } + + @Test + fun `cancel discards buffered chunks and execute then yields null`() { + val device = "AA:BB:CC:DD:EE:05" + buffer.append(device, 0, bytes(1, 2, 3)) + buffer.cancel(device) + assertNull(buffer.execute(device)) + } + + @Test + fun `execute with nothing buffered yields null`() { + assertNull(buffer.execute("never-seen")) + } + + @Test + fun `execute clears the buffer so a second execute yields null`() { + val device = "AA:BB:CC:DD:EE:06" + buffer.append(device, 0, bytes(7, 7)) + assertArrayEquals(bytes(7, 7), buffer.execute(device)) + assertNull(buffer.execute(device)) + assertEquals(0, buffer.activeDeviceCount()) + } + + @Test + fun `oversize write is rejected and dropped`() { + val small = GattPreparedWriteBuffer(maxPayloadSize = 16) + val device = "AA:BB:CC:DD:EE:07" + assertTrue(small.append(device, 0, ByteArray(10))) + // This chunk would push the payload past the 16-byte cap + assertFalse(small.append(device, 10, ByteArray(10))) + // Once overflowed the whole payload is dropped + assertNull(small.execute(device)) + } + + @Test + fun `negative offset is rejected`() { + val device = "AA:BB:CC:DD:EE:08" + assertFalse(buffer.append(device, -1, bytes(1, 2, 3))) + } + + @Test + fun `writes exactly at the cap are accepted`() { + val small = GattPreparedWriteBuffer(maxPayloadSize = 16) + val device = "AA:BB:CC:DD:EE:09" + assertTrue(small.append(device, 0, ByteArray(16) { it.toByte() })) + val result = small.execute(device) + assertEquals(16, result?.size) + } +} From e16ca70ff7b20521100a6d5bf369324e1262b6f0 Mon Sep 17 00:00:00 2001 From: Gunjan Jaswal Date: Sun, 26 Jul 2026 07:07:32 +0530 Subject: [PATCH 2/2] Clear prepared-write buffers when the GATT server stops stop() marks the server inactive before connections are torn down, and the disconnect callback returns early once inactive, so it never reaches the per-device cancel path. Any half-finished prepared write would linger until the process ends. Clear all buffers directly in stop(). --- .../android/mesh/BluetoothGattServerManager.kt | 4 ++++ .../bitchat/android/mesh/GattPreparedWriteBuffer.kt | 9 +++++++++ .../android/mesh/GattPreparedWriteBufferTest.kt | 13 +++++++++++++ 3 files changed, 26 insertions(+) diff --git a/app/src/main/java/com/bitchat/android/mesh/BluetoothGattServerManager.kt b/app/src/main/java/com/bitchat/android/mesh/BluetoothGattServerManager.kt index 9c3c7672..bf311956 100644 --- a/app/src/main/java/com/bitchat/android/mesh/BluetoothGattServerManager.kt +++ b/app/src/main/java/com/bitchat/android/mesh/BluetoothGattServerManager.kt @@ -123,6 +123,10 @@ class BluetoothGattServerManager( * Stop GATT server */ fun stop() { + // Drop any half-finished prepared writes. A disconnect after the server is marked + // inactive returns early before the per-device cancel path, so clear them here. + preparedWriteBuffer.clearAll() + if (!isActive) { // Idempotent stop stopAdvertising() diff --git a/app/src/main/java/com/bitchat/android/mesh/GattPreparedWriteBuffer.kt b/app/src/main/java/com/bitchat/android/mesh/GattPreparedWriteBuffer.kt index 9dccb614..ba6b9ba9 100644 --- a/app/src/main/java/com/bitchat/android/mesh/GattPreparedWriteBuffer.kt +++ b/app/src/main/java/com/bitchat/android/mesh/GattPreparedWriteBuffer.kt @@ -97,6 +97,15 @@ class GattPreparedWriteBuffer( buffers.remove(deviceKey) } + /** + * Drop every device's buffered chunks. Used when the GATT server shuts down, since a + * disconnect that happens after the server is already marked inactive never reaches the + * per-device cancel path. + */ + fun clearAll() { + buffers.clear() + } + /** Number of devices currently holding buffered chunks (diagnostics/tests). */ fun activeDeviceCount(): Int = buffers.size } diff --git a/app/src/test/java/com/bitchat/android/mesh/GattPreparedWriteBufferTest.kt b/app/src/test/java/com/bitchat/android/mesh/GattPreparedWriteBufferTest.kt index 25dfb3c3..bc62d742 100644 --- a/app/src/test/java/com/bitchat/android/mesh/GattPreparedWriteBufferTest.kt +++ b/app/src/test/java/com/bitchat/android/mesh/GattPreparedWriteBufferTest.kt @@ -144,4 +144,17 @@ class GattPreparedWriteBufferTest { val result = small.execute(device) assertEquals(16, result?.size) } + + @Test + fun `clearAll drops every device's buffered chunks`() { + buffer.append("AA:BB:CC:DD:EE:0A", 0, bytes(1, 2, 3)) + buffer.append("AA:BB:CC:DD:EE:0B", 0, bytes(4, 5, 6)) + assertEquals(2, buffer.activeDeviceCount()) + + buffer.clearAll() + + assertEquals(0, buffer.activeDeviceCount()) + assertNull(buffer.execute("AA:BB:CC:DD:EE:0A")) + assertNull(buffer.execute("AA:BB:CC:DD:EE:0B")) + } }