Skip to content

Commit 77b5181

Browse files
authored
Wrap decoding targetToInflightPackages in a try/catch (#3689)
It's not apparent how a key could be stored with a nil object in the dictionary. Maybe it's possible that somehow a GDTUploadPackage fails to decode, but there aren't any indications that this is the case. This PR adds a check for contents of the dictionary, though encoding an empty dictionary should be fine. A try/catch is utilized to catch exceptions thrown in iOS's implementation of NSSecureCoding in NSMutableDictionary.
1 parent 51ded3f commit 77b5181

File tree

2 files changed

+23
-4
lines changed

2 files changed

+23
-4
lines changed

GoogleDataTransport/GDTLibrary/GDTUploadCoordinator.m

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -163,16 +163,23 @@ + (BOOL)supportsSecureCoding {
163163

164164
- (instancetype)initWithCoder:(NSCoder *)aDecoder {
165165
GDTUploadCoordinator *sharedCoordinator = [GDTUploadCoordinator sharedInstance];
166-
sharedCoordinator->_targetToInFlightPackages =
167-
[aDecoder decodeObjectOfClass:[NSMutableDictionary class]
168-
forKey:ktargetToInFlightPackagesKey];
166+
@try {
167+
sharedCoordinator->_targetToInFlightPackages =
168+
[aDecoder decodeObjectOfClass:[NSMutableDictionary class]
169+
forKey:ktargetToInFlightPackagesKey];
170+
171+
} @catch (NSException *exception) {
172+
sharedCoordinator->_targetToInFlightPackages = [NSMutableDictionary dictionary];
173+
}
169174
return sharedCoordinator;
170175
}
171176

172177
- (void)encodeWithCoder:(NSCoder *)aCoder {
173178
// All packages that have been given to uploaders need to be tracked so that their expiration
174179
// timers can be called.
175-
[aCoder encodeObject:_targetToInFlightPackages forKey:ktargetToInFlightPackagesKey];
180+
if (_targetToInFlightPackages.count > 0) {
181+
[aCoder encodeObject:_targetToInFlightPackages forKey:ktargetToInFlightPackagesKey];
182+
}
176183
}
177184

178185
#pragma mark - GDTLifecycleProtocol

GoogleDataTransport/GDTTests/Unit/GDTUploadCoordinatorTest.m

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,4 +146,16 @@ - (void)testThatAFailedUploadResultsInAnEventualRetry {
146146
});
147147
}
148148

149+
/** Tests that encoding and decoding works without crashing. */
150+
- (void)testNSSecureCoding {
151+
GDTUploadPackage *package = [[GDTUploadPackage alloc] initWithTarget:kGDTTargetTest];
152+
GDTUploadCoordinator *coordinator = [[GDTUploadCoordinator alloc] init];
153+
coordinator.targetToInFlightPackages[@(kGDTTargetTest)] = package;
154+
NSData *data = [NSKeyedArchiver archivedDataWithRootObject:coordinator];
155+
156+
// Unarchiving the coordinator always ends up altering the singleton instance.
157+
GDTUploadCoordinator *unarchivedCoordinator = [NSKeyedUnarchiver unarchiveObjectWithData:data];
158+
XCTAssertEqualObjects([GDTUploadCoordinator sharedInstance], unarchivedCoordinator);
159+
}
160+
149161
@end

0 commit comments

Comments
 (0)