From e7a517e35997bd982d13e99b1dccbdd1c96015c4 Mon Sep 17 00:00:00 2001 From: Pushkar N Kulkarni Date: Fri, 3 May 2019 07:08:30 +0100 Subject: [PATCH] Don't fail a URLSessionTask on a 401 response The basic authentication implementation fails a URLSessionTask with an error if a status code of 401 is received but we aren't able to build a URLProtectionSpace, possibly due to the absence of a the WWW-Authenticate response header. A valid HTTP response object is received in this case. It may not possible to go ahead with the `urlSession(_,task: task, didReceive:)` callback, either because a delegate isn't being used or because the `WWW-Authenticate` response header is absent. In such cases, the completion handler is invoked with the received response, NOT with an error. Note that basic authentication (https://github.com/apple/swift-corelibs-foundation/commit/4c1e8c4271c4e21000528 d3910f20a958d03f308#diff-14c6cb724eead661596e4468ae5ca638) wasn't available with Swift 4.x. It was first made available in development builds for Swift 5. Initially, when we failed to build a URLProtectionSpace, we failed with an error, which resulted in removing the task from the TaskRegistry. Further we went ahead and invoked the completion handler, which again tried to remove the same task from the TaskRegistry, resulting in a fatal error reported against Swift 5.0. As a fix to this, we decided to simply return after failing with an error (see https://github.com/apple/swift-corelibs-foundation/commit/69abd6dd396e1985a882a9 a39079801f84658fb3#diff-14c6cb724eead661596e4468ae5ca638) which means we skipped invoking the completion handler. It seems that the right thing to do is NOT fail with an error, but simply invoke the completion handler with the 401 response received. --- Foundation/URLSession/URLSessionTask.swift | 5 +---- TestFoundation/TestURLSession.swift | 5 +++-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/Foundation/URLSession/URLSessionTask.swift b/Foundation/URLSession/URLSessionTask.swift index f6f5fc128b..494c219b5b 100644 --- a/Foundation/URLSession/URLSessionTask.swift +++ b/Foundation/URLSession/URLSessionTask.swift @@ -574,11 +574,8 @@ extension _ProtocolClient : URLProtocolClient { sender: `protocol` as! _HTTPURLProtocol) task.previousFailureCount += 1 urlProtocol(`protocol`, didReceive: authenticationChallenge) - } else { - let urlError = URLError(_nsError: NSError(domain: NSURLErrorDomain, code: NSURLErrorUserAuthenticationRequired, userInfo: nil)) - urlProtocol(`protocol`, didFailWithError: urlError) + return } - return } switch session.behaviour(for: task) { case .taskDelegate(let delegate): diff --git a/TestFoundation/TestURLSession.swift b/TestFoundation/TestURLSession.swift index 48f8265fdb..4e5943926d 100644 --- a/TestFoundation/TestURLSession.swift +++ b/TestFoundation/TestURLSession.swift @@ -766,9 +766,10 @@ class TestURLSession : LoopbackServerTest { let expect = expectation(description: "GET \(urlString): with a completion handler") var expectedResult = "unknown" let session = URLSession(configuration: URLSessionConfiguration.default) - let task = session.dataTask(with: url) { _, _, error in + let task = session.dataTask(with: url) { _, response, error in defer { expect.fulfill() } - XCTAssertNotNil(error) + XCTAssertNotNil(response) + XCTAssertNil(error) } task.resume() waitForExpectations(timeout: 12, handler: nil)