This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Make FlutterStandardCodec handle writing NSData #6207
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
iOS developers can accidentally pass an `NSData` object directly to `FlutterStandardCodec` and forget to wrap it in `FlutterStandardTypedData`. This failure won't be caught until runtime. Let's make `FlutterStandardCodec` more tolerant by making it assume that `NSData` should be treated as a binary blob and by wrapping it automatically. Fixes flutter/flutter#17449 Testing Done: * Made a sample Flutter iOS application that created a `FlutterMethodChannel` and invoked a method using an `NSData*` argument. Verified that the Flutter method handler received the message and that the `MethodCall.arguments` was a `UInt8Array` that preserved the original byte order. * Verified that without this change, the same sample application crashed when run in debug mode.
Contributor
|
Should this documentation be updated (https://flutter.io/platform-channels)? If cc @johnfesa who's more familiar with Objective-C and recommended filing the issue I filed. |
alanrussian
reviewed
Sep 10, 2018
Contributor
Author
I lean toward no. This change adds a convenience to tolerate passing |
alanrussian
approved these changes
Sep 10, 2018
Contributor
alanrussian
left a comment
There was a problem hiding this comment.
LGTM but please get approval from someone with more Objective-C familiarity (I have very little). Thanks!
chinmaygarde
approved these changes
Sep 21, 2018
Contributor
|
Sorry about the late reply. Can this be committed? |
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Sep 21, 2018
flutter/engine@a8890fd...5b8e8c3 git log a8890fd..5b8e8c3 --no-merges --oneline 5b8e8c3 Make FlutterStandardCodec handle writing NSData (flutter/engine#6207) 02901b7 Decouple PlatformViewsController from FlutterView. (flutter/engine#6303) cc3009c Revert 'Dart SDK roll for 2018/09/20' 8471862 (flutter/engine#6309) bbdf7c1 Revert "Fix a compilation problem when using iPhoneOS12.0sdk(Xcode10) && clang version 7.0.0." (flutter/engine#6307) dea0921 Roll src/third_party/skia c25f440d537e..358558a4cecc (17 commits) (flutter/engine#6308) d29c7db Add logging if FlutterDartProject fails to load the application kernel snapshot (flutter/engine#6257) 2a1debf Update deprecated subtags from language subtag registry. (flutter/engine#6280) 540cd96 Add Xib splashscreen support (flutter/engine#6289) 05f21e6 Fix a compilation problem when using iPhoneOS12.0sdk(Xcode10) && clang version 7.0.0. (flutter/engine#6279) ca6f103 Roll src/third_party/skia d842557c0724..c25f440d537e (10 commits) (flutter/engine#6304) 3b46705 Roll src/third_party/skia 38ca6d509d9f..d842557c0724 (5 commits) (flutter/engine#6302) 0c166fe Roll src/third_party/skia 05cf051f0252..38ca6d509d9f (1 commits) (flutter/engine#6301) cf0fbad Roll src/third_party/skia 44c6167c4125..05cf051f0252 (4 commits) (flutter/engine#6299) 2ec20aa Remove bottom safe-area padding when keyboard up (flutter/engine#6297) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary.
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Sep 22, 2018
flutter/engine@cc3009c...7648d21 git log cc3009c..7648d21 --no-merges --oneline 7648d21 Dart SDK roll for 2018-09-21 (flutter/engine#6311) 5b8e8c3 Make FlutterStandardCodec handle writing NSData (flutter/engine#6207) 02901b7 Decouple PlatformViewsController from FlutterView. (flutter/engine#6303) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary.
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Sep 22, 2018
flutter/engine@cc3009c...19ac3e1 git log cc3009c..19ac3e1 --no-merges --oneline 19ac3e1 Roll src/third_party/skia 358558a4cecc..11f4994b84e1 (2 commits) (flutter/engine#6312) 7648d21 Dart SDK roll for 2018-09-21 (flutter/engine#6311) 5b8e8c3 Make FlutterStandardCodec handle writing NSData (flutter/engine#6207) 02901b7 Decouple PlatformViewsController from FlutterView. (flutter/engine#6303) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary.
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Sep 22, 2018
flutter/engine@cc3009c...f3a3d0c git log cc3009c..f3a3d0c --no-merges --oneline f3a3d0c Roll src/third_party/skia 11f4994b84e1..175b587a634d (1 commits) (flutter/engine#6313) 19ac3e1 Roll src/third_party/skia 358558a4cecc..11f4994b84e1 (2 commits) (flutter/engine#6312) 7648d21 Dart SDK roll for 2018-09-21 (flutter/engine#6311) 5b8e8c3 Make FlutterStandardCodec handle writing NSData (flutter/engine#6207) 02901b7 Decouple PlatformViewsController from FlutterView. (flutter/engine#6303) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary.
engine-flutter-autoroll
added a commit
to flutter/flutter
that referenced
this pull request
Sep 22, 2018
flutter/engine@cc3009c...f3a3d0c git log cc3009c..f3a3d0c --no-merges --oneline f3a3d0c Roll src/third_party/skia 11f4994b84e1..175b587a634d (1 commits) (flutter/engine#6313) 19ac3e1 Roll src/third_party/skia 358558a4cecc..11f4994b84e1 (2 commits) (flutter/engine#6312) 7648d21 Dart SDK roll for 2018-09-21 (flutter/engine#6311) 5b8e8c3 Make FlutterStandardCodec handle writing NSData (flutter/engine#6207) 02901b7 Decouple PlatformViewsController from FlutterView. (flutter/engine#6303) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
iOS developers can accidentally pass an
NSDataobject directly toFlutterStandardCodecand forget to wrap it inFlutterStandardTypedData. This failure won't be caught untilruntime.
Let's make
FlutterStandardCodecmore tolerant by making it assumethat
NSDatashould be treated as a binary blob and by wrapping itautomatically.
Fixes flutter/flutter#17449
Testing Done:
FlutterMethodChanneland invoked a method using anNSData*argument. Verified that the Flutter method handler received the
message and that the
MethodCall.argumentswas aUInt8Arraythatpreserved the original byte order.
crashed when run in debug mode.