diff --git a/CHANGELOG.md b/CHANGELOG.md index c1d98c5e..b5b55938 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,89 @@ This file holds unreleased changes and the current release. Older releases are archived by series under [docs/changelog/](docs/changelog/); see the [archive index](docs/changelog/README.md). +## [Unreleased] + +### Fixed + +- **Eight React Native methods were unreachable on iOS, and the bridge now + proves it cannot happen again.** `RCT_EXTERN_METHOD` does not declare a Swift + method, it records a selector that React Native resolves against the class at + module load; one it cannot find is dropped with a log line and the JS method + is simply absent. Neither compiler sees both halves, and + `OfflineProtocolModule.swift` is the one bridge source no CI job compiles, so + three separate drifts shipped. `wipePersistedState` kept the pre-rename + `userId:` label and had been uncallable since 0.21.0, which meant logging out + could not erase the account it had just signed out of and every prior + account's MLS identity and sealed state stayed on disk. `setBatteryState`, + `getIsCharging`, `updateRelayConfig` and `getRelayConfig` were written in + Swift, Kotlin and TypeScript and never declared in the bridge at all, so + since 0.22.0 every relay setting an application passed to `create()` was + discarded on iOS behind a `console.warn`: **applications that configure + `allowRelay`, `minBatteryForRelay` or `relayPriority` will see those settings + take effect on iOS for the first time on this release.** `dataListSpaces`, + `dataFlushAll` and `dataWipeAll` took a labelled first parameter, which Swift + exports as `dataListSpacesWithResolver:` rather than `dataListSpaces:`, and + stopped resolving in 0.23.0. Android was never affected: its dispatch is by + method name and position, and the Kotlin side was correct throughout. + `react_native_ios_objc_shim_and_swift_agree_on_every_selector` now reads both + bridge halves and the TypeScript, and fails on any selector one side has and + another does not. + +- **Seven more iOS methods resolved but ran on the wrong argument bits.** The + bridge declares each parameter's type as text, and React Native picks the + `RCTConvert` converter from that text and the calling convention from the + Swift parameter's runtime encoding, then calls the one through a function + pointer cast to the other. `nonnull NSNumber *` against a Swift `Int` + therefore hands the method an object pointer read as a 64-bit integer, which + is the pointer bits of a tagged `NSNumber` and never the number. The type + table in `BRIDGE_MAINTENANCE.md` had recommended exactly that pairing since + v0.3.3, the release that also introduced the first of these methods, so + `sendMessage`, `sendMessageRich` and `sendPresenceUpdate` silently pinned + every priority and status to their + `default:` arm, `setBatteryLevel` and `setBatteryState` recorded a clamp bound + rather than the level, and `processFileChunk` and `blePeerDiscovered` reached + a narrowing conversion that traps, aborting the application. Nothing was + logged in any of the seven cases. The type table is corrected, the two + conversions that now receive real values reject or clamp out-of-range input + instead of trapping, and the selector guard gained a third direction that + compares the ABI class of every parameter behind a shared selector. + +- **Fifteen iOS conversions aborted the app instead of rejecting the call.** + A narrowing conversion like `UInt8(_:)` traps on out-of-range input rather + than returning a value the bridge could reject, and every number reaching + these conversions came straight from JavaScript. Twelve of them turned a + `[NSNumber]` argument into bytes, so any array element outside 0...255 + crashed the application: reachable from a malformed BLE fragment, a Wi-Fi + Direct or internet frame, an MLS ciphertext or Welcome, a key package, or a + file chunk. The thirteenth was the `initialTtl` config field, which made + `create()` abort on iOS for an application passing a value above 255, where + Android truncated the same value and started normally. The last two narrowed + the DORS `historyWindowSize` to `Int` before clamping it, which is too late + to help: a negative number from JavaScript arrives at `uint64Value` as + `UInt64.max`, so the conversion traps before the surrounding clamp can run, + and both `create()` and `updateDorsConfig` aborted on a negative value. Byte + arrays now convert through a helper that throws into the rejection each call + site already had, `initialTtl` and `historyWindowSize` are clamped in the + domain they arrive in, and + `react_native_ios_bridge_bounds_every_byte_it_builds_from_javascript` fails + on any byte conversion in the bridge that does not carry its own bound. + + Unlike the ABI mismatches above, these were never masked by anything. Array + arguments cross as `NSArray *` against `[NSNumber]`, which has agreed since + the UniFFI migration, so every one of these has been reachable in every + release that shipped the method, and the transport ones are reachable by a + remote peer rather than only by the application's own code. + +- **`forwardMessage` hangs on iOS in debug builds** rather than forwarding. + React Native forces every `NSNumber` argument to non-null, because numbers + are not nullable on Android, and refuses a null one before the Swift method + is entered, so neither the resolver nor the rejecter ever runs. The + TypeScript passes `null` whenever a caller omits the priority. No declaration + in the bridge can fix this; it needs a contract change across TypeScript, + Swift and Kotlin, and is tracked in + [#417](https://github.com/Offline-Protocol/offline-protocol-sdk/issues/417). + Release builds are unaffected, as the check is compiled out. + ## [0.24.0] — 2026-08-24 > **A door lock speaks this protocol now, and not a smaller version of it.** diff --git a/bindings/react-native/ios/BRIDGE_MAINTENANCE.md b/bindings/react-native/ios/BRIDGE_MAINTENANCE.md index da7d7733..9f3bf92f 100644 --- a/bindings/react-native/ios/BRIDGE_MAINTENANCE.md +++ b/bindings/react-native/ios/BRIDGE_MAINTENANCE.md @@ -42,7 +42,7 @@ Add or update the corresponding `RCT_EXTERN_METHOD` in `OfflineProtocolModule.m` ```objective-c RCT_EXTERN_METHOD(sendMessage:(NSString *)recipient content:(NSString *)content - priority:(nonnull NSNumber *)priority + priority:(NSInteger)priority replyToMsg:(NSString *)replyToMsg resolver:(RCTPromiseResolveBlock)resolve rejecter:(RCTPromiseRejectBlock)reject) @@ -56,13 +56,88 @@ Map Swift types to Objective-C types: |------------|------------------| | `String` | `NSString *` | | `String?` | `NSString *` (nullable) | -| `Int` | `nonnull NSNumber *` | +| `Int` | `NSInteger` | +| `Double` | `double` | | `Bool` | `BOOL` | +| `NSNumber` | `nonnull NSNumber *` | | `[NSNumber]` | `NSArray *` | | `NSDictionary?` | `NSDictionary *` (nullable) | +**A primitive and an object are not interchangeable here, and mixing them is +silent.** React Native picks the `RCTConvert` converter from the type text you +write above and the calling convention from the Swift parameter's runtime +encoding, then calls the first through a function pointer cast to the second. +Write `nonnull NSNumber *` against a Swift `Int` and `+[RCTConvert NSNumber:]` +returns an object pointer that is then read as a 64-bit integer, so the method +runs with the pointer bits of a tagged `NSNumber` where the number should be. +Write it against a Swift `Double` and an integer register is read as a floating +point one. Nothing is logged either way. This row read `Int` to +`nonnull NSNumber *` from v0.3.3 until this release, and seven methods +followed it. + +Take an `NSNumber` on the Swift side only where the argument is genuinely +optional, and know that React Native does not really support that: it forces +every `NSNumber` argument to non-null whatever you declare, because numbers are +not nullable on Android. A null one is then refused before the Swift method is +entered, so neither the resolver nor the rejecter runs and the promise never +settles. `forwardMessage` is the one method in this bridge that relies on a +nullable number, and it hangs on iOS debug builds for that reason; there is no +spelling of the declaration that fixes it, so it needs a contract change across +all three languages. That is tracked in +[#417](https://github.com/Offline-Protocol/offline-protocol-sdk/issues/417). +Until it lands, do not add a second nullable-number argument. + **Note**: All `@objc` methods must include `resolver` and `rejecter` parameters (React Native Promise pattern). +### Step 4: Leave the first parameter unlabelled + +Write `_ recipient: String`, not `recipient: String`. Swift exports a labelled +first parameter with a `With` infix, so `dataListSpaces(resolver:rejecter:)` +becomes the selector `dataListSpacesWithResolver:rejecter:` and no longer +matches `RCT_EXTERN_METHOD(dataListSpaces:...)`. Three data-layer methods +drifted into that shape in 0.23.0 and stopped resolving. + +Repair it by dropping the label in Swift. Do not write the `With` form in the +bridge instead: React Native derives the JS method name from the selector text +before its first colon, so that spelling renames the JS method rather than +fixing it. + +### Step 5: Bound every number you narrow + +`UInt8(someInt)` traps. It does not return nil, throw, or truncate: it aborts +the process, and every number reaching this file came from JavaScript, so an +out-of-range value is a caller mistake that must reject the promise instead. + +Convert byte arrays through the `jsBytes` helper, which throws an `NSError` +into the rejection your `do`/`catch` already has: + +```swift +let bytes = try jsBytes(data, "data") // not data.map { UInt8($0.intValue) } +let optional = try maybe.map { try jsBytes($0, "keyPackage") } +``` + +For a scalar, bound it where you write it (`min`/`max`, `UInt8(exactly:)`, +`UInt8(clamping:)`) or `guard` the range before the conversion, as +`processFileChunk` does for its `UInt32` and `UInt64` arguments. Twelve array +conversions and the `initialTtl` config field were unbounded until this +release: a peer sending a malformed fragment, or an application passing `initialTtl: 300` +to `create()`, aborted the app on iOS where Android truncated. + +**The clamp has to sit inside the conversion, not around it.** Wrapping a +narrowing conversion in `min`/`max` reads as bounded and is not: the conversion +runs first, so it traps before any of the clamp applies. This reaches unsigned +values too, because a negative JavaScript number arrives at `uint64Value` as +`UInt64.max`, and narrowing that to `Int` aborts. Clamp in the domain the value +arrives in, or convert with `Int(clamping:)`. Two DORS config paths carried the +wrong order until this release, so a `historyWindowSize` of `-1` passed to +`create()` aborted the app on iOS. + +`react_native_ios_bridge_bounds_every_byte_it_builds_from_javascript` in +`offline-protocol-uniffi` fails on any `UInt8(...)` in this file whose argument +does not carry its own bound. It reads bytes only: a scalar narrowing like the +one above is held by this checklist and by review, because the text of +`Int(raw)` cannot say whether `raw` is already bounded. + ## Common Issues ### Missing Parameter @@ -81,6 +156,26 @@ Map Swift types to Objective-C types: **Fix**: Check the type mapping table above +### Renamed Parameter Label + +**Error (boot log)**: ``The Objective-C `...` method signature for the JS method +`...` can not be found in the Objective-C definition of the +OfflineProtocolModule module.`` + +**Cause**: The selector here and the selector Swift exports differ. Renaming a +parameter in Swift renames the selector, so a bridge left on the old label +declares a method that no longer exists. + +**Fix**: Rename the label here too. This is not caught by any compiler; it is +caught by `react_native_ios_objc_shim_and_swift_agree_on_every_selector` in +`offline-protocol-uniffi`, which compares the selector sets of both files and +also fails when the TypeScript calls a method this bridge never exports. Run +it with: + +```bash +cargo test -p offline-protocol-uniffi --lib react_native_ios_objc_shim +``` + ## Threading contract for the transport managers `BleManager` (and the same reasoning applies to the other transport managers) @@ -153,6 +248,9 @@ Before committing changes: - [ ] All `@objc func` methods in Swift have corresponding `RCT_EXTERN_METHOD` declarations - [ ] Parameter names and types match between Swift and Objective-C - [ ] All methods include `resolver` and `rejecter` parameters +- [ ] The first Swift parameter is unlabelled (`_`) +- [ ] Every narrowing conversion is bounded, and byte arrays go through `jsBytes` +- [ ] `cargo test -p offline-protocol-uniffi --lib react_native_ios` passes - [ ] Build succeeds without warnings - [ ] Test the method from JavaScript to ensure it works diff --git a/bindings/react-native/ios/OfflineProtocolModule.m b/bindings/react-native/ios/OfflineProtocolModule.m index 100d60cd..83cd6076 100644 --- a/bindings/react-native/ios/OfflineProtocolModule.m +++ b/bindings/react-native/ios/OfflineProtocolModule.m @@ -19,7 +19,7 @@ @interface RCT_EXTERN_MODULE(OfflineProtocolModule, RCTEventEmitter) rejecter:(RCTPromiseRejectBlock)reject) RCT_EXTERN_METHOD(wipePersistedState:(NSString *)appId - userId:(NSString *)userId + profile:(NSString *)profile resolver:(RCTPromiseResolveBlock)resolve rejecter:(RCTPromiseRejectBlock)reject) @@ -47,14 +47,14 @@ @interface RCT_EXTERN_MODULE(OfflineProtocolModule, RCTEventEmitter) RCT_EXTERN_METHOD(sendMessage:(NSString *)recipient content:(NSString *)content - priority:(nonnull NSNumber *)priority + priority:(NSInteger)priority replyToMsg:(NSString *)replyToMsg resolver:(RCTPromiseResolveBlock)resolve rejecter:(RCTPromiseRejectBlock)reject) RCT_EXTERN_METHOD(sendMessageRich:(NSString *)recipient content:(NSString *)content - priority:(nonnull NSNumber *)priority + priority:(NSInteger)priority replyToMsg:(NSString *)replyToMsg options:(NSDictionary *)options resolver:(RCTPromiseResolveBlock)resolve @@ -164,7 +164,7 @@ @interface RCT_EXTERN_MODULE(OfflineProtocolModule, RCTEventEmitter) // BLE transport methods RCT_EXTERN_METHOD(blePeerDiscovered:(NSString *)peerId - rssi:(nonnull NSNumber *)rssi + rssi:(NSInteger)rssi resolver:(RCTPromiseResolveBlock)resolve rejecter:(RCTPromiseRejectBlock)reject) @@ -194,13 +194,21 @@ @interface RCT_EXTERN_MODULE(OfflineProtocolModule, RCTEventEmitter) rejecter:(RCTPromiseRejectBlock)reject) // Battery management -RCT_EXTERN_METHOD(setBatteryLevel:(nonnull NSNumber *)level +RCT_EXTERN_METHOD(setBatteryLevel:(NSInteger)level + resolver:(RCTPromiseResolveBlock)resolve + rejecter:(RCTPromiseRejectBlock)reject) + +RCT_EXTERN_METHOD(setBatteryState:(NSInteger)level + isCharging:(BOOL)isCharging resolver:(RCTPromiseResolveBlock)resolve rejecter:(RCTPromiseRejectBlock)reject) RCT_EXTERN_METHOD(getBatteryLevel:(RCTPromiseResolveBlock)resolve rejecter:(RCTPromiseRejectBlock)reject) +RCT_EXTERN_METHOD(getIsCharging:(RCTPromiseResolveBlock)resolve + rejecter:(RCTPromiseRejectBlock)reject) + // Relay management RCT_EXTERN_METHOD(setRelayPriority:(NSString *)priorityString resolver:(RCTPromiseResolveBlock)resolve @@ -212,6 +220,13 @@ @interface RCT_EXTERN_MODULE(OfflineProtocolModule, RCTEventEmitter) RCT_EXTERN_METHOD(isRelay:(RCTPromiseResolveBlock)resolve rejecter:(RCTPromiseRejectBlock)reject) +RCT_EXTERN_METHOD(updateRelayConfig:(NSString *)configJson + resolver:(RCTPromiseResolveBlock)resolve + rejecter:(RCTPromiseRejectBlock)reject) + +RCT_EXTERN_METHOD(getRelayConfig:(RCTPromiseResolveBlock)resolve + rejecter:(RCTPromiseRejectBlock)reject) + // Transport metrics RCT_EXTERN_METHOD(getTransportMetrics:(NSString *)transportType resolver:(RCTPromiseResolveBlock)resolve @@ -267,9 +282,9 @@ @interface RCT_EXTERN_MODULE(OfflineProtocolModule, RCTEventEmitter) // File Transfer Operations RCT_EXTERN_METHOD(processFileChunk:(NSString *)fileId - chunkIndex:(nonnull NSNumber *)chunkIndex - totalChunks:(nonnull NSNumber *)totalChunks - fileSize:(nonnull NSNumber *)fileSize + chunkIndex:(NSInteger)chunkIndex + totalChunks:(NSInteger)totalChunks + fileSize:(double)fileSize fileName:(NSString *)fileName fileChecksum:(NSString *)fileChecksum data:(NSArray *)data @@ -681,7 +696,7 @@ @interface RCT_EXTERN_MODULE(OfflineProtocolModule, RCTEventEmitter) // Presence, Typing, Read Receipts RCT_EXTERN_METHOD(sendPresenceUpdate:(NSString *)recipient - status:(nonnull NSNumber *)status + status:(NSInteger)status resolver:(RCTPromiseResolveBlock)resolve rejecter:(RCTPromiseRejectBlock)reject) diff --git a/bindings/react-native/ios/OfflineProtocolModule.swift b/bindings/react-native/ios/OfflineProtocolModule.swift index d178ee47..9e20b1f5 100644 --- a/bindings/react-native/ios/OfflineProtocolModule.swift +++ b/bindings/react-native/ios/OfflineProtocolModule.swift @@ -451,7 +451,11 @@ class OfflineProtocolModule: RCTEventEmitter { reticulumEnabled: raw["reticulumEnabled"] as? Bool ?? raw["reticulum_enabled"] as? Bool ?? false, nostrEnabled: raw["nostrEnabled"] as? Bool ?? raw["nostr_enabled"] as? Bool ?? false, preferOnline: raw["preferOnline"] as? Bool ?? raw["prefer_online"] as? Bool ?? false, - initialTtl: UInt8(raw["initialTtl"] as? Int ?? raw["initial_ttl"] as? Int ?? 8), + // Clamped, not converted: `initialTtl` is a public config field + // of unbounded `number` type in TypeScript, so an application + // passing 300 would trap here and abort inside `create()`. + // Android truncates the same value through `toUByte()`. + initialTtl: UInt8(min(255, max(0, raw["initialTtl"] as? Int ?? raw["initial_ttl"] as? Int ?? 8))), encryptionEnabled: encryption.enabled, autoKeyExchange: encryption.autoKeyExchange, storePending: encryption.storePending, @@ -536,6 +540,31 @@ class OfflineProtocolModule: RCTEventEmitter { } } + /// The bytes behind a JavaScript number array. + /// + /// `UInt8(_:)` traps on anything outside 0...255, and every array these + /// methods receive came straight from JavaScript, so an out-of-range + /// element is a caller mistake rather than an impossible state. A caller + /// mistake has to reject the promise; the trapping form aborts the + /// application instead, which makes a crash reachable from any JavaScript + /// call site. Throwing reaches the rejection each of these methods + /// already has. + /// + /// Fractional elements still truncate, as they always have: JavaScript + /// byte arrays are integral, and a caller passing 3.7 is not the failure + /// this guards. + private func jsBytes(_ numbers: [NSNumber], _ field: String) throws -> [UInt8] { + try numbers.map { number in + guard let byte = UInt8(exactly: number.intValue) else { + throw NSError(domain: "OfflineProtocol", code: -1, userInfo: [ + NSLocalizedDescriptionKey: + "\(field) must hold byte values in 0...255, found \(number.intValue)" + ]) + } + return byte + } + } + private func applyInitialRuntimeConfig(_ proto: OfflineProtocol, rawConfig: [String: Any]) { if let dorsDict = rawConfig["dors"] as? [String: Any] { let preferOnline = dorsDict["preferOnline"] as? Bool ?? dorsDict["prefer_online"] as? Bool ?? false @@ -579,7 +608,11 @@ class OfflineProtocolModule: RCTEventEmitter { let historyWindowRaw = UInt64((dorsDict["historyWindowSize"] as? NSNumber)?.uint64Value ?? (dorsDict["history_window_size"] as? NSNumber)?.uint64Value ?? 10) - let historyWindow = max(1, min(100, Int(historyWindowRaw))) + // Clamped in the unsigned domain, never through `Int`: a negative + // `historyWindowSize` from JavaScript reaches `uint64Value` as + // `UInt64.max`, and narrowing that to `Int` traps and aborts the + // application before the clamp around it can run. + let historyWindow = max(1, min(100, historyWindowRaw)) let rawQueueRecovery = Float((dorsDict["queueRecoveryRatio"] as? NSNumber)?.floatValue ?? (dorsDict["queue_recovery_ratio"] as? NSNumber)?.floatValue ?? 0.5) @@ -608,7 +641,7 @@ class OfflineProtocolModule: RCTEventEmitter { ttlEscalationThreshold: ttlThreshold, congestionDurationSecs: congestionDuration, ttlEscalationHoldSecs: ttlHold, - historyWindowSize: UInt64(historyWindow), + historyWindowSize: historyWindow, queueRecoveryRatio: queueRecovery, lowBatteryThreshold: lowBattery, relayMinBatteryLevel: relayMinBattery, @@ -1486,7 +1519,7 @@ class OfflineProtocolModule: RCTEventEmitter { userInfo: [NSLocalizedDescriptionKey: "Protocol not initialized"]) } - let keyPackageData = keyPackage?.map { UInt8($0.intValue) } + let keyPackageData = try keyPackage.map { try jsBytes($0, "keyPackage") } let messageId = try proto.sendConnectionRequest( recipient: recipient, senderName: senderName, @@ -1510,7 +1543,7 @@ class OfflineProtocolModule: RCTEventEmitter { userInfo: [NSLocalizedDescriptionKey: "Protocol not initialized"]) } - let keyPackageData = keyPackage?.map { UInt8($0.intValue) } + let keyPackageData = try keyPackage.map { try jsBytes($0, "keyPackage") } let messageId = try proto.acceptConnectionRequest( recipient: recipient, accepterName: accepterName, @@ -1623,8 +1656,8 @@ class OfflineProtocolModule: RCTEventEmitter { } } - @objc func dataListSpaces(resolver: @escaping RCTPromiseResolveBlock, - rejecter: @escaping RCTPromiseRejectBlock) { + @objc func dataListSpaces(_ resolver: @escaping RCTPromiseResolveBlock, + rejecter: @escaping RCTPromiseRejectBlock) { do { guard let store = dataStoreInstance else { throw NSError(domain: "OfflineProtocol", code: -1, @@ -1909,8 +1942,8 @@ class OfflineProtocolModule: RCTEventEmitter { } } - @objc func dataFlushAll(resolver: @escaping RCTPromiseResolveBlock, - rejecter: @escaping RCTPromiseRejectBlock) { + @objc func dataFlushAll(_ resolver: @escaping RCTPromiseResolveBlock, + rejecter: @escaping RCTPromiseRejectBlock) { do { guard let store = dataStoreInstance else { throw NSError(domain: "OfflineProtocol", code: -1, @@ -2040,8 +2073,8 @@ class OfflineProtocolModule: RCTEventEmitter { } } - @objc func dataWipeAll(resolver: @escaping RCTPromiseResolveBlock, - rejecter: @escaping RCTPromiseRejectBlock) { + @objc func dataWipeAll(_ resolver: @escaping RCTPromiseResolveBlock, + rejecter: @escaping RCTPromiseRejectBlock) { do { guard let store = dataStoreInstance else { throw NSError(domain: "OfflineProtocol", code: -1, @@ -2868,7 +2901,7 @@ class OfflineProtocolModule: RCTEventEmitter { return } do { - try proto.blePeerDiscovered(peerId: peerId, rssi: Int16(rssi)) + try proto.blePeerDiscovered(peerId: peerId, rssi: Int16(clamping: rssi)) resolver(nil) } catch { rejecter("ERROR_BLE", "BLE peer discovered failed: \(error.localizedDescription)", error) @@ -2914,7 +2947,7 @@ class OfflineProtocolModule: RCTEventEmitter { return } do { - let fragment = fragmentData.map { UInt8($0.intValue) } + let fragment = try jsBytes(fragmentData, "fragment") try proto.bleFragmentReceived(senderId: senderId, fragment: fragment) resolver(nil) } catch { @@ -3287,7 +3320,11 @@ class OfflineProtocolModule: RCTEventEmitter { let congestionDuration = max((config["congestionDurationSecs"] as? NSNumber)?.uint64Value ?? current.congestionDurationSecs, 0) let ttlHold = max((config["ttlEscalationHoldSecs"] as? NSNumber)?.uint64Value ?? current.ttlEscalationHoldSecs, 1) let historyWindowRaw = (config["historyWindowSize"] as? NSNumber)?.uint64Value ?? current.historyWindowSize - let historyWindow = max(1, min(100, Int(historyWindowRaw))) + // Clamped in the unsigned domain, never through `Int`: a negative + // `historyWindowSize` from JavaScript reaches `uint64Value` as + // `UInt64.max`, and narrowing that to `Int` traps and aborts the + // application before the clamp around it can run. + let historyWindow = max(1, min(100, historyWindowRaw)) let rawQueueRecovery = (config["queueRecoveryRatio"] as? NSNumber)?.floatValue ?? current.queueRecoveryRatio let queueRecovery = min(max(rawQueueRecovery, 0.0), 1.0) @@ -3308,7 +3345,7 @@ class OfflineProtocolModule: RCTEventEmitter { ttlEscalationThreshold: ttlThreshold, congestionDurationSecs: UInt64(congestionDuration), ttlEscalationHoldSecs: UInt64(ttlHold), - historyWindowSize: UInt64(historyWindow), + historyWindowSize: historyWindow, queueRecoveryRatio: queueRecovery, lowBatteryThreshold: UInt8(min(100, max(0, (config["lowBatteryThreshold"] as? NSNumber)?.intValue ?? Int(current.lowBatteryThreshold)))), relayMinBatteryLevel: UInt8(min(100, max(0, (config["relayMinBatteryLevel"] as? NSNumber)?.intValue ?? Int(current.relayMinBatteryLevel)))), @@ -3559,8 +3596,22 @@ class OfflineProtocolModule: RCTEventEmitter { rejecter("ERROR_FILE", "Protocol not initialized", nil) return } + // Every one of these narrows, and each narrowing traps rather than + // returning a value React Native could reject. The arguments come + // straight from JavaScript, so out-of-range is a caller mistake and + // must surface as a rejected promise, not as an abort. + guard chunkIndex >= 0, chunkIndex <= Int(UInt32.max), + totalChunks >= 0, totalChunks <= Int(UInt32.max), + // NaN fails both comparisons. The ceiling is 2^64 exactly, and + // every non-negative Double below it truncates into UInt64. + fileSize >= 0, fileSize < 18_446_744_073_709_551_616.0 else { + rejecter("ERROR_FILE", + "Chunk index, chunk count and file size must be non-negative and in range", + nil) + return + } do { - let bytes = data.map { UInt8($0.intValue) } + let bytes = try jsBytes(data, "data") try proto.processFileChunk( fileId: fileId, chunkIndex: UInt32(chunkIndex), @@ -3617,7 +3668,7 @@ class OfflineProtocolModule: RCTEventEmitter { return } do { - let bytes = data.map { UInt8($0.intValue) } + let bytes = try jsBytes(data, "data") try proto.wifiDirectMessageReceived(senderId: senderId, data: bytes) resolver(nil) } catch { @@ -3698,7 +3749,7 @@ class OfflineProtocolModule: RCTEventEmitter { return } do { - let bytes = data.map { UInt8($0.intValue) } + let bytes = try jsBytes(data, "data") try proto.internetMessageReceived(senderId: senderId, data: bytes) resolver(nil) } catch { @@ -4142,7 +4193,7 @@ class OfflineProtocolModule: RCTEventEmitter { } let welcomeDataNumbers = json["welcomeData"] as? [NSNumber] ?? [] - let welcomeData = welcomeDataNumbers.map { UInt8($0.intValue) } + let welcomeData = try jsBytes(welcomeDataNumbers, "welcomeData") let welcome = MlsWelcomeMessage( groupId: json["groupId"] as? String ?? "", @@ -4183,7 +4234,7 @@ class OfflineProtocolModule: RCTEventEmitter { } let ciphertextNumbers = json["ciphertext"] as? [NSNumber] ?? [] - let ciphertext = ciphertextNumbers.map { UInt8($0.intValue) } + let ciphertext = try jsBytes(ciphertextNumbers, "ciphertext") let encrypted = MlsEncryptedMessage( groupId: json["groupId"] as? String ?? "", @@ -4230,7 +4281,7 @@ class OfflineProtocolModule: RCTEventEmitter { return } do { - let data = keyPackageData.map { UInt8($0.intValue) } + let data = try jsBytes(keyPackageData, "keyPackageData") try proto.mlsImportKeyPackage(userId: userId, keyPackageData: data) resolver(nil) } catch { @@ -4349,7 +4400,7 @@ class OfflineProtocolModule: RCTEventEmitter { return } do { - let data = plaintext.map { UInt8($0.intValue) } + let data = try jsBytes(plaintext, "plaintext") let encrypted = try proto.mlsEncryptForUser(otherUserId: otherUserId, plaintext: data) let result: [String: Any] = [ "groupId": encrypted.groupId, @@ -4380,7 +4431,7 @@ class OfflineProtocolModule: RCTEventEmitter { } let ciphertextNumbers = json["ciphertext"] as? [NSNumber] ?? [] - let ciphertext = ciphertextNumbers.map { UInt8($0.intValue) } + let ciphertext = try jsBytes(ciphertextNumbers, "ciphertext") let encrypted = MlsEncryptedMessage( groupId: json["groupId"] as? String ?? "", @@ -4426,7 +4477,7 @@ class OfflineProtocolModule: RCTEventEmitter { } let welcomeDataNumbers = json["welcomeData"] as? [NSNumber] ?? [] - let welcomeData = welcomeDataNumbers.map { UInt8($0.intValue) } + let welcomeData = try jsBytes(welcomeDataNumbers, "welcomeData") let welcome = MlsWelcomeMessage( groupId: json["groupId"] as? String ?? "", diff --git a/crates/offline-protocol-uniffi/src/lib.rs b/crates/offline-protocol-uniffi/src/lib.rs index 4f6f4e88..d06b38a6 100644 --- a/crates/offline-protocol-uniffi/src/lib.rs +++ b/crates/offline-protocol-uniffi/src/lib.rs @@ -12130,6 +12130,668 @@ mod tests { ); } + /// Every method React Native can reach on iOS is spelled the same way in + /// both halves of the hand-written bridge, and hands over each argument + /// the same way. + /// + /// `RCT_EXTERN_METHOD` does not declare the Swift method. It stringifies + /// its argument and stores the text; React Native parses that text at + /// module load, resolves the selector against the class, and marshals each + /// call through `NSInvocation`. Nothing in either compiler sees both + /// halves: the `.m` never compiles the text it stores, and + /// `OfflineProtocolModule.swift` is the one bridge source no CI job + /// compiles at all, because it needs real React headers. So a + /// source-reading guard is the only reachable pin, which is C5 + /// (`docs/bridges/README.md`) applied to a selector table rather than to a + /// constant. + /// + /// Two independent things have to agree, and each has shipped broken. + /// + /// **The selector**, or the binding does not exist. A selector no Swift + /// method implements is dropped at module load behind an `RCTLogWarn` and + /// the JS method is simply absent, so the application fails at the call + /// site with `undefined is not a function`, or, where the caller wraps the + /// call in a `catch` that only logs, does not fail at all and quietly + /// keeps its defaults. Three shapes of this shipped to npm: + /// + /// 1. **A renamed label.** The `userId` to `profile` rename reached Swift, + /// Kotlin and TypeScript and missed the shim, so `wipePersistedState` + /// was uncallable from 0.21.0 through 0.24.0 and logging out could not + /// erase the account it had just signed out of. + /// 2. **No declaration at all.** `setBatteryState`, `getIsCharging`, + /// `updateRelayConfig` and `getRelayConfig` were written in Swift, + /// Kotlin and TypeScript and never added to the shim, so from 0.22.0 + /// every relay setting an application passed to `create()` was + /// discarded on iOS behind a `console.warn`. + /// 3. **A labelled first parameter.** Swift exports `f(resolver:rejecter:)` + /// as `fWithResolver:rejecter:`, not as `f:rejecter:`. Three data-layer + /// methods drifted into that shape in 0.23.0 and stopped resolving. The + /// fix for that shape is always to drop the label in Swift rather than + /// to spell the `With` form in the shim: React Native takes the JS + /// method name from the selector text before its first colon, so + /// spelling it in the shim renames the JS method instead of repairing + /// it. + /// + /// **The argument ABI**, or the binding exists and lies. This is the + /// quieter of the two, because the call arrives and the method runs. + /// `RCTModuleMethod` picks the *converter* from the macro's type text + /// (`+[RCTConvert NSNumber:]` for `NSNumber *`, `+[RCTConvert NSInteger:]` + /// for `NSInteger`) and the *calling convention* from the Swift + /// parameter's runtime type encoding, then calls the one through a + /// function pointer cast to the other. Pair `(nonnull NSNumber *)` with a + /// Swift `Int` and the `NSNumber *` the converter returns is reinterpreted + /// as a 64-bit integer, so the method runs with the pointer bits of a + /// tagged `NSNumber` in place of the number. Pair it with a Swift `Double` + /// and the integer register is read as a floating-point one, which is not + /// even deterministic. Neither logs anything. + /// + /// So the two type texts must land in the same ABI class, which is what + /// the `Abi` enum below names. Both classifiers are exhaustive over the + /// vocabulary the bridge actually uses and refuse anything else rather + /// than guessing: a wrong equivalence here is invisible in exactly the way + /// this guard exists to prevent, so a new type must be classified + /// deliberately. + /// + /// The set of Swift methods held to all of this is **derived, not + /// listed**: an `@objc` method is one React Native exports exactly when it + /// takes the promise pair. That keeps the guard blind to the `@objc` + /// methods which are not bridge entry points (two `NotificationCenter` + /// targets, and React Native's own `addListener`/`removeListeners` + /// overrides) without naming them, and it puts a *new* bridge method + /// inside the invariant the moment it is written, which a hand-maintained + /// list could not. + #[test] + fn react_native_ios_objc_shim_and_swift_agree_on_every_selector() { + /// Modifiers that may sit between `@objc` and `func`. + const MODIFIERS: &[&str] = &[ + "private", + "fileprivate", + "internal", + "public", + "open", + "override", + "static", + "class", + "final", + "dynamic", + "nonisolated", + ]; + + /// How React Native passes one argument slot, which is the property + /// the two type texts have to share. The names are ABI classes, not + /// types: what matters is the register the converter's return value + /// arrives in and the width it is read at, so `NSString *` and + /// `NSDictionary *` are one class and `NSInteger` and `double` are two + /// despite both being 64 bits. + #[derive(Clone, Copy, PartialEq, Eq, Debug)] + enum Abi { + /// `_C_ID`: the converter returns a retained object pointer. + Object, + /// `_C_LNG_LNG`: the converter returns a signed 64-bit integer. + Integer, + /// `_C_DBL`: the converter returns a double, in a float register. + Double, + /// `_C_BOOL`: the converter returns a single byte. + Bool, + /// The promise pair, which `RCTModuleMethod` special-cases by name + /// rather than routing through `RCTConvert` at all. + Block, + } + + /// The macro's type text, as React Native parses it to choose a + /// converter. + /// + /// Deliberately not exhaustive over Objective-C: it covers what this + /// bridge uses and returns `None` for everything else, so a type + /// nobody has classified fails the test instead of defaulting into a + /// class that might be wrong. + fn objc_abi(ty: &str) -> Option { + let bare = ty + .replace("nonnull", " ") + .replace("nullable", " ") + .replace("_Nonnull", " ") + .replace("_Nullable", " ") + .split_whitespace() + .collect::>() + .join(" "); + if bare.ends_with('*') { + return Some(Abi::Object); + } + match bare.as_str() { + "NSInteger" => Some(Abi::Integer), + "double" => Some(Abi::Double), + "BOOL" => Some(Abi::Bool), + "RCTPromiseResolveBlock" | "RCTPromiseRejectBlock" => Some(Abi::Block), + _ => None, + } + } + + /// The Swift parameter type, as the Objective-C runtime encodes it. + /// + /// Every optional is [`Abi::Object`], and only a reference type can + /// reach that branch: `@objc` refuses a method outright when a + /// parameter is an optional value type, because Objective-C has + /// nowhere to put the nil, so an optional here is always a nullable + /// object pointer. + fn swift_abi(ty: &str) -> Option { + let bare = ty + .replace("@escaping", " ") + .split_whitespace() + .collect::>() + .join(" "); + if bare.ends_with('?') { + return Some(Abi::Object); + } + match bare.as_str() { + "Int" => Some(Abi::Integer), + "Double" => Some(Abi::Double), + "Bool" => Some(Abi::Bool), + "RCTPromiseResolveBlock" | "RCTPromiseRejectBlock" => Some(Abi::Block), + "String" | "NSNumber" | "NSDictionary" | "NSArray" | "[NSNumber]" | "[String]" => { + Some(Abi::Object) + } + _ => None, + } + } + + /// One `RCT_EXTERN_METHOD` declaration: the selector it names, and the + /// macro type text of each of its parameters, in order. + struct ObjcMethod { + selector: String, + types: Vec, + } + + /// The selectors `RCT_EXTERN_METHOD` declares, with their types. + /// + /// A label is an identifier that precedes a `:` at the depth of the + /// macro's own argument list; the parenthesised type always follows + /// it, and the parameter name follows that. Stepping the cursor over + /// the whole `(type)` group is what keeps the type's own words out of + /// the label stream. Do not collapse this to a whitespace-stripped + /// scan: that glues each parameter's name onto the next label and + /// every selector comes out wrong in a way that still looks plausible. + fn objc_methods(src: &str) -> Vec { + const MARKER: &str = "RCT_EXTERN_METHOD("; + let bytes = src.as_bytes(); + let mut out = Vec::new(); + let mut from = 0usize; + while let Some(hit) = src[from..].find(MARKER) { + let open = from + hit + MARKER.len(); + let mut depth = 1usize; + let mut end = open; + while end < bytes.len() && depth > 0 { + match bytes[end] { + b'(' => depth += 1, + b')' => depth -= 1, + _ => {} + } + end += 1; + } + let body = &src[open..end - 1]; + from = end; + + let body_bytes = body.as_bytes(); + let mut labels: Vec = Vec::new(); + let mut types: Vec = Vec::new(); + let mut ident = String::new(); + let mut i = 0usize; + while i < body_bytes.len() { + let c = body_bytes[i] as char; + if c == ':' && !ident.is_empty() { + labels.push(std::mem::take(&mut ident)); + let mut j = i + 1; + while j < body_bytes.len() && (body_bytes[j] as char).is_ascii_whitespace() + { + j += 1; + } + if body_bytes.get(j) == Some(&b'(') { + let mut d = 1usize; + let mut k = j + 1; + while k < body_bytes.len() && d > 0 { + match body_bytes[k] { + b'(' => d += 1, + b')' => d -= 1, + _ => {} + } + k += 1; + } + types.push(body[j + 1..k - 1].trim().to_string()); + i = k; + } else { + types.push(String::new()); + i = j; + } + continue; + } + if c.is_ascii_alphanumeric() || c == '_' { + ident.push(c); + } else { + ident.clear(); + } + i += 1; + } + if !labels.is_empty() { + out.push(ObjcMethod { + selector: format!("{}:", labels.join(":")), + types, + }); + } + } + out + } + + /// One `@objc` method: the selector Swift exports it under, the type + /// of each parameter, and whether React Native exports it at all. + struct SwiftMethod { + selector: String, + types: Vec, + exported: bool, + } + + fn swift_methods(src: &str) -> Vec { + let bytes = src.as_bytes(); + let ident_char = |c: u8| (c as char).is_ascii_alphanumeric() || c == b'_'; + let skip_spaces = |mut i: usize| { + while i < bytes.len() && (bytes[i] as char).is_ascii_whitespace() { + i += 1; + } + i + }; + let close_paren = |open: usize| { + let mut depth = 1usize; + let mut i = open + 1; + while i < bytes.len() && depth > 0 { + match bytes[i] { + b'(' => depth += 1, + b')' => depth -= 1, + _ => {} + } + i += 1; + } + i + }; + + let mut out = Vec::new(); + let mut from = 0usize; + while let Some(hit) = src[from..].find("@objc") { + let start = from + hit; + let mut i = start + "@objc".len(); + // Bail past this `@objc` by default; only a real method + // declaration advances further. + from = i; + + // `@objc(explicitSelector:)` pins the selector outright, which + // is how the two methods whose Swift names differ from their + // JS names stay bound to the right one. + let mut explicit: Option = None; + if bytes.get(i) == Some(&b'(') { + let end = close_paren(i); + explicit = Some(src[i + 1..end - 1].trim().to_string()); + i = end; + } + + // Modifiers, then `func`. Anything else means this `@objc` + // decorates something other than a method: the class + // declaration itself, most importantly. + let mut cursor = i; + let is_func = loop { + cursor = skip_spaces(cursor); + let tok_start = cursor; + while cursor < bytes.len() && ident_char(bytes[cursor]) { + cursor += 1; + } + if tok_start == cursor { + break false; + } + match &src[tok_start..cursor] { + "func" => break true, + token if MODIFIERS.contains(&token) => continue, + _ => break false, + } + }; + if !is_func { + continue; + } + + cursor = skip_spaces(cursor); + let name_start = cursor; + while cursor < bytes.len() && ident_char(bytes[cursor]) { + cursor += 1; + } + let name = &src[name_start..cursor]; + cursor = skip_spaces(cursor); + if bytes.get(cursor) != Some(&b'(') { + continue; + } + let params_end = close_paren(cursor); + let params = &src[cursor + 1..params_end - 1]; + from = params_end; + + // Split the parameter list on its top-level commas. Brackets + // count toward depth (`[String: Any]`), angle brackets do not: + // a `->` in a closure type would otherwise unbalance them. + let mut parts: Vec<&str> = Vec::new(); + let mut depth = 0i32; + let mut seg = 0usize; + for (idx, ch) in params.char_indices() { + match ch { + '(' | '[' | '{' => depth += 1, + ')' | ']' | '}' => depth -= 1, + ',' if depth == 0 => { + parts.push(¶ms[seg..idx]); + seg = idx + 1; + } + _ => {} + } + } + if !params[seg..].trim().is_empty() { + parts.push(¶ms[seg..]); + } + let label_of = |part: &str| -> String { + part.trim() + .split_whitespace() + .next() + .unwrap_or_default() + .trim_end_matches(':') + .to_string() + }; + // The type is everything past the parameter's own colon, which + // is the first one at bracket depth zero: `[String: Any]` + // carries a colon that is not this one. + let type_of = |part: &str| -> String { + let mut depth = 0i32; + for (idx, ch) in part.char_indices() { + match ch { + '(' | '[' | '{' => depth += 1, + ')' | ']' | '}' => depth -= 1, + ':' if depth == 0 => return part[idx + 1..].trim().to_string(), + _ => {} + } + } + String::new() + }; + + let selector = if let Some(pinned) = explicit { + pinned + } else if parts.is_empty() { + name.to_string() + } else { + let mut selector = String::from(name); + let first = label_of(parts[0]); + // An unlabelled first parameter gives `name:`; a labelled + // one gives `nameWithLabel:`. This is the shape that broke + // three data-layer methods while reading correctly. + if first != "_" { + selector.push_str("With"); + let mut chars = first.chars(); + if let Some(initial) = chars.next() { + selector.extend(initial.to_uppercase()); + selector.push_str(chars.as_str()); + } + } + selector.push(':'); + for part in &parts[1..] { + selector.push_str(&label_of(part)); + selector.push(':'); + } + selector + }; + + out.push(SwiftMethod { + selector, + types: parts.iter().map(|part| type_of(part)).collect(), + exported: params.contains("RCTPromiseResolveBlock") + && params.contains("RCTPromiseRejectBlock"), + }); + } + out + } + + let objc = rn_source_code_only("ios/OfflineProtocolModule.m"); + let swift = rn_source_code_only("ios/OfflineProtocolModule.swift"); + let js = rn_source_code_only("src/index.ts"); + + let objc_declarations = objc_methods(&objc); + let implemented = swift_methods(&swift); + + // Both parsers have to prove they found something before any set + // comparison below means anything: a moved file, a renamed macro or a + // reformatted signature would otherwise leave two empty sets agreeing + // perfectly. + assert!( + objc_declarations.len() >= 150, + "only parsed {} RCT_EXTERN_METHOD selectors out of OfflineProtocolModule.m; \ + the parser is broken, not the bridge", + objc_declarations.len() + ); + let exported: std::collections::BTreeSet<&str> = implemented + .iter() + .filter(|method| method.exported) + .map(|method| method.selector.as_str()) + .collect(); + assert!( + exported.len() >= 150, + "only parsed {} promise-taking @objc methods out of OfflineProtocolModule.swift; \ + the parser is broken, not the bridge", + exported.len() + ); + + let declared: std::collections::BTreeSet<&str> = objc_declarations + .iter() + .map(|method| method.selector.as_str()) + .collect(); + + let dangling: Vec<&str> = declared.difference(&exported).copied().collect(); + assert!( + dangling.is_empty(), + "OfflineProtocolModule.m declares {dangling:?}, which no Swift method implements. \ + React Native resolves every declared selector against the class at module load, \ + logs that the JS method will not be available, and drops the binding, so the \ + application sees `undefined is not a function` at the call site" + ); + + let invisible: Vec<&str> = exported.difference(&declared).copied().collect(); + assert!( + invisible.is_empty(), + "OfflineProtocolModule.swift implements {invisible:?} with no matching \ + RCT_EXTERN_METHOD. The Swift compiles and the method is unreachable from \ + JavaScript, with no diagnostic at build time and none at run time either" + ); + + // Same selector on both sides, so now the arguments behind it. A + // mismatch here is not a missing method but a method that runs on the + // wrong bits, which is why it is worth a distinct failure message. + let by_selector: std::collections::BTreeMap<&str, &ObjcMethod> = objc_declarations + .iter() + .map(|method| (method.selector.as_str(), method)) + .collect(); + let mut mismatched: Vec = Vec::new(); + for method in implemented.iter().filter(|method| method.exported) { + let Some(declaration) = by_selector.get(method.selector.as_str()) else { + continue; + }; + assert_eq!( + declaration.types.len(), + method.types.len(), + "{} is declared with {} parameters and implemented with {}", + method.selector, + declaration.types.len(), + method.types.len() + ); + for (index, (objc_type, swift_type)) in + declaration.types.iter().zip(&method.types).enumerate() + { + let objc_class = objc_abi(objc_type).unwrap_or_else(|| { + panic!( + "{} parameter {index} is declared `{objc_type}`, which `objc_abi` does \ + not classify. Decide which ABI class React Native passes it in and add \ + it there; a guess would be invisibly wrong", + method.selector + ) + }); + let swift_class = swift_abi(swift_type).unwrap_or_else(|| { + panic!( + "{} parameter {index} is implemented as `{swift_type}`, which \ + `swift_abi` does not classify. Decide which ABI class the Objective-C \ + runtime encodes it in and add it there; a guess would be invisibly wrong", + method.selector + ) + }); + if objc_class != swift_class { + mismatched.push(format!( + "{} parameter {index}: `{objc_type}` ({objc_class:?}) in the shim, \ + `{swift_type}` ({swift_class:?}) in Swift", + method.selector + )); + } + } + } + assert!( + mismatched.is_empty(), + "the two halves disagree on how React Native should pass an argument:\n {}\n\ + React Native chooses the RCTConvert converter from the shim's type text and the \ + calling convention from the Swift parameter's runtime encoding, then calls the one \ + through a function pointer cast to the other. The selector still resolves and the \ + method still runs, on the converter's return value read as the wrong kind of \ + register: an `NSNumber *` reinterpreted as an `Int` arrives as the pointer bits of \ + a tagged pointer, not as the number. See the type table in \ + bindings/react-native/ios/BRIDGE_MAINTENANCE.md", + mismatched.join("\n ") + ); + + // The TypeScript is the third copy of the same table. React Native + // names the JS method after the selector text before its first colon, + // so that prefix is what a call site has to match. + let heads: std::collections::BTreeSet<&str> = declared + .iter() + .map(|selector| selector.split(':').next().unwrap_or(selector)) + .collect(); + const CALL: &str = "OfflineProtocolNativeModule."; + let mut uncallable: Vec<&str> = Vec::new(); + let mut calls = 0usize; + let mut from = 0usize; + while let Some(hit) = js[from..].find(CALL) { + let start = from + hit + CALL.len(); + let rest = &js[start..]; + let end = rest + .find(|c: char| !(c.is_ascii_alphanumeric() || c == '_')) + .unwrap_or(rest.len()); + // Only an immediate `(` is a call; `typeof mod.foo === ...` and + // other property reads are not. + if rest[end..].starts_with('(') { + calls += 1; + if !heads.contains(&rest[..end]) { + uncallable.push(&rest[..end]); + } + } + from = start + end; + } + // The same floor the two parsers above carry, for the same reason and + // against a likelier trigger: this scan keys off one identifier, so + // renaming the binding, destructuring it, or moving to a TurboModule + // spec finds nothing and passes while checking nothing. The other two + // directions cannot go quiet this way; this one can, so it says so. + assert!( + calls >= 150, + "only found {calls} native-module calls in bindings/react-native/src/index.ts; \ + the scanner is broken, not the TypeScript" + ); + uncallable.sort_unstable(); + uncallable.dedup(); + assert!( + uncallable.is_empty(), + "bindings/react-native/src/index.ts calls {uncallable:?} on the native module, \ + which the Objective-C shim never exports, so the call is undefined on iOS \ + however well it works on Android" + ); + } + + /// Every byte the iOS bridge builds out of a JavaScript number is bounded + /// where it is built. + /// + /// `UInt8(_:)` traps. It does not return nil, throw, or truncate: it + /// aborts the process. Every number this module converts arrived from + /// JavaScript, either as an element of an array argument or as a field of + /// the config object, so out-of-range input is a caller mistake, and a + /// caller mistake that aborts the application is a crash any JavaScript + /// caller can reach on purpose or by accident. + /// + /// Twelve array conversions were written `UInt8($0.intValue)` and one + /// config field `UInt8(raw["initialTtl"] as? Int ?? 8)`, so a peer sending + /// a malformed fragment, or an application passing `initialTtl: 300` to + /// `create()`, aborted on iOS where Android truncated. They now route + /// through `jsBytes`, which throws into the rejection each call site + /// already had, or clamp inline. + /// + /// The rule is bounded-at-the-site, which is why this reads text rather + /// than counting call sites: a conversion is fine when its own argument + /// carries the bound (`exactly:`, `clamping:`, `min(`, an already-`UInt8` + /// `uint8Value`, or a mask or shift), and suspect otherwise. The scalar + /// narrowings in `processFileChunk` are bounded by a `guard` several lines + /// above instead, which no textual rule can see; they are held by review + /// and by that guard clause, not by this test. + #[test] + fn react_native_ios_bridge_bounds_every_byte_it_builds_from_javascript() { + let swift = rn_source_code_only("ios/OfflineProtocolModule.swift"); + + /// Markers that bound a conversion at the point it is written. + const BOUNDED: &[&str] = &[ + "exactly:", + "clamping:", + "truncatingIfNeeded:", + "min(", + "uint8Value", + ">>", + "&", + ]; + + let bytes = swift.as_bytes(); + let mut unbounded: Vec = Vec::new(); + let mut found = 0usize; + let mut from = 0usize; + while let Some(hit) = swift[from..].find("UInt8(") { + let open = from + hit + "UInt8(".len(); + let mut depth = 1usize; + let mut end = open; + while end < bytes.len() && depth > 0 { + match bytes[end] { + b'(' => depth += 1, + b')' => depth -= 1, + _ => {} + } + end += 1; + } + let argument = &swift[open..end - 1]; + from = open; + // `UInt8()` is the default initializer: it is zero, and there + // is no argument to bound. Note that `[UInt8](...)` is not this + // case and never reaches here at all, because the `]` before its + // paren keeps it from matching the search above. + if argument.trim().is_empty() { + continue; + } + found += 1; + if !BOUNDED.iter().any(|marker| argument.contains(marker)) { + unbounded.push(argument.trim().to_string()); + } + } + + assert!( + found >= 8, + "only found {found} UInt8 conversions in OfflineProtocolModule.swift; \ + the scanner is broken, not the bridge" + ); + assert!( + unbounded.is_empty(), + "these iOS bridge conversions trap instead of rejecting:\n {}\n\ + `UInt8(_:)` aborts the process on out-of-range input, and every value here \ + came from JavaScript. Route an array through `jsBytes`, which throws into the \ + rejection the call site already has, or bound the value inline with `min`/`max` \ + as the config fields do", + unbounded.join("\n ") + ); + } + /// No transport manager may take its ordering from the app's main looper. /// /// This is OFF-2123 as an invariant. Every call these managers make into diff --git a/docs/bridges/README.md b/docs/bridges/README.md index 95bf1141..64940719 100644 --- a/docs/bridges/README.md +++ b/docs/bridges/README.md @@ -157,6 +157,29 @@ than the engine hands every silent-relay resolution to the sweep instead of to the bridge that knows which relays replied. Nothing on either side of the boundary would show that, so the guard asserts the ordering too. +**The iOS selector table** is the seventh, and the only one that is not a +constant. `OfflineProtocolModule.m` mirrors every `@objc` method of +`OfflineProtocolModule.swift` as an `RCT_EXTERN_METHOD` selector, and React +Native resolves those selectors against the class at module load rather than at +build time, so a method the two files spell differently is dropped there and is +absent from JavaScript. Eight methods were unreachable on iOS across 0.21.0 to +0.24.0 this way, in three shapes: a parameter label renamed on one side only, a +method never declared in the bridge at all, and a labelled first parameter, +which Swift exports with a `With` infix. `react_native_ios_objc_shim_and_swift_agree_on_every_selector` +reads both files plus `src/index.ts` and compares the three as sets. It is a +Rust guard for the C5 reason: the `.m` never compiles the text it stores, and +its Swift counterpart is the one bridge source no CI job compiles at all. + +The same guard pins a second agreement behind each shared selector: **the ABI +class of every parameter**. React Native chooses the `RCTConvert` converter from +the bridge's type text and the calling convention from the Swift parameter's +runtime encoding, then calls the one through a function pointer cast to the +other, so `nonnull NSNumber *` against a Swift `Int` hands the method the +pointer bits of a tagged `NSNumber` in place of the number. That one is quieter +than a missing selector, because the call arrives and the method runs: it cost +seven more methods, whose type table had recommended the wrong mapping since +before any of them were written. + ## C6. Config parsers must not default to literals A bridge parsing a config object must distinguish "the caller did not supply this diff --git a/docs/bridges/swift.md b/docs/bridges/swift.md index 06f55abd..d796d9a8 100644 --- a/docs/bridges/swift.md +++ b/docs/bridges/swift.md @@ -30,13 +30,78 @@ Type mapping: |-------|-------------| | `String` | `NSString *` | | `String?` | `NSString *` (nullable) | -| `Int` | `nonnull NSNumber *` | +| `Int` | `NSInteger` | +| `Double` | `double` | | `Bool` | `BOOL` | +| `NSNumber` | `nonnull NSNumber *` | | Promise | `RCTPromiseResolveBlock` / `RCTPromiseRejectBlock` | A method present in Swift and absent from the bridge is simply not callable from JavaScript. There is no error at build time. +**A primitive and an object are not interchangeable, and mixing them does not +fail, it lies.** React Native chooses the `RCTConvert` converter from the +bridge's type text and the calling convention from the Swift parameter's runtime +encoding, then calls the one through a function pointer cast to the other. Pair +`nonnull NSNumber *` with a Swift `Int` and the returned object pointer is read +as a 64-bit integer, so the method runs on the pointer bits of a tagged +`NSNumber` rather than on the number; pair it with a `Double` and an integer +register is read as a floating-point one. The selector still resolves, the +method still runs, and nothing is logged. This table said `Int` to +`nonnull NSNumber *` from v0.3.3 until this release, and seven methods +followed it: message and +presence priorities were silently pinned to their `default:` arm, the battery +level to a clamp bound, and the three file-transfer scalars aborted the app on +a trapping conversion. + +**The two halves must agree on the whole selector, not just the method name.** +React Native resolves each declared selector against the class when it parses +the module, drops any it cannot find, and logs that the JS method will not be +available. A renamed parameter label is therefore as fatal as a missing +declaration, and it is the easier of the two to ship: the `userId` to `profile` +rename reached Swift, Kotlin and TypeScript and missed this file, which left +`wipePersistedState` uncallable on iOS from 0.21.0 through 0.24.0. + +**The first parameter must be unlabelled (`_`).** Swift exports +`f(resolver:rejecter:)` as `fWithResolver:rejecter:`, not as `f:rejecter:`, so a +labelled first parameter silently changes the selector. Fix that shape by +dropping the label in Swift, never by spelling the `With` form here: React +Native takes the JS method name from the selector text before its first colon, +so writing `fWithResolver:` in the bridge renames the JS method instead of +repairing it. + +**An argument that arrives is still not a value you can narrow.** `UInt8(_:)` +and its siblings trap on out-of-range input: they abort the process rather than +returning something the bridge could reject. Every number crossing here came +from JavaScript, so out-of-range is a caller mistake, and a caller mistake that +aborts is a crash any caller can reach. Byte arrays go through the `jsBytes` +helper, which throws into the rejection the call site already has; scalars are +bounded where they are written, or behind a `guard` that rejects. A clamp +*around* a narrowing conversion does not count, because the conversion runs +first and traps before the clamp applies; that reaches unsigned values too, +since a negative JavaScript number arrives at `uint64Value` as `UInt64.max`. +Twelve array conversions, the `initialTtl` config field and two DORS config +paths were unbounded until this release, which made a malformed BLE fragment, +an `initialTtl: 300` and a `historyWindowSize: -1` all fatal on iOS and +harmless on Android. + +All of these are pinned in `offline-protocol-uniffi`, in `cargo test`, because +neither compiler sees both halves and this file's Swift counterpart is the one +bridge source no CI job compiles. +`react_native_ios_objc_shim_and_swift_agree_on_every_selector` reads both files +and compares them as sets: the selectors in both directions, and then, behind +each shared selector, the ABI class of every parameter. It also checks that the +TypeScript only calls methods the bridge exports, and refuses to pass when its +own scan finds nothing to check. +`react_native_ios_bridge_bounds_every_byte_it_builds_from_javascript` fails on +any byte conversion whose argument does not carry its own bound. + +One gap is known and unfixable here: React Native forces every `NSNumber` +argument to non-null, so `forwardMessage`'s optional priority is refused before +the Swift method runs and its promise never settles in a debug build. It needs +a contract change across all three languages, tracked in +[#417](https://github.com/Offline-Protocol/offline-protocol-sdk/issues/417). + ## S2. Five registration points per new Swift file A new Swift source file in the React Native iOS package must be registered in