From 9de168d0d7666e9e9414b7ecdb840bcc74a5fa0c Mon Sep 17 00:00:00 2001 From: Gunjan Jaswal Date: Sat, 25 Jul 2026 22:32:30 +0530 Subject: [PATCH 1/3] 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/3] 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")) + } } From fb2bb0afd5010230b33457f13b5f03aa9a89f334 Mon Sep 17 00:00:00 2001 From: gunjanjaswal Date: Sun, 16 Aug 2026 11:31:47 +0530 Subject: [PATCH 3/3] Surface prepared-write rejections and reject gapped payloads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two robustness fixes from review: - Rejections now reach the peer. A negative offset and an over-cap chunk both answered GATT_SUCCESS, so the client saw every prepare accepted and only lost the payload at execute, silently — the failure mode #90 was about. The server now returns GATT_INVALID_OFFSET for a negative offset and ATT "Prepare Queue Full" (0x09) when the chunk would exceed the buffer cap, so the peer can abort or resend. - A gap is no longer mistaken for zeros. Chunks land in an array grown to the highest offset, so an unwritten hole (write offset 0 then offset 200, nothing between) reassembled as zero-filled and parsed as complete. The buffer now tracks covered byte ranges and execute() refuses a payload that is not a single contiguous run over [0, length); overlaps are fine (last write wins). Tests: an unwritten gap yields null on execute, adjacent chunks that meet exactly reassemble, and an overlapping-but-contiguous write succeeds with the later bytes winning. --- .../mesh/BluetoothGattServerManager.kt | 19 +++++++++-- .../android/mesh/GattPreparedWriteBuffer.kt | 34 +++++++++++++++++++ .../mesh/GattPreparedWriteBufferTest.kt | 28 +++++++++++++++ 3 files changed, 78 insertions(+), 3 deletions(-) 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 e233bd9b..0ac5b43d 100644 --- a/app/src/main/java/com/bitchat/android/mesh/BluetoothGattServerManager.kt +++ b/app/src/main/java/com/bitchat/android/mesh/BluetoothGattServerManager.kt @@ -34,6 +34,10 @@ class BluetoothGattServerManager( // Self-healing advertising recovery tuning private const val ADVERTISE_RETRY_BASE_MS = 3_000L // base backoff for transient advertise failures private const val ADVERTISE_MAX_RETRY_DELAY_MS = 30_000L // cap on backoff delay + // ATT error code 0x09 "Prepare Queue Full" — Android's BluetoothGatt has no + // constant for it (it exposes GATT_INVALID_OFFSET but not this), so it is + // defined here. Returned when a prepared-write chunk can't be buffered. + private const val ATT_ERROR_PREPARE_QUEUE_FULL = 0x09 } // Core Bluetooth components @@ -275,11 +279,20 @@ class BluetoothGattServerManager( 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") + val reason = if (offset < 0) "invalid offset" else "buffer cap exceeded" + Log.w(TAG, "Server: Rejected prepared-write chunk from ${device.address} (offset=$offset, size=${value.size}); $reason") } - // Prepared-write protocol requires echoing the offset and value back + // On success the prepared-write protocol echoes the offset and value + // back; on rejection send the matching ATT error so the peer can + // abort or resend, instead of only discovering the loss at execute + // (where the chunk is silently dropped). if (responseNeeded) { - gattServer?.sendResponse(device, requestId, BluetoothGatt.GATT_SUCCESS, offset, value) + val status = when { + accepted -> BluetoothGatt.GATT_SUCCESS + offset < 0 -> BluetoothGatt.GATT_INVALID_OFFSET + else -> ATT_ERROR_PREPARE_QUEUE_FULL + } + gattServer?.sendResponse(device, requestId, status, offset, if (accepted) value else null) } return } 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 ba6b9ba9..fc120dec 100644 --- a/app/src/main/java/com/bitchat/android/mesh/GattPreparedWriteBuffer.kt +++ b/app/src/main/java/com/bitchat/android/mesh/GattPreparedWriteBuffer.kt @@ -37,6 +37,11 @@ class GattPreparedWriteBuffer( var data: ByteArray = ByteArray(0) var length: Int = 0 var overflowed: Boolean = false + // Byte ranges [start, end) actually written, kept sorted and merged. `data` + // is grown to the highest offset seen, so without this an unwritten gap + // would reassemble as zero-filled and parse as a complete packet. Tracking + // coverage lets execute() tell "not yet written" from "wrote zeros". + val covered: MutableList = mutableListOf() } private val buffers = ConcurrentHashMap() @@ -63,6 +68,7 @@ class GattPreparedWriteBuffer( buf.overflowed = true buf.data = ByteArray(0) buf.length = 0 + buf.covered.clear() return false } val endInt = end.toInt() @@ -72,10 +78,32 @@ class GattPreparedWriteBuffer( } System.arraycopy(value, 0, buf.data, offset, value.size) if (endInt > buf.length) buf.length = endInt + if (value.isNotEmpty()) addCovered(buf.covered, offset, endInt) return true } } + /** + * Merge the range [start, end) into [ranges], keeping it sorted and coalesced + * so [execute] can check whether the payload was fully covered. + */ + private fun addCovered(ranges: MutableList, start: Int, end: Int) { + if (end <= start) return + ranges.add(intArrayOf(start, end)) + ranges.sortBy { it[0] } + val merged = ArrayList(ranges.size) + for (r in ranges) { + val last = merged.lastOrNull() + if (last != null && r[0] <= last[1]) { + last[1] = maxOf(last[1], r[1]) + } else { + merged.add(r) + } + } + ranges.clear() + ranges.addAll(merged) + } + /** * Finalize the prepared write for [deviceKey] and return the reassembled * payload, or null if nothing was buffered or the buffer overflowed. The @@ -85,6 +113,12 @@ class GattPreparedWriteBuffer( val buf = buffers.remove(deviceKey) ?: return null synchronized(buf) { if (buf.overflowed || buf.length == 0) return null + // Reject a payload with a hole: a missing chunk would otherwise be + // handed up as zero-filled and parse as if it were complete. Require a + // single contiguous run covering [0, length). + if (buf.covered.size != 1 || buf.covered[0][0] != 0 || buf.covered[0][1] != buf.length) { + return null + } return buf.data.copyOf(buf.length) } } 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 bc62d742..1107e2ea 100644 --- a/app/src/test/java/com/bitchat/android/mesh/GattPreparedWriteBufferTest.kt +++ b/app/src/test/java/com/bitchat/android/mesh/GattPreparedWriteBufferTest.kt @@ -157,4 +157,32 @@ class GattPreparedWriteBufferTest { assertNull(buffer.execute("AA:BB:CC:DD:EE:0A")) assertNull(buffer.execute("AA:BB:CC:DD:EE:0B")) } + + @Test + fun `execute rejects a payload with an unwritten gap`() { + val device = "AA:BB:CC:DD:EE:0C" + buffer.append(device, 0, bytes(1, 2, 3)) + // A later chunk lands past a hole — bytes 3..99 were never written. `data` + // is grown to offset 100, so without coverage tracking this would come back + // zero-filled and parse as if it were complete. + assertTrue(buffer.append(device, 100, bytes(4, 5, 6))) + assertNull(buffer.execute(device)) + } + + @Test + fun `execute accepts adjacent chunks that meet exactly`() { + val device = "AA:BB:CC:DD:EE:0D" + assertTrue(buffer.append(device, 0, bytes(1, 2))) + assertTrue(buffer.append(device, 2, bytes(3, 4))) + assertArrayEquals(bytes(1, 2, 3, 4), buffer.execute(device)) + } + + @Test + fun `overlapping chunks leave no gap and the later write wins`() { + val device = "AA:BB:CC:DD:EE:0E" + assertTrue(buffer.append(device, 0, bytes(1, 2, 3, 4))) + // Overlaps bytes 2..3; coverage stays contiguous so execute still succeeds. + assertTrue(buffer.append(device, 2, bytes(9, 9))) + assertArrayEquals(bytes(1, 2, 9, 9), buffer.execute(device)) + } }