Skip to content

fix: iOS GATT promise mix-ups and crashes, Android short UUIDs, plugin scan permission - #49

Merged
sheehanmunim merged 5 commits into
mainfrom
audit/2026-10-04
Oct 5, 2026
Merged

fix: iOS GATT promise mix-ups and crashes, Android short UUIDs, plugin scan permission#49
sheehanmunim merged 5 commits into
mainfrom
audit/2026-10-04

Conversation

@sheehanmunim

@sheehanmunim sheehanmunim commented Oct 5, 2026

Copy link
Copy Markdown
Member

Audit fixes (2026-10-04). Each fix is a separate commit.

Bugs fixed

  1. iOS: GATT promises were settled with another operation's result, or never settled. Each delegate handler called completeGattOperation() before taking its promise out of the pending dictionary. Completing an operation starts the next queued one, and its start closure stores a new promise under the same key. The handler then settled that new promise with the old result, and the original promise was dropped.
    • Example: Promise.all(chunks.map(c => writeCharacteristic(id, s, ch, c))) never settles.
    • Reads, descriptor reads and writes, subscribe/unsubscribe, readRSSI and discoverServices had the same problem.
    • Handlers now take their promise first.
  2. iOS: crashes on ordinary input.
    • CBUUID(string:) raises an Objective-C exception, which Swift cannot catch, for anything that isn't a Bluetooth UUID ("180", "xyz", a trailing space). startScan, startAdvertising, updateAdvertisingData and setServices now validate UUIDs from JS and throw instead.
    • setServices with two services sharing a UUID trapped in Dictionary(uniqueKeysWithValues:).
    • writeDescriptor on the CCCD (0x2902) makes CoreBluetooth raise an exception. It now rejects and points to subscribeToCharacteristic().
  3. iOS: subscribing to deviceLost caused an RCTLogError (a red box in dev builds), because the event is in the TS event map but missing from supportedEvents.
  4. Android: the README's short UUIDs didn't work, and advertising could crash the app. The README uses '180D'/'2A37' everywhere, but Android passed them to UUID.fromString/ParcelUuid.fromString.
    • Lookups rejected with "Characteristic not found".
    • Scans, setServices and the background session threw.
    • startAdvertising/updateAdvertisingData build their payload in a Main-dispatcher coroutine with no exception handler, so the whole app crashed.
    • A new parseBleUuid() expands 16- and 32-bit forms onto the Bluetooth base UUID. Lookups compare parsed UUIDs. The advertising coroutine emits advertisingStartFailed instead of crashing.
  5. Expo plugin: Android 12+ scans returned nothing. The default scan capability declared BLUETOOTH_SCAN without neverForLocation while capping fine location at API 30, so the Bluetooth stack filtered out every scan result. It now matches the README's manual setup and the example manifest.

How each fix was verified

  • Plugin: ran the manifest mod with @expo/config-plugins before and after. BLUETOOTH_SCAN now carries android:usesPermissionFlags="neverForLocation".
  • iOS UUID check: a macOS test against real CoreBluetooth.
    • Invalid strings crash CBUUID(string:) (uncaught NSException).
    • The new check rejects exactly those strings and accepts every form CBUUID accepts: 16/32-bit, 0x-prefixed and 128-bit.
  • parseBleUuid: compiled with kotlinc 2.2.20 and tested (short, 0x-prefixed, 32-bit, full and invalid inputs, plus the equality helper). The other Kotlin edits are type-identical substitutions.
  • Swift: swiftc -parse passes. The GATT ordering fix was verified by reading the code.
  • This PR's CI: runs the Android and iOS example builds.
  • npm ci, tsc and npm run build pass, and npm pack --dry-run contents are correct.
  • Not verified: nothing was run on a physical device.

Found, not fixed

Each of these needs a design decision or broader change:

  • iOS: UUIDs are compared as text against CBUUID.uuidString, so full-form base UUIDs (0000180f-…) don't match the short form CoreBluetooth reports.
  • iOS: stopAdvertising() also removes all GATT services, unlike Android.
  • iOS: central state isn't cleared when the adapter powers off, which leaves pending promises and connected-device state stale.
  • iOS: hexStringToData rejects "" and left-pads odd-length input, while Android rejects odd-length input.
  • iOS: the deviceFound payload differs from Android (serviceData and the manufacturer-data prefix).
  • Both platforms: calling connect() twice for the same device orphans the first promise. On Android it also leaks a BluetoothGatt.
  • Android: the background service adds several GATT services back to back (only the first is published) and doesn't handle prepared/long writes (onExecuteWrite).
  • Android: isScanning stays true after Bluetooth is toggled off, so the next startScan() silently does nothing.
  • Android: advertising renames the system Bluetooth adapter and only restores the name in stopAdvertising.
  • Android: broadcast receivers registered in init are never unregistered, so they leak across JS reloads.
  • Android: appearance/solicitation advertising fields are encoded as service data/UUIDs.
  • Plugin: requires @expo/config-plugins without declaring it, which can fail under strict (pnpm/isolated) installs. munim-ffmpeg uses expo/config-plugins.

🤖 Generated with Claude Code

sheehanmunim and others added 5 commits October 4, 2026 22:37
@sheehanmunim @claude
fix(ios): settle each GATT promise with its own result
d76b4b7 
The GATT delegate handlers called completeGattOperation() before taking
the pending promise out of its dictionary. Completing starts the next
queued operation, whose start closure stores its promise under the same
key (same characteristic, descriptor or device). The handler then removed
and settled that new promise with the previous operation's result, and
the earlier promise was never settled.

So two writes to one characteristic without awaiting (for example
Promise.all over chunked writes) left the first promise pending forever
and resolved the second before its own ATT response arrived. Reads,
descriptor reads/writes, subscribe/unsubscribe, readRSSI and
discoverServices behaved the same way.

Each handler now takes its promise before completing the operation.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@sheehanmunim @claude
fix(ios): reject invalid UUIDs, duplicate services and CCCD writes in…
21647fa 
…stead of crashing

- CBUUID(string:) raises an Objective-C exception for strings that are not
  a Bluetooth UUID ("180", "xyz", a trailing space), which Swift cannot
  catch, so startScan/startAdvertising/updateAdvertisingData/setServices
  crashed the app on such input. UUIDs from JS are now validated and the
  call throws instead. Every form CBUUID accepts (16/32-bit, optional 0x,
  full 128-bit) is still accepted.
- setServices built its include map with Dictionary(uniqueKeysWithValues:),
  which traps when two services share a UUID (valid GATT, e.g. two Battery
  services, or "180F" and "180f").
- writeDescriptor on the Client Characteristic Configuration descriptor
  (0x2902), the normal way to enable notifications on Android, makes
  CoreBluetooth raise an exception. It now rejects and points to
  subscribeToCharacteristic().

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@sheehanmunim @claude
fix(ios): list deviceLost in supportedEvents
abddce1 
deviceLost is part of the typed event map, but subscribing to it on iOS
made RCTEventEmitter log "is not a supported event type" (a red box in
development builds). iOS never emits it; it is listed so subscribing is
harmless, as on Android.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@sheehanmunim @claude
fix(android): accept 16/32-bit UUIDs and stop advertising from crashi…
f66132c 
…ng the app

The README uses short UUIDs throughout (startScan({ serviceUUIDs: ['180D'] }),
readCharacteristic(id, '180D', '2A37'), setServices([{ uuid: '180D', ... }])),
and iOS accepts them, but Android passed them to UUID.fromString /
ParcelUuid.fromString, which only accept full 128-bit UUIDs:

- reads, writes, subscribes and descriptor calls rejected with
  "Characteristic not found" because lookups compared strings;
- startScan, setServices and the background session threw;
- startAdvertising / updateAdvertisingData build their payload inside a
  Main-dispatcher coroutine with no exception handler, so a short (or any
  invalid) UUID there crashed the whole app.

UUIDs from JS now go through parseBleUuid(), which expands 16/32-bit forms
onto the Bluetooth base UUID, lookups compare parsed UUIDs, and an invalid
UUID in the advertising coroutine emits advertisingStartFailed instead of
crashing.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@sheehanmunim @claude
fix(plugin): declare BLUETOOTH_SCAN with neverForLocation
75fcd10 
The Expo plugin's default "scan" capability added BLUETOOTH_SCAN without
usesPermissionFlags while capping ACCESS_FINE_LOCATION at API 30. On
Android 12+ the Bluetooth stack only delivers scan results to apps that
hold fine location or disavow it, so Expo apps using the plugin defaults
started scans that never reported a device. The flag matches the README's
manual setup and the example app.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 5, 2026 02:56
Copilot AI reviewed Oct 5, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@sheehanmunim
sheehanmunim merged commit 943959d into main Oct 5, 2026
3 checks passed
@sheehanmunim
sheehanmunim deleted the audit/2026-10-04 branch October 5, 2026 12:20
@saty9 saty9 mentioned this pull request Oct 8, 2026
Merged
sheehanmunim pushed a commit that referenced this pull request Oct 8, 2026
@semantic-release-bot
chore(release): 0.9.1 [skip ci]
3221e09 
## [0.9.1](v0.9.0...v0.9.1) (2026-10-08)

### 🐛 Bug Fixes

* iOS GATT promise mix-ups and crashes, Android short UUIDs, plugin scan permission ([#49](#49)) ([943959d](943959d))
* populate manufacturer data entries in scan results ([#50](#50)) ([480d6d8](480d6d8))

### 📚 Documentation

* move Installation up and list Expo first ([b31f5fd](b31f5fd))
@sheehanmunim

sheehanmunim commented Oct 8, 2026

Copy link
Copy Markdown
Member Author

🎉 This PR is included in version 0.9.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

Assignees

No one assigned

Labels

released

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@sheehanmunim