Add regression tests for WebDAV task registration race

Author: tobihagemann

Sources/CryptomatorCloudAccess/WebDAV/WebDAVSession.swift

@@ -224,9 +224,13 @@ class WebDAVSession {
 		progress.addChild(task.progress, withPendingUnitCount: 1)
 		let pendingPromise = Promise<HTTPURLResponse>.pending()
 		let webDAVDownloadTask = WebDAVDownloadTask(promise: pendingPromise, localURL: localURL)
-		startTask(task, register: { downloadTask in
-			self.delegate?.addRunningDownloadTask(key: downloadTask, value: webDAVDownloadTask)
-		}, onTaskCreation: onTaskCreation)
+		// Register before invoking `onTaskCreation` so a synchronous resume cannot race the delegate's lookup.
+		delegate?.addRunningDownloadTask(key: task, value: webDAVDownloadTask)
+		if let onTaskCreation {
+			onTaskCreation(task)
+		} else {
+			task.resume()
+		}
 		return pendingPromise
 	}
 
@@ -237,21 +241,13 @@ class WebDAVSession {
 		progress.addChild(task.progress, withPendingUnitCount: 1)
 		let pendingPromise = Promise<(HTTPURLResponse, Data?)>.pending()
 		let webDAVDataTask = WebDAVDataTask(promise: pendingPromise)
-		startTask(task, register: { uploadTask in
-			self.delegate?.addRunningDataTask(key: uploadTask, value: webDAVDataTask)
-		}, onTaskCreation: onTaskCreation)
-		return pendingPromise
-	}
-
-	/// Registers the task with the delegate's running-tasks dictionary, then yields to `onTaskCreation`.
-	/// Registration must happen before `onTaskCreation` (which may resume the task) so the delegate can
-	/// match response callbacks without racing an empty dictionary. If the caller does not provide an
-	/// `onTaskCreation` closure, the task is resumed directly.
-	private func startTask<Task: URLSessionTask>(_ task: Task, register: (Task) -> Void, onTaskCreation: ((Task?) -> Void)?) {
-		register(task)
-		onTaskCreation?(task)
-		if onTaskCreation == nil {
+		// Register before invoking `onTaskCreation` so a synchronous resume cannot race the delegate's lookup.
+		delegate?.addRunningDataTask(key: task, value: webDAVDataTask)
+		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() {}
+}