Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce the types for sessionUserInfo. #433

Merged
merged 1 commit into from
Jan 29, 2025
+191 −52
Merged

Enforce the types for sessionUserInfo. #433

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 66 additions & 52 deletions Sources/Core/GTMSessionFetcher.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1594,56 +1594,65 @@ - (void)setSessionIdentifierInternal:(nullable NSString *)sessionIdentifier {
if (_sessionUserInfo == nil) {
// We'll return the metadata dictionary with internal keys removed. This avoids the user
// re-using the userInfo dictionary later and accidentally including the internal keys.
// Just incase something got corrupted in storage and parsed back out differently, ensure
// the api contract on types is still valid.
NSMutableDictionary *metadata = [[self sessionIdentifierMetadataUnsynchronized] mutableCopy];
NSSet *keysToRemove = [metadata keysOfEntriesPassingTest:^BOOL(id key, id obj, BOOL *stop) {
return [key hasPrefix:@"_"];
return ![key isKindOfClass:[NSString class]] || ![obj isKindOfClass:[NSString class]] ||
[key hasPrefix:@"_"];
}];
[metadata removeObjectsForKeys:[keysToRemove allObjects]];
if (metadata.count > 0) {
_sessionUserInfo = metadata;

#if DEBUG
// If we're pruning, give warnings about the things that were invalid as some bug has slipped
// through.
if (keysToRemove.count) {
[metadata enumerateKeysAndObjectsUsingBlock:^(id _Nonnull key, id _Nonnull obj,
BOOL *_Nonnull stop) {
GTMSESSION_ASSERT_DEBUG([key isKindOfClass:[NSString class]],
@"sessionUserInfo keys must be NSStrings: %@", key);
if (![obj isKindOfClass:[NSString class]]) {
GTMSESSION_LOG_DEBUG(@"WARNING: sessionUserInfo included a non String value, this will "
@"be an error in the future: %@: %@",
key, obj);
if (![key isKindOfClass:[NSString class]]) {
GTMSESSION_LOG_DEBUG(
@"InternalError: restoring sessionUserInfo is pruning a non String key: %@", key);
} else if (![obj isKindOfClass:[NSString class]]) {
GTMSESSION_LOG_DEBUG(
@"InternalError: restoring sessionUserInfo is pruning a non String value: %@: %@",
key, obj);
}
}];
}
#endif // DEBUG
[metadata removeObjectsForKeys:[keysToRemove allObjects]];

if (metadata.count > 0) {
_sessionUserInfo = metadata;
}
}
return _sessionUserInfo;
} // @synchronized(self)
}

- (void)setSessionUserInfo:(nullable NSDictionary<NSString *, NSString *> *)dictionary {
[dictionary enumerateKeysAndObjectsUsingBlock:^(id _Nonnull key, id _Nonnull obj,
BOOL *_Nonnull stop) {
if (![key isKindOfClass:[NSString class]]) {
[NSException raise:NSInvalidArgumentException
format:@"sessionUserInfo keys must be NSStrings: %@", key];
}
if ([key hasPrefix:@"_"]) {
[NSException
raise:NSInvalidArgumentException
format:
@"sessionUserInfo keys starting with an underscore are reserved for the library: %@",
key];
}
if (![obj isKindOfClass:[NSString class]]) {
[NSException raise:NSInvalidArgumentException
format:@"Values in sessionUserInfo must be NSStrings: %@: %@", key, obj];
}
}];

@synchronized(self) {
GTMSessionMonitorSynchronized(self);

GTMSESSION_ASSERT_DEBUG(_sessionIdentifier == nil, @"Too late to assign userInfo");
_sessionUserInfo = dictionary;

#if DEBUG
[dictionary enumerateKeysAndObjectsUsingBlock:^(id _Nonnull key, id _Nonnull obj,
BOOL *_Nonnull stop) {
GTMSESSION_ASSERT_DEBUG([key isKindOfClass:[NSString class]],
@"sessionUserInfo keys must be NSStrings: %@", key);
if ([key hasPrefix:@"_"]) {
GTMSESSION_LOG_DEBUG(@"WARNING: sessionUserInfo keys starting with an underscore are "
@"reserved for the library, this will become an error in the future: "
@"%@: %@", key, obj);
}
if (![obj isKindOfClass:[NSString class]]) {
GTMSESSION_LOG_DEBUG(@"WARNING: sessionUserInfo included a non String value, this will be "
@"an error in the future: %@: %@",
key, obj);
}
}];
#endif // DEBUG
} // @synchronized(self)
}

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

#if DEBUG
// _sessionUserInfo was declared as `strong` (not `copy`, so it could have been modifed after
// having been set.
[_sessionUserInfo enumerateKeysAndObjectsUsingBlock:^(id _Nonnull key, id _Nonnull obj,
BOOL *_Nonnull stop) {
GTMSESSION_ASSERT_DEBUG([key isKindOfClass:[NSString class]],
@"sessionUserInfo keys must be NSStrings: %@", key);
if ([key hasPrefix:@"_"]) {
GTMSESSION_LOG_DEBUG(@"WARNING: sessionUserInfo keys starting with an underscore are "
@"reserved for the library, this will become an error in the future: "
@"%@: %@", key, obj);
}
if (![obj isKindOfClass:[NSString class]]) {
GTMSESSION_LOG_DEBUG(@"WARNING: sessionUserInfo included a non String value, this will "
@"be an error in the future: %@: %@",
key, obj);
}
}];
#endif // DEBUG

// Start with user-supplied keys so they cannot accidentally override the fetcher's keys.
NSMutableDictionary *metadataDict =
[NSMutableDictionary dictionaryWithDictionary:(NSDictionary *_Nonnull)_sessionUserInfo];

// sessionUserInfo was declared as `strong` (not `copy`), so it could have been modifed after
// having been set. So remove anything that breaks the contract.
NSSet *keysToRemove = [metadataDict keysOfEntriesPassingTest:^BOOL(id key, id obj, BOOL *stop) {
return ![key isKindOfClass:[NSString class]] || ![obj isKindOfClass:[NSString class]] ||
[key hasPrefix:@"_"];
}];
#if DEBUG
// If we're pruning, give warnings about the things that were invalid as some bug has slipped
// through.
if (keysToRemove.count) {
[metadataDict enumerateKeysAndObjectsUsingBlock:^(id _Nonnull key, id _Nonnull obj,
BOOL *_Nonnull stop) {
if (![key isKindOfClass:[NSString class]]) {
GTMSESSION_LOG_DEBUG(
@"Warning: sessionUserInfo has been modifed to include a non String key: %@", key);
} else if (![obj isKindOfClass:[NSString class]]) {
GTMSESSION_LOG_DEBUG(
@"Warning: sessionUserInfo has been modifed to include a non String value: %@: %@",
key, obj);
}
}];
}
#endif // DEBUG
[metadataDict removeObjectsForKeys:[keysToRemove allObjects]];

if (metadataToInclude) {
#if DEBUG
[metadataToInclude enumerateKeysAndObjectsUsingBlock:^(id _Nonnull key, id _Nonnull obj,
BOOL * _Nonnull stop) {
[metadataToInclude enumerateKeysAndObjectsUsingBlock:^(id _Nonnull key, id _Nonnull obj,
BOOL *_Nonnull stop) {
GTMSESSION_ASSERT_DEBUG([key isKindOfClass:[NSString class]],
@"metadataToInclude keys must be NSStrings: %@", key);
GTMSESSION_ASSERT_DEBUG([key hasPrefix:@"_"],
Expand All @@ -1774,7 +1788,7 @@ - (NSString *)createSessionIdentifierWithMetadata:(nullable NSDictionary *)metad
NSDictionary *defaultMetadataDict = [self sessionIdentifierDefaultMetadata];
if (defaultMetadataDict) {
#if DEBUG
[defaultMetadataDict enumerateKeysAndObjectsUsingBlock:^(id _Nonnull key, id _Nonnull obj,
[defaultMetadataDict enumerateKeysAndObjectsUsingBlock:^(id _Nonnull key, id _Nonnull obj,
BOOL * _Nonnull stop) {
GTMSESSION_ASSERT_DEBUG([key isKindOfClass:[NSString class]],
@"defaultMetadataDict keys must be NSStrings: %@", key);
Expand Down
125 changes: 125 additions & 0 deletions UnitTests/GTMSessionFetcherFetchingTest.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,17 @@
@interface GTMSessionFetcher (ExposedForTesting)
+ (nullable NSURL *)redirectURLWithOriginalRequestURL:(nullable NSURL *)originalRequestURL
redirectRequestURL:(nullable NSURL *)redirectRequestURL;
- (NSString *)createSessionIdentifierWithMetadata:(NSDictionary *)metadata;
@end

@interface TestIdentifierMetadataFecher : GTMSessionFetcher
@property(strong, nullable) NSDictionary *testIdentifierMetadata;
@end

@implementation TestIdentifierMetadataFecher
- (nullable NSDictionary *)sessionIdentifierMetadataUnsynchronized {
return self.testIdentifierMetadata;
}
@end

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

@implementation GTMSessionFetcherFetchingTest

#pragma mark - SessionUserInfo Tests

- (void)testSetSessionUserInfo {
GTMSessionFetcher *fetcher = [GTMSessionFetcher fetcherWithURLString:@"file://not_needed"];

@try {
NSDictionary *dict = @{@1 : @"invalid non string key"};
fetcher.sessionUserInfo = dict;
XCTFail(@"Shouldn't get here");
} @catch (NSException *exception) {
XCTAssertEqualObjects(exception.name, NSInvalidArgumentException);
XCTAssertEqualObjects(exception.reason, @"sessionUserInfo keys must be NSStrings: 1");
}
XCTAssertNil(fetcher.sessionUserInfo);

// Key with a leading underscore
@try {
fetcher.sessionUserInfo = @{@"_invalidUnderscore" : @"value"};
XCTFail(@"Shouldn't get here");
} @catch (NSException *exception) {
XCTAssertEqualObjects(exception.name, NSInvalidArgumentException);
XCTAssertEqualObjects(exception.reason, @"sessionUserInfo keys starting with an underscore are "
@"reserved for the library: _invalidUnderscore");
}
XCTAssertNil(fetcher.sessionUserInfo);

@try {
NSDictionary *dict = @{@"InvalidNonStringValue" : @1};
fetcher.sessionUserInfo = dict;
XCTFail(@"Shouldn't get here");
} @catch (NSException *exception) {
XCTAssertEqualObjects(exception.name, NSInvalidArgumentException);
XCTAssertEqualObjects(exception.reason,
@"Values in sessionUserInfo must be NSStrings: InvalidNonStringValue: 1");
}
XCTAssertNil(fetcher.sessionUserInfo);

NSDictionary *validDict = @{@"key" : @"value"};
@try {
fetcher.sessionUserInfo = validDict;
} @catch (NSException *exception) {
XCTFail(@"Shouldn't have gotten here: %@", exception);
}
XCTAssertEqualObjects(fetcher.sessionUserInfo, validDict);
XCTAssertTrue(fetcher.sessionUserInfo == validDict); // Ptr equality, property is strong.
}

- (void)testCreateSessionIdentifierWithMetadata {
// Since `sessionUserInfo` is a `strong` property, the value can be modified after being set
// and thus `createSessionIdentifierWithMetadata:` has to deal with late arriving invalide
// values. Everything else should get encoded into the identifier.

GTMSessionFetcher *fetcher = [GTMSessionFetcher fetcherWithURLString:@"file://not_needed"];
NSMutableDictionary *dict = [NSMutableDictionary dictionaryWithObject:@"GoodValue"
forKey:@"GoodKey"];
fetcher.sessionUserInfo = dict;

// Values that make it through end up in the identifier
NSString *identifier = [fetcher createSessionIdentifierWithMetadata:nil];
XCTAssertTrue([identifier containsString:@"{\"GoodKey\":\"GoodValue\"}"], @"%@", identifier);

[dict removeAllObjects];
fetcher = [GTMSessionFetcher fetcherWithURLString:@"file://not_needed"];
fetcher.sessionUserInfo = dict;
[dict setObject:@"NonStringKey" forKey:@2];
identifier = [fetcher createSessionIdentifierWithMetadata:nil];
XCTAssertFalse([identifier containsString:@"NonStringKey"], @"%@", identifier);
XCTAssertTrue(fetcher.sessionUserInfo == dict); // Ptr compared, dict still set.

[dict removeAllObjects];
fetcher = [GTMSessionFetcher fetcherWithURLString:@"file://not_needed"];
fetcher.sessionUserInfo = dict;
[dict setObject:@"KeysCantHaveLeadingUnderscore" forKey:@"_InvalidKey"];
identifier = [fetcher createSessionIdentifierWithMetadata:nil];
XCTAssertFalse([identifier containsString:@"InvalidKey"], @"%@", identifier);
XCTAssertFalse([identifier containsString:@"KeysCantHaveLeadingUnderscore"], @"%@", identifier);
XCTAssertTrue(fetcher.sessionUserInfo == dict); // Ptr compared, dict still set.

[dict removeAllObjects];
fetcher = [GTMSessionFetcher fetcherWithURLString:@"file://not_needed"];
fetcher.sessionUserInfo = dict;
[dict setObject:@1 forKey:@"ValuesMustBeStrings"];
identifier = [fetcher createSessionIdentifierWithMetadata:nil];
XCTAssertFalse([identifier containsString:@"ValuesMustBeStrings"], @"%@", identifier);
XCTAssertTrue(fetcher.sessionUserInfo == dict); // Ptr compared, dict still set.
}

- (void)testSessionUserInfoFromSessionIdentifierMetadata {
NSDictionary *expected = @{@"GoodKey" : @"GoodValue"};
NSMutableDictionary *dict = [expected mutableCopy];

TestIdentifierMetadataFecher *fetcher =
[TestIdentifierMetadataFecher fetcherWithURLString:@"file://not_needed"];
fetcher.testIdentifierMetadata = dict;
XCTAssertEqualObjects(fetcher.sessionUserInfo, expected);

// Add these additions will get dropped

[dict setObject:@"NonStringKey" forKey:@1];
fetcher = [TestIdentifierMetadataFecher fetcherWithURLString:@"file://not_needed"];
fetcher.testIdentifierMetadata = dict;
XCTAssertEqualObjects(fetcher.sessionUserInfo, expected);

[dict setObject:@"InvalidKey" forKey:@"_UnderscorePrefixedKey"];
fetcher = [TestIdentifierMetadataFecher fetcherWithURLString:@"file://not_needed"];
fetcher.testIdentifierMetadata = dict;
XCTAssertEqualObjects(fetcher.sessionUserInfo, expected);

[dict setObject:@5 forKey:@"NonStringValue"];
fetcher = [TestIdentifierMetadataFecher fetcherWithURLString:@"file://not_needed"];
fetcher.testIdentifierMetadata = dict;
XCTAssertEqualObjects(fetcher.sessionUserInfo, expected);
}

#pragma mark - Fetcher Tests

- (void)assertSuccessfulGettysburgFetchWithFetcher:(GTMSessionFetcher *)fetcher
Expand Down