Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

fix RCTNetworking contentType issue #35640

Closed
wants to merge 1 commit into from
Closed

Conversation

malacca
Copy link
Contributor

@malacca malacca commented Dec 14, 2022

Summary

error, data ? @{@"body" : data, @"contentType" : RCTNullIfNil(response.MIMEType)} : nil);

because RCTNullIfNil(response.MIMEType), contentType may be [NSNull null]

if (partContentType != nil) {

Here only check partContentType is not nil, This causes the custom type never used

BOOL isMultipart = [dataContentType hasPrefix:@"multipart"];

more serious here, if dataContentType is [NSNull null] , it will crash the application

-[NSNull hasPrefix:]: unrecognized selector sent to instance 0x7fff8004b700

Changelog

[iOS] [Fixed] - fix dataContentType may be [NSNull null] issue

Test Plan

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Dec 14, 2022
@react-native-bot react-native-bot added Bug Platform: iOS iOS applications. labels Dec 14, 2022
@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 7,777,859 +0
android hermes armeabi-v7a 7,097,471 +0
android hermes x86 8,252,264 +0
android hermes x86_64 8,110,376 +0
android jsc arm64-v8a 8,970,518 +0
android jsc armeabi-v7a 7,703,386 +0
android jsc x86 9,034,232 +0
android jsc x86_64 9,511,773 +0

Base commit: 9f9111b
Branch: main

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: 83afdaf
Branch: main

@pull-bot
Copy link

PR build artifact for fb63097 is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@pull-bot
Copy link

PR build artifact for fb63097 is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@facebook-github-bot
Copy link
Contributor

@dmytrorykun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Dec 15, 2022
@facebook-github-bot
Copy link
Contributor

@dmytrorykun merged this pull request in c0834b8.

@malacca malacca mentioned this pull request Dec 26, 2022
OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
Summary:
https://github.com/facebook/react-native/blob/96027f787ade1a465569975059426d2c03ee5ad0/Libraries/Network/RCTNetworking.mm#L418

because  `RCTNullIfNil(response.MIMEType)`,  `contentType` may be `[NSNull null]`

https://github.com/facebook/react-native/blob/96027f787ade1a465569975059426d2c03ee5ad0/Libraries/Network/RCTNetworking.mm#L114

Here only check `partContentType` is not nil, This causes the custom type never used

https://github.com/facebook/react-native/blob/96027f787ade1a465569975059426d2c03ee5ad0/Libraries/Network/RCTNetworking.mm#L334

more serious here, if `dataContentType` is `[NSNull null]` ,  it will crash the application

`-[NSNull hasPrefix:]: unrecognized selector sent to instance 0x7fff8004b700`

## Changelog

[iOS] [Fixed] - fix dataContentType may be [NSNull null] issue

Pull Request resolved: facebook#35640

Reviewed By: cipolleschi

Differential Revision: D42067206

Pulled By: dmytrorykun

fbshipit-source-id: 073e6589111f5117486b69b8206a07ef1995c5b8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Platform: iOS iOS applications.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants