use none-deprecated archive functions when possible #394
Conversation
This file contains 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
ShiCheng-Lu
force-pushed
the
shicheng/fence-2079-switch-from-unarchiveobjectwithdata-to
branch
from
758811d
to
7e87e5e
Compare
| [[NSUserDefaults standardUserDefaults] setObject:replaysData forKey:@"radar-replays"]; | ||
| } | ||
|
|
||
| - (void)loadReplaysFromPersistentStore { | ||
| NSData *replaysData = [[NSUserDefaults standardUserDefaults] objectForKey:@"radar-replays"]; | ||
| if (replaysData) { | ||
| NSArray *replays = [NSKeyedUnarchiver unarchiveObjectWithData:replaysData]; | ||
| NSArray *replays; | ||
| if (@available(iOS 11.0, *)) { |
There was a problem hiding this comment.
I'm a little confused about iOS 11 version gate since NSSecureCoding is available from iOS 6
There was a problem hiding this comment.
the archivedDataWithRootObject: requireingSecureCoding: function is only introduced in iOS 11.0. the function that we were using (without the second param) is the one raising security alerts.
| @@ -18,4 +18,181 @@ + (NSDictionary *)jsonDictionaryFromResource:(NSString *)resource { | |||
| return jsonDict; | |||
| } | |||
|
|
|||
| + (NSMutableDictionary *)createTrackParamWithLocation:(CLLocation *_Nonnull)location | |||
There was a problem hiding this comment.
I guess using the default values returned by radarState and radarSettings is perfectly fine in this case, but can we just put a comment noting that?
There was a problem hiding this comment.
added function docstring in the header file for this behaviour
| @@ -1489,6 +1490,22 @@ - (void)test_RadarLogBuffer_purge { | |||
| [[RadarLogBuffer sharedInstance]clearBuffer]; | |||
| } | |||
|
|
|||
| - (void)test_RadarReplayBuffer_writeAndRead { | |||
There was a problem hiding this comment.
thank you for adding this test
…used for RadarTestUtils.createTrackParamWithLocation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The archive function that we were using was deprecated, and it was giving security warnings to customers integrating the SDK. The new, secure archive functions were introduces with iOS 11.0. So for iOS 11.0 +, we are switching to using the secure archive functions in order to avoid security issues.
I the archiving and unarchiving of RadarLog is already tested. I tested it with original archiving with new unarchiving just to make sure that previously saved data do not become invalid after this function change.
For RadarReplay, I added the test for archiving and unarchiving by creating the replayParam the same way it was created in /track, then using the
writeNewReplayToBufferandloadReplaysFromPersistentStoremethods to verify the archiving/unarchiving is working. Same test with old archiving to verify updating sdk doesn't invalidate existing stored data.(+267 lines might look scary, but most of it is in
RadarTestUtilto construct the track param for replay test)