mirror of
https://github.com/permissionlesstech/bitchat-android.git
synced 2026-09-19 04:59:59 +00:00
Surface prepared-write rejections and reject gapped payloads
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.
This commit is contained in:
parent
7ff62563e1
commit
fb2bb0afd5
@ -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
|
||||
}
|
||||
|
||||
@ -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<IntArray> = mutableListOf()
|
||||
}
|
||||
|
||||
private val buffers = ConcurrentHashMap<String, DeviceBuffer>()
|
||||
@ -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<IntArray>, start: Int, end: Int) {
|
||||
if (end <= start) return
|
||||
ranges.add(intArrayOf(start, end))
|
||||
ranges.sortBy { it[0] }
|
||||
val merged = ArrayList<IntArray>(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)
|
||||
}
|
||||
}
|
||||
|
||||
@ -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))
|
||||
}
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user