Skip to content

Commit

Permalink
Update test for iOS 17 (and related OSes) NSURL changes. (#378)
Browse files Browse the repository at this point in the history
+[NSURL URLWithString:] changed with iOS 17 (and the related OSes), it used
to always fail (return nil) for some invalid urls. The test depended on that
behavior. Now NSURL will escape the characters that would have failed things
before.

Since the test is calling an internal helper it will only ever see urls that
return in redirect responses from servers. So there is no real way a server
should ever return something matched the test case. Rather that change the
logic in the handling, revise the test to explicitly test the nil case that
was desired.

If there ever is a concern that a server might get some other redirect from
"secure" to "insecure", then we can revise the logic at that time. (it would
only be around protocol changes)

Fixes #368
  • Loading branch information
thomasvl committed Feb 8, 2024
1 parent 0498f59 commit 79653ec
Showing 1 changed file with 16 additions and 8 deletions.
24 changes: 16 additions & 8 deletions UnitTests/GTMSessionFetcherFetchingTest.m
Original file line number Diff line number Diff line change
Expand Up @@ -2769,20 +2769,28 @@ - (void)testFetcherRedirectURLHandling {
@[ @"http://original_host/", @"https://redirect_host/", @"https://redirect_host/" ],
// Secure to insecure = disallowed.
@[ @"https://original_host/", @"http://redirect_host/", @"https://redirect_host/" ],
// Arbitrary change = disallowed.
// Arbitrary change = disallowed. This really shouldn't happen since there
// would be a redirect from a server to a different protocol.
@[ @"http://original_host/", @"fake://redirect_host/", @"http://redirect_host/" ],
// Validate the behavior of nil URLs in the redirect. This should not happen under
// real conditions, but if one of the redirect URLs are nil, the other one should
// always be returned. For these tests, use a string that will not parse to a URL
// due to invalid characters (the backslash \).
@[ @"invalid:\\url", @"https://redirect_host/", @"https://redirect_host/" ],
@[ @"http://original_host/", @"invalid:\\url", @"http://original_host/" ],
// real conditions, but iOS 17 (and the related OSes) changes their behavior for
// +[NSURL URLWithString:] for invalid characters, and what used to be a otherwise
// malformed URL now gets the characters encoded. This maintains the testing of
// the internal helper for these cases, but since the helper is only calls from
// an NSURLSession redirect handing, that path should never really see these
// sort of cases.
@[ @"[nil]", @"https://redirect_host/", @"https://redirect_host/" ],
@[ @"http://original_host/", @"[nil]", @"http://original_host/" ],
];

NSURL *(^toNSURL)(NSString*) = ^NSURL*(NSString *s) {
return [s isEqual:@"[nil]"] ? nil : [NSURL URLWithString:s];
};

for (NSArray<NSString *> *testCase in testCases) {
NSURL *redirectURL =
[GTMSessionFetcher redirectURLWithOriginalRequestURL:[NSURL URLWithString:testCase[0]]
redirectRequestURL:[NSURL URLWithString:testCase[1]]];
[GTMSessionFetcher redirectURLWithOriginalRequestURL:toNSURL(testCase[0])
redirectRequestURL:toNSURL(testCase[1])];
XCTAssertEqualObjects(redirectURL, [NSURL URLWithString:testCase[2]]);
}
}
Expand Down

0 comments on commit 79653ec

Please sign in to comment.