Merge pull request #1001 from nethraravindran/httpRedirection-branch

[SR-2679] Fix for URLSession delivers redirection delegate methods in wrong order

Author: Pushkar N Kulkarni

Foundation/NSURLSession/http/HTTPURLProtocol.swift

@@ -776,10 +776,15 @@ internal extension _HTTPURLProtocol {
             // For now, we'll notify the delegate, but won't pause the transfer,
             // and we'll disregard the completion handler:
             guard let s = session as? URLSession else { fatalError() }
-            s.delegateQueue.addOperation {
-                delegate.urlSession(s, dataTask: dt, didReceive: response, completionHandler: { _ in
-                    URLSession.printDebug("warning: Ignoring disposition from completion handler.")
-                })
+            switch response.statusCode {
+            case 301, 302, 303, 307:
+                break
+            default:
+                s.delegateQueue.addOperation {
+                    delegate.urlSession(s, dataTask: dt, didReceive: response, completionHandler: { _ in
+                        URLSession.printDebug("warning: Ignoring disposition from completion handler.")
+                    })
+                }
             }
         case .taskDelegate:
             break
@@ -859,7 +864,7 @@ internal extension _HTTPURLProtocol {
         //TODO: Do we ever want to redirect for HEAD requests?
         func methodAndURL() -> (String, URL)? {
             guard
-                let location = response.value(forHeaderField: .location),
+                let location = response.value(forHeaderField: .location, response: response),
                 let targetURL = URL(string: location)
                 else {
                     // Can't redirect when there's no location to redirect to.
@@ -882,8 +887,24 @@ internal extension _HTTPURLProtocol {
         guard let (method, targetURL) = methodAndURL() else { return nil }
         var request = fromRequest
         request.httpMethod = method
-        request.url = targetURL
-        return request
+ 
+        // If targetURL has only relative path of url, create a new valid url with relative path
+        // Otherwise, return request with targetURL ie.url from location field
+        guard targetURL.scheme == nil || targetURL.host == nil else {
+            request.url = targetURL
+            return request
+        }
+    
+        let scheme = request.url?.scheme
+        let host = request.url?.host
+
+        var components = URLComponents()
+        components.scheme = scheme
+        components.host = host
+        components.path = targetURL.relativeString
+        guard let urlString = components.string else { fatalError("Invalid URL") }
+        request.url = URL(string: urlString)
+	return request
     }
 }
 
@@ -894,7 +915,12 @@ fileprivate extension HTTPURLResponse {
         /// - SeeAlso: RFC 2616 section 14.30 <https://tools.ietf.org/html/rfc2616#section-14.30>
         case location = "Location"
     }
-    func value(forHeaderField field: _Field) -> String? {
-        return field.rawValue
+    func value(forHeaderField field: _Field, response: HTTPURLResponse?) -> String? {
+        let value = field.rawValue
+	guard let response = response else { fatalError("Response is nil") }
+        if let location = response.allHeaderFields[value] as? String {
+            return location
+        }
+        return nil
     }
 }

TestFoundation/HTTPServer.swift

@@ -207,6 +207,7 @@ struct _HTTPRequest {
 struct _HTTPResponse {
     enum Response : Int {
         case OK = 200
+        case REDIRECT = 302
     }
     private let responseCode: Response
     private let headers: String
@@ -229,6 +230,7 @@ public class TestURLSessionServer {
                                      "Peru":"Lima",
                                      "Italy":"Rome",
                                      "USA":"Washington, D.C.",
+				     "UnitedStates": "USA",
                                      "country.txt": "A country is a region that is identified as a distinct national entity in political geography"]
     let httpServer: _HTTPServer
     let startDelay: TimeInterval?
@@ -269,6 +271,18 @@ public class TestURLSessionServer {
             let text = request.getCommaSeparatedHeaders()
             return _HTTPResponse(response: .OK, headers: "Content-Length: \(text.characters.count)", body: text)
         }
+
+	if uri == "/UnitedStates" {
+            let value = capitals[String(uri.characters.dropFirst())]!
+            let text = request.getCommaSeparatedHeaders()
+            let host = request.headers[1].components(separatedBy: " ")[1]
+            let ip = host.components(separatedBy: ":")[0]
+            let port = host.components(separatedBy: ":")[1]
+            let newPort = Int(port)! + 1
+            let newHost = ip + ":" + String(newPort)
+            let httpResponse = _HTTPResponse(response: .REDIRECT, headers: "Location: http://\(newHost + "/" + value)", body: text)
+            return httpResponse 
+        }
         return _HTTPResponse(response: .OK, body: capitals[String(uri.characters.dropFirst())]!) 
     }
 

TestFoundation/TestNSURLSession.swift

@@ -39,6 +39,7 @@ class TestURLSession : XCTestCase {
             ("test_verifyHttpAdditionalHeaders", test_verifyHttpAdditionalHeaders),
             ("test_timeoutInterval", test_timeoutInterval),
 	    ("test_customProtocol", test_customProtocol),
+	    ("test_httpRedirection", test_httpRedirection),
         ]
     }
 
@@ -467,6 +468,24 @@ class TestURLSession : XCTestCase {
         task.resume()
         waitForExpectations(timeout: 12)
     }
+
+    func test_httpRedirection() {
+        let serverReady = ServerSemaphore()
+        globalDispatchQueue.async {
+            do {
+                try self.runServer(with: serverReady)
+            } catch {
+                XCTAssertTrue(true)
+                return
+            }
+        }
+        serverReady.wait()
+        let urlString = "http://127.0.0.1:\(serverPort)/UnitedStates"
+        let url = URL(string: urlString)!
+        let d = HTTPRedirectionDataTask(with: expectation(description: "data task"))
+        d.run(with: url)
+        waitForExpectations(timeout: 12)
+    }
 }
 
 class SessionDelegate: NSObject, URLSessionDelegate {
@@ -516,14 +535,13 @@ class DataTask : NSObject {
 extension DataTask : URLSessionDataDelegate {
     public func urlSession(_ session: URLSession, dataTask: URLSessionDataTask, didReceive data: Data) {
         capital = String(data: data, encoding: String.Encoding.utf8)!
-        dataTaskExpectation.fulfill()
     }
 }
 
 extension DataTask : URLSessionTaskDelegate {
     public func urlSession(_ session: URLSession, task: URLSessionTask, didCompleteWithError error: Error?) {
-         guard (error as? URLError) != nil else { return }
          dataTaskExpectation.fulfill()
+	 guard (error as? URLError) != nil else { return }
          if let cancellation = cancelExpectation {
              cancellation.fulfill()
          }
@@ -608,3 +626,61 @@ class CustomProtocol : URLProtocol {
         return
     }
 }
+
+class HTTPRedirectionDataTask : NSObject {
+    let dataTaskExpectation: XCTestExpectation!
+    var session: URLSession! = nil
+    var task: URLSessionDataTask! = nil
+    var cancelExpectation: XCTestExpectation?
+
+    public var error = false
+
+    init(with expectation: XCTestExpectation) {
+        dataTaskExpectation = expectation
+    }
+
+    func run(with request: URLRequest) {
+        let config = URLSessionConfiguration.default
+        config.timeoutIntervalForRequest = 8
+        session = URLSession(configuration: config, delegate: self, delegateQueue: nil)
+        task = session.dataTask(with: request)
+        task.resume()
+    }
+
+    func run(with url: URL) {
+        let config = URLSessionConfiguration.default
+        config.timeoutIntervalForRequest = 8
+        session = URLSession(configuration: config, delegate: self, delegateQueue: nil)
+        task = session.dataTask(with: url)
+        task.resume()
+    }
+
+    func cancel() {
+        task.cancel()
+    }
+}
+
+extension HTTPRedirectionDataTask : URLSessionDataDelegate {
+    public func urlSession(_ session: URLSession, dataTask: URLSessionDataTask, didReceive response: URLResponse, completionHandler: @escaping (URLSession.ResponseDisposition) -> Void) {
+        guard let httpresponse = response as? HTTPURLResponse else { fatalError() }
+	XCTAssertNotNil(response)
+        XCTAssertEqual(200, httpresponse.statusCode, "HTTP response code is not 200")
+    }
+}
+
+extension HTTPRedirectionDataTask : URLSessionTaskDelegate {
+    public func urlSession(_ session: URLSession, task: URLSessionTask, didCompleteWithError error: Error?) {
+        dataTaskExpectation.fulfill()
+        guard (error as? URLError) != nil else { return }
+        if let cancellation = cancelExpectation {
+            cancellation.fulfill()
+        }
+        self.error = true
+    }
+
+    public func urlSession(_ session: URLSession, task: URLSessionTask, willPerformHTTPRedirection response: HTTPURLResponse, newRequest request: URLRequest, completionHandler: @escaping (URLRequest?) -> Void) {
+        XCTAssertNotNil(response)
+        XCTAssertEqual(302, response.statusCode, "HTTP response code is not 302")
+        completionHandler(request)
+    }
+}