Fix IOS crash in Network Plugin due to incorrect processing of data U… #978
Conversation
…RLs (#974) Summary: Fix #974 by skipping response processing in FlipperKitNetworkPlugin.mm if the response is not an instance of NSHTTPURLResponse, which data URLs are not. My assumption is that data URLs are not the ones Flipper Network Plugin users are interested in, given the type of information being extracted in the didObserveResponse method which are only present in NSHTTPURLReponse types.
facebook-github-bot
added
the
CLA Signed
There was a problem hiding this comment.
@cekkaewnumchai has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
Thank you for the PR! |
| @@ -29,6 +29,11 @@ - (instancetype)initWithIndentifier:(int64_t)identifier | |||
| } | |||
|
|
|||
| + (BOOL)shouldStripReponseBodyWithResponse:(NSURLResponse*)response { | |||
| // Only HTTP(S) responses have Content-Type headers | |||
| if (![response isKindOfClass:[NSHTTPURLResponse class]]) { | |||
There was a problem hiding this comment.
I'm wondering it this shouldn't return YES instead, given that this method returns if the response data should be stripped from the network request details, which is the case for large binary data as images, videos and zips. I can imagine we don't want to send the details fo data: urls to Flipper either? (see also line 50)
There was a problem hiding this comment.
That makes sense. I updated this PR.
|
@mark-plukkido has updated the pull request. Re-import the pull request |
There was a problem hiding this comment.
@cekkaewnumchai has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@cekkaewnumchai merged this pull request in e44c7f4. |
Fix IOS crash in Network Plugin due to incorrect processing of data URLs (#974)
Summary
Fix #974 by skipping response processing in FlipperKitNetworkPlugin.mm if the response is not an instance of NSHTTPURLResponse, which data URLs are not. My assumption is that data URLs are not the ones Flipper Network Plugin users are interested in, given the type of information being extracted in the didObserveResponse method which are only present in NSHTTPURLReponse types.
Changelog
Fix IOS crash in Network Plugin due to unchecked casting of data URLs
Test plan
npx react-native init issue974 --version react-native@0.62.1cd issue974/ios/Pods/FlipperKit/iOS/Plugins/curl -L https://patch-diff.githubusercontent.com/raw/facebook/flipper/pull/978.patch | git apply -v -p3cd ../../../../../curl -L https://github.com/facebook/flipper/files/4434063/rn-data-uri-test.patch.txt | git apply -v react-native run-iosThe React Native app patch used to verify the PR working and resolving the issue #974
rn-data-uri-test.patch.txt