diff --git a/patches/react-native/details.md b/patches/react-native/details.md index 4395fa022dbb..72e27e549fc4 100644 --- a/patches/react-native/details.md +++ b/patches/react-native/details.md @@ -339,3 +339,20 @@ - Upstream PR/issue: https://github.com/react/react-native/pull/57546 (merged as `06eb1fe`) - E/App issue: https://github.com/Expensify/App/issues/97127 - PR introducing patch: https://github.com/Expensify/App/pull/98095 + +### [react-native+0.86.0+042+fix-jsi-string-truncated-at-nul-APP-KDN.patch](react-native+0.86.0+042+fix-jsi-string-truncated-at-nul-APP-KDN.patch) + +- Reason: Strings crossing the JS/native boundary were truncated at the first NUL byte, because the converters passed `.c_str()` to NUL-terminated C-string APIs (`stringWithUTF8String:`, `stringWithCString:`, `NewStringUTF`) instead of using the byte length they already had. That is the producer behind the APP-KDN crash: `BlobManager.js` records the blob's `size` from the whole string, native stores the truncated bytes, and `subdataWithRange:` then overruns the stored data the next time that blob is used as a part of another blob. Backports upstream's fix, which replaces those calls with length-aware equivalents — `initWithBytes:length:encoding:` and `dataUsingEncoding:` on iOS, UTF-16 in both directions on Android. +- Blast radius: this covers **every** string argument crossing into a TurboModule on both platforms, plus Fabric text, not just blobs. Strings without NULs are unaffected; strings that previously arrived truncated now arrive whole. Two call sites (`convertJSIStringToNSString`, `RCTNSAttributedStringFragmentFromFragment`) now return `@""` where invalid UTF-8 previously gave `nil`, which is upstream's behaviour, not a deviation. +- Upstream PR/issue: [react/react-native#57906](https://github.com/react/react-native/pull/57906) (merged as [`5906cfb`](https://github.com/react/react-native/commit/5906cfb06085a4ae7ae2e5ac5bb190e0c1e90a42), fixes [#24129](https://github.com/react/react-native/issues/24129)) +- E/App issue: https://github.com/Expensify/App/issues/100843 +- PR introducing patch: this PR. +- 0.86.0 migration note: **drop this patch on the RN 0.88 upgrade** — `5906cfb` is absent from `v0.87.1` and ships in `v0.88.0`. Every hunk here is byte-identical to the 0.88 sources, so the drop needs no re-derivation. The commit's `React/Tests/Text/RCTAttributedTextUtilsTest.mm` hunks are omitted: that file ships in the npm package but is not part of any target the App builds, so patching it would add noise and test nothing. + +### [react-native+0.86.0+043+clamp-blob-range-APP-KDN.patch](react-native+0.86.0+043+clamp-blob-range-APP-KDN.patch) + +- Reason: Defence in depth for the same crash (APP-KDN, `NSRangeException: -[NSConcreteMutableData subdataWithRange:]: range {0, 321} exceeds data length 318`). `RCTBlobManager resolve:offset:size:` sliced the stored `NSData` with the JS-supplied offset and size without range-checking them, so any descriptor/data desync raised an `NSRangeException` on a background queue where no JS frame can catch it and the app dies. This clamps the range to the bytes actually stored and logs when it has to — an out-of-bounds offset returns `nil`, an oversized size is truncated — and adds the same logging to the silently-0-byte part paths in `createFromParts:withId:`. `size < 0` still means "the whole blob", preserving the existing `-1` contract. Android is already forgiving here ([`Arrays.copyOfRange`](https://github.com/facebook/react-native/blob/v0.86.0/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobModule.kt)), which is why the crash is iOS-only. +- Why both patches: patch 042 fixes the producer; on its own this one would turn the crash into silent data loss (a leading NUL resolves to 0 bytes and uploads as an empty attachment as if it had succeeded). It is here so any *future* desync degrades and logs instead of killing the app. `RCTLogWarn` rather than `RCTLogError` deliberately: this runs on a background queue and `RCTLogError` red-boxes on every occurrence in dev. +- Upstream PR/issue: [react/react-native#58566](https://github.com/react/react-native/pull/58566) — open, not merged. Still unfixed in every release to date: `v0.88.0-rc.1` has the same unchecked `subdataWithRange:`. Re-check on each RN upgrade and drop this patch once the adopted release contains it. +- E/App issue: https://github.com/Expensify/App/issues/100843 +- PR introducing patch: this PR. diff --git a/patches/react-native/react-native+0.86.0+042+fix-jsi-string-truncated-at-nul-APP-KDN.patch b/patches/react-native/react-native+0.86.0+042+fix-jsi-string-truncated-at-nul-APP-KDN.patch new file mode 100644 index 000000000000..1c811a2d5d26 --- /dev/null +++ b/patches/react-native/react-native+0.86.0+042+fix-jsi-string-truncated-at-nul-APP-KDN.patch @@ -0,0 +1,90 @@ +diff --git a/node_modules/react-native/React/Fabric/RCTConversions.h b/node_modules/react-native/React/Fabric/RCTConversions.h +--- a/node_modules/react-native/React/Fabric/RCTConversions.h ++++ b/node_modules/react-native/React/Fabric/RCTConversions.h +@@ -35,4 +35,5 @@ + const NSStringEncoding &encoding = NSUTF8StringEncoding) + { +- return [NSString stringWithCString:string.c_str() encoding:encoding] ?: @""; ++ NSString *result = [[NSString alloc] initWithBytes:string.data() length:string.size() encoding:encoding]; ++ return result != nil ? result : @""; + } +@@ -47,4 +48,11 @@ + inline std::string RCTStringFromNSString(NSString *string) + { +- return std::string{string.UTF8String ?: ""}; ++ if (string == nil) { ++ return ""; ++ } ++ NSData *data = [string dataUsingEncoding:NSUTF8StringEncoding]; ++ if (data == nil) { ++ return ""; ++ } ++ return std::string{static_cast(data.bytes), data.length}; + } +diff --git a/node_modules/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp b/node_modules/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp +--- a/node_modules/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp ++++ b/node_modules/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp +@@ -403,5 +403,7 @@ + "string", argIndex, methodName, arg, &rt); + } +- jarg->l = makeGlobalIfNecessary( +- env->NewStringUTF(arg->getString(rt).utf8(rt).c_str())); ++ auto utf16 = rt.utf16(arg->getString(rt)); ++ jarg->l = makeGlobalIfNecessary(env->NewString( ++ reinterpret_cast(utf16.data()), ++ static_cast(utf16.size()))); + } else if (type == "Lcom/facebook/react/bridge/Callback;") { +@@ -751,8 +753,11 @@ + jsi::Value returnValue = jsi::Value::null(); + if (returnString != nullptr) { +- const char* js = env->GetStringUTFChars(returnString, nullptr); +- std::string result = js; +- env->ReleaseStringUTFChars(returnString, js); +- returnValue = +- jsi::Value(runtime, jsi::String::createFromUtf8(runtime, result)); ++ jsize length = env->GetStringLength(returnString); ++ const jchar* chars = env->GetStringChars(returnString, nullptr); ++ auto jsiString = jsi::String::createFromUtf16( ++ runtime, ++ reinterpret_cast(chars), ++ static_cast(length)); ++ env->ReleaseStringChars(returnString, chars); ++ returnValue = jsi::Value(runtime, jsiString); + } +diff --git a/node_modules/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm b/node_modules/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm +--- a/node_modules/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm ++++ b/node_modules/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm +@@ -111,4 +111,6 @@ + static NSString *convertJSIStringToNSString(jsi::Runtime &runtime, const jsi::String &value) + { +- return [NSString stringWithUTF8String:value.utf8(runtime).c_str()]; ++ auto utf8 = value.utf8(runtime); ++ NSString *result = [[NSString alloc] initWithBytes:utf8.data() length:utf8.size() encoding:NSUTF8StringEncoding]; ++ return result != nil ? result : @""; + } +diff --git a/node_modules/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTAttributedTextUtils.mm b/node_modules/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTAttributedTextUtils.mm +--- a/node_modules/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTAttributedTextUtils.mm ++++ b/node_modules/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTAttributedTextUtils.mm +@@ -431,5 +431,8 @@ + return [[NSMutableAttributedString attributedStringWithAttachment:attachment] mutableCopy]; + } else { +- NSString *string = [NSString stringWithUTF8String:fragment.string.c_str()]; ++ NSString *decoded = [[NSString alloc] initWithBytes:fragment.string.data() ++ length:fragment.string.size() ++ encoding:NSUTF8StringEncoding]; ++ NSString *string = decoded != nil ? decoded : @""; + + if (fragment.textAttributes.textTransform.has_value()) { +diff --git a/node_modules/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTTextLayoutManager.mm b/node_modules/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTTextLayoutManager.mm +--- a/node_modules/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTTextLayoutManager.mm ++++ b/node_modules/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTTextLayoutManager.mm +@@ -211,5 +211,7 @@ + CGFloat baseline = [layoutManager locationForGlyphAtIndex:range.location].y; +- const char *renderedUTF8 = [renderedString UTF8String]; ++ NSData *renderedData = [renderedString dataUsingEncoding:NSUTF8StringEncoding]; + auto line = LineMeasurement{ +- std::string(renderedUTF8 != nullptr ? renderedUTF8 : ""), ++ std::string( ++ renderedData != nil ? static_cast(renderedData.bytes) : "", ++ renderedData != nil ? renderedData.length : 0), + rect, diff --git a/patches/react-native/react-native+0.86.0+043+clamp-blob-range-APP-KDN.patch b/patches/react-native/react-native+0.86.0+043+clamp-blob-range-APP-KDN.patch new file mode 100644 index 000000000000..a4ff9a5abb29 --- /dev/null +++ b/patches/react-native/react-native+0.86.0+043+clamp-blob-range-APP-KDN.patch @@ -0,0 +1,68 @@ +diff --git a/node_modules/react-native/Libraries/Blob/RCTBlobManager.mm b/node_modules/react-native/Libraries/Blob/RCTBlobManager.mm +--- a/node_modules/react-native/Libraries/Blob/RCTBlobManager.mm ++++ b/node_modules/react-native/Libraries/Blob/RCTBlobManager.mm +@@ -12,5 +12,6 @@ + #import + #import ++#import + #import + #import + #import +@@ -111,8 +112,33 @@ + if (!data) { + return nil; + } +- if (offset != 0 || (size != -1 && size != data.length)) { +- data = [data subdataWithRange:NSMakeRange(offset, size)]; ++ // The descriptor is computed in JS and can disagree with the bytes that were actually stored. ++ // Clamp the range to what is really here instead of letting `subdataWithRange:` raise an ++ // NSRangeException, which is fatal on this background queue because no JS frame can catch it. ++ const NSInteger length = (NSInteger)data.length; ++ if (offset < 0 || offset > length) { ++ RCTLogWarn( ++ @"[BlobManager] blob %@: offset %ld out of bounds for %ld stored bytes", ++ blobId, ++ (long)offset, ++ (long)length); ++ return nil; ++ } ++ const NSInteger available = length - offset; ++ // A negative size means "the rest of the blob", which is the existing contract for `-1`. ++ if (size < 0 || size > available) { ++ if (size > available) { ++ RCTLogWarn( ++ @"[BlobManager] blob %@: asked for %ld bytes at offset %ld, only %ld stored; truncating", ++ blobId, ++ (long)size, ++ (long)offset, ++ (long)available); ++ } ++ size = available; ++ } ++ if (offset != 0 || size != length) { ++ data = [data subdataWithRange:NSMakeRange((NSUInteger)offset, (NSUInteger)size)]; + } + return data; + } +@@ -198,8 +224,21 @@ + if ([type isEqualToString:@"blob"]) { + NSData *partData = [self resolve:part[@"data"]]; ++ if (!partData) { ++ // A part that cannot be resolved contributes zero bytes, which is how a blob ends up ++ // shorter than the size JS recorded for it. Say so instead of failing silently. ++ RCTLogWarn( ++ @"[BlobManager] blob %@: could not resolve blob part %@, it contributes 0 bytes", ++ blobId, ++ [RCTConvert NSString:part[@"data"][@"blobId"]]); ++ continue; ++ } + [data appendData:partData]; + } else if ([type isEqualToString:@"string"]) { + NSData *partData = [[RCTConvert NSString:part[@"data"]] dataUsingEncoding:NSUTF8StringEncoding]; ++ if (!partData) { ++ RCTLogWarn(@"[BlobManager] blob %@: string part could not be encoded, it contributes 0 bytes", blobId); ++ continue; ++ } + [data appendData:partData]; + } else { + [NSException raise:@"Invalid type for blob" format:@"%@ is invalid", type];