From f6b0728024735c3dc49beda99f3a9fae0ef8d5bf Mon Sep 17 00:00:00 2001 From: Michal Smaga Date: Mon, 23 Sep 2024 14:17:35 +0200 Subject: [PATCH 01/12] Remove unused session --- DuckDuckGo.xcodeproj/project.pbxproj | 4 -- DuckDuckGo/Base64DownloadSession.swift | 63 -------------------------- 2 files changed, 67 deletions(-) delete mode 100644 DuckDuckGo/Base64DownloadSession.swift diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index 11934ff00d..6625ae61f5 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -816,7 +816,6 @@ B6BA95C328891E33004ABA20 /* BrowsingMenuAnimator.swift in Sources */ = {isa = PBXBuildFile; fileRef = B6BA95C228891E33004ABA20 /* BrowsingMenuAnimator.swift */; }; B6BA95C528894A28004ABA20 /* BrowsingMenuViewController.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = B6BA95C428894A28004ABA20 /* BrowsingMenuViewController.storyboard */; }; B6BA95E828924730004ABA20 /* JSAlertController.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = B6BA95E728924730004ABA20 /* JSAlertController.storyboard */; }; - B6CB93E5286445AB0090FEB4 /* Base64DownloadSession.swift in Sources */ = {isa = PBXBuildFile; fileRef = B6CB93E4286445AB0090FEB4 /* Base64DownloadSession.swift */; }; BBFF18B12C76448100C48D7D /* QuerySubmittedTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = BBFF18B02C76448100C48D7D /* QuerySubmittedTests.swift */; }; BD10B8AA2C7629740033115D /* Logger+Subscription.swift in Sources */ = {isa = PBXBuildFile; fileRef = BD10B8A92C7629740033115D /* Logger+Subscription.swift */; }; BD15DB852B959CFD00821457 /* BundleExtension.swift in Sources */ = {isa = PBXBuildFile; fileRef = BD15DB842B959CFD00821457 /* BundleExtension.swift */; }; @@ -2617,7 +2616,6 @@ B6BA95C228891E33004ABA20 /* BrowsingMenuAnimator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BrowsingMenuAnimator.swift; sourceTree = ""; }; B6BA95C428894A28004ABA20 /* BrowsingMenuViewController.storyboard */ = {isa = PBXFileReference; lastKnownFileType = file.storyboard; path = BrowsingMenuViewController.storyboard; sourceTree = ""; }; B6BA95E728924730004ABA20 /* JSAlertController.storyboard */ = {isa = PBXFileReference; lastKnownFileType = file.storyboard; path = JSAlertController.storyboard; sourceTree = ""; }; - B6CB93E4286445AB0090FEB4 /* Base64DownloadSession.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Base64DownloadSession.swift; sourceTree = ""; }; B6DFE6CF2BC7E47500A9CE59 /* SwiftLintTool.bundle */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = SwiftLintTool.bundle; sourceTree = BUILT_PRODUCTS_DIR; }; B6DFE6D92BC7E61B00A9CE59 /* SwiftLintToolBundleConfiguration.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; path = SwiftLintToolBundleConfiguration.xcconfig; sourceTree = ""; }; BBFF18B02C76448100C48D7D /* QuerySubmittedTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = QuerySubmittedTests.swift; sourceTree = ""; }; @@ -3484,7 +3482,6 @@ B623C1C12862CA9E0043013E /* DownloadSession.swift */, 31C138A727A3E9C900FFD4B2 /* URLDownloadSession.swift */, B623C1C32862CD670043013E /* WKDownloadSession.swift */, - B6CB93E4286445AB0090FEB4 /* Base64DownloadSession.swift */, B609D5512862EAFF0088CAC2 /* InlineWKDownloadDelegate.swift */, 310D09202799FD1A00DC0060 /* MIMEType.swift */, 3161D13127AC161B00285CF6 /* DownloadMetadata.swift */, @@ -7720,7 +7717,6 @@ EE01EB432AFC1E0A0096AAC9 /* NetworkProtectionVPNLocationView.swift in Sources */, 7BC571202BDBB877003B0CCE /* VPNActivationDateStore.swift in Sources */, 9820EAF522613CD30089094D /* WebProgressWorker.swift in Sources */, - B6CB93E5286445AB0090FEB4 /* Base64DownloadSession.swift in Sources */, 1EEF387D285B1A1100383393 /* TrackerImageCache.swift in Sources */, 3151F0EC27357FEE00226F58 /* VoiceSearchFeedbackViewModel.swift in Sources */, 1E53508F2C7C9A1F00818DAA /* DefaultSubscriptionManager+AccountManagerKeychainAccessDelegate.swift in Sources */, diff --git a/DuckDuckGo/Base64DownloadSession.swift b/DuckDuckGo/Base64DownloadSession.swift deleted file mode 100644 index 6796aa95d9..0000000000 --- a/DuckDuckGo/Base64DownloadSession.swift +++ /dev/null @@ -1,63 +0,0 @@ -// -// Base64DownloadSession.swift -// DuckDuckGo -// -// Copyright © 2022 DuckDuckGo. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -import Foundation - -final class Base64DownloadSession: DownloadSession { - private var base64: String? - weak var delegate: DownloadSessionDelegate? - private(set) var isRunning: Bool = false - - init(base64: String) { - self.base64 = base64 - } - - func start() { - guard let base64 = base64 else { - self.delegate?.downloadSession(self, didFinishWith: .failure(CancellationError())) - return - } - self.isRunning = true - self.base64 = nil - - DispatchQueue.global().async { [self] in - do { - guard let data = Data(base64Encoded: base64) else { throw CocoaError(.fileReadCorruptFile) } - let localURL = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) - - try data.write(to: localURL) - - DispatchQueue.main.async { - self.delegate?.downloadSession(self, didFinishWith: .success(localURL)) - self.isRunning = false - } - } catch { - DispatchQueue.main.async { - self.delegate?.downloadSession(self, didFinishWith: .failure(error)) - self.isRunning = false - } - } - } - } - - func cancel() { - self.base64 = nil - } - -} From 8424a6ef00d6d03f62053d9aac8794ea5c7f8b12 Mon Sep 17 00:00:00 2001 From: Michal Smaga Date: Thu, 26 Sep 2024 11:38:44 +0200 Subject: [PATCH 02/12] Do not trigger GPC flow for same document navigation --- DuckDuckGo.xcodeproj/project.pbxproj | 8 ++++++++ DuckDuckGo/TabViewController.swift | 2 ++ 2 files changed, 10 insertions(+) diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index 6625ae61f5..df56711281 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -74,6 +74,7 @@ 1E4FAA6427D8DFB900ADC5B3 /* OngoingDownloadRowViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1E4FAA6327D8DFB900ADC5B3 /* OngoingDownloadRowViewModel.swift */; }; 1E4FAA6627D8DFC800ADC5B3 /* CompleteDownloadRowViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1E4FAA6527D8DFC800ADC5B3 /* CompleteDownloadRowViewModel.swift */; }; 1E53508F2C7C9A1F00818DAA /* DefaultSubscriptionManager+AccountManagerKeychainAccessDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1E53508E2C7C9A1F00818DAA /* DefaultSubscriptionManager+AccountManagerKeychainAccessDelegate.swift */; }; + 1E5918472CA422A7008ED2B3 /* Navigation in Frameworks */ = {isa = PBXBuildFile; productRef = 1E5918462CA422A7008ED2B3 /* Navigation */; }; 1E60989B290009C700A508F9 /* Common in Frameworks */ = {isa = PBXBuildFile; productRef = 1E7060BD28F88EE200E4CCDB /* Common */; }; 1E60989D290011E600A508F9 /* ContentBlocking in Frameworks */ = {isa = PBXBuildFile; productRef = 1E60989C290011E600A508F9 /* ContentBlocking */; }; 1E6098A1290011E600A508F9 /* UserScript in Frameworks */ = {isa = PBXBuildFile; productRef = 1E6098A0290011E600A508F9 /* UserScript */; }; @@ -3040,6 +3041,7 @@ 31E69A63280F4CB600478327 /* DuckUI in Frameworks */, CB941A6E2B96AB08000F9E7A /* PrivacyDashboard in Frameworks */, F42D541D29DCA40B004C4FF1 /* DesignResourcesKit in Frameworks */, + 1E5918472CA422A7008ED2B3 /* Navigation in Frameworks */, 85875B6129912A9900115F05 /* SyncUI in Frameworks */, F4D7F634298C00C3006C3AE9 /* FindInPageIOSJSSupport in Frameworks */, 9F96F73B2C9144D5009E45D5 /* Onboarding in Frameworks */, @@ -6514,6 +6516,7 @@ F1D43AF92B99C1D300BAB743 /* BareBonesBrowserKit */, 9F8FE9482BAE50E50071E372 /* Lottie */, 9F96F73A2C9144D5009E45D5 /* Onboarding */, + 1E5918462CA422A7008ED2B3 /* Navigation */, ); productName = DuckDuckGo; productReference = 84E341921E2F7EFB00BDBA6F /* DuckDuckGo.app */; @@ -11016,6 +11019,11 @@ package = F486D2FD25069744002D07D7 /* XCRemoteSwiftPackageReference "OHHTTPStubs" */; productName = OHHTTPStubsSwift; }; + 1E5918462CA422A7008ED2B3 /* Navigation */ = { + isa = XCSwiftPackageProductDependency; + package = 98A16C2928A11BDE00A6C003 /* XCRemoteSwiftPackageReference "BrowserServicesKit" */; + productName = Navigation; + }; 1E60989C290011E600A508F9 /* ContentBlocking */ = { isa = XCSwiftPackageProductDependency; package = 98A16C2928A11BDE00A6C003 /* XCRemoteSwiftPackageReference "BrowserServicesKit" */; diff --git a/DuckDuckGo/TabViewController.swift b/DuckDuckGo/TabViewController.swift index 514b8931de..510741c30a 100644 --- a/DuckDuckGo/TabViewController.swift +++ b/DuckDuckGo/TabViewController.swift @@ -40,6 +40,7 @@ import SpecialErrorPages import NetworkProtection import Onboarding import os.log +import Navigation class TabViewController: UIViewController { @@ -1742,6 +1743,7 @@ extension TabViewController: WKNavigationDelegate { } if navigationAction.isTargetingMainFrame(), + !navigationAction.isSameDocumentNavigation, !(navigationAction.request.url?.isCustomURLScheme() ?? false), navigationAction.navigationType != .backForward, let request = requestForDoNotSell(basedOn: navigationAction.request) { From df92b4b850fae85cc78cbb8b0e4ab9cdd2b71c4a Mon Sep 17 00:00:00 2001 From: Michal Smaga Date: Thu, 26 Sep 2024 13:03:35 +0200 Subject: [PATCH 03/12] Use safely unwrapped requests --- DuckDuckGo/TabViewController.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/DuckDuckGo/TabViewController.swift b/DuckDuckGo/TabViewController.swift index 510741c30a..93cb26811c 100644 --- a/DuckDuckGo/TabViewController.swift +++ b/DuckDuckGo/TabViewController.swift @@ -1256,7 +1256,7 @@ extension TabViewController: WKNavigationDelegate { private func handleServerTrustChallenge(_ challenge: URLAuthenticationChallenge, completionHandler: @escaping (URLSession.AuthChallengeDisposition, URLCredential?) -> Void) { guard shouldBypassSSLError, - let credential = urlCredentialCreator.urlCredentialFrom(trust: challenge.protectionSpace.serverTrust) else { + let credential = urlCredentialCreator.urlCredentialFrom(trust: challenge.protectionSpace.serverTrust) else { completionHandler(.performDefaultHandling, nil) return } @@ -2340,7 +2340,7 @@ extension TabViewController: WKUIDelegate { return } - let alert = WebJSAlert(domain: frame.request.url?.host + let alert = WebJSAlert(domain: frame.safeRequest?.url?.host // in case the web view is navigating to another host ?? webView.backForwardList.currentItem?.url.host ?? self.url?.absoluteString @@ -2360,7 +2360,7 @@ extension TabViewController: WKUIDelegate { return } - let alert = WebJSAlert(domain: frame.request.url?.host + let alert = WebJSAlert(domain: frame.safeRequest?.url?.host // in case the web view is navigating to another host ?? webView.backForwardList.currentItem?.url.host ?? self.url?.absoluteString From 2ac18a73d1544ec01c408f1385913739dbe9cab0 Mon Sep 17 00:00:00 2001 From: Michal Smaga Date: Thu, 26 Sep 2024 13:08:11 +0200 Subject: [PATCH 04/12] Store URL for recent WKNavigationAction that had shouldPerformDownload set to true --- DuckDuckGo/TabViewController.swift | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/DuckDuckGo/TabViewController.swift b/DuckDuckGo/TabViewController.swift index 93cb26811c..5447c35cf2 100644 --- a/DuckDuckGo/TabViewController.swift +++ b/DuckDuckGo/TabViewController.swift @@ -168,6 +168,9 @@ class TabViewController: UIViewController { var mostRecentAutoPreviewDownloadID: UUID? private var blobDownloadTargetFrame: WKFrameInfo? + // Recent request's URL if its WKNavigationAction had shouldPerformDownload set to true + private var recentNavigationActionShouldPerformDownloadURL: URL? + let userAgentManager: UserAgentManager = DefaultUserAgentManager.shared let bookmarksDatabase: CoreDataDatabase @@ -1818,6 +1821,8 @@ extension TabViewController: WKNavigationDelegate { let tld = storageCache.tld + // If WKNavigationAction requests to shouldPerformDownload prepare for handling it in decidePolicyFor:navigationResponse: + recentNavigationActionShouldPerformDownloadURL = navigationAction.shouldPerformDownload ? navigationAction.request.url : nil if navigationAction.isTargetingMainFrame() && tld.domain(navigationAction.request.mainDocumentURL?.host) != tld.domain(lastUpgradedURL?.host) { From 7c773654b4a51529d447b9422e04714943dfb266 Mon Sep 17 00:00:00 2001 From: Michal Smaga Date: Thu, 26 Sep 2024 13:08:52 +0200 Subject: [PATCH 05/12] Fix triggering the downloads when appropriate --- DuckDuckGo/TabViewController.swift | 65 +++++++++++++++++++++--------- 1 file changed, 47 insertions(+), 18 deletions(-) diff --git a/DuckDuckGo/TabViewController.swift b/DuckDuckGo/TabViewController.swift index 5447c35cf2..2554b46daa 100644 --- a/DuckDuckGo/TabViewController.swift +++ b/DuckDuckGo/TabViewController.swift @@ -1310,6 +1310,7 @@ extension TabViewController: WKNavigationDelegate { decisionHandler: @escaping (WKNavigationResponsePolicy) -> Void) { let mimeType = MIMEType(from: navigationResponse.response.mimeType) + let urlSchemeType = navigationResponse.response.url.map { SchemeHandler.schemeType(for: $0) } ?? .unknown let httpResponse = navigationResponse.response as? HTTPURLResponse let isSuccessfulResponse = httpResponse?.isSuccessfulResponse ?? false @@ -1321,11 +1322,36 @@ extension TabViewController: WKNavigationDelegate { NotificationCenter.default.post(Notification(name: AppUserDefaults.Notifications.didVerifyInternalUser)) } - if navigationResponse.canShowMIMEType && !FilePreviewHelper.canAutoPreviewMIMEType(mimeType) { + // HTTP response has "Content-Disposition: attachment" header + let hasContentDispositionAttachment = httpResponse?.shouldDownload ?? false + // If preceding WKNavigationAction requested to start the download + let hasNavigationActionRequestedDownload = recentNavigationActionShouldPerformDownloadURL == navigationResponse.response.url + + let shouldTriggerDownloadAction = hasContentDispositionAttachment || hasNavigationActionRequestedDownload + + if shouldTriggerDownloadAction { + if urlSchemeType == .blob { + Swift.print("==== BLOB that should be downloaded") + decisionHandler(.download) + return + } else if let downloadMetadata = AppDependencyProvider.shared.downloadManager.downloadMetaData(for: navigationResponse.response) { + + Swift.print("=== request url: \(navigationResponse.response.url?.absoluteString ?? "")") + + self.presentSaveToDownloadsAlert(with: downloadMetadata) { + self.startDownload(with: navigationResponse, decisionHandler: decisionHandler) + } cancelHandler: { + decisionHandler(.cancel) + } + return + } + } else if navigationResponse.canShowMIMEType && !FilePreviewHelper.canAutoPreviewMIMEType(mimeType) { url = webView.url if navigationResponse.isForMainFrame, let decision = setupOrClearTemporaryDownload(for: navigationResponse.response) { + // Loading a file in WebView decisionHandler(decision) } else { + // Loading HTML if navigationResponse.isForMainFrame && isSuccessfulResponse { adClickAttributionDetection.on2XXResponse(url: url) } @@ -1339,8 +1365,7 @@ extension TabViewController: WKNavigationDelegate { mostRecentAutoPreviewDownloadID = download?.id Pixel.fire(pixel: .downloadStarted, withAdditionalParameters: [PixelParameters.canAutoPreviewMIMEType: "1"]) - } else if let url = navigationResponse.response.url, - case .blob = SchemeHandler.schemeType(for: url) { + } else if urlSchemeType == .blob { decisionHandler(.download) } else if let downloadMetadata = AppDependencyProvider.shared.downloadManager @@ -1860,7 +1885,7 @@ extension TabViewController: WKNavigationDelegate { } } - + let schemeType = SchemeHandler.schemeType(for: url) self.blobDownloadTargetFrame = nil switch schemeType { @@ -2147,31 +2172,35 @@ extension TabViewController { return } - let isTemporary = navigationResponse.canShowMIMEType - || FilePreviewHelper.canAutoPreviewMIMEType(downloadMetadata.mimeType) - if isTemporary { - // restart blob request loading for preview that was interrupted by .download callback - if navigationResponse.canShowMIMEType { - self.webView.load(navigationResponse.response.url!, in: self.blobDownloadTargetFrame) - } - callback(self.transfer(download, - to: downloadManager, - with: navigationResponse.response, - suggestedFilename: suggestedFilename, - isTemporary: isTemporary)) + let hasNavigationActionRequestedDownload = self.recentNavigationActionShouldPerformDownloadURL == navigationResponse.response.url + let canShowOrPreview = navigationResponse.canShowMIMEType || FilePreviewHelper.canAutoPreviewMIMEType(downloadMetadata.mimeType) - } else { + let shouldTriggerDownloadAction = hasNavigationActionRequestedDownload || !canShowOrPreview + + if shouldTriggerDownloadAction { + // Show alert to start the file download self.presentSaveToDownloadsAlert(with: downloadMetadata) { callback(self.transfer(download, to: downloadManager, with: navigationResponse.response, suggestedFilename: suggestedFilename, - isTemporary: isTemporary)) + isTemporary: false)) } cancelHandler: { callback(nil) } self.temporaryDownloadForPreviewedFile = nil + } else { + // Showing file in the webview or in preview view + if navigationResponse.canShowMIMEType { + // restart blob request loading for preview that was interrupted by .download callback + self.webView.load(navigationResponse.response.url!, in: self.blobDownloadTargetFrame) + } + callback(self.transfer(download, + to: downloadManager, + with: navigationResponse.response, + suggestedFilename: suggestedFilename, + isTemporary: true)) } delegate.decideDestinationCallback = nil From 0bb7a0d7de50e5fd16224df62c6af7c1389498d1 Mon Sep 17 00:00:00 2001 From: Michal Smaga Date: Thu, 26 Sep 2024 20:07:12 +0200 Subject: [PATCH 06/12] Blob type URLs need to be downloaded first --- DuckDuckGo/TabViewController.swift | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/DuckDuckGo/TabViewController.swift b/DuckDuckGo/TabViewController.swift index 2554b46daa..788bb0769c 100644 --- a/DuckDuckGo/TabViewController.swift +++ b/DuckDuckGo/TabViewController.swift @@ -1329,22 +1329,19 @@ extension TabViewController: WKNavigationDelegate { let shouldTriggerDownloadAction = hasContentDispositionAttachment || hasNavigationActionRequestedDownload - if shouldTriggerDownloadAction { - if urlSchemeType == .blob { - Swift.print("==== BLOB that should be downloaded") - decisionHandler(.download) - return - } else if let downloadMetadata = AppDependencyProvider.shared.downloadManager.downloadMetaData(for: navigationResponse.response) { - - Swift.print("=== request url: \(navigationResponse.response.url?.absoluteString ?? "")") - - self.presentSaveToDownloadsAlert(with: downloadMetadata) { - self.startDownload(with: navigationResponse, decisionHandler: decisionHandler) - } cancelHandler: { - decisionHandler(.cancel) - } - return + if urlSchemeType == .blob { + // To further handle BLOB we need to trigger download action + decisionHandler(.download) + return + } else if shouldTriggerDownloadAction, + let downloadMetadata = AppDependencyProvider.shared.downloadManager.downloadMetaData(for: navigationResponse.response) { + // We should handle the response as download + self.presentSaveToDownloadsAlert(with: downloadMetadata) { + self.startDownload(with: navigationResponse, decisionHandler: decisionHandler) + } cancelHandler: { + decisionHandler(.cancel) } + return } else if navigationResponse.canShowMIMEType && !FilePreviewHelper.canAutoPreviewMIMEType(mimeType) { url = webView.url if navigationResponse.isForMainFrame, let decision = setupOrClearTemporaryDownload(for: navigationResponse.response) { From 5b7429a533f8d41e1d8f64064b3275f7549e4125 Mon Sep 17 00:00:00 2001 From: Michal Smaga Date: Thu, 26 Sep 2024 21:04:47 +0200 Subject: [PATCH 07/12] Fix checks order --- Core/PixelEvent.swift | 2 -- DuckDuckGo/TabViewController.swift | 52 +++++++++++------------------- 2 files changed, 18 insertions(+), 36 deletions(-) diff --git a/Core/PixelEvent.swift b/Core/PixelEvent.swift index 2f67e300f1..b4cc7d9384 100644 --- a/Core/PixelEvent.swift +++ b/Core/PixelEvent.swift @@ -525,7 +525,6 @@ extension Pixel { case cachedTabPreviewRemovalError case missingDownloadedFile - case unhandledDownload case compilationResult(result: CompileRulesResult, waitTime: BucketAggregation, appState: AppState) @@ -1326,7 +1325,6 @@ extension Pixel.Event { case .cachedTabPreviewRemovalError: return "m_d_tpre" case .missingDownloadedFile: return "m_d_missing_downloaded_file" - case .unhandledDownload: return "m_d_unhandled_download" case .compilationResult(result: let result, waitTime: let waitTime, appState: let appState): return "m_compilation_result_\(result)_time_\(waitTime)_state_\(appState)" diff --git a/DuckDuckGo/TabViewController.swift b/DuckDuckGo/TabViewController.swift index 788bb0769c..b2a2249a38 100644 --- a/DuckDuckGo/TabViewController.swift +++ b/DuckDuckGo/TabViewController.swift @@ -1325,27 +1325,37 @@ extension TabViewController: WKNavigationDelegate { // HTTP response has "Content-Disposition: attachment" header let hasContentDispositionAttachment = httpResponse?.shouldDownload ?? false // If preceding WKNavigationAction requested to start the download - let hasNavigationActionRequestedDownload = recentNavigationActionShouldPerformDownloadURL == navigationResponse.response.url + let hasNavigationActionRequestedDownload = (recentNavigationActionShouldPerformDownloadURL != nil) && recentNavigationActionShouldPerformDownloadURL == navigationResponse.response.url + // File can be rendered by web view or in custom preview handled by FilePreviewHelper + let canLoadOrPreviewTheFile = navigationResponse.canShowMIMEType || FilePreviewHelper.canAutoPreviewMIMEType(mimeType) - let shouldTriggerDownloadAction = hasContentDispositionAttachment || hasNavigationActionRequestedDownload + let shouldTriggerDownloadAction = hasContentDispositionAttachment || hasNavigationActionRequestedDownload || !canLoadOrPreviewTheFile + // Important: Order of these checks matter! if urlSchemeType == .blob { - // To further handle BLOB we need to trigger download action + // 1. If it is BLOB we need to trigger download to handle it then in webView:navigationAction:didBecomeDownload decisionHandler(.download) return } else if shouldTriggerDownloadAction, let downloadMetadata = AppDependencyProvider.shared.downloadManager.downloadMetaData(for: navigationResponse.response) { - // We should handle the response as download + // 2. We know the response should trigger the file download prompt self.presentSaveToDownloadsAlert(with: downloadMetadata) { self.startDownload(with: navigationResponse, decisionHandler: decisionHandler) } cancelHandler: { decisionHandler(.cancel) } return - } else if navigationResponse.canShowMIMEType && !FilePreviewHelper.canAutoPreviewMIMEType(mimeType) { + } else if FilePreviewHelper.canAutoPreviewMIMEType(mimeType) { + // 3. For this MIME type we are able to provide our custom preview via FilePreviewHelper + let download = self.startDownload(with: navigationResponse, decisionHandler: decisionHandler) + mostRecentAutoPreviewDownloadID = download?.id + Pixel.fire(pixel: .downloadStarted, + withAdditionalParameters: [PixelParameters.canAutoPreviewMIMEType: "1"]) + } else if navigationResponse.canShowMIMEType { + // 4. WebView can preview the MIME type and it is not to be handled by our custom FilePreviewHelper url = webView.url if navigationResponse.isForMainFrame, let decision = setupOrClearTemporaryDownload(for: navigationResponse.response) { - // Loading a file in WebView + // Loading a file preview in WebView decisionHandler(decision) } else { // Loading HTML @@ -1356,35 +1366,8 @@ extension TabViewController: WKNavigationDelegate { decisionHandler(.allow) } } - } else if isSuccessfulResponse { - if FilePreviewHelper.canAutoPreviewMIMEType(mimeType) { - let download = self.startDownload(with: navigationResponse, decisionHandler: decisionHandler) - mostRecentAutoPreviewDownloadID = download?.id - Pixel.fire(pixel: .downloadStarted, - withAdditionalParameters: [PixelParameters.canAutoPreviewMIMEType: "1"]) - } else if urlSchemeType == .blob { - decisionHandler(.download) - - } else if let downloadMetadata = AppDependencyProvider.shared.downloadManager - .downloadMetaData(for: navigationResponse.response) { - if view.window == nil { - decisionHandler(.cancel) - } else { - self.presentSaveToDownloadsAlert(with: downloadMetadata) { - self.startDownload(with: navigationResponse, decisionHandler: decisionHandler) - } cancelHandler: { - decisionHandler(.cancel) - } - // Rewrite the current URL to prevent spoofing from download URLs - self.chromeDelegate?.omniBar.textField.text = "about:blank" - } - } else { - Pixel.fire(pixel: .unhandledDownload) - decisionHandler(.cancel) - } - } else { - // MIME type should trigger download but response has no 2xx status code + // Fallback decisionHandler(.allow) } } @@ -1769,6 +1752,7 @@ extension TabViewController: WKNavigationDelegate { if navigationAction.isTargetingMainFrame(), !navigationAction.isSameDocumentNavigation, + !navigationAction.shouldDownload, !(navigationAction.request.url?.isCustomURLScheme() ?? false), navigationAction.navigationType != .backForward, let request = requestForDoNotSell(basedOn: navigationAction.request) { From 780d323a60b00a329320284bc7b885e0fcb7a903 Mon Sep 17 00:00:00 2001 From: Michal Smaga Date: Fri, 27 Sep 2024 11:18:00 +0200 Subject: [PATCH 08/12] Clean up naming --- DuckDuckGo/TabViewController.swift | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/DuckDuckGo/TabViewController.swift b/DuckDuckGo/TabViewController.swift index b2a2249a38..bb47e42c41 100644 --- a/DuckDuckGo/TabViewController.swift +++ b/DuckDuckGo/TabViewController.swift @@ -1355,7 +1355,7 @@ extension TabViewController: WKNavigationDelegate { // 4. WebView can preview the MIME type and it is not to be handled by our custom FilePreviewHelper url = webView.url if navigationResponse.isForMainFrame, let decision = setupOrClearTemporaryDownload(for: navigationResponse.response) { - // Loading a file preview in WebView + // Loading a file preview in web view decisionHandler(decision) } else { // Loading HTML @@ -2153,13 +2153,13 @@ extension TabViewController { return } - let hasNavigationActionRequestedDownload = self.recentNavigationActionShouldPerformDownloadURL == navigationResponse.response.url - let canShowOrPreview = navigationResponse.canShowMIMEType || FilePreviewHelper.canAutoPreviewMIMEType(downloadMetadata.mimeType) + let hasNavigationActionRequestedDownload = (self.recentNavigationActionShouldPerformDownloadURL != nil) && self.recentNavigationActionShouldPerformDownloadURL == navigationResponse.response.url + let canLoadOrPreviewTheFile = navigationResponse.canShowMIMEType || FilePreviewHelper.canAutoPreviewMIMEType(downloadMetadata.mimeType) - let shouldTriggerDownloadAction = hasNavigationActionRequestedDownload || !canShowOrPreview + let shouldTriggerDownloadAction = hasNavigationActionRequestedDownload || !canLoadOrPreviewTheFile if shouldTriggerDownloadAction { - // Show alert to start the file download + // Show alert to the file download self.presentSaveToDownloadsAlert(with: downloadMetadata) { callback(self.transfer(download, to: downloadManager, From 7e731fd55c4b3717a5ecbf3221f73f66583a244a Mon Sep 17 00:00:00 2001 From: Michal Smaga Date: Fri, 27 Sep 2024 14:20:44 +0200 Subject: [PATCH 09/12] Clean up path for handling BLOB downloads --- DuckDuckGo/TabViewController.swift | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/DuckDuckGo/TabViewController.swift b/DuckDuckGo/TabViewController.swift index bb47e42c41..2303652772 100644 --- a/DuckDuckGo/TabViewController.swift +++ b/DuckDuckGo/TabViewController.swift @@ -1333,8 +1333,15 @@ extension TabViewController: WKNavigationDelegate { // Important: Order of these checks matter! if urlSchemeType == .blob { - // 1. If it is BLOB we need to trigger download to handle it then in webView:navigationAction:didBecomeDownload - decisionHandler(.download) + // 1. To properly handle BLOB we need to trigger its download, if temporaryDownloadForPreviewedFile is set we allow its load in the web view + if let temporaryDownloadForPreviewedFile, temporaryDownloadForPreviewedFile.url == navigationResponse.response.url { + // BLOB already has a temporary downloaded so and we can allow loading it + blobDownloadTargetFrame = nil + decisionHandler(.allow) + } else { + // First we need to trigger download to handle it then in webView:navigationAction:didBecomeDownload + decisionHandler(.download) + } return } else if shouldTriggerDownloadAction, let downloadMetadata = AppDependencyProvider.shared.downloadManager.downloadMetaData(for: navigationResponse.response) { @@ -2118,17 +2125,6 @@ extension TabViewController { temporaryDownloadForPreviewedFile = nil return nil } - guard SchemeHandler.schemeType(for: url) != .blob else { - // suggestedFilename is empty for blob: downloads unless handled via completion(.download) - // WKNavigationResponse._downloadAttribute private API could be used instead of it :( - if self.temporaryDownloadForPreviewedFile?.url != url { // if temporary download not setup yet, preview otherwise - // calls webView:navigationAction:didBecomeDownload: - return .download - } else { - self.blobDownloadTargetFrame = nil - return .allow - } - } let cookieStore = webView.configuration.websiteDataStore.httpCookieStore temporaryDownloadForPreviewedFile = downloadManager.makeDownload(response: response, @@ -2173,8 +2169,11 @@ extension TabViewController { self.temporaryDownloadForPreviewedFile = nil } else { // Showing file in the webview or in preview view - if navigationResponse.canShowMIMEType { - // restart blob request loading for preview that was interrupted by .download callback + if FilePreviewHelper.canAutoPreviewMIMEType(downloadMetadata.mimeType) { + // If FilePreviewHelper can handle format we do not need to load as it will be handled by setting + // temporaryDownloadForPreviewedFile and mostRecentAutoPreviewDownloadID + } else if navigationResponse.canShowMIMEType { + // To load BLOB in web view we need to restart the request loading as it was interrupted by .download callback self.webView.load(navigationResponse.response.url!, in: self.blobDownloadTargetFrame) } callback(self.transfer(download, @@ -2212,6 +2211,7 @@ extension TabViewController { temporary: isTemporary) self.temporaryDownloadForPreviewedFile = isTemporary ? download : nil + self.mostRecentAutoPreviewDownloadID = isTemporary ? download?.id : nil if let download = download { downloadManager.startDownload(download) } From 5f83fc87fe74b5759988753fb80ff2b43346ea2a Mon Sep 17 00:00:00 2001 From: Michal Smaga Date: Mon, 14 Oct 2024 18:27:31 +0200 Subject: [PATCH 10/12] Make showing custom preview for specific mimetypes a higher priority --- DuckDuckGo/TabViewController.swift | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/DuckDuckGo/TabViewController.swift b/DuckDuckGo/TabViewController.swift index 2303652772..a34698fc0b 100644 --- a/DuckDuckGo/TabViewController.swift +++ b/DuckDuckGo/TabViewController.swift @@ -1342,22 +1342,20 @@ extension TabViewController: WKNavigationDelegate { // First we need to trigger download to handle it then in webView:navigationAction:didBecomeDownload decisionHandler(.download) } - return + } else if FilePreviewHelper.canAutoPreviewMIMEType(mimeType) { + // 2. For this MIME type we are able to provide a better custom preview via FilePreviewHelper so it takes priority + let download = self.startDownload(with: navigationResponse, decisionHandler: decisionHandler) + mostRecentAutoPreviewDownloadID = download?.id + Pixel.fire(pixel: .downloadStarted, + withAdditionalParameters: [PixelParameters.canAutoPreviewMIMEType: "1"]) } else if shouldTriggerDownloadAction, let downloadMetadata = AppDependencyProvider.shared.downloadManager.downloadMetaData(for: navigationResponse.response) { - // 2. We know the response should trigger the file download prompt + // 3. We know the response should trigger the file download prompt self.presentSaveToDownloadsAlert(with: downloadMetadata) { self.startDownload(with: navigationResponse, decisionHandler: decisionHandler) } cancelHandler: { decisionHandler(.cancel) } - return - } else if FilePreviewHelper.canAutoPreviewMIMEType(mimeType) { - // 3. For this MIME type we are able to provide our custom preview via FilePreviewHelper - let download = self.startDownload(with: navigationResponse, decisionHandler: decisionHandler) - mostRecentAutoPreviewDownloadID = download?.id - Pixel.fire(pixel: .downloadStarted, - withAdditionalParameters: [PixelParameters.canAutoPreviewMIMEType: "1"]) } else if navigationResponse.canShowMIMEType { // 4. WebView can preview the MIME type and it is not to be handled by our custom FilePreviewHelper url = webView.url From 83cdecf1edd19753710d041ba575348f04c08568 Mon Sep 17 00:00:00 2001 From: Michal Smaga Date: Wed, 16 Oct 2024 11:12:36 +0200 Subject: [PATCH 11/12] Share logic for determining if should trigger download --- DuckDuckGo/TabViewController.swift | 34 ++++++++++++++++-------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/DuckDuckGo/TabViewController.swift b/DuckDuckGo/TabViewController.swift index a34698fc0b..5d444d735e 100644 --- a/DuckDuckGo/TabViewController.swift +++ b/DuckDuckGo/TabViewController.swift @@ -1322,15 +1322,6 @@ extension TabViewController: WKNavigationDelegate { NotificationCenter.default.post(Notification(name: AppUserDefaults.Notifications.didVerifyInternalUser)) } - // HTTP response has "Content-Disposition: attachment" header - let hasContentDispositionAttachment = httpResponse?.shouldDownload ?? false - // If preceding WKNavigationAction requested to start the download - let hasNavigationActionRequestedDownload = (recentNavigationActionShouldPerformDownloadURL != nil) && recentNavigationActionShouldPerformDownloadURL == navigationResponse.response.url - // File can be rendered by web view or in custom preview handled by FilePreviewHelper - let canLoadOrPreviewTheFile = navigationResponse.canShowMIMEType || FilePreviewHelper.canAutoPreviewMIMEType(mimeType) - - let shouldTriggerDownloadAction = hasContentDispositionAttachment || hasNavigationActionRequestedDownload || !canLoadOrPreviewTheFile - // Important: Order of these checks matter! if urlSchemeType == .blob { // 1. To properly handle BLOB we need to trigger its download, if temporaryDownloadForPreviewedFile is set we allow its load in the web view @@ -1348,7 +1339,7 @@ extension TabViewController: WKNavigationDelegate { mostRecentAutoPreviewDownloadID = download?.id Pixel.fire(pixel: .downloadStarted, withAdditionalParameters: [PixelParameters.canAutoPreviewMIMEType: "1"]) - } else if shouldTriggerDownloadAction, + } else if shouldTriggerDownloadAction(for: navigationResponse), let downloadMetadata = AppDependencyProvider.shared.downloadManager.downloadMetaData(for: navigationResponse.response) { // 3. We know the response should trigger the file download prompt self.presentSaveToDownloadsAlert(with: downloadMetadata) { @@ -1377,6 +1368,22 @@ extension TabViewController: WKNavigationDelegate { } } + private func shouldTriggerDownloadAction(for navigationResponse: WKNavigationResponse) -> Bool { + let mimeType = MIMEType(from: navigationResponse.response.mimeType) + let httpResponse = navigationResponse.response as? HTTPURLResponse + + // HTTP response has "Content-Disposition: attachment" header + let hasContentDispositionAttachment = httpResponse?.shouldDownload ?? false + + // If preceding WKNavigationAction requested to start the download (e.g. link `download` attribute or BLOB object) + let hasNavigationActionRequestedDownload = (recentNavigationActionShouldPerformDownloadURL != nil) && recentNavigationActionShouldPerformDownloadURL == navigationResponse.response.url + + // File can be rendered by web view or in custom preview handled by FilePreviewHelper + let canLoadOrPreviewTheFile = navigationResponse.canShowMIMEType || FilePreviewHelper.canAutoPreviewMIMEType(mimeType) + + return hasContentDispositionAttachment || hasNavigationActionRequestedDownload || !canLoadOrPreviewTheFile + } + func webView(_ webView: WKWebView, didStartProvisionalNavigation navigation: WKNavigation!) { lastError = nil lastRenderedURL = webView.url @@ -2147,12 +2154,7 @@ extension TabViewController { return } - let hasNavigationActionRequestedDownload = (self.recentNavigationActionShouldPerformDownloadURL != nil) && self.recentNavigationActionShouldPerformDownloadURL == navigationResponse.response.url - let canLoadOrPreviewTheFile = navigationResponse.canShowMIMEType || FilePreviewHelper.canAutoPreviewMIMEType(downloadMetadata.mimeType) - - let shouldTriggerDownloadAction = hasNavigationActionRequestedDownload || !canLoadOrPreviewTheFile - - if shouldTriggerDownloadAction { + if self.shouldTriggerDownloadAction(for: navigationResponse) { // Show alert to the file download self.presentSaveToDownloadsAlert(with: downloadMetadata) { callback(self.transfer(download, From 8a359fb7a602fc616b7af7b3f89356ddf3f3cde7 Mon Sep 17 00:00:00 2001 From: Michal Smaga Date: Wed, 16 Oct 2024 11:27:54 +0200 Subject: [PATCH 12/12] Extend logic to resolve download MIMEtype by also looking at file extension --- DuckDuckGo/DownloadMetadata.swift | 2 +- DuckDuckGo/MIMEType.swift | 12 +++++++++++- DuckDuckGo/TabViewController.swift | 4 ++-- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/DuckDuckGo/DownloadMetadata.swift b/DuckDuckGo/DownloadMetadata.swift index 0b793ee795..e5dc290a38 100644 --- a/DuckDuckGo/DownloadMetadata.swift +++ b/DuckDuckGo/DownloadMetadata.swift @@ -32,7 +32,7 @@ struct DownloadMetadata { self.filename = filename self.expectedContentLength = response.expectedContentLength self.mimeTypeSource = response.mimeType ?? "" - self.mimeType = MIMEType(from: response.mimeType) + self.mimeType = MIMEType(from: response.mimeType, fileExtension: filename.pathExtension) self.url = url } } diff --git a/DuckDuckGo/MIMEType.swift b/DuckDuckGo/MIMEType.swift index 29a7c47813..93a0c8b4bf 100644 --- a/DuckDuckGo/MIMEType.swift +++ b/DuckDuckGo/MIMEType.swift @@ -33,7 +33,17 @@ enum MIMEType: String { init(from string: String?) { self = MIMEType(rawValue: string ?? "") ?? .unknown } - + + init(from string: String?, fileExtension: String?) { + let initialMIMEType = MIMEType(from: string) + + switch (initialMIMEType, fileExtension) { + case (.octetStream, "pkpass"): self = .passbook + case (.octetStream, "pkpasses"): self = .multipass + default: self = initialMIMEType + } + } + var isHTML: Bool { switch self { case .html, .xhtml: diff --git a/DuckDuckGo/TabViewController.swift b/DuckDuckGo/TabViewController.swift index 5d444d735e..2983f6ba57 100644 --- a/DuckDuckGo/TabViewController.swift +++ b/DuckDuckGo/TabViewController.swift @@ -1309,7 +1309,7 @@ extension TabViewController: WKNavigationDelegate { decidePolicyFor navigationResponse: WKNavigationResponse, decisionHandler: @escaping (WKNavigationResponsePolicy) -> Void) { - let mimeType = MIMEType(from: navigationResponse.response.mimeType) + let mimeType = MIMEType(from: navigationResponse.response.mimeType, fileExtension: navigationResponse.response.url?.pathExtension) let urlSchemeType = navigationResponse.response.url.map { SchemeHandler.schemeType(for: $0) } ?? .unknown let httpResponse = navigationResponse.response as? HTTPURLResponse @@ -1369,7 +1369,7 @@ extension TabViewController: WKNavigationDelegate { } private func shouldTriggerDownloadAction(for navigationResponse: WKNavigationResponse) -> Bool { - let mimeType = MIMEType(from: navigationResponse.response.mimeType) + let mimeType = MIMEType(from: navigationResponse.response.mimeType, fileExtension: navigationResponse.response.url?.pathExtension) let httpResponse = navigationResponse.response as? HTTPURLResponse // HTTP response has "Content-Disposition: attachment" header