Skip to content

Commit 0f7ef08

Browse files
thomasvlthomasvl
committed
Enforce the types for sessionUserInfo.
Ensure the types are correct for the api contract in all places possible. Add some testing also to ensure things are enforced.
1 parent 3cdb78e commit 0f7ef08

File tree

2 files changed

+191
-52
lines changed

2 files changed

+191
-52
lines changed

‎Sources/Core/GTMSessionFetcher.m

+66-52
Original file line numberDiff line numberDiff line change
@@ -1594,56 +1594,65 @@ - (void)setSessionIdentifierInternal:(nullable NSString *)sessionIdentifier {
15941594
if (_sessionUserInfo == nil) {
15951595
// We'll return the metadata dictionary with internal keys removed. This avoids the user
15961596
// re-using the userInfo dictionary later and accidentally including the internal keys.
1597+
// Just incase something got corrupted in storage and parsed back out differently, ensure
1598+
// the api contract on types is still valid.
15971599
NSMutableDictionary *metadata = [[self sessionIdentifierMetadataUnsynchronized] mutableCopy];
15981600
NSSet *keysToRemove = [metadata keysOfEntriesPassingTest:^BOOL(id key, id obj, BOOL *stop) {
1599-
return [key hasPrefix:@"_"];
1601+
return ![key isKindOfClass:[NSString class]] || ![obj isKindOfClass:[NSString class]] ||
1602+
[key hasPrefix:@"_"];
16001603
}];
1601-
[metadata removeObjectsForKeys:[keysToRemove allObjects]];
1602-
if (metadata.count > 0) {
1603-
_sessionUserInfo = metadata;
1604-
16051604
#if DEBUG
1605+
// If we're pruning, give warnings about the things that were invalid as some bug has slipped
1606+
// through.
1607+
if (keysToRemove.count) {
16061608
[metadata enumerateKeysAndObjectsUsingBlock:^(id _Nonnull key, id _Nonnull obj,
16071609
BOOL *_Nonnull stop) {
1608-
GTMSESSION_ASSERT_DEBUG([key isKindOfClass:[NSString class]],
1609-
@"sessionUserInfo keys must be NSStrings: %@", key);
1610-
if (![obj isKindOfClass:[NSString class]]) {
1611-
GTMSESSION_LOG_DEBUG(@"WARNING: sessionUserInfo included a non String value, this will "
1612-
@"be an error in the future: %@: %@",
1613-
key, obj);
1610+
if (![key isKindOfClass:[NSString class]]) {
1611+
GTMSESSION_LOG_DEBUG(
1612+
@"InternalError: restoring sessionUserInfo is pruning a non String key: %@", key);
1613+
} else if (![obj isKindOfClass:[NSString class]]) {
1614+
GTMSESSION_LOG_DEBUG(
1615+
@"InternalError: restoring sessionUserInfo is pruning a non String value: %@: %@",
1616+
key, obj);
16141617
}
16151618
}];
1619+
}
16161620
#endif // DEBUG
1621+
[metadata removeObjectsForKeys:[keysToRemove allObjects]];
1622+
1623+
if (metadata.count > 0) {
1624+
_sessionUserInfo = metadata;
16171625
}
16181626
}
16191627
return _sessionUserInfo;
16201628
} // @synchronized(self)
16211629
}
16221630

16231631
- (void)setSessionUserInfo:(nullable NSDictionary<NSString *, NSString *> *)dictionary {
1632+
[dictionary enumerateKeysAndObjectsUsingBlock:^(id _Nonnull key, id _Nonnull obj,
1633+
BOOL *_Nonnull stop) {
1634+
if (![key isKindOfClass:[NSString class]]) {
1635+
[NSException raise:NSInvalidArgumentException
1636+
format:@"sessionUserInfo keys must be NSStrings: %@", key];
1637+
}
1638+
if ([key hasPrefix:@"_"]) {
1639+
[NSException
1640+
raise:NSInvalidArgumentException
1641+
format:
1642+
@"sessionUserInfo keys starting with an underscore are reserved for the library: %@",
1643+
key];
1644+
}
1645+
if (![obj isKindOfClass:[NSString class]]) {
1646+
[NSException raise:NSInvalidArgumentException
1647+
format:@"Values in sessionUserInfo must be NSStrings: %@: %@", key, obj];
1648+
}
1649+
}];
1650+
16241651
@synchronized(self) {
16251652
GTMSessionMonitorSynchronized(self);
16261653

16271654
GTMSESSION_ASSERT_DEBUG(_sessionIdentifier == nil, @"Too late to assign userInfo");
16281655
_sessionUserInfo = dictionary;
1629-
1630-
#if DEBUG
1631-
[dictionary enumerateKeysAndObjectsUsingBlock:^(id _Nonnull key, id _Nonnull obj,
1632-
BOOL *_Nonnull stop) {
1633-
GTMSESSION_ASSERT_DEBUG([key isKindOfClass:[NSString class]],
1634-
@"sessionUserInfo keys must be NSStrings: %@", key);
1635-
if ([key hasPrefix:@"_"]) {
1636-
GTMSESSION_LOG_DEBUG(@"WARNING: sessionUserInfo keys starting with an underscore are "
1637-
@"reserved for the library, this will become an error in the future: "
1638-
@"%@: %@", key, obj);
1639-
}
1640-
if (![obj isKindOfClass:[NSString class]]) {
1641-
GTMSESSION_LOG_DEBUG(@"WARNING: sessionUserInfo included a non String value, this will be "
1642-
@"an error in the future: %@: %@",
1643-
key, obj);
1644-
}
1645-
}];
1646-
#endif // DEBUG
16471656
} // @synchronized(self)
16481657
}
16491658

@@ -1734,34 +1743,39 @@ - (NSString *)createSessionIdentifierWithMetadata:(nullable NSDictionary *)metad
17341743
_sessionIdentifier =
17351744
[NSString stringWithFormat:@"%@_%@", kGTMSessionIdentifierPrefix, _sessionIdentifierUUID];
17361745

1737-
#if DEBUG
1738-
// _sessionUserInfo was declared as `strong` (not `copy`, so it could have been modifed after
1739-
// having been set.
1740-
[_sessionUserInfo enumerateKeysAndObjectsUsingBlock:^(id _Nonnull key, id _Nonnull obj,
1741-
BOOL *_Nonnull stop) {
1742-
GTMSESSION_ASSERT_DEBUG([key isKindOfClass:[NSString class]],
1743-
@"sessionUserInfo keys must be NSStrings: %@", key);
1744-
if ([key hasPrefix:@"_"]) {
1745-
GTMSESSION_LOG_DEBUG(@"WARNING: sessionUserInfo keys starting with an underscore are "
1746-
@"reserved for the library, this will become an error in the future: "
1747-
@"%@: %@", key, obj);
1748-
}
1749-
if (![obj isKindOfClass:[NSString class]]) {
1750-
GTMSESSION_LOG_DEBUG(@"WARNING: sessionUserInfo included a non String value, this will "
1751-
@"be an error in the future: %@: %@",
1752-
key, obj);
1753-
}
1754-
}];
1755-
#endif // DEBUG
1756-
17571746
// Start with user-supplied keys so they cannot accidentally override the fetcher's keys.
17581747
NSMutableDictionary *metadataDict =
17591748
[NSMutableDictionary dictionaryWithDictionary:(NSDictionary *_Nonnull)_sessionUserInfo];
17601749

1750+
// sessionUserInfo was declared as `strong` (not `copy`), so it could have been modifed after
1751+
// having been set. So remove anything that breaks the contract.
1752+
NSSet *keysToRemove = [metadataDict keysOfEntriesPassingTest:^BOOL(id key, id obj, BOOL *stop) {
1753+
return ![key isKindOfClass:[NSString class]] || ![obj isKindOfClass:[NSString class]] ||
1754+
[key hasPrefix:@"_"];
1755+
}];
1756+
#if DEBUG
1757+
// If we're pruning, give warnings about the things that were invalid as some bug has slipped
1758+
// through.
1759+
if (keysToRemove.count) {
1760+
[metadataDict enumerateKeysAndObjectsUsingBlock:^(id _Nonnull key, id _Nonnull obj,
1761+
BOOL *_Nonnull stop) {
1762+
if (![key isKindOfClass:[NSString class]]) {
1763+
GTMSESSION_LOG_DEBUG(
1764+
@"Warning: sessionUserInfo has been modifed to include a non String key: %@", key);
1765+
} else if (![obj isKindOfClass:[NSString class]]) {
1766+
GTMSESSION_LOG_DEBUG(
1767+
@"Warning: sessionUserInfo has been modifed to include a non String value: %@: %@",
1768+
key, obj);
1769+
}
1770+
}];
1771+
}
1772+
#endif // DEBUG
1773+
[metadataDict removeObjectsForKeys:[keysToRemove allObjects]];
1774+
17611775
if (metadataToInclude) {
17621776
#if DEBUG
1763-
[metadataToInclude enumerateKeysAndObjectsUsingBlock:^(id _Nonnull key, id _Nonnull obj,
1764-
BOOL * _Nonnull stop) {
1777+
[metadataToInclude enumerateKeysAndObjectsUsingBlock:^(id _Nonnull key, id _Nonnull obj,
1778+
BOOL *_Nonnull stop) {
17651779
GTMSESSION_ASSERT_DEBUG([key isKindOfClass:[NSString class]],
17661780
@"metadataToInclude keys must be NSStrings: %@", key);
17671781
GTMSESSION_ASSERT_DEBUG([key hasPrefix:@"_"],
@@ -1774,7 +1788,7 @@ - (NSString *)createSessionIdentifierWithMetadata:(nullable NSDictionary *)metad
17741788
NSDictionary *defaultMetadataDict = [self sessionIdentifierDefaultMetadata];
17751789
if (defaultMetadataDict) {
17761790
#if DEBUG
1777-
[defaultMetadataDict enumerateKeysAndObjectsUsingBlock:^(id _Nonnull key, id _Nonnull obj,
1791+
[defaultMetadataDict enumerateKeysAndObjectsUsingBlock:^(id _Nonnull key, id _Nonnull obj,
17781792
BOOL * _Nonnull stop) {
17791793
GTMSESSION_ASSERT_DEBUG([key isKindOfClass:[NSString class]],
17801794
@"defaultMetadataDict keys must be NSStrings: %@", key);

‎UnitTests/GTMSessionFetcherFetchingTest.m

+125
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,17 @@
5454
@interface GTMSessionFetcher (ExposedForTesting)
5555
+ (nullable NSURL *)redirectURLWithOriginalRequestURL:(nullable NSURL *)originalRequestURL
5656
redirectRequestURL:(nullable NSURL *)redirectRequestURL;
57+
- (NSString *)createSessionIdentifierWithMetadata:(NSDictionary *)metadata;
58+
@end
59+
60+
@interface TestIdentifierMetadataFecher : GTMSessionFetcher
61+
@property(strong, nullable) NSDictionary *testIdentifierMetadata;
62+
@end
63+
64+
@implementation TestIdentifierMetadataFecher
65+
- (nullable NSDictionary *)sessionIdentifierMetadataUnsynchronized {
66+
return self.testIdentifierMetadata;
67+
}
5768
@end
5869

5970
// Base class for fetcher and chunked upload tests.
@@ -258,6 +269,120 @@ @interface GTMSessionFetcherFetchingTest : GTMSessionFetcherBaseTest
258269

259270
@implementation GTMSessionFetcherFetchingTest
260271

272+
#pragma mark - SessionUserInfo Tests
273+
274+
- (void)testSetSessionUserInfo {
275+
GTMSessionFetcher *fetcher = [GTMSessionFetcher fetcherWithURLString:@"file://not_needed"];
276+
277+
@try {
278+
NSDictionary *dict = @{@1 : @"invalid non string key"};
279+
fetcher.sessionUserInfo = dict;
280+
XCTFail(@"Shouldn't get here");
281+
} @catch (NSException *exception) {
282+
XCTAssertEqualObjects(exception.name, NSInvalidArgumentException);
283+
XCTAssertEqualObjects(exception.reason, @"sessionUserInfo keys must be NSStrings: 1");
284+
}
285+
XCTAssertNil(fetcher.sessionUserInfo);
286+
287+
// Key with a leading underscore
288+
@try {
289+
fetcher.sessionUserInfo = @{@"_invalidUnderscore" : @"value"};
290+
XCTFail(@"Shouldn't get here");
291+
} @catch (NSException *exception) {
292+
XCTAssertEqualObjects(exception.name, NSInvalidArgumentException);
293+
XCTAssertEqualObjects(exception.reason, @"sessionUserInfo keys starting with an underscore are "
294+
@"reserved for the library: _invalidUnderscore");
295+
}
296+
XCTAssertNil(fetcher.sessionUserInfo);
297+
298+
@try {
299+
NSDictionary *dict = @{@"InvalidNonStringValue" : @1};
300+
fetcher.sessionUserInfo = dict;
301+
XCTFail(@"Shouldn't get here");
302+
} @catch (NSException *exception) {
303+
XCTAssertEqualObjects(exception.name, NSInvalidArgumentException);
304+
XCTAssertEqualObjects(exception.reason,
305+
@"Values in sessionUserInfo must be NSStrings: InvalidNonStringValue: 1");
306+
}
307+
XCTAssertNil(fetcher.sessionUserInfo);
308+
309+
NSDictionary *validDict = @{@"key" : @"value"};
310+
@try {
311+
fetcher.sessionUserInfo = validDict;
312+
} @catch (NSException *exception) {
313+
XCTFail(@"Shouldn't have gotten here: %@", exception);
314+
}
315+
XCTAssertEqualObjects(fetcher.sessionUserInfo, validDict);
316+
XCTAssertTrue(fetcher.sessionUserInfo == validDict); // Ptr equality, property is strong.
317+
}
318+
319+
- (void)testCreateSessionIdentifierWithMetadata {
320+
// Since `sessionUserInfo` is a `strong` property, the value can be modified after being set
321+
// and thus `createSessionIdentifierWithMetadata:` has to deal with late arriving invalide
322+
// values. Everything else should get encoded into the identifier.
323+
324+
GTMSessionFetcher *fetcher = [GTMSessionFetcher fetcherWithURLString:@"file://not_needed"];
325+
NSMutableDictionary *dict = [NSMutableDictionary dictionaryWithObject:@"GoodValue"
326+
forKey:@"GoodKey"];
327+
fetcher.sessionUserInfo = dict;
328+
329+
// Values that make it through end up in the identifier
330+
NSString *identifier = [fetcher createSessionIdentifierWithMetadata:nil];
331+
XCTAssertTrue([identifier containsString:@"{\"GoodKey\":\"GoodValue\"}"], @"%@", identifier);
332+
333+
[dict removeAllObjects];
334+
fetcher = [GTMSessionFetcher fetcherWithURLString:@"file://not_needed"];
335+
fetcher.sessionUserInfo = dict;
336+
[dict setObject:@"NonStringKey" forKey:@2];
337+
identifier = [fetcher createSessionIdentifierWithMetadata:nil];
338+
XCTAssertFalse([identifier containsString:@"NonStringKey"], @"%@", identifier);
339+
XCTAssertTrue(fetcher.sessionUserInfo == dict); // Ptr compared, dict still set.
340+
341+
[dict removeAllObjects];
342+
fetcher = [GTMSessionFetcher fetcherWithURLString:@"file://not_needed"];
343+
fetcher.sessionUserInfo = dict;
344+
[dict setObject:@"KeysCantHaveLeadingUnderscore" forKey:@"_InvalidKey"];
345+
identifier = [fetcher createSessionIdentifierWithMetadata:nil];
346+
XCTAssertFalse([identifier containsString:@"InvalidKey"], @"%@", identifier);
347+
XCTAssertFalse([identifier containsString:@"KeysCantHaveLeadingUnderscore"], @"%@", identifier);
348+
XCTAssertTrue(fetcher.sessionUserInfo == dict); // Ptr compared, dict still set.
349+
350+
[dict removeAllObjects];
351+
fetcher = [GTMSessionFetcher fetcherWithURLString:@"file://not_needed"];
352+
fetcher.sessionUserInfo = dict;
353+
[dict setObject:@1 forKey:@"ValuesMustBeStrings"];
354+
identifier = [fetcher createSessionIdentifierWithMetadata:nil];
355+
XCTAssertFalse([identifier containsString:@"ValuesMustBeStrings"], @"%@", identifier);
356+
XCTAssertTrue(fetcher.sessionUserInfo == dict); // Ptr compared, dict still set.
357+
}
358+
359+
- (void)testSessionUserInfoFromSessionIdentifierMetadata {
360+
NSDictionary *expected = @{@"GoodKey" : @"GoodValue"};
361+
NSMutableDictionary *dict = [expected mutableCopy];
362+
363+
TestIdentifierMetadataFecher *fetcher =
364+
[TestIdentifierMetadataFecher fetcherWithURLString:@"file://not_needed"];
365+
fetcher.testIdentifierMetadata = dict;
366+
XCTAssertEqualObjects(fetcher.sessionUserInfo, expected);
367+
368+
// Add these additions will get dropped
369+
370+
[dict setObject:@"NonStringKey" forKey:@1];
371+
fetcher = [TestIdentifierMetadataFecher fetcherWithURLString:@"file://not_needed"];
372+
fetcher.testIdentifierMetadata = dict;
373+
XCTAssertEqualObjects(fetcher.sessionUserInfo, expected);
374+
375+
[dict setObject:@"InvalidKey" forKey:@"_UnderscorePrefixedKey"];
376+
fetcher = [TestIdentifierMetadataFecher fetcherWithURLString:@"file://not_needed"];
377+
fetcher.testIdentifierMetadata = dict;
378+
XCTAssertEqualObjects(fetcher.sessionUserInfo, expected);
379+
380+
[dict setObject:@5 forKey:@"NonStringValue"];
381+
fetcher = [TestIdentifierMetadataFecher fetcherWithURLString:@"file://not_needed"];
382+
fetcher.testIdentifierMetadata = dict;
383+
XCTAssertEqualObjects(fetcher.sessionUserInfo, expected);
384+
}
385+
261386
#pragma mark - Fetcher Tests
262387

263388
- (void)assertSuccessfulGettysburgFetchWithFetcher:(GTMSessionFetcher *)fetcher

0 commit comments

Comments
 (0)