From 5b24c936a0c0cdb85f6737db8b70e0fc2e727c18 Mon Sep 17 00:00:00 2001
From: Thomas Van Lenten <thomasvl@google.com>
Date: Thu, 8 Feb 2024 15:20:58 -0500
Subject: [PATCH] Update test for iOS 17 (and related OSes) NSURL changes.

+[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
---
 UnitTests/GTMSessionFetcherFetchingTest.m | 24 +++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/UnitTests/GTMSessionFetcherFetchingTest.m b/UnitTests/GTMSessionFetcherFetchingTest.m
index 6a68fec..c98a41e 100644
--- a/UnitTests/GTMSessionFetcherFetchingTest.m
+++ b/UnitTests/GTMSessionFetcherFetchingTest.m
@@ -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]]);
   }
 }