Skip to content

Commit

Permalink
convert NSNull to nil before checking type in readAsDataURL (#…
Browse files Browse the repository at this point in the history
…46635)

Summary:
This issue original arose out of bluesky-social/social-app#5100. Copying the description (with my general understanding of the problem) from the patch PR to here as well.

There's a crash that comes up in the following, pretty specific scenario:

- Have a response that has an empty body
- Do not include a `content-type` header in the response
- Set the `x-content-type-options` header to `nosniff`

RN 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`.

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 and `[RCTConvert NSString]` seems to just return the input if it is `NSNull`.

## Changelog:

[IOS] [FIXED] - Convert `NSNull` to `nil` before checking `type` in `readAsDataURL`

Pull Request resolved: #46635

Test Plan:
This is a little awkward to test, but essentially this comes up in the following scenario that is described (and "tested" as being fixed by tweaking) in bluesky-social/social-app#5100. I have personally tested by using Cloudflare rules to add/remove that particular header from an empty body response. You could also test this with a little local web server if you want.

### Before

https://github.com/user-attachments/assets/deb86c68-2251-4fef-9705-a1c93584e83e

### After

https://github.com/user-attachments/assets/9ffab11b-b2c8-4a83-afd6-0a55fed3ae9b

Reviewed By: dmytrorykun

Differential Revision: D63381947

Pulled By: cipolleschi

fbshipit-source-id: b2b4944d998133611592eed8d112faa6195587bd
  • Loading branch information
haileyok authored and facebook-github-bot committed Sep 25, 2024
1 parent 65575e8 commit 99ab845
Showing 1 changed file with 4 additions and 3 deletions.
7 changes: 4 additions & 3 deletions packages/react-native/Libraries/Blob/RCTFileReaderModule.mm
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,10 @@ @implementation RCTFileReaderModule
nil);
} else {
NSString *type = [RCTConvert NSString:blob[@"type"]];
NSString *text = [NSString stringWithFormat:@"data:%@;base64,%@",
type != nil && [type length] > 0 ? type : @"application/octet-stream",
[data base64EncodedStringWithOptions:0]];
NSString *text = [NSString
stringWithFormat:@"data:%@;base64,%@",
![type isEqual:[NSNull null]] && [type length] > 0 ? type : @"application/octet-stream",
[data base64EncodedStringWithOptions:0]];

resolve(text);
}
Expand Down

0 comments on commit 99ab845

Please # to comment.