Merge pull request #39 from cryptomator/feature/fix-webdav-task-registration-race

Fix WebDAV task registration race causing stuck downloads and uploads

Author: tobihagemann

Sources/CryptomatorCloudAccess/WebDAV/WebDAVSession.swift

@@ -221,12 +221,14 @@ class WebDAVSession {
 		HTTPDebugLogger.logRequest(request)
 		let progress = Progress(totalUnitCount: 1)
 		let task = urlSession.downloadTask(with: request)
-		onTaskCreation?(task)
 		progress.addChild(task.progress, withPendingUnitCount: 1)
 		let pendingPromise = Promise<HTTPURLResponse>.pending()
 		let webDAVDownloadTask = WebDAVDownloadTask(promise: pendingPromise, localURL: localURL)
+		// Register before invoking `onTaskCreation` so a synchronous resume cannot race the delegate's lookup.
 		delegate?.addRunningDownloadTask(key: task, value: webDAVDownloadTask)
-		if onTaskCreation == nil {
+		if let onTaskCreation {
+			onTaskCreation(task)
+		} else {
 			task.resume()
 		}
 		return pendingPromise
@@ -236,12 +238,14 @@ class WebDAVSession {
 		HTTPDebugLogger.logRequest(request)
 		let progress = Progress(totalUnitCount: 1)
 		let task = urlSession.uploadTask(with: request, fromFile: fileURL)
-		onTaskCreation?(task)
 		progress.addChild(task.progress, withPendingUnitCount: 1)
 		let pendingPromise = Promise<(HTTPURLResponse, Data?)>.pending()
 		let webDAVDataTask = WebDAVDataTask(promise: pendingPromise)
+		// Register before invoking `onTaskCreation` so a synchronous resume cannot race the delegate's lookup.
 		delegate?.addRunningDataTask(key: task, value: webDAVDataTask)
-		if onTaskCreation == nil {
+		if let onTaskCreation {
+			onTaskCreation(task)
+		} else {
 			task.resume()
 		}
 		return pendingPromise

Tests/CryptomatorCloudAccessTests/WebDAV/URLProtocolMock.swift

@@ -10,6 +10,7 @@ import Foundation
 
 enum URLProtocolMockError: Error {
 	case unexpectedRequest
+	case simulatedTransportFailure
 }
 
 class URLProtocolMock: URLProtocol {
@@ -71,3 +72,47 @@ class URLProtocolAuthenticationMock: URLProtocol, URLAuthenticationChallengeSend
 
 	override func stopLoading() {}
 }
+
+/// Test-only `URLProtocol` that emits a queued sequence of either HTTP responses or auth challenges
+/// in order. Used to target an auth challenge at a specific request (e.g. the transfer task) while
+/// letting earlier requests (e.g. the preflight `PROPFIND`) resolve normally.
+class URLProtocolSequenceMock: URLProtocol, URLAuthenticationChallengeSender {
+	enum Step {
+		case response(HTTPURLResponse, Data?)
+		case authChallenge(URLAuthenticationChallengeMock)
+	}
+
+	func use(_ credential: URLCredential, for challenge: URLAuthenticationChallenge) {}
+
+	func continueWithoutCredential(for challenge: URLAuthenticationChallenge) {}
+
+	func cancel(_ challenge: URLAuthenticationChallenge) {}
+
+	static var steps = [Step]()
+
+	override func startLoading() {
+		let step = URLProtocolSequenceMock.steps.removeFirst()
+		switch step {
+		case let .response(response, data):
+			client?.urlProtocol(self, didReceive: response, cacheStoragePolicy: .notAllowed)
+			if let data = data {
+				client?.urlProtocol(self, didLoad: data)
+			}
+			client?.urlProtocolDidFinishLoading(self)
+		case let .authChallenge(settings):
+			let protectionSpace = URLProtectionSpace(host: "", port: 443, protocol: nil, realm: nil, authenticationMethod: nil)
+			let challenge = URLAuthenticationChallenge(protectionSpace: protectionSpace, proposedCredential: nil, previousFailureCount: settings.previousFailureCount, failureResponse: settings.failureResponse, error: nil, sender: self)
+			client?.urlProtocol(self, didReceive: challenge)
+		}
+	}
+
+	override class func canInit(with request: URLRequest) -> Bool {
+		return true
+	}
+
+	override class func canonicalRequest(for request: URLRequest) -> URLRequest {
+		return request
+	}
+
+	override func stopLoading() {}
+}