Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[React Native] Patch RCTFileReaderModule readAsDataURL to prevent crash when type is nil #5475

Merged
merged 1 commit into from
Sep 24, 2024

Conversation

haileyok
Copy link
Member

@haileyok haileyok commented Sep 24, 2024

Fixes #5100

Why

The null check for type doesn't actually do anything here, because type will never be nil, but rather it will be either NSString or NSNull. We need to actually convert NSNull to nil first before the check.

Breakdown

React Native handles the response for a request in this block of code: https://github.com/facebook/react-native/blob/303e0ed7641409acf2d852c077f6be426afd7a0c/packages/react-native/Libraries/Blob/RCTBlobManager.mm#L314-L326

Here, we see that values of nil - which [response MIMEType] will return when no content-type is provided in the response and the actual type cannot be determined (https://developer.apple.com/documentation/foundation/nsurlresponse/1411613-mimetype) - gets converted to NSNull by RCTNullIfNil. This is fine, since we cannot have nil in a dictionary so we have to do the conversion.

When we get back over to readAsDataURL, we see that we grab the type from the dictionary and check if its nil before calling length on the string. https://github.com/facebook/react-native/blob/303e0ed7641409acf2d852c077f6be426afd7a0c/packages/react-native/Libraries/Blob/RCTFileReaderModule.mm#L74-L77

However, this check is dubious, because the value will never actually be nil. It will always either be NSString or NSNull because of the RCTNullIfNil call made above. The [RCTConvert NSString] made when getting the type from the dictionary will simply return NSNull here, because NSNull is an objc class, which will merely be returned as such by the call (https://github.com/facebook/react-native/blob/303e0ed7641409acf2d852c077f6be426afd7a0c/packages/react-native/React/Base/RCTConvert.mm#L38-L60)

The trick here is to first use RCTNilIfNull to convert to nil if needed, then continue making the != nil check.

Test Plan

Nothing should break. The only way I can repro the crash right now is to add nosniff to the response headers in x-content-type-options for something like registerPush or updateSeen. Verified that this does fix the problem though.

@arcalinea arcalinea temporarily deployed to hailey/patch-rn-type-crash - social-app PR #5475 September 24, 2024 20:07 — with Render Destroyed
Copy link

Old size New size Diff
10.37 MB 10.37 MB 0 B (0.00%)

@haileyok haileyok merged commit 6338083 into main Sep 24, 2024
6 checks passed
@haileyok haileyok deleted the hailey/patch-rn-type-crash branch September 24, 2024 20:25
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ios: crashing when opening the app with notifications allowed
3 participants