From 3a696d588367630e51375781959b209e441c6d7c Mon Sep 17 00:00:00 2001 From: bahdotsh Date: Wed, 26 Aug 2026 10:26:06 +0530 Subject: [PATCH 1/5] fix(bindings): restore the eight iOS methods React Native could not reach `RCT_EXTERN_METHOD` does not declare a Swift method, it records a selector 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 of the bridge, 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, so logging out could not erase the account it had just signed out of. - `setBatteryState`, `getIsCharging`, `updateRelayConfig` and `getRelayConfig` were written in Swift, Kotlin and TypeScript and never declared in the shim, so since 0.22.0 every relay setting an application passed to `create()` was discarded on iOS behind a `console.warn`. - `dataListSpaces`, `dataFlushAll` and `dataWipeAll` took a labelled first parameter, which Swift exports as `dataListSpacesWithResolver:` rather than `dataListSpaces:`, and stopped resolving in 0.23.0. These are fixed in Swift rather than in the shim: React Native names the JS method after the selector text before its first colon, so writing the `With` form in the shim would rename the JS method instead of repairing it. Android was never affected; its dispatch is by method name and position. `react_native_ios_objc_shim_and_swift_agree_on_every_selector` now reads both bridge halves plus `src/index.ts` and compares them as sets, in all three directions. The set of Swift methods it holds to this is derived rather than listed: an `@objc` method is one React Native exports exactly when it takes the promise pair, so a new bridge method is inside the invariant the moment it is written. --- CHANGELOG.md | 28 ++ .../react-native/ios/BRIDGE_MAINTENANCE.md | 33 ++ .../react-native/ios/OfflineProtocolModule.m | 17 +- .../ios/OfflineProtocolModule.swift | 12 +- crates/offline-protocol-uniffi/src/lib.rs | 345 ++++++++++++++++++ docs/bridges/README.md | 13 + docs/bridges/swift.md | 22 ++ 7 files changed, 463 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c1d98c5e..955344b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,34 @@ 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. + ## [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..22e8c667 100644 --- a/bindings/react-native/ios/BRIDGE_MAINTENANCE.md +++ b/bindings/react-native/ios/BRIDGE_MAINTENANCE.md @@ -63,6 +63,19 @@ Map Swift types to Objective-C types: **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. + ## Common Issues ### Missing Parameter @@ -81,6 +94,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) diff --git a/bindings/react-native/ios/OfflineProtocolModule.m b/bindings/react-native/ios/OfflineProtocolModule.m index 100d60cd..fe685684 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) @@ -198,9 +198,17 @@ @interface RCT_EXTERN_MODULE(OfflineProtocolModule, RCTEventEmitter) resolver:(RCTPromiseResolveBlock)resolve rejecter:(RCTPromiseRejectBlock)reject) +RCT_EXTERN_METHOD(setBatteryState:(nonnull NSNumber *)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 diff --git a/bindings/react-native/ios/OfflineProtocolModule.swift b/bindings/react-native/ios/OfflineProtocolModule.swift index d178ee47..288cac5b 100644 --- a/bindings/react-native/ios/OfflineProtocolModule.swift +++ b/bindings/react-native/ios/OfflineProtocolModule.swift @@ -1623,8 +1623,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 +1909,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 +2040,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, diff --git a/crates/offline-protocol-uniffi/src/lib.rs b/crates/offline-protocol-uniffi/src/lib.rs index 4f6f4e88..8d8db932 100644 --- a/crates/offline-protocol-uniffi/src/lib.rs +++ b/crates/offline-protocol-uniffi/src/lib.rs @@ -12130,6 +12130,351 @@ mod tests { ); } + /// Every method React Native can reach on iOS is spelled the same way in + /// both halves of the hand-written bridge. + /// + /// `RCT_EXTERN_METHOD` does not declare the Swift method. It records a + /// selector string that React Native resolves against the class at module + /// load, in `parseExportedMethods`. A selector no Swift method implements + /// is dropped there 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. + /// + /// Neither half's compiler can see the other. The `.m` compiles standalone + /// against the macro, 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. + /// + /// Three ways this has actually broken, all of them 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 set of Swift methods held to 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. + /// + /// What this does not check is parameter types. A selector pins the + /// argument count and the labels, not that `(BOOL)` on one side is `Bool` + /// on the other; that mapping is S1 in `docs/bridges/swift.md`. + #[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", + ]; + + /// The selectors `RCT_EXTERN_METHOD` declares. + /// + /// A label is an identifier that precedes a `:` at the depth of the + /// macro's own argument list. Parameter types sit one paren deeper and + /// parameter names are never followed by a colon, so tracking depth is + /// what separates the label `profile:` both from the type + /// `(NSString *)` and from the `profile` that names the variable after + /// it. 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_selectors(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 mut i = from + hit + MARKER.len(); + let mut depth = 1usize; + let mut labels: Vec = Vec::new(); + let mut ident = String::new(); + while i < bytes.len() && depth > 0 { + let c = bytes[i] as char; + match c { + '(' => { + depth += 1; + ident.clear(); + } + ')' => { + depth -= 1; + ident.clear(); + } + ':' if depth == 1 && !ident.is_empty() => { + labels.push(std::mem::take(&mut ident)); + } + _ if depth == 1 && (c.is_ascii_alphanumeric() || c == '_') => ident.push(c), + _ => ident.clear(), + } + i += 1; + } + if !labels.is_empty() { + out.push(format!("{}:", labels.join(":"))); + } + from = i; + } + out + } + + /// One `@objc` method: the selector Swift exports it under, and + /// whether React Native exports it at all. + struct SwiftMethod { + selector: String, + 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() + }; + + 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, + 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 declared = objc_selectors(&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!( + declared.len() >= 150, + "only parsed {} RCT_EXTERN_METHOD selectors out of OfflineProtocolModule.m; \ + the parser is broken, not the bridge", + declared.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> = + declared.iter().map(String::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" + ); + + // 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 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('(') && !heads.contains(&rest[..end]) { + uncallable.push(&rest[..end]); + } + from = start + end; + } + 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" + ); + } + /// 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..a76d5c82 100644 --- a/docs/bridges/README.md +++ b/docs/bridges/README.md @@ -157,6 +157,19 @@ 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` compiles standalone, and its Swift +counterpart is the one bridge source no CI job compiles at all. + ## 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..78bdbfd3 100644 --- a/docs/bridges/swift.md +++ b/docs/bridges/swift.md @@ -37,6 +37,28 @@ Type mapping: A method present in Swift and absent from the bridge is simply not callable from JavaScript. There is no error at build time. +**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. + +All three shapes are pinned by +`react_native_ios_objc_shim_and_swift_agree_on_every_selector` in +`offline-protocol-uniffi`, which reads both files and compares selector sets. +It runs in `cargo test`, because neither compiler sees both halves and this +file's Swift counterpart is the one bridge source no CI job compiles. + ## S2. Five registration points per new Swift file A new Swift source file in the React Native iOS package must be registered in From 8d9b7e0fdeb95affed492d17c3d4c45e714ffe91 Mon Sep 17 00:00:00 2001 From: bahdotsh Date: Wed, 26 Aug 2026 11:58:26 +0530 Subject: [PATCH 2/5] fix(bindings): make the iOS bridge agree on argument types, not just selectors The selector fix in the parent commit restored eight methods React Native could not reach. Extending its guard to compare parameter types found seven more that resolved and then ran on the wrong bits. `RCT_EXTERN_METHOD` stringifies its argument. React Native parses that text at module load, picks the `RCTConvert` converter from the type it reads there, and picks 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 `+[RCTConvert NSNumber:]` returns an object pointer that is then read as a 64-bit integer, so the method runs on the pointer bits of a tagged `NSNumber` and never 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. The type table in `BRIDGE_MAINTENANCE.md` had mapped `Int` to `nonnull NSNumber *` since v0.3.3, the release that also introduced the first method to follow it, so every method that took a number followed it too: - `sendMessage`, `sendMessageRich`, `sendPresenceUpdate`: every priority and status fell through to the `default:` arm, so an app's choice was discarded; - `setBatteryLevel`, `setBatteryState`: the level clamped to a bound rather than recording the battery, which is an input to relay eligibility; - `processFileChunk`, `blePeerDiscovered`: the value reached a narrowing conversion that traps, aborting the application. Fixed in the shim rather than in Swift: the selector is unchanged either way, and this direction leaves all seven Swift bodies alone. The two conversions that now receive real values are made safe for them, since out-of-range input from JavaScript is a caller mistake and belongs in a rejected promise rather than in an abort. The guard gains a third direction, comparing the ABI class of every parameter behind a shared selector. Both classifiers are exhaustive over the vocabulary the bridge uses and refuse anything else rather than guessing, so a new type has to be classified deliberately: a wrong equivalence here is invisible in exactly the way the guard exists to prevent. Verified: the type direction was written first and run against the unfixed tree, where it named all nine mismatched parameters. Two mutations, each restored and checksum-verified: reverting one type fires the ABI direction naming it, and an unclassified type fires the refusal. `cargo fmt`, `clippy`, `cargo test --workspace` and rustdoc are clean; the full hand-written iOS source set typechecks (exit 0) under the `BRIDGE_MAINTENANCE.md` recipe, itself negative-controlled first, and `swift test` passes 252 tests. --- CHANGELOG.md | 19 + .../react-native/ios/BRIDGE_MAINTENANCE.md | 23 +- .../react-native/ios/OfflineProtocolModule.m | 18 +- .../ios/OfflineProtocolModule.swift | 16 +- crates/offline-protocol-uniffi/src/lib.rs | 333 ++++++++++++++---- docs/bridges/README.md | 14 +- docs/bridges/swift.md | 29 +- 7 files changed, 374 insertions(+), 78 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 955344b1..8e83b024 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,25 @@ archived by series under [docs/changelog/](docs/changelog/); see the 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. + ## [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 22e8c667..4dab9edd 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,11 +56,30 @@ 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 note that React Native does not support that: it forces every +`NSNumber` argument to non-null and rejects a null one, so `forwardMessage`'s +absent priority is refused in a debug build. + **Note**: All `@objc` methods must include `resolver` and `rejecter` parameters (React Native Promise pattern). ### Step 4: Leave the first parameter unlabelled diff --git a/bindings/react-native/ios/OfflineProtocolModule.m b/bindings/react-native/ios/OfflineProtocolModule.m index fe685684..83cd6076 100644 --- a/bindings/react-native/ios/OfflineProtocolModule.m +++ b/bindings/react-native/ios/OfflineProtocolModule.m @@ -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,11 +194,11 @@ @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:(nonnull NSNumber *)level +RCT_EXTERN_METHOD(setBatteryState:(NSInteger)level isCharging:(BOOL)isCharging resolver:(RCTPromiseResolveBlock)resolve rejecter:(RCTPromiseRejectBlock)reject) @@ -282,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 @@ -696,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 288cac5b..ad41b35f 100644 --- a/bindings/react-native/ios/OfflineProtocolModule.swift +++ b/bindings/react-native/ios/OfflineProtocolModule.swift @@ -2868,7 +2868,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) @@ -3559,6 +3559,20 @@ 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) } try proto.processFileChunk( diff --git a/crates/offline-protocol-uniffi/src/lib.rs b/crates/offline-protocol-uniffi/src/lib.rs index 8d8db932..870eb25a 100644 --- a/crates/offline-protocol-uniffi/src/lib.rs +++ b/crates/offline-protocol-uniffi/src/lib.rs @@ -12131,24 +12131,28 @@ mod tests { } /// Every method React Native can reach on iOS is spelled the same way in - /// both halves of the hand-written bridge. - /// - /// `RCT_EXTERN_METHOD` does not declare the Swift method. It records a - /// selector string that React Native resolves against the class at module - /// load, in `parseExportedMethods`. A selector no Swift method implements - /// is dropped there 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. - /// - /// Neither half's compiler can see the other. The `.m` compiles standalone - /// against the macro, 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 + /// 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. /// - /// Three ways this has actually broken, all of them shipped to npm: + /// 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` @@ -12168,17 +12172,34 @@ mod tests { /// spelling it in the shim renames the JS method instead of repairing /// it. /// - /// The set of Swift methods held to 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. - /// - /// What this does not check is parameter types. A selector pins the - /// argument count and the labels, not that `(BOOL)` on one side is `Bool` - /// on the other; that mapping is S1 in `docs/bridges/swift.md`. + /// **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`. @@ -12196,57 +12217,172 @@ mod tests { "nonisolated", ]; - /// The selectors `RCT_EXTERN_METHOD` declares. + /// 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`], including `Int?`: an optional + /// value type crosses into Objective-C as a boxed `NSNumber`, not as + /// the primitive it wraps. + 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. Parameter types sit one paren deeper and - /// parameter names are never followed by a colon, so tracking depth is - /// what separates the label `profile:` both from the type - /// `(NSString *)` and from the `profile` that names the variable after - /// it. 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_selectors(src: &str) -> Vec { + /// 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 mut i = from + hit + MARKER.len(); + 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(); - while i < bytes.len() && depth > 0 { - let c = bytes[i] as char; - match c { - '(' => { - depth += 1; - ident.clear(); - } - ')' => { - depth -= 1; - ident.clear(); + 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 depth == 1 && !ident.is_empty() => { - labels.push(std::mem::take(&mut ident)); + 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; } - _ if depth == 1 && (c.is_ascii_alphanumeric() || c == '_') => ident.push(c), - _ => ident.clear(), + continue; + } + if c.is_ascii_alphanumeric() || c == '_' { + ident.push(c); + } else { + ident.clear(); } i += 1; } if !labels.is_empty() { - out.push(format!("{}:", labels.join(":"))); + out.push(ObjcMethod { + selector: format!("{}:", labels.join(":")), + types, + }); } - from = i; } out } - /// One `@objc` method: the selector Swift exports it under, and - /// whether React Native exports it at all. + /// 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, } @@ -12357,6 +12493,21 @@ mod tests { .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 @@ -12386,6 +12537,7 @@ mod tests { out.push(SwiftMethod { selector, + types: parts.iter().map(|part| type_of(part)).collect(), exported: params.contains("RCTPromiseResolveBlock") && params.contains("RCTPromiseRejectBlock"), }); @@ -12397,7 +12549,7 @@ mod tests { let swift = rn_source_code_only("ios/OfflineProtocolModule.swift"); let js = rn_source_code_only("src/index.ts"); - let declared = objc_selectors(&objc); + let objc_declarations = objc_methods(&objc); let implemented = swift_methods(&swift); // Both parsers have to prove they found something before any set @@ -12405,10 +12557,10 @@ mod tests { // reformatted signature would otherwise leave two empty sets agreeing // perfectly. assert!( - declared.len() >= 150, + objc_declarations.len() >= 150, "only parsed {} RCT_EXTERN_METHOD selectors out of OfflineProtocolModule.m; \ the parser is broken, not the bridge", - declared.len() + objc_declarations.len() ); let exported: std::collections::BTreeSet<&str> = implemented .iter() @@ -12422,8 +12574,10 @@ mod tests { exported.len() ); - let declared: std::collections::BTreeSet<&str> = - declared.iter().map(String::as_str).collect(); + 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!( @@ -12442,6 +12596,67 @@ mod tests { 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. diff --git a/docs/bridges/README.md b/docs/bridges/README.md index a76d5c82..64940719 100644 --- a/docs/bridges/README.md +++ b/docs/bridges/README.md @@ -167,8 +167,18 @@ absent from JavaScript. Eight methods were unreachable on iOS across 0.21.0 to 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` compiles standalone, and its Swift -counterpart is the one bridge source no CI job compiles at all. +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 diff --git a/docs/bridges/swift.md b/docs/bridges/swift.md index 78bdbfd3..eb4b24ad 100644 --- a/docs/bridges/swift.md +++ b/docs/bridges/swift.md @@ -30,13 +30,30 @@ 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 @@ -53,11 +70,13 @@ 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. -All three shapes are pinned by +All of these are pinned by `react_native_ios_objc_shim_and_swift_agree_on_every_selector` in -`offline-protocol-uniffi`, which reads both files and compares selector sets. -It runs in `cargo test`, because neither compiler sees both halves and this -file's Swift counterpart is the one bridge source no CI job compiles. +`offline-protocol-uniffi`, which 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 runs in `cargo test`, because neither compiler +sees both halves and this file's Swift counterpart is the one bridge source no +CI job compiles. ## S2. Five registration points per new Swift file From 21f73e0a87df9653104ce75f8677e48479694786 Mon Sep 17 00:00:00 2001 From: bahdotsh Date: Wed, 26 Aug 2026 12:40:58 +0530 Subject: [PATCH 3/5] test(bindings): make the iOS selector guard fail when it checks nothing The selector guard reads three files and compares them in three directions. Two of those directions assert up front that their parser actually found something, on the grounds that two empty sets agree with each other perfectly and prove absolutely nothing. The third direction doesn't, and it is the one most likely to need it. The TypeScript scan hunts for a single identifier string, `OfflineProtocolNativeModule.`. Rename that binding, destructure it, or move the package to a TurboModule spec, and the scan matches nothing at all. No matches means no findings, no findings means the assertion passes, and a guard that passes because it read an empty set is worse than no guard, because by then you trust it. So count the calls and require a plausible number of them, the same way the other two directions do. There are 167 today. Checked the same way the rest of this guard was: renaming the identifier in index.ts makes it fail with "only found 0 native-module calls in bindings/react-native/src/index.ts", which is the sentence you want to be reading at that point. --- crates/offline-protocol-uniffi/src/lib.rs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/crates/offline-protocol-uniffi/src/lib.rs b/crates/offline-protocol-uniffi/src/lib.rs index 870eb25a..22dbeb82 100644 --- a/crates/offline-protocol-uniffi/src/lib.rs +++ b/crates/offline-protocol-uniffi/src/lib.rs @@ -12666,6 +12666,7 @@ mod tests { .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(); @@ -12675,11 +12676,24 @@ mod tests { .unwrap_or(rest.len()); // Only an immediate `(` is a call; `typeof mod.foo === ...` and // other property reads are not. - if rest[end..].starts_with('(') && !heads.contains(&rest[..end]) { - uncallable.push(&rest[..end]); + 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!( From c4365e2f0dc74aa170ef35ce277851454d2cd94c Mon Sep 17 00:00:00 2001 From: bahdotsh Date: Wed, 26 Aug 2026 12:41:17 +0530 Subject: [PATCH 4/5] fix(bindings): stop aborting the iOS app when JavaScript sends a bad byte `UInt8(_:)` traps. It does not return nil, it does not throw, it does not truncate: it kills the process. Thirteen conversions in the iOS bridge used it on numbers that came straight from JavaScript. Twelve of them turn an array argument into bytes, so any element outside 0...255 aborts the app. That covers a BLE fragment, a Wi-Fi Direct or internet frame, an MLS ciphertext, a Welcome, a key package and a file chunk, which means most of them are reachable by a peer sending us something malformed rather than only by our own caller being careless. The thirteenth is `initialTtl`, typed as an unbounded `number` in TypeScript, so an application passing 300 to `create()` takes the app down on iOS while Android quietly truncates it and starts. Nobody notices until someone's field device starts crashing on boot and you get to explain why. This is not great. Route the arrays through a helper that throws instead, landing in the rejection every one of these call sites already had, and clamp `initialTtl` the way every other numeric config field in that file is already clamped. An out-of-range value from JavaScript is a caller mistake, and a caller mistake belongs in a rejected promise, not in a crash report. Worth being clear that these were never hiding behind the argument ABI bug fixed earlier on this branch. Array arguments cross as `NSArray *` against `[NSNumber]`, which has agreed since the UniFFI migration, so every one of these has been live in every release that shipped the method. Pin it with a guard that fails on any `UInt8` conversion in that file whose argument doesn't carry its own bound. It has to read text rather than count call sites, because the thing that makes a conversion safe is local to where it's written. The scalar narrowings in `processFileChunk` are bounded by a `guard` several lines above instead, which no textual rule can see, so they stay held by that clause and by review. While at it, document the one case that can't be fixed here. `forwardMessage` takes an optional priority, React Native forces every `NSNumber` argument to non-null because Android can't express a nullable number, and it drops the call before the Swift method is entered. Neither the resolver nor the rejecter runs, so the promise never settles and the caller waits forever. No spelling of the declaration fixes that one; it needs a contract change in three languages. See #417 for the contract change that would actually fix it. --- CHANGELOG.md | 30 +++++++ .../react-native/ios/BRIDGE_MAINTENANCE.md | 41 ++++++++- .../ios/OfflineProtocolModule.swift | 55 +++++++++--- crates/offline-protocol-uniffi/src/lib.rs | 84 +++++++++++++++++++ docs/bridges/swift.md | 34 ++++++-- 5 files changed, 221 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e83b024..724a02e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,36 @@ archived by series under [docs/changelog/](docs/changelog/); see the instead of trapping, and the selector guard gained a third direction that compares the ABI class of every parameter behind a shared selector. +- **Thirteen iOS conversions aborted the app instead of rejecting the call.** + `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. Byte arrays now convert through a helper that throws + into the rejection each call site already had, `initialTtl` is clamped, 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 4dab9edd..af90e4f0 100644 --- a/bindings/react-native/ios/BRIDGE_MAINTENANCE.md +++ b/bindings/react-native/ios/BRIDGE_MAINTENANCE.md @@ -76,9 +76,16 @@ point one. Nothing is logged either way. This row read `Int` to followed it. Take an `NSNumber` on the Swift side only where the argument is genuinely -optional, and note that React Native does not support that: it forces every -`NSNumber` argument to non-null and rejects a null one, so `forwardMessage`'s -absent priority is refused in a debug build. +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). @@ -95,6 +102,31 @@ 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. + +`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. + ## Common Issues ### Missing Parameter @@ -205,6 +237,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.swift b/bindings/react-native/ios/OfflineProtocolModule.swift index ad41b35f..8bb9d059 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 @@ -1486,7 +1515,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 +1539,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, @@ -2914,7 +2943,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 { @@ -3574,7 +3603,7 @@ class OfflineProtocolModule: RCTEventEmitter { return } do { - let bytes = data.map { UInt8($0.intValue) } + let bytes = try jsBytes(data, "data") try proto.processFileChunk( fileId: fileId, chunkIndex: UInt32(chunkIndex), @@ -3631,7 +3660,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 { @@ -3712,7 +3741,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 { @@ -4156,7 +4185,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 ?? "", @@ -4197,7 +4226,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 ?? "", @@ -4244,7 +4273,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 { @@ -4363,7 +4392,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, @@ -4394,7 +4423,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 ?? "", @@ -4440,7 +4469,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 22dbeb82..ef6b3b01 100644 --- a/crates/offline-protocol-uniffi/src/lib.rs +++ b/crates/offline-protocol-uniffi/src/lib.rs @@ -12704,6 +12704,90 @@ mod tests { ); } + /// 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](...)` and `-> [UInt8]` are not conversions; only a + // construction from something has an argument to bound. + 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/swift.md b/docs/bridges/swift.md index eb4b24ad..428a8e98 100644 --- a/docs/bridges/swift.md +++ b/docs/bridges/swift.md @@ -70,13 +70,33 @@ 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. -All of these are pinned by -`react_native_ios_objc_shim_and_swift_agree_on_every_selector` in -`offline-protocol-uniffi`, which 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 runs in `cargo test`, because neither compiler -sees both halves and this file's Swift counterpart is the one bridge source no -CI job compiles. +**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. Twelve array +conversions and the `initialTtl` config field were unbounded until this +release, which made a malformed BLE fragment and an `initialTtl: 300` both 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 From 068da39d7efce4d879d481ea02f37a8fa462ea66 Mon Sep 17 00:00:00 2001 From: bahdotsh Date: Wed, 26 Aug 2026 13:28:35 +0530 Subject: [PATCH 5/5] fix(bindings): stop iOS aborting on a negative DORS config number The previous commit swept this bridge for conversions that trap on JavaScript input, fixed thirteen of them, and added a guard that fails on any `UInt8(...)` whose argument is not bounded where it is written. It missed two, and the guard could not have caught them, because they are not byte conversions. Both DORS config paths clamp `historyWindowSize` with `max(1, min(100, Int(raw)))`, which reads as bounded and is not: the narrowing runs *first*. `raw` is a `UInt64` straight off `NSNumber.uint64Value`, and a negative JavaScript number lands there as `UInt64.max`, so the `Int` conversion traps and aborts the process before the clamp around it ever runs. A `historyWindowSize` of -1 kills the app through `create()`, and again through `updateDorsConfig`. So clamp in the domain the value arrives in. `historyWindow` is only ever consumed as a `UInt64`, so dropping the round-trip through `Int` deletes the trapping conversion outright instead of making a pointless one safe, and the two `UInt64(historyWindow)` call sites become identity conversions and go with it. The symlink-farm typecheck agrees it now infers as `UInt64`; the negative control proved the harness by complaining about exactly that type. No new guard for this one. Whether `Int(raw)` is safe depends on what `raw` already is, which the text cannot say, and this file is full of legitimate widenings like `Int(hops)` that a textual rule would flag. Review and the checklist hold it instead, and both bridge docs now say that a clamp wrapped *around* a narrowing conversion does not count. While at it, two comments on the new guards were simply wrong. The byte guard's empty-argument skip claimed to exempt `[UInt8](...)`, which never reaches it at all because the `]` stops it matching the search; what it actually exempts is the zero-argument `UInt8()`. And the ABI classifier claimed `Int?` boxes into an `NSNumber`. It cannot: `@objc` refuses an optional value-type parameter outright, so only reference types ever reach that branch. --- CHANGELOG.md | 32 +++++++++++-------- .../react-native/ios/BRIDGE_MAINTENANCE.md | 13 +++++++- .../ios/OfflineProtocolModule.swift | 16 +++++++--- crates/offline-protocol-uniffi/src/lib.rs | 14 +++++--- docs/bridges/swift.md | 12 ++++--- 5 files changed, 60 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 724a02e2..b5b55938 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,19 +58,25 @@ archived by series under [docs/changelog/](docs/changelog/); see the instead of trapping, and the selector guard gained a third direction that compares the ABI class of every parameter behind a shared selector. -- **Thirteen iOS conversions aborted the app instead of rejecting the call.** - `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. Byte arrays now convert through a helper that throws - into the rejection each call site already had, `initialTtl` is clamped, 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. +- **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 diff --git a/bindings/react-native/ios/BRIDGE_MAINTENANCE.md b/bindings/react-native/ios/BRIDGE_MAINTENANCE.md index af90e4f0..9f3bf92f 100644 --- a/bindings/react-native/ios/BRIDGE_MAINTENANCE.md +++ b/bindings/react-native/ios/BRIDGE_MAINTENANCE.md @@ -123,9 +123,20 @@ 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. +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 diff --git a/bindings/react-native/ios/OfflineProtocolModule.swift b/bindings/react-native/ios/OfflineProtocolModule.swift index 8bb9d059..9e20b1f5 100644 --- a/bindings/react-native/ios/OfflineProtocolModule.swift +++ b/bindings/react-native/ios/OfflineProtocolModule.swift @@ -608,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) @@ -637,7 +641,7 @@ class OfflineProtocolModule: RCTEventEmitter { ttlEscalationThreshold: ttlThreshold, congestionDurationSecs: congestionDuration, ttlEscalationHoldSecs: ttlHold, - historyWindowSize: UInt64(historyWindow), + historyWindowSize: historyWindow, queueRecoveryRatio: queueRecovery, lowBatteryThreshold: lowBattery, relayMinBatteryLevel: relayMinBattery, @@ -3316,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) @@ -3337,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)))), diff --git a/crates/offline-protocol-uniffi/src/lib.rs b/crates/offline-protocol-uniffi/src/lib.rs index ef6b3b01..d06b38a6 100644 --- a/crates/offline-protocol-uniffi/src/lib.rs +++ b/crates/offline-protocol-uniffi/src/lib.rs @@ -12268,9 +12268,11 @@ mod tests { /// The Swift parameter type, as the Objective-C runtime encodes it. /// - /// Every optional is [`Abi::Object`], including `Int?`: an optional - /// value type crosses into Objective-C as a boxed `NSNumber`, not as - /// the primitive it wraps. + /// 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", " ") @@ -12761,8 +12763,10 @@ mod tests { } let argument = &swift[open..end - 1]; from = open; - // `[UInt8](...)` and `-> [UInt8]` are not conversions; only a - // construction from something has an argument to bound. + // `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; } diff --git a/docs/bridges/swift.md b/docs/bridges/swift.md index 428a8e98..d796d9a8 100644 --- a/docs/bridges/swift.md +++ b/docs/bridges/swift.md @@ -76,10 +76,14 @@ 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. Twelve array -conversions and the `initialTtl` config field were unbounded until this -release, which made a malformed BLE fragment and an `initialTtl: 300` both fatal on iOS -and harmless on Android. +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