diff --git a/RSWeb/Sources/RSWeb/DownloadSession.swift b/RSWeb/Sources/RSWeb/DownloadSession.swift index d12e7305c..fb9c0b868 100755 --- a/RSWeb/Sources/RSWeb/DownloadSession.swift +++ b/RSWeb/Sources/RSWeb/DownloadSession.swift @@ -28,12 +28,18 @@ public protocol DownloadSessionDelegate { private var taskIdentifierToInfoDictionary = [Int: DownloadInfo]() private var urlsInSession = Set() private let delegate: DownloadSessionDelegate - private var redirectCache = [String: String]() + private var redirectCache = [URL: URL]() private var queue = [URL]() + + // 429 Too Many Requests responses private var retryAfterMessages = [String: HTTPResponse429]() + /// URLs with 400-499 responses (except for 429). + /// These URLs are skipped for the rest of the session. + private var urlsWith400s = Set() + public init(delegate: DownloadSessionDelegate) { - + self.delegate = delegate super.init() @@ -94,14 +100,16 @@ extension DownloadSession: URLSessionTaskDelegate { delegate.downloadSession(self, downloadDidComplete: info.url, response: info.urlResponse, data: info.data as Data, error: error as NSError?) } + private static let redirectStatusCodes = Set([HTTPResponseCode.redirectPermanent, HTTPResponseCode.redirectTemporary, HTTPResponseCode.redirectVeryTemporary, HTTPResponseCode.redirectPermanentPreservingMethod]) + public func urlSession(_ session: URLSession, task: URLSessionTask, willPerformHTTPRedirection response: HTTPURLResponse, newRequest request: URLRequest, completionHandler: @escaping (URLRequest?) -> Void) { - - if response.statusCode == 301 || response.statusCode == 308 { - if let oldURLString = task.originalRequest?.url?.absoluteString, let newURLString = request.url?.absoluteString { - cacheRedirect(oldURLString, newURLString) + + if Self.redirectStatusCodes.contains(response.statusCode) { + if let oldURL = task.originalRequest?.url, let newURL = request.url { + cacheRedirect(oldURL, newURL) } } - + completionHandler(request) } } @@ -128,8 +136,12 @@ extension DownloadSession: URLSessionDataDelegate { completionHandler(.cancel) removeTask(dataTask) - if response.forcedStatusCode == HTTPResponseCode.tooManyRequests { + let statusCode = response.forcedStatusCode + + if statusCode == HTTPResponseCode.tooManyRequests { handle429Response(dataTask, response) + } else if (400...499).contains(statusCode), let url = response.url { + urlsWith400s.insert(url) } return @@ -158,24 +170,21 @@ extension DownloadSession: URLSessionDataDelegate { private extension DownloadSession { func addDataTask(_ url: URL) { + guard tasksPending.count < 500 else { queue.insert(url, at: 0) return } - var urlToUse = url - // If received permanent redirect earlier, use that URL. - let urlString = url.absoluteString - if let redirectedURLString = cachedRedirectForURLString(urlString) { - if let redirectedURL = URL(string: redirectedURLString) { - urlToUse = redirectedURL - } - } + let urlToUse = cachedRedirect(for: url) ?? url if requestShouldBeDroppedDueToActive429(urlToUse) { return } + if requestShouldBeDroppedDueToPrevious400(urlToUse) { + return + } let task = urlSession.dataTask(with: urlToUse) @@ -221,32 +230,32 @@ private extension DownloadSession { return false } - func cacheRedirect(_ oldURLString: String, _ newURLString: String) { - if urlStringIsBlackListedRedirect(newURLString) { + func cacheRedirect(_ oldURL: URL, _ newURL: URL) { + if urlStringIsBlackListedRedirect(newURL.absoluteString) { return } - redirectCache[oldURLString] = newURLString + redirectCache[oldURL] = newURL } - func cachedRedirectForURLString(_ urlString: String) -> String? { + func cachedRedirect(for url: URL) -> URL? { // Follow chains of redirects, but avoid loops. - var urlStrings = Set() - urlStrings.insert(urlString) + var urls = Set() + urls.insert(url) - var currentString = urlString + var currentURL = url while(true) { - if let oneRedirectString = redirectCache[currentString] { + if let oneRedirectURL = redirectCache[currentURL] { - if urlStrings.contains(oneRedirectString) { + if urls.contains(oneRedirectURL) { // Cycle. Bail. return nil } - urlStrings.insert(oneRedirectString) - currentString = oneRedirectString + urls.insert(oneRedirectURL) + currentURL = oneRedirectURL } else { @@ -254,7 +263,10 @@ private extension DownloadSession { } } - return currentString == urlString ? nil : currentString + if currentURL == url { + return nil + } + return currentURL } // MARK: - Download Progress @@ -344,6 +356,20 @@ private extension DownloadSession { return true } + + // MARK: - 400-499 responses + + func requestShouldBeDroppedDueToPrevious400(_ url: URL) -> Bool { + + if urlsWith400s.contains(url) { + return true + } + if let redirectedURL = cachedRedirect(for: url), urlsWith400s.contains(redirectedURL) { + return true + } + + return false + } } extension URLSessionTask {