Merge pull request #2756 from spevans/pr_max_http_redirects

URLSession: Add a limit on the number of HTTP Redirects that are followed

Author: spevans

Sources/FoundationNetworking/URLSession/HTTP/HTTPURLProtocol.swift

@@ -25,6 +25,7 @@ internal class _HTTPURLProtocol: _NativeProtocol {
     // and the 3xx response is returned to the client and the data is sent via the delegate notify
     // mechanism. `lastRedirectBody` holds the body of the redirect currently being processed.
     var lastRedirectBody: Data? = nil
+    private var redirectCount = 0
 
     public required init(task: URLSessionTask, cachedResponse: CachedURLResponse?, client: URLProtocolClient?) {
         super.init(task: task, cachedResponse: cachedResponse, client: client)
@@ -440,13 +441,24 @@ internal class _HTTPURLProtocol: _NativeProtocol {
     }
 
     override func redirectFor(request: URLRequest) {
-        //TODO: Should keep track of the number of redirects that this
-        // request has gone through and err out once it's too large, i.e.
-        // call into `failWith(errorCode: )` with NSURLErrorHTTPTooManyRedirects
         guard case .transferCompleted(response: let response, bodyDataDrain: let bodyDataDrain) = self.internalState else {
             fatalError("Trying to redirect, but the transfer is not complete.")
         }
 
+        // Avoid a never ending redirect chain by having a hard limit on the number of redirects.
+        // This value mirrors Darwin.
+        redirectCount += 1
+        if redirectCount > 16 {
+            self.internalState = .transferFailed
+            let error = NSError(domain: NSURLErrorDomain, code: NSURLErrorHTTPTooManyRedirects,
+                                userInfo: [NSLocalizedDescriptionKey: "too many HTTP redirects"])
+            guard let request = task?.currentRequest else {
+                fatalError("In a redirect chain but no current task/request")
+            }
+            failWith(error: error, request: request)
+            return
+        }
+
         guard let session = task?.session as? URLSession else { fatalError() }
         switch session.behaviour(for: task!) {
         case .taskDelegate(let delegate):

Tests/Foundation/Tests/TestURLSession.swift

@@ -798,6 +798,47 @@ class TestURLSession: LoopbackServerTest {
         }
     }
 
+    func test_httpRedirectionExceededMaxRedirects() throws {
+        let expectedMaxRedirects = 16
+        let urlString = "http://127.0.0.1:\(TestURLSession.serverPort)/redirect/99"
+        let url = try XCTUnwrap(URL(string: urlString))
+        let exceededCountUrlString = "http://127.0.0.1:\(TestURLSession.serverPort)/redirect/\(99 - expectedMaxRedirects)"
+        let exceededCountUrl = try XCTUnwrap(URL(string: exceededCountUrlString))
+
+        for method in httpMethods {
+            var request = URLRequest(url: url)
+            request.httpMethod = method
+            let delegate = SessionDelegate(with: expectation(description: "\(method) \(urlString): with HTTP redirection"))
+
+            var redirectRequests: [(HTTPURLResponse, URLRequest)] = []
+            delegate.redirectionHandler = { (response: HTTPURLResponse, request: URLRequest, completionHandler: @escaping (URLRequest?) -> Void) in
+                redirectRequests.append((response, request))
+                completionHandler(request)
+            }
+            delegate.run(with: request, timeoutInterval: 5)
+            waitForExpectations(timeout: 20)
+
+            XCTAssertNil(delegate.response)
+            XCTAssertNil(delegate.receivedData)
+
+            XCTAssertNotNil(delegate.error)
+            let error = delegate.error as? URLError
+            XCTAssertEqual(error?.code.rawValue, NSURLErrorHTTPTooManyRedirects)
+            XCTAssertEqual(error?.localizedDescription, "too many HTTP redirects")
+            let userInfo = error?.userInfo as? [String: Any]
+            let errorURL = userInfo?[NSURLErrorFailingURLErrorKey] as? URL
+            XCTAssertEqual(errorURL, exceededCountUrl)
+
+            // Check the last Redirection response/request received.
+            XCTAssertEqual(redirectRequests.count, expectedMaxRedirects)
+            let lastResponse = redirectRequests.last?.0
+            let lastRequest = redirectRequests.last?.1
+
+            XCTAssertEqual(lastResponse?.statusCode, 302)
+            XCTAssertEqual(lastRequest?.url, exceededCountUrl)
+        }
+    }
+
     func test_httpNotFound() throws {
         let urlString = "http://127.0.0.1:\(TestURLSession.serverPort)/404"
         let url = try XCTUnwrap(URL(string: urlString))
@@ -1691,6 +1732,7 @@ class TestURLSession: LoopbackServerTest {
             ("test_httpRedirectionWithDefaultPort", test_httpRedirectionWithDefaultPort),
             ("test_httpRedirectionTimeout", test_httpRedirectionTimeout),
             ("test_httpRedirectionChainInheritsTimeoutInterval", test_httpRedirectionChainInheritsTimeoutInterval),
+            ("test_httpRedirectionExceededMaxRedirects", test_httpRedirectionExceededMaxRedirects),
             ("test_httpNotFound", test_httpNotFound),
             ("test_http0_9SimpleResponses", test_http0_9SimpleResponses),
             ("test_outOfRangeButCorrectlyFormattedHTTPCode", test_outOfRangeButCorrectlyFormattedHTTPCode),