From 4b9fa1a3e5c80234e6ac4a4be0e43d7415a05d70 Mon Sep 17 00:00:00 2001 From: Dax Mobile <44842493+daxmobile@users.noreply.github.com> Date: Tue, 9 Apr 2024 18:08:37 +0200 Subject: [PATCH 01/14] Update BSK with autofill 11.0.1 (#2588) Task/Issue URL: https://app.asana.com/0/1207038392239720/1207038392239720 Autofill Release: https://github.com/duckduckgo/duckduckgo-autofill/releases/tag/11.0.1 BSK PR: duckduckgo/BrowserServicesKit#769 Description Updates Autofill to version 11.0.1. --- DuckDuckGo.xcodeproj/project.pbxproj | 2 +- .../xcshareddata/swiftpm/Package.resolved | 8 ++++---- LocalPackages/DataBrokerProtection/Package.swift | 2 +- LocalPackages/NetworkProtectionMac/Package.swift | 2 +- LocalPackages/SubscriptionUI/Package.swift | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index dfa32b6122..17154ba5d0 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -14452,7 +14452,7 @@ repositoryURL = "https://github.com/duckduckgo/BrowserServicesKit"; requirement = { kind = exactVersion; - version = 133.0.0; + version = 133.0.1; }; }; 9FF521422BAA8FF300B9819B /* XCRemoteSwiftPackageReference "lottie-spm" */ = { diff --git a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 0e02888fb7..12fabf86b4 100644 --- a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -32,8 +32,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/duckduckgo/BrowserServicesKit", "state" : { - "revision" : "c0b0cb55e7ac2f69d10452e1a5c06713155d798e", - "version" : "133.0.0" + "revision" : "39d74829150a9ecffea2f503c01851e54eda8ad1", + "version" : "133.0.1" } }, { @@ -50,8 +50,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/duckduckgo/duckduckgo-autofill.git", "state" : { - "revision" : "6493e296934bf09277c03df45f11f4619711cb24", - "version" : "10.2.0" + "revision" : "6053999d6af384a716ab0ce7205dbab5d70ed1b3", + "version" : "11.0.1" } }, { diff --git a/LocalPackages/DataBrokerProtection/Package.swift b/LocalPackages/DataBrokerProtection/Package.swift index 46c094664b..9418e01778 100644 --- a/LocalPackages/DataBrokerProtection/Package.swift +++ b/LocalPackages/DataBrokerProtection/Package.swift @@ -29,7 +29,7 @@ let package = Package( targets: ["DataBrokerProtection"]) ], dependencies: [ - .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "133.0.0"), + .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "133.0.1"), .package(path: "../PixelKit"), .package(path: "../SwiftUIExtensions"), .package(path: "../XPCHelper"), diff --git a/LocalPackages/NetworkProtectionMac/Package.swift b/LocalPackages/NetworkProtectionMac/Package.swift index d0ea0235c3..4e62507c16 100644 --- a/LocalPackages/NetworkProtectionMac/Package.swift +++ b/LocalPackages/NetworkProtectionMac/Package.swift @@ -31,7 +31,7 @@ let package = Package( .library(name: "NetworkProtectionUI", targets: ["NetworkProtectionUI"]), ], dependencies: [ - .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "133.0.0"), + .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "133.0.1"), .package(path: "../XPCHelper"), .package(path: "../SwiftUIExtensions"), .package(path: "../LoginItems"), diff --git a/LocalPackages/SubscriptionUI/Package.swift b/LocalPackages/SubscriptionUI/Package.swift index 556d4f308e..1c064eb962 100644 --- a/LocalPackages/SubscriptionUI/Package.swift +++ b/LocalPackages/SubscriptionUI/Package.swift @@ -12,7 +12,7 @@ let package = Package( targets: ["SubscriptionUI"]), ], dependencies: [ - .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "133.0.0"), + .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "133.0.1"), .package(path: "../SwiftUIExtensions") ], targets: [ From 532ec92fc6bd42a85007b5868640601713e449cd Mon Sep 17 00:00:00 2001 From: Halle <378795+Halle@users.noreply.github.com> Date: Tue, 9 Apr 2024 09:13:06 -0700 Subject: [PATCH 02/14] Adds a series of UI tests for State Restoration Task/Issue URL: https://app.asana.com/0/1199230911884351/1205717021705374/f Tech Design URL: CC: **Description**: Adds a series of UI tests for State Restoration **Steps to test this PR**: 1. Open the scheme **UI Tests** 2. Navigate to the test pane 3. Run `StateRestorationTests` **Note**: If this builds a `review` build that is treated as a first run on your system during every test (for instance, asking you where to put the application), please **build and run** the scheme once instead of running its tests, and go through the new app install first run steps once before running the tests, and answer any first install questions. **UI Tests general guidelines for everyone**: * It (unfortunately!) isn't possible to multitask while running UI tests on your local machine, because your mousing/interface usage will change or intercept screen interactions that the tests depend on. * Much as we all want UI testing to work on multi-monitor setups, we have noticed specific focus issues related to waiting for focus across two monitors simultaneously, so we have decided to test the tests on a single monitor. * We are currently testing with English as the app destination language. * If you experience test failures in your local environment, please always send the `xcresult` bundle so it is possible to view what happened differently on your machine, even if it seems like the same failure as a previous failure (it may be subtly different). Please give its screenshots or screen recording a look first before sending, just to rule out one-off failure causes like an unrelated app unexpectedly throwing up an update helper (or something similar) in front of where `XCUITest` is waiting for a UI element to exist. * Since the `xcresult` bundle will include a screen recording or screenshots, for your privacy, please consider things like your chats, calls, and open documents that you don't want to send to a contractor when you are running the tests. Thank you very much for your help! --- DuckDuckGo.xcodeproj/project.pbxproj | 4 + .../View/PreferencesGeneralView.swift | 8 +- UITests/StateRestorationTests.swift | 134 ++++++++++++++++++ 3 files changed, 143 insertions(+), 3 deletions(-) create mode 100644 UITests/StateRestorationTests.swift diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index 17154ba5d0..d7da5c939e 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -3303,6 +3303,7 @@ EE7295ED2A545C0A008C0991 /* NetworkProtection in Frameworks */ = {isa = PBXBuildFile; productRef = EE7295EC2A545C0A008C0991 /* NetworkProtection */; }; EE7295EF2A545C12008C0991 /* NetworkProtection in Frameworks */ = {isa = PBXBuildFile; productRef = EE7295EE2A545C12008C0991 /* NetworkProtection */; }; EE7F74912BB5D76600CD9456 /* BookmarksBarTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = EE7F74902BB5D76600CD9456 /* BookmarksBarTests.swift */; }; + EE9D81C32BC57A3700338BE3 /* StateRestorationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = EE9D81C22BC57A3700338BE3 /* StateRestorationTests.swift */; }; EEA3EEB12B24EBD000E8333A /* NetworkProtectionVPNCountryLabelsModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = EEA3EEB02B24EBD000E8333A /* NetworkProtectionVPNCountryLabelsModel.swift */; }; EEA3EEB32B24EC0600E8333A /* VPNLocationViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = EEA3EEB22B24EC0600E8333A /* VPNLocationViewModel.swift */; }; EEAD7A7C2A1D3E20002A24E7 /* AppLauncher.swift in Sources */ = {isa = PBXBuildFile; fileRef = EEAD7A6E2A1D3E1F002A24E7 /* AppLauncher.swift */; }; @@ -4777,6 +4778,7 @@ EE66418B2B9B1981005BCD17 /* NetworkProtectionTokenStore+SubscriptionTokenKeychainStorage.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "NetworkProtectionTokenStore+SubscriptionTokenKeychainStorage.swift"; sourceTree = ""; }; EE66666E2B56EDE4001D898D /* VPNLocationsHostingViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VPNLocationsHostingViewController.swift; sourceTree = ""; }; EE7F74902BB5D76600CD9456 /* BookmarksBarTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = BookmarksBarTests.swift; sourceTree = ""; }; + EE9D81C22BC57A3700338BE3 /* StateRestorationTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = StateRestorationTests.swift; sourceTree = ""; }; EEA3EEB02B24EBD000E8333A /* NetworkProtectionVPNCountryLabelsModel.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = NetworkProtectionVPNCountryLabelsModel.swift; sourceTree = ""; }; EEA3EEB22B24EC0600E8333A /* VPNLocationViewModel.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = VPNLocationViewModel.swift; sourceTree = ""; }; EEAD7A6E2A1D3E1F002A24E7 /* AppLauncher.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AppLauncher.swift; sourceTree = ""; }; @@ -6562,6 +6564,7 @@ EE7F74902BB5D76600CD9456 /* BookmarksBarTests.swift */, EE02D41B2BB460A600DBE6B3 /* BrowsingHistoryTests.swift */, EE0429DF2BA31D2F009EB20F /* FindInPageTests.swift */, + EE9D81C22BC57A3700338BE3 /* StateRestorationTests.swift */, 7B4CE8E626F02134009134B1 /* TabBarTests.swift */, ); path = UITests; @@ -12282,6 +12285,7 @@ EE02D41A2BB4609900DBE6B3 /* UITests.swift in Sources */, EE0429E02BA31D2F009EB20F /* FindInPageTests.swift in Sources */, EE02D4212BB460FE00DBE6B3 /* StringExtension.swift in Sources */, + EE9D81C32BC57A3700338BE3 /* StateRestorationTests.swift in Sources */, EE54F7B32BBFEA49006218DB /* BookmarksAndFavoritesTests.swift in Sources */, EE02D4222BB4611A00DBE6B3 /* TestsURLExtension.swift in Sources */, 7B4CE8E726F02135009134B1 /* TabBarTests.swift in Sources */, diff --git a/DuckDuckGo/Preferences/View/PreferencesGeneralView.swift b/DuckDuckGo/Preferences/View/PreferencesGeneralView.swift index 591961ee75..521893ba2e 100644 --- a/DuckDuckGo/Preferences/View/PreferencesGeneralView.swift +++ b/DuckDuckGo/Preferences/View/PreferencesGeneralView.swift @@ -39,11 +39,13 @@ extension Preferences { PreferencePaneSubSection { Picker(selection: $startupModel.restorePreviousSession, content: { Text(UserText.showHomePage).tag(false) - .padding(.bottom, 4) + .padding(.bottom, 4).accessibilityIdentifier("PreferencesGeneralView.stateRestorePicker.openANewWindow") Text(UserText.reopenAllWindowsFromLastSession).tag(true) + .accessibilityIdentifier("PreferencesGeneralView.stateRestorePicker.reopenAllWindowsFromLastSession") }, label: {}) - .pickerStyle(.radioGroup) - .offset(x: PreferencesViews.Const.pickerHorizontalOffset) + .pickerStyle(.radioGroup) + .offset(x: PreferencesViews.Const.pickerHorizontalOffset) + .accessibilityIdentifier("PreferencesGeneralView.stateRestorePicker") } } diff --git a/UITests/StateRestorationTests.swift b/UITests/StateRestorationTests.swift new file mode 100644 index 0000000000..32f86a0d2e --- /dev/null +++ b/UITests/StateRestorationTests.swift @@ -0,0 +1,134 @@ +// +// StateRestorationTests.swift +// +// Copyright © 2024 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 XCTest + +class StateRestorationTests: XCTestCase { + private var app: XCUIApplication! + private var firstPageTitle: String! + private var secondPageTitle: String! + private var firstURLForBookmarksBar: URL! + private var secondURLForBookmarksBar: URL! + private let titleStringLength = 12 + private var addressBarTextField: XCUIElement! + private var settingsGeneralButton: XCUIElement! + private var openANewWindowPreference: XCUIElement! + private var reopenAllWindowsFromLastSessionPreference: XCUIElement! + + override func setUpWithError() throws { + continueAfterFailure = false + app = XCUIApplication() + app.launchEnvironment["UITEST_MODE"] = "1" + firstPageTitle = UITests.randomPageTitle(length: titleStringLength) + secondPageTitle = UITests.randomPageTitle(length: titleStringLength) + firstURLForBookmarksBar = UITests.simpleServedPage(titled: firstPageTitle) + secondURLForBookmarksBar = UITests.simpleServedPage(titled: secondPageTitle) + addressBarTextField = app.windows.textFields["AddressBarViewController.addressBarTextField"] + settingsGeneralButton = app.buttons["PreferencesSidebar.generalButton"] + openANewWindowPreference = app.radioButtons["PreferencesGeneralView.stateRestorePicker.openANewWindow"] + reopenAllWindowsFromLastSessionPreference = app.radioButtons["PreferencesGeneralView.stateRestorePicker.reopenAllWindowsFromLastSession"] + + app.launch() + app.typeKey("w", modifierFlags: [.command, .option, .shift]) // Let's enforce a single window + app.typeKey("n", modifierFlags: .command) + } + + override func tearDownWithError() throws { + app.terminate() + } + + func test_tabStateAtRelaunch_shouldContainTwoSitesVisitedInPreviousSession_whenReopenAllWindowsFromLastSessionIsSet() { + app.typeKey(",", modifierFlags: [.command]) // Open settings + settingsGeneralButton.click(forDuration: 0.5, thenDragTo: settingsGeneralButton) + reopenAllWindowsFromLastSessionPreference.clickAfterExistenceTestSucceeds() + app.typeKey("w", modifierFlags: [.command, .option, .shift]) // Close windows + app.typeKey("n", modifierFlags: .command) + XCTAssertTrue( + addressBarTextField.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The address bar text field didn't become available in a reasonable timeframe." + ) + addressBarTextField.typeURL(firstURLForBookmarksBar) + XCTAssertTrue( + app.windows.webViews[firstPageTitle].waitForExistence(timeout: UITests.Timeouts.elementExistence), + "Site didn't load with the expected title in a reasonable timeframe." + ) + app.typeKey("t", modifierFlags: [.command]) + app.typeURL(secondURLForBookmarksBar) + XCTAssertTrue( + app.windows.webViews[secondPageTitle].waitForExistence(timeout: UITests.Timeouts.elementExistence), + "Site didn't load with the expected title in a reasonable timeframe." + ) + + app.terminate() + app.launch() + + XCTAssertTrue( + app.windows.webViews[secondPageTitle].waitForExistence(timeout: UITests.Timeouts.elementExistence), + "Second visited site wasn't found in a webview with the expected title in a reasonable timeframe." + ) + app.typeKey("w", modifierFlags: [.command]) + XCTAssertTrue( + app.windows.webViews[firstPageTitle].waitForExistence(timeout: UITests.Timeouts.elementExistence), + "First visited site wasn't found in a webview with the expected title in a reasonable timeframe." + ) + } + + func test_tabStateAtRelaunch_shouldContainNoSitesVisitedInPreviousSession_whenReopenAllWindowsFromLastSessionIsUnset() { + app.typeKey(",", modifierFlags: [.command]) // Open settings + settingsGeneralButton.click(forDuration: 0.5, thenDragTo: settingsGeneralButton) + openANewWindowPreference.clickAfterExistenceTestSucceeds() + app.typeKey("w", modifierFlags: [.command, .option, .shift]) // Close windows + app.typeKey("n", modifierFlags: .command) + XCTAssertTrue( + addressBarTextField.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The address bar text field didn't become available in a reasonable timeframe." + ) + addressBarTextField.typeURL(firstURLForBookmarksBar) + XCTAssertTrue( + app.windows.webViews[firstPageTitle].waitForExistence(timeout: UITests.Timeouts.elementExistence), + "Site didn't load with the expected title in a reasonable timeframe." + ) + app.typeKey("t", modifierFlags: [.command]) + app.typeURL(secondURLForBookmarksBar) + XCTAssertTrue( + app.windows.webViews[secondPageTitle].waitForExistence(timeout: UITests.Timeouts.elementExistence), + "Site didn't load with the expected title in a reasonable timeframe." + ) + + app.terminate() + app.launch() + + XCTAssertTrue( + app.windows.webViews[secondPageTitle].waitForNonExistence(timeout: UITests.Timeouts.elementExistence), + "Second visited site from previous session should not be in any webview." + ) + XCTAssertTrue( + app.windows.webViews[firstPageTitle].waitForNonExistence(timeout: UITests.Timeouts.elementExistence), + "First visited site from previous session should not be in any webview." + ) + app.typeKey("w", modifierFlags: [.command]) + XCTAssertTrue( + app.windows.webViews[firstPageTitle].waitForNonExistence(timeout: UITests.Timeouts.elementExistence), + "First visited site from previous session should not be in any webview." + ) + XCTAssertTrue( + app.windows.webViews[secondPageTitle].waitForNonExistence(timeout: UITests.Timeouts.elementExistence), + "Second visited site from previous session should not be in any webview." + ) + } +} From 04e56c9ecf961163b269c9db65399a2012685095 Mon Sep 17 00:00:00 2001 From: Dax the Duck Date: Tue, 9 Apr 2024 18:01:43 +0000 Subject: [PATCH 03/14] Bump version to 1.82.1 (155) --- Configuration/BuildNumber.xcconfig | 2 +- Configuration/Version.xcconfig | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Configuration/BuildNumber.xcconfig b/Configuration/BuildNumber.xcconfig index 92feb2ccd4..8d555e6249 100644 --- a/Configuration/BuildNumber.xcconfig +++ b/Configuration/BuildNumber.xcconfig @@ -1 +1 @@ -CURRENT_PROJECT_VERSION = 152 +CURRENT_PROJECT_VERSION = 155 diff --git a/Configuration/Version.xcconfig b/Configuration/Version.xcconfig index 4aebe00d0f..0868ba0c18 100644 --- a/Configuration/Version.xcconfig +++ b/Configuration/Version.xcconfig @@ -1 +1 @@ -MARKETING_VERSION = 1.82.0 +MARKETING_VERSION = 1.82.1 From fb1e132c664f973e1c0acd88e6c609c3c6b00b14 Mon Sep 17 00:00:00 2001 From: Diego Rey Mendez Date: Tue, 9 Apr 2024 20:43:37 +0200 Subject: [PATCH 04/14] Hotfixing an issue with VPN visibility for certain waitlist users (#2591) Task/Issue URL: https://app.asana.com/0/0/1207040067636855/f Description Fixes an issue where some users were seeing waitlist UI when the subscription was avilable. --- .../View/NavigationBarViewController.swift | 30 ++++++++++++------- .../NetworkProtectionFeatureDisabler.swift | 4 ++- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/DuckDuckGo/NavigationBar/View/NavigationBarViewController.swift b/DuckDuckGo/NavigationBar/View/NavigationBarViewController.swift index 2148a66473..fbf3cc1ae8 100644 --- a/DuckDuckGo/NavigationBar/View/NavigationBarViewController.swift +++ b/DuckDuckGo/NavigationBar/View/NavigationBarViewController.swift @@ -338,18 +338,26 @@ final class NavigationBarViewController: NSViewController { } #endif - // 1. If the user is on the waitlist but hasn't been invited or accepted terms and conditions, show the waitlist screen. - // 2. If the user has no waitlist state but has an auth token, show the NetP popover. - // 3. If the user has no state of any kind, show the waitlist screen. - - if NetworkProtectionWaitlist().shouldShowWaitlistViewController { - NetworkProtectionWaitlistViewControllerPresenter.show() - DailyPixel.fire(pixel: .networkProtectionWaitlistIntroDisplayed, frequency: .dailyAndCount) - } else if NetworkProtectionKeychainTokenStore().isFeatureActivated { - popovers.toggleNetworkProtectionPopover(usingView: networkProtectionButton, withDelegate: networkProtectionButtonModel) + // Note: the following code is quite contrived but we're aiming to hotfix issues without mixing subscription and + // waitlist logic. This should be cleaned up once waitlist can safely be removed. + + if DefaultSubscriptionFeatureAvailability().isFeatureAvailable { + if NetworkProtectionKeychainTokenStore().isFeatureActivated { + popovers.toggleNetworkProtectionPopover(usingView: networkProtectionButton, withDelegate: networkProtectionButtonModel) + } } else { - NetworkProtectionWaitlistViewControllerPresenter.show() - DailyPixel.fire(pixel: .networkProtectionWaitlistIntroDisplayed, frequency: .dailyAndCount) + // 1. If the user is on the waitlist but hasn't been invited or accepted terms and conditions, show the waitlist screen. + // 2. If the user has no waitlist state but has an auth token, show the NetP popover. + // 3. If the user has no state of any kind, show the waitlist screen. + + if NetworkProtectionWaitlist().shouldShowWaitlistViewController { + NetworkProtectionWaitlistViewControllerPresenter.show() + DailyPixel.fire(pixel: .networkProtectionWaitlistIntroDisplayed, frequency: .dailyAndCount) + } else if NetworkProtectionKeychainTokenStore().isFeatureActivated { + popovers.toggleNetworkProtectionPopover(usingView: networkProtectionButton, withDelegate: networkProtectionButtonModel) + } else { + NetworkProtectionWaitlistViewControllerPresenter.show() + } } } #endif diff --git a/DuckDuckGo/Waitlist/NetworkProtectionFeatureDisabler.swift b/DuckDuckGo/Waitlist/NetworkProtectionFeatureDisabler.swift index ba12c9cdd2..d7cc337991 100644 --- a/DuckDuckGo/Waitlist/NetworkProtectionFeatureDisabler.swift +++ b/DuckDuckGo/Waitlist/NetworkProtectionFeatureDisabler.swift @@ -76,6 +76,9 @@ final class NetworkProtectionFeatureDisabler: NetworkProtectionFeatureDisabling @MainActor @discardableResult func disable(keepAuthToken: Bool, uninstallSystemExtension: Bool) async -> Bool { + // We can do this optimistically as it has little if any impact. + unpinNetworkProtection() + // To disable NetP we need the login item to be running // This should be fine though as we'll disable them further down below guard canUninstall(includingSystemExtension: uninstallSystemExtension) else { @@ -85,7 +88,6 @@ final class NetworkProtectionFeatureDisabler: NetworkProtectionFeatureDisabling isDisabling = true defer { - unpinNetworkProtection() resetUserDefaults(uninstallSystemExtension: uninstallSystemExtension) } From e562f1166d12ea4d095b45ae28f210a2fc7f215f Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Tue, 9 Apr 2024 22:32:31 +0200 Subject: [PATCH 05/14] Remove non-existent networkProtectionWaitlistIntroDisplayed pixel --- DuckDuckGo/NavigationBar/View/NavigationBarViewController.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/DuckDuckGo/NavigationBar/View/NavigationBarViewController.swift b/DuckDuckGo/NavigationBar/View/NavigationBarViewController.swift index 55eb55aa20..0274af6b08 100644 --- a/DuckDuckGo/NavigationBar/View/NavigationBarViewController.swift +++ b/DuckDuckGo/NavigationBar/View/NavigationBarViewController.swift @@ -323,7 +323,6 @@ final class NavigationBarViewController: NSViewController { if NetworkProtectionWaitlist().shouldShowWaitlistViewController { NetworkProtectionWaitlistViewControllerPresenter.show() - DailyPixel.fire(pixel: .networkProtectionWaitlistIntroDisplayed, frequency: .dailyAndCount) } else if NetworkProtectionKeychainTokenStore().isFeatureActivated { popovers.toggleNetworkProtectionPopover(usingView: networkProtectionButton, withDelegate: networkProtectionButtonModel) } else { From faadae581ef25e7e2734b1a45da1abc7ccc36282 Mon Sep 17 00:00:00 2001 From: Dax the Duck Date: Tue, 9 Apr 2024 20:42:26 +0000 Subject: [PATCH 06/14] Bump version to 1.83.0 (156) --- Configuration/BuildNumber.xcconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Configuration/BuildNumber.xcconfig b/Configuration/BuildNumber.xcconfig index 8d555e6249..1896f3713a 100644 --- a/Configuration/BuildNumber.xcconfig +++ b/Configuration/BuildNumber.xcconfig @@ -1 +1 @@ -CURRENT_PROJECT_VERSION = 155 +CURRENT_PROJECT_VERSION = 156 From 22484b7755599b29e400c0cd9337179a6586fd35 Mon Sep 17 00:00:00 2001 From: Sam Symons Date: Tue, 9 Apr 2024 21:48:02 -0700 Subject: [PATCH 07/14] Hide the `DuckDuckGo on Other Platforms` section on the App Store (#2592) Task/Issue URL: https://app.asana.com/0/1199230911884351/1207033225544215/f Description: This PR updates the "DuckDuckGo on Other Platforms" section of the Settings page to only show on Sparkle builds. This is done because Apple forbids references to other platforms on the App Store. --- DuckDuckGo/Preferences/Model/PreferencesSection.swift | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/DuckDuckGo/Preferences/Model/PreferencesSection.swift b/DuckDuckGo/Preferences/Model/PreferencesSection.swift index 161ffb3440..edf564d458 100644 --- a/DuckDuckGo/Preferences/Model/PreferencesSection.swift +++ b/DuckDuckGo/Preferences/Model/PreferencesSection.swift @@ -49,7 +49,12 @@ struct PreferencesSection: Hashable, Identifiable { return panes }() +#if APPSTORE + // App Store guidelines don't allow references to other platforms, so the Mac App Store build omits the otherPlatforms section. + let otherPanes: [PreferencePaneIdentifier] = [.about] +#else let otherPanes: [PreferencePaneIdentifier] = [.about, .otherPlatforms] +#endif var sections: [PreferencesSection] = [ .init(id: .privacyProtections, panes: privacyPanes), From 86f799f8e0a2bf4babeb693490357649c4eb2257 Mon Sep 17 00:00:00 2001 From: Dax the Duck Date: Wed, 10 Apr 2024 05:12:08 +0000 Subject: [PATCH 08/14] Bump version to 1.83.0 (157) --- Configuration/BuildNumber.xcconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Configuration/BuildNumber.xcconfig b/Configuration/BuildNumber.xcconfig index 1896f3713a..eb40c52354 100644 --- a/Configuration/BuildNumber.xcconfig +++ b/Configuration/BuildNumber.xcconfig @@ -1 +1 @@ -CURRENT_PROJECT_VERSION = 156 +CURRENT_PROJECT_VERSION = 157 From 39922beda185a96d7b1a2b4fcb37ecdb80cf0d6f Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Wed, 10 Apr 2024 10:36:38 +0500 Subject: [PATCH 09/14] Percent-decode download filenames (#2584) Task/Issue URL: https://app.asana.com/0/1199230911884351/1202199027702557/f --- DuckDuckGo/Common/Extensions/StringExtension.swift | 4 ++++ DuckDuckGo/FileDownload/Extensions/WKWebView+Download.swift | 6 +----- DuckDuckGo/FileDownload/Model/WebKitDownloadTask.swift | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/DuckDuckGo/Common/Extensions/StringExtension.swift b/DuckDuckGo/Common/Extensions/StringExtension.swift index eeb8e5bedb..6128d9906a 100644 --- a/DuckDuckGo/Common/Extensions/StringExtension.swift +++ b/DuckDuckGo/Common/Extensions/StringExtension.swift @@ -71,6 +71,10 @@ extension String { return result } + func replacingInvalidFileNameCharacters(with replacement: String = "_") -> String { + replacingOccurrences(of: "[~#@*+%{}<>\\[\\]|\"\\_^\\/:\\\\]", with: replacement, options: .regularExpression) + } + init(_ staticString: StaticString) { self = staticString.withUTF8Buffer { String(decoding: $0, as: UTF8.self) diff --git a/DuckDuckGo/FileDownload/Extensions/WKWebView+Download.swift b/DuckDuckGo/FileDownload/Extensions/WKWebView+Download.swift index ad211f4eba..757f50f845 100644 --- a/DuckDuckGo/FileDownload/Extensions/WKWebView+Download.swift +++ b/DuckDuckGo/FileDownload/Extensions/WKWebView+Download.swift @@ -24,11 +24,7 @@ import WebKit extension WKWebView { var suggestedFilename: String? { - guard let title = self.title?.replacingOccurrences(of: "[~#@*+%{}<>\\[\\]|\"\\_^\\/:\\\\]", - with: "_", - options: .regularExpression), - !title.isEmpty - else { + guard let title = self.title?.replacingInvalidFileNameCharacters(), !title.isEmpty else { return url?.suggestedFilename } return title.appending(".html") diff --git a/DuckDuckGo/FileDownload/Model/WebKitDownloadTask.swift b/DuckDuckGo/FileDownload/Model/WebKitDownloadTask.swift index a39f173fa1..ca3d3e26c9 100644 --- a/DuckDuckGo/FileDownload/Model/WebKitDownloadTask.swift +++ b/DuckDuckGo/FileDownload/Model/WebKitDownloadTask.swift @@ -560,7 +560,7 @@ extension WebKitDownloadTask: WKDownloadDelegate { progress.totalUnitCount = response.expectedContentLength } - var suggestedFilename = suggestedFilename + var suggestedFilename = (suggestedFilename.removingPercentEncoding ?? suggestedFilename).replacingInvalidFileNameCharacters() // sometimes suggesteFilename has an extension appended to already present URL file extension // e.g. feed.xml.rss for www.domain.com/rss.xml if let urlSuggestedFilename = response.url?.suggestedFilename, From 77f8b144ff9b4357beab09463cc3af7a168854b6 Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Wed, 10 Apr 2024 10:39:07 +0500 Subject: [PATCH 10/14] Update Privacy Dashboard URL on navigation commit (#2583) Task/Issue URL: https://app.asana.com/0/1177771139624306/1205436170208663/f --- .../TabExtensions/PrivacyDashboardTabExtension.swift | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/DuckDuckGo/Tab/TabExtensions/PrivacyDashboardTabExtension.swift b/DuckDuckGo/Tab/TabExtensions/PrivacyDashboardTabExtension.swift index ab33546a06..070374f0a2 100644 --- a/DuckDuckGo/Tab/TabExtensions/PrivacyDashboardTabExtension.swift +++ b/DuckDuckGo/Tab/TabExtensions/PrivacyDashboardTabExtension.swift @@ -136,13 +136,14 @@ extension PrivacyDashboardTabExtension: NavigationResponder { } @MainActor - func didReceiveRedirect(_ navigationAction: NavigationAction, for navigation: Navigation) { - resetDashboardInfo(for: navigationAction.url, didGoBackForward: false) + func didCommit(_ navigation: Navigation) { + resetDashboardInfo(for: navigation.url, didGoBackForward: navigation.navigationAction.navigationType.isBackForward) } - @MainActor - func didStart(_ navigation: Navigation) { - resetDashboardInfo(for: navigation.url, didGoBackForward: navigation.navigationAction.navigationType.isBackForward) + func navigationDidFinish(_ navigation: Navigation) { + if privacyInfo?.url != navigation.url { + resetDashboardInfo(for: navigation.url, didGoBackForward: navigation.navigationAction.navigationType.isBackForward) + } } } From b73b0b3f54bb8143861ee88f0fb2fa125d7c9334 Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Wed, 10 Apr 2024 12:02:29 +0500 Subject: [PATCH 11/14] Fix Open Downloads not working (#2576) Task/Issue URL: https://app.asana.com/0/1199230911884351/1207020636198102/f --- .../Model/DownloadViewModel.swift | 2 +- .../View/DownloadsViewController.swift | 22 +++++++++++++------ 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/DuckDuckGo/FileDownload/Model/DownloadViewModel.swift b/DuckDuckGo/FileDownload/Model/DownloadViewModel.swift index cdcdadf330..d22e8a9e6e 100644 --- a/DuckDuckGo/FileDownload/Model/DownloadViewModel.swift +++ b/DuckDuckGo/FileDownload/Model/DownloadViewModel.swift @@ -75,7 +75,7 @@ final class DownloadViewModel { } func update(with item: DownloadListItem) { - self.localURL = item.destinationURL + self.localURL = item.tempURL == nil ? item.destinationURL : nil // only return destination file URL for completed downloads self.filename = item.fileName let oldState = self.state let newState = State(item: item, shouldAnimateOnAppear: state.shouldAnimateOnAppear ?? true) diff --git a/DuckDuckGo/FileDownload/View/DownloadsViewController.swift b/DuckDuckGo/FileDownload/View/DownloadsViewController.swift index fb33a0788d..dac2e0bec7 100644 --- a/DuckDuckGo/FileDownload/View/DownloadsViewController.swift +++ b/DuckDuckGo/FileDownload/View/DownloadsViewController.swift @@ -158,35 +158,43 @@ final class DownloadsViewController: NSViewController { @IBAction func openDownloadsFolderAction(_ sender: Any) { let prefs = DownloadsPreferences.shared + let downloads = FileManager.default.urls(for: .downloadsDirectory, in: .userDomainMask)[0] var url: URL? var itemToSelect: URL? if prefs.alwaysRequestDownloadLocation { - url = FileManager.default.urls(for: .downloadsDirectory, in: .userDomainMask).first + url = prefs.lastUsedCustomDownloadLocation + // reveal the last completed download if let lastDownloaded = viewModel.items.first/* last added */(where: { // should still exist - $0.localURL != nil && FileManager.default.fileExists(atPath: $0.localURL!.deletingLastPathComponent().path) + if let url = $0.localURL, FileManager.default.fileExists(atPath: url.path) { true } else { false } }), let lastDownloadedURL = lastDownloaded.localURL, - // if no downloads are from the default Downloads folder - open the last downloaded item folder !viewModel.items.contains(where: { $0.localURL?.deletingLastPathComponent().path == url?.path }) || url == nil { url = lastDownloadedURL.deletingLastPathComponent() // select last downloaded item itemToSelect = lastDownloadedURL - } /* else fallback to default User‘s Downloads */ + } /* else fallback to the last location chosen in the Save Panel */ } else { // open preferred downlod location - url = prefs.effectiveDownloadLocation ?? FileManager.default.urls(for: .downloadsDirectory, in: .userDomainMask).first + url = prefs.effectiveDownloadLocation } - guard let url else { return } + let folder = url ?? downloads + + _=NSWorkspace.shared.selectFile(itemToSelect?.path, inFileViewerRootedAtPath: folder.path) + // hack for the sandboxed environment: + // when we have no permission to open a folder we don‘t have access to + // try to guess a file that would most probably exist and reveal it: it‘s the ".DS_Store" file + || NSWorkspace.shared.selectFile(folder.appendingPathComponent(".DS_Store").path, inFileViewerRootedAtPath: folder.path) + // fallback to default Downloads folder + || NSWorkspace.shared.selectFile(nil, inFileViewerRootedAtPath: downloads.path) self.dismiss() - NSWorkspace.shared.selectFile(itemToSelect?.path, inFileViewerRootedAtPath: url.path) } @IBAction func clearDownloadsAction(_ sender: Any) { From c3b0c7ba6e42f682f696a80bbcec8777c68d1152 Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Wed, 10 Apr 2024 12:10:20 +0500 Subject: [PATCH 12/14] Allow choosing downloads location in App Store builds (#2532) Task/Issue URL: https://app.asana.com/0/1199230911884351/1205665411589753/f - https://app.asana.com/0/0/1205084529216312/f - https://app.asana.com/0/1199178362774117/1201154025406383/f - https://app.asana.com/0/1199230911884351/1207020636198094/f - https://app.asana.com/0/1199230911884351/1207020636198108/f --- Configuration/App/DuckDuckGo.xcconfig | 8 +- Configuration/AppStore.xcconfig | 8 +- DuckDuckGo.xcodeproj/project.pbxproj | 22 ++ .../Extensions/FileManagerExtension.swift | 12 + .../Common/Extensions/URLExtension.swift | 44 +++ DuckDuckGo/Common/Logging/Logging.swift | 42 +++ .../FileDownload/Model/FilePresenter.swift | 256 ++++++++++-------- .../Model/NSURL+sandboxExtensionRetainCount.m | 40 +++ .../SecurityScopedFileURLController.swift | 179 ++++++++++++ .../Model/WebKitDownloadTask.swift | 101 ++++--- .../Services/DownloadListCoordinator.swift | 32 ++- .../Model/DownloadsPreferences.swift | 64 +++-- .../View/PreferencesGeneralView.swift | 4 +- .../Tab/View/BrowserTabViewController.swift | 19 +- DuckDuckGo/Tab/View/WebView.swift | 1 + .../Downloads/DownloadsIntegrationTests.swift | 18 +- .../FileDownload/FilePresenterTests.swift | 81 ++---- .../DownloadsPreferencesTests.swift | 9 +- sandbox-test-tool/SandboxTestTool.swift | 39 ++- 19 files changed, 714 insertions(+), 265 deletions(-) create mode 100644 DuckDuckGo/FileDownload/Model/NSURL+sandboxExtensionRetainCount.m create mode 100644 DuckDuckGo/FileDownload/Model/SecurityScopedFileURLController.swift diff --git a/Configuration/App/DuckDuckGo.xcconfig b/Configuration/App/DuckDuckGo.xcconfig index e0490eb81d..778bc1e09d 100644 --- a/Configuration/App/DuckDuckGo.xcconfig +++ b/Configuration/App/DuckDuckGo.xcconfig @@ -34,10 +34,10 @@ PROVISIONING_PROFILE_SPECIFIER[sdk=macosx*] = PROVISIONING_PROFILE_SPECIFIER[config=Release][sdk=macosx*] = MacOS Browser PROVISIONING_PROFILE_SPECIFIER[config=Review][sdk=macosx*] = MacOS Browser Product Review -GCC_PREPROCESSOR_DEFINITIONS[arch=*][sdk=*] = NETP_SYSTEM_EXTENSION=1 -GCC_PREPROCESSOR_DEFINITIONS[config=CI][arch=*][sdk=*] = NETP_SYSTEM_EXTENSION=1 DEBUG=1 CI=1 $(inherited) -GCC_PREPROCESSOR_DEFINITIONS[config=Debug][arch=*][sdk=*] = NETP_SYSTEM_EXTENSION=1 DEBUG=1 $(inherited) -GCC_PREPROCESSOR_DEFINITIONS[config=Review][arch=*][sdk=*] = NETP_SYSTEM_EXTENSION=1 REVIEW=1 $(inherited) +GCC_PREPROCESSOR_DEFINITIONS[arch=*][sdk=*] = NETP_SYSTEM_EXTENSION=1 SWIFT_OBJC_INTERFACE_HEADER_NAME=$(SWIFT_OBJC_INTERFACE_HEADER_NAME) +GCC_PREPROCESSOR_DEFINITIONS[config=CI][arch=*][sdk=*] = NETP_SYSTEM_EXTENSION=1 DEBUG=1 CI=1 SWIFT_OBJC_INTERFACE_HEADER_NAME=$(SWIFT_OBJC_INTERFACE_HEADER_NAME) $(inherited) +GCC_PREPROCESSOR_DEFINITIONS[config=Debug][arch=*][sdk=*] = NETP_SYSTEM_EXTENSION=1 DEBUG=1 SWIFT_OBJC_INTERFACE_HEADER_NAME=$(SWIFT_OBJC_INTERFACE_HEADER_NAME) $(inherited) +GCC_PREPROCESSOR_DEFINITIONS[config=Review][arch=*][sdk=*] = NETP_SYSTEM_EXTENSION=1 REVIEW=1 SWIFT_OBJC_INTERFACE_HEADER_NAME=$(SWIFT_OBJC_INTERFACE_HEADER_NAME) $(inherited) SWIFT_ACTIVE_COMPILATION_CONDITIONS[arch=*][sdk=*] = NETP_SYSTEM_EXTENSION $(FEATURE_FLAGS) SWIFT_ACTIVE_COMPILATION_CONDITIONS[config=CI][arch=*][sdk=*] = NETP_SYSTEM_EXTENSION DEBUG CI $(FEATURE_FLAGS) diff --git a/Configuration/AppStore.xcconfig b/Configuration/AppStore.xcconfig index 1fbb567afd..30ec838615 100644 --- a/Configuration/AppStore.xcconfig +++ b/Configuration/AppStore.xcconfig @@ -23,10 +23,10 @@ MAIN_BUNDLE_IDENTIFIER[config=Debug][sdk=*] = $(MAIN_BUNDLE_IDENTIFIER_PREFIX).d MAIN_BUNDLE_IDENTIFIER[config=CI][sdk=*] = $(MAIN_BUNDLE_IDENTIFIER_PREFIX).debug MAIN_BUNDLE_IDENTIFIER[config=Review][sdk=*] = $(MAIN_BUNDLE_IDENTIFIER_PREFIX).review -GCC_PREPROCESSOR_DEFINITIONS[arch=*][sdk=*] = APPSTORE=1 -GCC_PREPROCESSOR_DEFINITIONS[config=CI][arch=*][sdk=*] = APPSTORE=1 DEBUG=1 CI=1 $(inherited) -GCC_PREPROCESSOR_DEFINITIONS[config=Debug][arch=*][sdk=*] = APPSTORE=1 DEBUG=1 $(inherited) -GCC_PREPROCESSOR_DEFINITIONS[config=Review][arch=*][sdk=*] = APPSTORE=1 REVIEW=1 $(inherited) +GCC_PREPROCESSOR_DEFINITIONS[arch=*][sdk=*] = APPSTORE=1 SWIFT_OBJC_INTERFACE_HEADER_NAME=$(SWIFT_OBJC_INTERFACE_HEADER_NAME) +GCC_PREPROCESSOR_DEFINITIONS[config=CI][arch=*][sdk=*] = APPSTORE=1 DEBUG=1 CI=1 SWIFT_OBJC_INTERFACE_HEADER_NAME=$(SWIFT_OBJC_INTERFACE_HEADER_NAME) $(inherited) +GCC_PREPROCESSOR_DEFINITIONS[config=Debug][arch=*][sdk=*] = APPSTORE=1 DEBUG=1 SWIFT_OBJC_INTERFACE_HEADER_NAME=$(SWIFT_OBJC_INTERFACE_HEADER_NAME) $(inherited) +GCC_PREPROCESSOR_DEFINITIONS[config=Review][arch=*][sdk=*] = APPSTORE=1 REVIEW=1 SWIFT_OBJC_INTERFACE_HEADER_NAME=$(SWIFT_OBJC_INTERFACE_HEADER_NAME) $(inherited) MACOSX_DEPLOYMENT_TARGET = 12.3 diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index d7da5c939e..0ece6426d8 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -3063,6 +3063,12 @@ B6ABC5962B4861D4008343B9 /* FocusableTextField.swift in Sources */ = {isa = PBXBuildFile; fileRef = B6ABC5952B4861D4008343B9 /* FocusableTextField.swift */; }; B6ABC5972B4861D4008343B9 /* FocusableTextField.swift in Sources */ = {isa = PBXBuildFile; fileRef = B6ABC5952B4861D4008343B9 /* FocusableTextField.swift */; }; B6ABC5982B4861D4008343B9 /* FocusableTextField.swift in Sources */ = {isa = PBXBuildFile; fileRef = B6ABC5952B4861D4008343B9 /* FocusableTextField.swift */; }; + B6ABD0CA2BC03F610000EB69 /* SecurityScopedFileURLController.swift in Sources */ = {isa = PBXBuildFile; fileRef = B6ABD0C92BC03F610000EB69 /* SecurityScopedFileURLController.swift */; }; + B6ABD0CB2BC03F610000EB69 /* SecurityScopedFileURLController.swift in Sources */ = {isa = PBXBuildFile; fileRef = B6ABD0C92BC03F610000EB69 /* SecurityScopedFileURLController.swift */; }; + B6ABD0CC2BC03F610000EB69 /* SecurityScopedFileURLController.swift in Sources */ = {isa = PBXBuildFile; fileRef = B6ABD0C92BC03F610000EB69 /* SecurityScopedFileURLController.swift */; }; + B6ABD0CE2BC042CE0000EB69 /* NSURL+sandboxExtensionRetainCount.m in Sources */ = {isa = PBXBuildFile; fileRef = B6ABD0CD2BC042CE0000EB69 /* NSURL+sandboxExtensionRetainCount.m */; }; + B6ABD0CF2BC042CE0000EB69 /* NSURL+sandboxExtensionRetainCount.m in Sources */ = {isa = PBXBuildFile; fileRef = B6ABD0CD2BC042CE0000EB69 /* NSURL+sandboxExtensionRetainCount.m */; }; + B6ABD0D02BC042CE0000EB69 /* NSURL+sandboxExtensionRetainCount.m in Sources */ = {isa = PBXBuildFile; fileRef = B6ABD0CD2BC042CE0000EB69 /* NSURL+sandboxExtensionRetainCount.m */; }; B6AE39F129373AF200C37AA4 /* EmptyAttributionRulesProver.swift in Sources */ = {isa = PBXBuildFile; fileRef = B6AE39F029373AF200C37AA4 /* EmptyAttributionRulesProver.swift */; }; B6AE39F329374AEC00C37AA4 /* OHHTTPStubs in Frameworks */ = {isa = PBXBuildFile; productRef = B6AE39F229374AEC00C37AA4 /* OHHTTPStubs */; }; B6AE39F529374AEC00C37AA4 /* OHHTTPStubsSwift in Frameworks */ = {isa = PBXBuildFile; productRef = B6AE39F429374AEC00C37AA4 /* OHHTTPStubsSwift */; }; @@ -3203,6 +3209,9 @@ B6EC37FC29B83E99001ACE79 /* TestsURLExtension.swift in Sources */ = {isa = PBXBuildFile; fileRef = B6EC37FB29B83E99001ACE79 /* TestsURLExtension.swift */; }; B6EC37FD29B83E99001ACE79 /* TestsURLExtension.swift in Sources */ = {isa = PBXBuildFile; fileRef = B6EC37FB29B83E99001ACE79 /* TestsURLExtension.swift */; }; B6EC37FF29B8D915001ACE79 /* Configuration in Frameworks */ = {isa = PBXBuildFile; productRef = B6EC37FE29B8D915001ACE79 /* Configuration */; }; + B6EECB302BC3FA5A00B3CB77 /* SecurityScopedFileURLController.swift in Sources */ = {isa = PBXBuildFile; fileRef = B6ABD0C92BC03F610000EB69 /* SecurityScopedFileURLController.swift */; }; + B6EECB312BC3FAB100B3CB77 /* NSURL+sandboxExtensionRetainCount.m in Sources */ = {isa = PBXBuildFile; fileRef = B6ABD0CD2BC042CE0000EB69 /* NSURL+sandboxExtensionRetainCount.m */; }; + B6EECB322BC40A1400B3CB77 /* FileManagerExtension.swift in Sources */ = {isa = PBXBuildFile; fileRef = B6E61EE2263AC0C8004E11AB /* FileManagerExtension.swift */; }; B6EEDD7D2B8C69E900637EBC /* TabContentTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B6EEDD7C2B8C69E900637EBC /* TabContentTests.swift */; }; B6EEDD7E2B8C69E900637EBC /* TabContentTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B6EEDD7C2B8C69E900637EBC /* TabContentTests.swift */; }; B6F1C80B2761C45400334924 /* LocalUnprotectedDomains.swift in Sources */ = {isa = PBXBuildFile; fileRef = 336B39E22726B4B700C417D3 /* LocalUnprotectedDomains.swift */; }; @@ -4645,6 +4654,8 @@ B6AAAC2C260330580029438D /* PublishedAfter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PublishedAfter.swift; sourceTree = ""; }; B6AAAC3D26048F690029438D /* RandomAccessCollectionExtension.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RandomAccessCollectionExtension.swift; sourceTree = ""; }; B6ABC5952B4861D4008343B9 /* FocusableTextField.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FocusableTextField.swift; sourceTree = ""; }; + B6ABD0C92BC03F610000EB69 /* SecurityScopedFileURLController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SecurityScopedFileURLController.swift; sourceTree = ""; }; + B6ABD0CD2BC042CE0000EB69 /* NSURL+sandboxExtensionRetainCount.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = "NSURL+sandboxExtensionRetainCount.m"; sourceTree = ""; }; B6AE39F029373AF200C37AA4 /* EmptyAttributionRulesProver.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EmptyAttributionRulesProver.swift; sourceTree = ""; }; B6AE74332609AFCE005B9B1A /* ProgressEstimationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProgressEstimationTests.swift; sourceTree = ""; }; B6B040072B95C4C80085279D /* Downloads 2.xcdatamodel */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcdatamodel; path = "Downloads 2.xcdatamodel"; sourceTree = ""; }; @@ -6690,6 +6701,8 @@ B6C0B23826E742610031CB7F /* FileDownloadError.swift */, 856C98DE257014BD00A22F1F /* FileDownloadManager.swift */, B6E6B9E22BA1F5F1008AA7E1 /* FilePresenter.swift */, + B6ABD0C92BC03F610000EB69 /* SecurityScopedFileURLController.swift */, + B6ABD0CD2BC042CE0000EB69 /* NSURL+sandboxExtensionRetainCount.m */, B6A924D82664C72D001A28CA /* WebKitDownloadTask.swift */, B6CC266B2BAD9CD800F53F8D /* FileProgressPresenter.swift */, ); @@ -10727,6 +10740,7 @@ 7B430EA22A71411A00BAC4A1 /* NetworkProtectionSimulateFailureMenu.swift in Sources */, 3706FBD5293F65D500E42796 /* TabCollection+NSSecureCoding.swift in Sources */, 3706FBD6293F65D500E42796 /* Instruments.swift in Sources */, + B6ABD0CF2BC042CE0000EB69 /* NSURL+sandboxExtensionRetainCount.m in Sources */, B62B483F2ADE48DE000DECE5 /* ArrayBuilder.swift in Sources */, 569277C229DDCBB500B633EF /* HomePageContinueSetUpModel.swift in Sources */, 3706FBD7293F65D500E42796 /* ContentBlockerRulesLists.swift in Sources */, @@ -10872,6 +10886,7 @@ 3706FC32293F65D500E42796 /* MoreOptionsMenu.swift in Sources */, 3706FC34293F65D500E42796 /* PermissionAuthorizationViewController.swift in Sources */, 3706FC35293F65D500E42796 /* BookmarkNode.swift in Sources */, + B6ABD0CB2BC03F610000EB69 /* SecurityScopedFileURLController.swift in Sources */, 31EF1E822B63FFC200E6DB17 /* DataBrokerProtectionLoginItemScheduler.swift in Sources */, B6B140892ABDBCC1004F8E85 /* HoverTrackingArea.swift in Sources */, 3706FC36293F65D500E42796 /* LongPressButton.swift in Sources */, @@ -12027,6 +12042,7 @@ 4B2F565D2B38F93E001214C0 /* NetworkProtectionSubscriptionEventHandler.swift in Sources */, 4B957B082AC7AE700062CA31 /* PixelDataStore.swift in Sources */, 4B957B092AC7AE700062CA31 /* WaitlistStorage.swift in Sources */, + B6ABD0D02BC042CE0000EB69 /* NSURL+sandboxExtensionRetainCount.m in Sources */, 4B957B0A2AC7AE700062CA31 /* Pixel.swift in Sources */, 4B957B0B2AC7AE700062CA31 /* PixelEvent.swift in Sources */, 4B957B0C2AC7AE700062CA31 /* TabBarFooter.swift in Sources */, @@ -12051,6 +12067,7 @@ F1B33DF42BAD929D001128B3 /* SubscriptionAppStoreRestorer.swift in Sources */, 4B957B1B2AC7AE700062CA31 /* ScriptSourceProviding.swift in Sources */, 4B957B1C2AC7AE700062CA31 /* CoreDataBookmarkImporter.swift in Sources */, + B6ABD0CC2BC03F610000EB69 /* SecurityScopedFileURLController.swift in Sources */, 4B957B1D2AC7AE700062CA31 /* SuggestionViewModel.swift in Sources */, 4B957B1E2AC7AE700062CA31 /* BookmarkManagedObject.swift in Sources */, 4B957B1F2AC7AE700062CA31 /* CSVLoginExporter.swift in Sources */, @@ -12895,6 +12912,7 @@ 4B37EE5F2B4CFC3C00A89A61 /* HomePageRemoteMessagingStorage.swift in Sources */, 4B9DB0472A983B24000927DB /* WaitlistRootView.swift in Sources */, 31F28C5128C8EEC500119F70 /* YoutubeOverlayUserScript.swift in Sources */, + B6ABD0CA2BC03F610000EB69 /* SecurityScopedFileURLController.swift in Sources */, B6040856274B830F00680351 /* DictionaryExtension.swift in Sources */, B684592725C93C0500DC17B6 /* Publishers.NestedObjectChanges.swift in Sources */, B6DA06E62913F39400225DE2 /* MenuItemSelectors.swift in Sources */, @@ -13016,6 +13034,7 @@ B6A9E46B2614618A0067D1B9 /* OperatingSystemVersionExtension.swift in Sources */, 4BDFA4AE27BF19E500648192 /* ToggleableScrollView.swift in Sources */, 1D36F4242A3B85C50052B527 /* TabCleanupPreparer.swift in Sources */, + B6ABD0CE2BC042CE0000EB69 /* NSURL+sandboxExtensionRetainCount.m in Sources */, 4B4D60DF2A0C875F00BCD287 /* NetworkProtectionOptionKeyExtension.swift in Sources */, 85AC3AEF25D5CE9800C7D2AA /* UserScripts.swift in Sources */, B643BF1427ABF772000BACEC /* NSWorkspaceExtension.swift in Sources */, @@ -13378,7 +13397,10 @@ B6E6BA062BA1FE10008AA7E1 /* NSApplicationExtension.swift in Sources */, B6E6B9F62BA1FD90008AA7E1 /* SandboxTestTool.swift in Sources */, B6E6BA252BA2EDDE008AA7E1 /* FileReadResult.swift in Sources */, + B6EECB322BC40A1400B3CB77 /* FileManagerExtension.swift in Sources */, + B6EECB302BC3FA5A00B3CB77 /* SecurityScopedFileURLController.swift in Sources */, B6E6BA052BA1FE09008AA7E1 /* URLExtension.swift in Sources */, + B6EECB312BC3FAB100B3CB77 /* NSURL+sandboxExtensionRetainCount.m in Sources */, B6E6BA202BA2E462008AA7E1 /* CollectionExtension.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; diff --git a/DuckDuckGo/Common/Extensions/FileManagerExtension.swift b/DuckDuckGo/Common/Extensions/FileManagerExtension.swift index c524d39c8c..b7e767fece 100644 --- a/DuckDuckGo/Common/Extensions/FileManagerExtension.swift +++ b/DuckDuckGo/Common/Extensions/FileManagerExtension.swift @@ -101,4 +101,16 @@ extension FileManager { return resolvedUrl.path.hasPrefix(trashUrl.path) } + /// Check if location pointed by the URL is writable by writing an empty data to it and removing the file if write succeeds + /// - Throws error if writing to the location fails + func checkWritability(_ url: URL) throws { + if fileExists(atPath: url.path), isWritableFile(atPath: url.path) { + return // we can write + } else { + // either we can‘t write or there‘s no file at the url – try writing throwing access error if no permission + try Data().write(to: url) + try removeItem(at: url) + } + } + } diff --git a/DuckDuckGo/Common/Extensions/URLExtension.swift b/DuckDuckGo/Common/Extensions/URLExtension.swift index 6ea129d2fb..b79720a9d5 100644 --- a/DuckDuckGo/Common/Extensions/URLExtension.swift +++ b/DuckDuckGo/Common/Extensions/URLExtension.swift @@ -469,6 +469,50 @@ extension URL { } + var isFileHidden: Bool { + get throws { + try self.resourceValues(forKeys: [.isHiddenKey]).isHidden ?? false + } + } + + mutating func setFileHidden(_ hidden: Bool) throws { + var resourceValues = URLResourceValues() + resourceValues.isHidden = true + try setResourceValues(resourceValues) + } + + /// Check if location pointed by the URL is writable + /// - Note: if there‘s no file at the URL, it will try to create a file and then remove it + func isWritableLocation() -> Bool { + do { + try FileManager.default.checkWritability(self) + return true + } catch { + return false + } + } + +#if DEBUG && APPSTORE + /// sandbox extension URL access should be stopped after SecurityScopedFileURLController is deallocated - this function validates it and breaks if the file is still writable + func ensureUrlIsNotWritable(or handler: () -> Void) { + let fm = FileManager.default + // is the URL ~/Downloads? + if self.resolvingSymlinksInPath() == fm.urls(for: .downloadsDirectory, in: .userDomainMask).first!.resolvingSymlinksInPath() { + assert(isWritableLocation()) + return + } + // is parent directory writable (e.g. ~/Downloads)? + if fm.isWritableFile(atPath: self.deletingLastPathComponent().path) + // trashed files are still accessible for some reason even after stopping access + || fm.isInTrash(self) + // other file is being saved at the same URL + || NSURL.activeSecurityScopedUrlUsages.contains(where: { $0.url !== self as NSURL && $0.url == self as NSURL }) + || !isWritableLocation() { return } + + handler() + } +#endif + // MARK: - System Settings static var fullDiskAccess = URL(string: "x-apple.systempreferences:com.apple.preference.security?Privacy_AllFiles")! diff --git a/DuckDuckGo/Common/Logging/Logging.swift b/DuckDuckGo/Common/Logging/Logging.swift index de04f91cb7..3740e0e223 100644 --- a/DuckDuckGo/Common/Logging/Logging.swift +++ b/DuckDuckGo/Common/Logging/Logging.swift @@ -140,3 +140,45 @@ func logOrAssertionFailure(_ message: String) { os_log("%{public}s", type: .error, message) #endif } + +#if DEBUG + +func breakByRaisingSigInt(_ description: String, file: StaticString = #file, line: Int = #line) { + let fileLine = "\(("\(file)" as NSString).lastPathComponent):\(line)" + os_log(""" + + + ------------------------------------------------------------------------------------------------------ + BREAK at %s: + ------------------------------------------------------------------------------------------------------ + + %s + + Hit Continue (^⌘Y) to continue program execution + ------------------------------------------------------------------------------------------------------ + + """, type: .debug, fileLine, description.components(separatedBy: "\n").map { " " + $0.trimmingWhitespace() }.joined(separator: "\n")) + raise(SIGINT) +} + +// get symbol from stack trace for a caller of a calling method +func callingSymbol() -> String { + let stackTrace = Thread.callStackSymbols + // find `callingSymbol` itself or dispatch_once_callout + var callingSymbolIdx = stackTrace.firstIndex(where: { $0.contains("_dispatch_once_callout") }) + ?? stackTrace.firstIndex(where: { $0.contains("callingSymbol") })! + // procedure calling `callingSymbol` + callingSymbolIdx += 1 + + var symbolName: String + repeat { + // caller for the procedure + callingSymbolIdx += 1 + let line = stackTrace[callingSymbolIdx].replacingOccurrences(of: Bundle.main.executableURL!.lastPathComponent, with: "DDG") + symbolName = String(line.split(separator: " ", maxSplits: 3)[3]).components(separatedBy: " + ")[0] + } while stackTrace[callingSymbolIdx - 1].contains(symbolName.dropping(suffix: "To")) // skip objc wrappers + + return symbolName +} + +#endif diff --git a/DuckDuckGo/FileDownload/Model/FilePresenter.swift b/DuckDuckGo/FileDownload/Model/FilePresenter.swift index be8d94e8a6..1770935af2 100644 --- a/DuckDuckGo/FileDownload/Model/FilePresenter.swift +++ b/DuckDuckGo/FileDownload/Model/FilePresenter.swift @@ -23,7 +23,6 @@ import Foundation private protocol FilePresenterDelegate: AnyObject { var logger: FilePresenterLogger { get } var url: URL? { get } - var primaryPresentedItemURL: URL? { get } func presentedItemDidMove(to newURL: URL) func accommodatePresentedItemDeletion() throws func accommodatePresentedItemEviction() throws @@ -57,12 +56,14 @@ internal class FilePresenter { final let presentedItemOperationQueue: OperationQueue fileprivate final weak var delegate: FilePresenterDelegate? - init(presentedItemOperationQueue: OperationQueue) { + init(presentedItemOperationQueue: OperationQueue, delegate: FilePresenterDelegate) { self.presentedItemOperationQueue = presentedItemOperationQueue + self.delegate = delegate } + final var fallbackPresentedItemURL: URL? final var presentedItemURL: URL? { - guard let delegate else { return nil } + guard let delegate else { return fallbackPresentedItemURL } FilePresenter.dispatchSourceQueue.async { // prevent owning FilePresenter deallocation inside the presentedItemURL getter withExtendedLifetime(delegate) {} @@ -72,12 +73,10 @@ internal class FilePresenter { } final func presentedItemDidMove(to newURL: URL) { - assert(delegate != nil) delegate?.presentedItemDidMove(to: newURL) } func accommodatePresentedItemDeletion(completionHandler: @escaping @Sendable ((any Error)?) -> Void) { - assert(delegate != nil) do { try delegate?.accommodatePresentedItemDeletion() completionHandler(nil) @@ -87,9 +86,8 @@ internal class FilePresenter { } func accommodatePresentedItemEviction(completionHandler: @escaping @Sendable ((any Error)?) -> Void) { - assert(delegate != nil) do { - try delegate?.accommodatePresentedItemEviction() + try delegate?.accommodatePresentedItemEviction() completionHandler(nil) } catch { completionHandler(error) @@ -100,74 +98,138 @@ internal class FilePresenter { final private class DelegatingRelatedFilePresenter: DelegatingFilePresenter { - var primaryPresentedItemURL: URL? { - let url = delegate?.primaryPresentedItemURL - return url + let primaryPresentedItemURL: URL? + + init(primaryPresentedItemURL: URL?, presentedItemOperationQueue: OperationQueue, delegate: FilePresenterDelegate) { + self.primaryPresentedItemURL = primaryPresentedItemURL + super.init(presentedItemOperationQueue: presentedItemOperationQueue, delegate: delegate) } } fileprivate let lock = NSLock() - private var innerPresenter: DelegatingFilePresenter? + private var innerPresenters = [DelegatingFilePresenter]() private var dispatchSourceCancellable: AnyCancellable? fileprivate let logger: any FilePresenterLogger - let primaryPresentedItemURL: URL? - - private var _url: URL? + private var urlController: SecurityScopedFileURLController? final var url: URL? { lock.withLock { - _url + urlController?.url } } private func setURL(_ newURL: URL?) { - guard let oldValue = lock.withLock({ () -> URL?? in - let oldValue = _url - guard oldValue != newURL else { return URL??.none } - _url = newURL - return oldValue - }) else { return } + guard let oldValue = lock.withLock({ _setURL(newURL) }) else { return } didSetURL(newURL, oldValue: oldValue) } + // inside locked scope + private func _setURL(_ newURL: URL?) -> URL?? /* returns old value (URL?) if new value was updated */ { + let oldValue = urlController?.url + guard oldValue != newURL else { return URL??.none } + guard let newURL else { + urlController = nil + return newURL + } + + // if the new url is pointing to the same path (only letter case has changed) - keep its sandbox extension in a new Controller + if let urlController, let oldValue, + oldValue.resolvingSymlinksInPath().path == newURL.resolvingSymlinksInPath().path, + urlController.isManagingSecurityScope { + urlController.updateUrlKeepingSandboxExtensionRetainCount(newURL) + } else { + urlController = SecurityScopedFileURLController(url: newURL, logger: logger) + } + + return oldValue + } + private var urlSubject = PassthroughSubject() final var urlPublisher: AnyPublisher { urlSubject.prepend(url).eraseToAnyPublisher() } - init(url: URL, primaryItemURL: URL? = nil, logger: FilePresenterLogger = OSLog.disabled, createIfNeededCallback: ((URL) throws -> URL)? = nil) throws { - self._url = url - self.primaryPresentedItemURL = primaryItemURL + /// - Parameter url: represented file URL access to which is coordinated by the File Presenter. + /// - Parameter consumeUnbalancedStartAccessingResource: assume the `url` is already accessible (e.g. after choosing the file using Open Panel). + /// would cause an unbalanced `stopAccessingSecurityScopedResource` call on the File Presenter deallocation. + /// - Note: see https://stackoverflow.com/questions/25627628/sandboxed-mac-app-exhausting-security-scoped-url-resources + init(url: URL, consumeUnbalancedStartAccessingResource: Bool = false, logger: FilePresenterLogger = OSLog.disabled, createIfNeededCallback: ((URL) throws -> URL)? = nil) throws { + self.urlController = SecurityScopedFileURLController(url: url, manageSecurityScope: consumeUnbalancedStartAccessingResource, logger: logger) + self.logger = logger - let innerPresenter: DelegatingFilePresenter - if primaryItemURL != nil { - innerPresenter = DelegatingRelatedFilePresenter(presentedItemOperationQueue: FilePresenter.presentedItemOperationQueue) + do { + try setupInnerPresenter(for: url, primaryItemURL: nil, createIfNeededCallback: createIfNeededCallback) + logger.log("🗄️ added file presenter for \"\(url.path)\"") + } catch { + removeFilePresenters() + throw error + } + } + + /// - Parameter url: represented file URL access to which is coordinated by the File Presenter. + /// - Parameter primaryItemURL: URL to a main file resource access to which has been granted. + /// Used to grant out-of-sandbox access to `url` representing a “related” resource like “download.duckload” where the `primaryItemURL` would point to “download.zip”. + /// - Note: the related (“duckload”) file extension should be registered in the Info.plist with `NSIsRelatedItemType` flag set to `true`. + /// - Note: when presenting a related item the security scoped resource access will always be stopped on the File Presenter deallocation + /// - Parameter consumeUnbalancedStartAccessingResource: assume the `url` is already accessible (e.g. after choosing the file using Open Panel). + /// would cause an unbalanced `stopAccessingSecurityScopedResource` call on the File Presenter deallocation. + init(url: URL, primaryItemURL: URL, logger: FilePresenterLogger = OSLog.disabled, createIfNeededCallback: ((URL) throws -> URL)? = nil) throws { + self.urlController = SecurityScopedFileURLController(url: url, logger: logger) + self.logger = logger + + do { + try setupInnerPresenter(for: url, primaryItemURL: primaryItemURL, createIfNeededCallback: createIfNeededCallback) + logger.log("🗄️ added file presenter for \"\(url.path) primary item: \"\(primaryItemURL.path)\"") + } catch { + removeFilePresenters() + throw error + } + } + + private func setupInnerPresenter(for url: URL, primaryItemURL: URL?, createIfNeededCallback: ((URL) throws -> URL)?) throws { + let innerPresenter = if let primaryItemURL { + DelegatingRelatedFilePresenter(primaryPresentedItemURL: primaryItemURL, presentedItemOperationQueue: FilePresenter.presentedItemOperationQueue, delegate: self) } else { - innerPresenter = DelegatingFilePresenter(presentedItemOperationQueue: FilePresenter.presentedItemOperationQueue) + DelegatingFilePresenter(presentedItemOperationQueue: FilePresenter.presentedItemOperationQueue, delegate: self) } - self.innerPresenter = innerPresenter - innerPresenter.delegate = self + self.innerPresenters = [innerPresenter] + NSFileCoordinator.addFilePresenter(innerPresenter) if !FileManager.default.fileExists(atPath: url.path) { - if let createFile = createIfNeededCallback { - logger.log("🗄️💥 creating file for presenter at \"\(url.path)\"") - self._url = try coordinateWrite(at: url, using: createFile) - - // re-add File Presenter for the updated URL + guard let createFile = createIfNeededCallback else { + throw CocoaError(.fileReadNoSuchFile, userInfo: [NSFilePathErrorKey: url.path]) + } + logger.log("🗄️💥 creating file for presenter at \"\(url.path)\"") + // create new file at the presented URL using the provided callback and update URL if needed + _=self._setURL( + try coordinateWrite(at: url, using: createFile) + ) + + if primaryItemURL == nil { + // Remove and re-add the file presenter for regular item presenters. NSFileCoordinator.removeFilePresenter(innerPresenter) NSFileCoordinator.addFilePresenter(innerPresenter) - - } else { - throw CocoaError(.fileReadNoSuchFile, userInfo: [NSFilePathErrorKey: url.path]) } } - addFSODispatchSource(for: url) + // to correctly handle file move events for a “related” item presenters we need to use a secondary presenter + if primaryItemURL != nil { + // set permanent original url without tracking file movements + // to correctly release the sandbox extension when the “related” presenter is removed + innerPresenter.fallbackPresentedItemURL = url + innerPresenter.delegate = nil + + let innerPresenter2 = DelegatingFilePresenter(presentedItemOperationQueue: FilePresenter.presentedItemOperationQueue, delegate: self) + NSFileCoordinator.addFilePresenter(innerPresenter2) + innerPresenters.append(innerPresenter2) + } - logger.log("🗄️ added file presenter for \"\(url.path)\"\(primaryPresentedItemURL != nil ? " primary item: \"\(primaryPresentedItemURL!.path)\"" : "")") + try coordinateRead(at: url, with: .withoutChanges) { url in + addFSODispatchSource(for: url) + } } private func addFSODispatchSource(for url: URL) { @@ -187,7 +249,7 @@ internal class FilePresenter { self.logger.log("🗄️⚠️ file delete event handler: \"\(url.path)\"") var resolvedBookmarkData: URL? { var isStale = false - guard let presenter = self as? SandboxFilePresenter, + guard let presenter = self as? BookmarkFilePresenter, let bookmarkData = presenter.fileBookmarkData, let url = try? URL(resolvingBookmarkData: bookmarkData, bookmarkDataIsStale: &isStale) else { if FileManager().fileExists(atPath: url.path) { return url } // file still exists but with different letter case ? @@ -212,25 +274,36 @@ internal class FilePresenter { dispatchSource.resume() } - private func removeFilePresenter() { - if let innerPresenter { - logger.log("🗄️ removing file presenter for \"\(url?.path ?? "")\"") + private func removeFilePresenters() { + for (idx, innerPresenter) in innerPresenters.enumerated() { + // innerPresenter delegate won‘t be available at this point when called from `deinit`, + // so set the final url here to correctly remove the presenter. + if innerPresenter.fallbackPresentedItemURL == nil { + innerPresenter.fallbackPresentedItemURL = urlController?.url + } + logger.log("🗄️ removing file presenter \(idx) for \"\(innerPresenter.presentedItemURL?.path ?? "")\"") NSFileCoordinator.removeFilePresenter(innerPresenter) - self.innerPresenter = nil } + if innerPresenters.count > 1 { + // ”related” item File Presenters make an unbalanced sandbox extension retain, + // release the actual file URL sandbox extension by calling an extra `stopAccessingSecurityScopedResource` + urlController?.url.consumeUnbalancedStartAccessingSecurityScopedResource() + } + innerPresenters = [] } fileprivate func didSetURL(_ newValue: URL?, oldValue: URL?) { - assert(newValue != oldValue) + assert(newValue == nil || newValue != oldValue) logger.log("🗄️ did update url from \"\(oldValue?.path ?? "")\" to \"\(newValue?.path ?? "")\"") urlSubject.send(newValue) } deinit { - removeFilePresenter() + removeFilePresenters() } } + extension FilePresenter: FilePresenterDelegate { func presentedItemDidMove(to newURL: URL) { @@ -240,8 +313,9 @@ extension FilePresenter: FilePresenterDelegate { func accommodatePresentedItemDeletion() throws { logger.log("🗄️ accommodatePresentedItemDeletion (\"\(url?.path ?? "")\")") + // should go before resetting the URL to correctly remove File Presenter + removeFilePresenters() setURL(nil) - removeFilePresenter() } func accommodatePresentedItemEviction() throws { @@ -254,9 +328,7 @@ extension FilePresenter: FilePresenterDelegate { /// Maintains File Bookmark Data for presented resource URL /// and manages its sandbox security scope access calling `stopAccessingSecurityScopedResource` on deinit /// balanced with preceding `startAccessingSecurityScopedResource` -final class SandboxFilePresenter: FilePresenter { - - private let securityScopedURL: URL? +final class BookmarkFilePresenter: FilePresenter { private var _fileBookmarkData: Data? final var fileBookmarkData: Data? { @@ -271,21 +343,31 @@ final class SandboxFilePresenter: FilePresenter { } /// - Parameter url: represented file URL access to which is coordinated by the File Presenter. - /// - Parameter primaryItemURL: URL to a main file resource access to which has been granted. - /// Used to grant out-of-sandbox access to `url` representing a “secondary” resource like “download.duckload” where the `primaryItemURL` would point to “download.zip”. - /// - Note: the secondary (“duckload”) file extension should be registered in the Info.plist with `NSIsRelatedItemType` flag set to `true`. /// - Parameter consumeUnbalancedStartAccessingResource: assume the `url` is already accessible (e.g. after choosing the file using Open Panel). /// would cause an unbalanced `stopAccessingSecurityScopedResource` call on the File Presenter deallocation. - init(url: URL, primaryItemURL: URL? = nil, consumeUnbalancedStartAccessingResource: Bool = false, logger: FilePresenterLogger = OSLog.disabled, createIfNeededCallback: ((URL) throws -> URL)? = nil) throws { + override init(url: URL, consumeUnbalancedStartAccessingResource: Bool = false, logger: FilePresenterLogger = OSLog.disabled, createIfNeededCallback: ((URL) throws -> URL)? = nil) throws { - if consumeUnbalancedStartAccessingResource || url.startAccessingSecurityScopedResource() == true { - self.securityScopedURL = url - logger.log("🏝️ \(consumeUnbalancedStartAccessingResource ? "consuming unbalanced startAccessingResource for" : "started resource access for") \"\(url.path)\"") - } else { - self.securityScopedURL = nil - logger.log("🏖️ didn‘t start resource access for \"\(url.path)\"") + try super.init(url: url, consumeUnbalancedStartAccessingResource: consumeUnbalancedStartAccessingResource, logger: logger, createIfNeededCallback: createIfNeededCallback) + + do { + try self.coordinateRead(at: url, with: .withoutChanges) { url in + logger.log("📒 updating bookmark data for \"\(url.path)\"") + self._fileBookmarkData = try url.bookmarkData(options: .withSecurityScope) + } + } catch { + logger.log("📕 bookmark data retreival failed for \"\(url.path)\": \(error)") + throw error } + } + /// - Parameter url: represented file URL access to which is coordinated by the File Presenter. + /// - Parameter primaryItemURL: URL to a main file resource access to which has been granted. + /// Used to grant out-of-sandbox access to `url` representing a “related” resource like “download.duckload” where the `primaryItemURL` would point to “download.zip”. + /// - Note: the related (“duckload”) file extension should be registered in the Info.plist with `NSIsRelatedItemType` flag set to `true`. + /// - Note: when presenting a related item the security scoped resource access will always be stopped on the File Presenter deallocation + /// - Parameter consumeUnbalancedStartAccessingResource: assume the `url` is already accessible (e.g. after choosing the file using Open Panel). + /// would cause an unbalanced `stopAccessingSecurityScopedResource` call on the File Presenter deallocation. + override init(url: URL, primaryItemURL: URL, logger: FilePresenterLogger = OSLog.disabled, createIfNeededCallback: ((URL) throws -> URL)? = nil) throws { try super.init(url: url, primaryItemURL: primaryItemURL, logger: logger, createIfNeededCallback: createIfNeededCallback) do { @@ -305,15 +387,8 @@ final class SandboxFilePresenter: FilePresenter { var isStale = false logger.log("📒 resolving url from bookmark data") let url = try URL(resolvingBookmarkData: fileBookmarkData, options: .withSecurityScope, bookmarkDataIsStale: &isStale) - if url.startAccessingSecurityScopedResource() == true { - self.securityScopedURL = url - logger.log("🏝️ started resource access for \"\(url.path)\"\(isStale ? " (stale)" : "")") - } else { - self.securityScopedURL = nil - logger.log("🏖️ didn‘t start resource access for \"\(url.path)\"\(isStale ? " (stale)" : "")") - } - try super.init(url: url, logger: logger) + try super.init(url: url, consumeUnbalancedStartAccessingResource: true, logger: logger) if isStale { DispatchQueue.global().async { [weak self] in @@ -346,25 +421,18 @@ final class SandboxFilePresenter: FilePresenter { fileBookmarkDataSubject.send(fileBookmarkData) } - deinit { - if let securityScopedURL { - logger.log("🗄️ stopAccessingSecurityScopedResource \"\(securityScopedURL.path)\"") - securityScopedURL.stopAccessingSecurityScopedResource() - } - } - } extension FilePresenter { func coordinateRead(at url: URL? = nil, with options: NSFileCoordinator.ReadingOptions = [], using reader: (URL) throws -> T) throws -> T { - guard let innerPresenter, let url = url ?? self.url else { throw CocoaError(.fileNoSuchFile) } + guard let innerPresenter = innerPresenters.last, let url = url ?? self.url else { throw CocoaError(.fileNoSuchFile) } return try NSFileCoordinator(filePresenter: innerPresenter).coordinateRead(at: url, with: options, using: reader) } func coordinateWrite(at url: URL? = nil, with options: NSFileCoordinator.WritingOptions = [], using writer: (URL) throws -> T) throws -> T { - guard let innerPresenter, let url = url ?? self.url else { throw CocoaError(.fileNoSuchFile) } + guard let innerPresenter = innerPresenters.last, let url = url ?? self.url else { throw CocoaError(.fileNoSuchFile) } // temporarily disable DispatchSource file removal events dispatchSourceCancellable?.cancel() @@ -377,7 +445,7 @@ extension FilePresenter { } public func coordinateMove(from url: URL? = nil, to: URL, with options2: NSFileCoordinator.WritingOptions = .forReplacing, using move: (URL, URL) throws -> T) throws -> T { - guard let innerPresenter, let url = url ?? self.url else { throw CocoaError(.fileNoSuchFile) } + guard let innerPresenter = innerPresenters.last, let url = url ?? self.url else { throw CocoaError(.fileNoSuchFile) } return try NSFileCoordinator(filePresenter: innerPresenter).coordinateMove(from: url, to: to, with: options2, using: move) } @@ -425,33 +493,3 @@ extension NSFileCoordinator { } } - -#if DEBUG -extension NSURL { - - private static var stopAccessingSecurityScopedResourceCallback: ((URL) -> Void)? - - private static let originalStopAccessingSecurityScopedResource = { - class_getInstanceMethod(NSURL.self, #selector(NSURL.stopAccessingSecurityScopedResource))! - }() - private static let swizzledStopAccessingSecurityScopedResource = { - class_getInstanceMethod(NSURL.self, #selector(NSURL.swizzled_stopAccessingSecurityScopedResource))! - }() - private static let swizzleStopAccessingSecurityScopedResourceOnce: Void = { - method_exchangeImplementations(originalStopAccessingSecurityScopedResource, swizzledStopAccessingSecurityScopedResource) - }() - - static func swizzleStopAccessingSecurityScopedResource(with stopAccessingSecurityScopedResourceCallback: ((URL) -> Void)?) { - _=swizzleStopAccessingSecurityScopedResourceOnce - self.stopAccessingSecurityScopedResourceCallback = stopAccessingSecurityScopedResourceCallback - } - - @objc private dynamic func swizzled_stopAccessingSecurityScopedResource() { - if let stopAccessingSecurityScopedResourceCallback = Self.stopAccessingSecurityScopedResourceCallback { - stopAccessingSecurityScopedResourceCallback(self as URL) - } - self.swizzled_stopAccessingSecurityScopedResource() // call original - } - -} -#endif diff --git a/DuckDuckGo/FileDownload/Model/NSURL+sandboxExtensionRetainCount.m b/DuckDuckGo/FileDownload/Model/NSURL+sandboxExtensionRetainCount.m new file mode 100644 index 0000000000..8a312c2fb0 --- /dev/null +++ b/DuckDuckGo/FileDownload/Model/NSURL+sandboxExtensionRetainCount.m @@ -0,0 +1,40 @@ +// +// NSURL+sandboxExtensionRetainCount.m +// +// Copyright © 2024 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 + +// Macro for adding quotes +#define STRINGIFY(X) STRINGIFY2(X) +#define STRINGIFY2(X) #X + +#import STRINGIFY(SWIFT_OBJC_INTERFACE_HEADER_NAME) + +@implementation NSURL (sandboxExtensionRetainCount) + +/** + * This method will be automatically called at app launch time to swizzle `startAccessingSecurityScopedResource` and + * `stopAccessingSecurityScopedResource` methods to accurately reflect the current number of start and stop calls + * stored in the associated `NSURL.sandboxExtensionRetainCount` value. + * + * See SecurityScopedFileURLController.swift + */ ++ (void)initialize { + [self swizzleStartStopAccessingSecurityScopedResourceOnce]; +} + +@end diff --git a/DuckDuckGo/FileDownload/Model/SecurityScopedFileURLController.swift b/DuckDuckGo/FileDownload/Model/SecurityScopedFileURLController.swift new file mode 100644 index 0000000000..7cdc7eecdf --- /dev/null +++ b/DuckDuckGo/FileDownload/Model/SecurityScopedFileURLController.swift @@ -0,0 +1,179 @@ +// +// SecurityScopedFileURLController.swift +// +// Copyright © 2024 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 +import Common + +/// Manages security-scoped resource access to a file URL. +/// +/// This class is designed to consume unbalanced `startAccessingSecurityScopedResource` calls and ensure proper +/// resource cleanup by calling `stopAccessingSecurityScopedResource` the appropriate number of times +/// to end the resource access securely. +/// +/// - Note: Used in conjunction with NSURL extension swizzling the `startAccessingSecurityScopedResource` and +/// `stopAccessingSecurityScopedResource` methods to accurately reflect the current number of start and stop calls. +/// The number is reflected in the associated `URL.sandboxExtensionRetainCount` value. +final class SecurityScopedFileURLController { + + fileprivate let logger: any FilePresenterLogger + + private(set) var url: URL + let isManagingSecurityScope: Bool + + /// Initializes a new instance of `SecurityScopedFileURLController` with the provided URL and security-scoped resource handling options. + /// + /// - Parameters: + /// - url: The URL of the file to manage. + /// - manageSecurityScope: A Boolean value indicating whether the controller should manage the URL security scope access (i.e. call stop and end accessing resource methods). + /// - logger: An optional logger instance for logging file operations. Defaults to disabled. + /// - Note: when `manageSecurityScope` is `true` access to the represented URL will be stopped for the whole app on the controller deallocation. + init(url: URL, manageSecurityScope: Bool = true, logger: any FilePresenterLogger = OSLog.disabled) { + assert(url.isFileURL) +#if APPSTORE + let didStartAccess = manageSecurityScope && url.startAccessingSecurityScopedResource() +#else + let didStartAccess = false +#endif + self.url = url + self.isManagingSecurityScope = didStartAccess + self.logger = logger + logger.log("\(didStartAccess ? "🧪 " : "")SecurityScopedFileURLController.init: \(url.sandboxExtensionRetainCount) – \"\(url.path)\"") + } + + func updateUrlKeepingSandboxExtensionRetainCount(_ newURL: URL) { + guard newURL as NSURL !== url as NSURL else { return } + + for _ in 0.. Bool { + if self.swizzled_startAccessingSecurityScopedResource() /* call original */ { + sandboxExtensionRetainCount += 1 + return true + } + return false + } + + @objc private dynamic func swizzled_stopAccessingSecurityScopedResource() { + self.swizzled_stopAccessingSecurityScopedResource() // call original + + var sandboxExtensionRetainCount = self.sandboxExtensionRetainCount + if sandboxExtensionRetainCount > 0 { + sandboxExtensionRetainCount -= 1 + self.sandboxExtensionRetainCount = sandboxExtensionRetainCount + } + } + + private static let sandboxExtensionRetainCountKey = UnsafeRawPointer(bitPattern: "sandboxExtensionRetainCountKey".hashValue)! + fileprivate(set) var sandboxExtensionRetainCount: Int { + get { + (objc_getAssociatedObject(self, Self.sandboxExtensionRetainCountKey) as? NSNumber)?.intValue ?? 0 + } + set { + objc_setAssociatedObject(self, Self.sandboxExtensionRetainCountKey, NSNumber(value: newValue), .OBJC_ASSOCIATION_RETAIN) +#if DEBUG + if newValue > 0 { + NSURL.activeSecurityScopedUrlUsages.insert(.init(url: self)) + } else { + NSURL.activeSecurityScopedUrlUsages.remove(.init(url: self)) + } +#endif + } + } + +#if DEBUG + struct SecurityScopedUrlUsage: Hashable { + let url: NSURL + // hash url as object address + func hash(into hasher: inout Hasher) { + hasher.combine(ObjectIdentifier(url)) + } + } + static var activeSecurityScopedUrlUsages: Set = [] +#endif + +} + +extension URL { + + /// The number of times the security-scoped resource associated with the URL has been accessed + /// using `startAccessingSecurityScopedResource` without a corresponding call to + /// `stopAccessingSecurityScopedResource`. This property provides a count of active accesses + /// to the security-scoped resource, helping manage resource cleanup and ensure proper + /// handling of security-scoped resources. + /// + /// - Note: Accessing this property requires NSURL extension swizzling of `startAccessingSecurityScopedResource` + /// and `stopAccessingSecurityScopedResource` methods to accurately track the count. + var sandboxExtensionRetainCount: Int { + (self as NSURL).sandboxExtensionRetainCount + } + + func consumeUnbalancedStartAccessingSecurityScopedResource() { + (self as NSURL).sandboxExtensionRetainCount += 1 + } + +} diff --git a/DuckDuckGo/FileDownload/Model/WebKitDownloadTask.swift b/DuckDuckGo/FileDownload/Model/WebKitDownloadTask.swift index ca3d3e26c9..b263df5915 100644 --- a/DuckDuckGo/FileDownload/Model/WebKitDownloadTask.swift +++ b/DuckDuckGo/FileDownload/Model/WebKitDownloadTask.swift @@ -116,6 +116,7 @@ final class WebKitDownloadTask: NSObject, ProgressReporting, @unchecked Sendable @MainActor private var itemReplacementDirectory: URL? @MainActor private var itemReplacementDirectoryFSOCancellable: AnyCancellable? @MainActor private var tempFileUrlCancellable: AnyCancellable? + @MainActor private(set) var selectedDestinationURL: URL? var originalRequest: URLRequest? { download.originalRequest @@ -226,6 +227,13 @@ final class WebKitDownloadTask: NSObject, ProgressReporting, @unchecked Sendable do { let fm = FileManager() guard let destinationURL else { throw URLError(.cancelled) } + // in case we‘re overwriting the URL – increment the access counter for the duration of the method + let accessStarted = destinationURL.startAccessingSecurityScopedResource() + defer { + if accessStarted { + destinationURL.stopAccessingSecurityScopedResource() + } + } os_log(.debug, log: log, "download task callback: creating temp directory for \"\(destinationURL.path)\"") switch cleanupStyle { @@ -333,15 +341,15 @@ final class WebKitDownloadTask: NSObject, ProgressReporting, @unchecked Sendable /// opens File Presenters for destination file and temp file private nonisolated func filePresenters(for destinationURL: URL, tempURL: URL) async throws -> (tempFile: FilePresenter, destinationFile: FilePresenter) { var destinationURL = destinationURL - let duckloadURL = destinationURL.deletingPathExtension().appendingPathExtension(Self.downloadExtension) - let fm = FileManager.default + var duckloadURL = destinationURL.deletingPathExtension().appendingPathExtension(Self.downloadExtension) + let fm = FileManager() // 🧙‍♂️ now we‘re doing do some magique here 🧙‍♂️ // -------------------------------------- os_log(.debug, log: log, "🧙‍♂️ magique.start: \"\(destinationURL.path)\" (\"\(duckloadURL.path)\") directory writable: \(fm.isWritableFile(atPath: destinationURL.deletingLastPathComponent().path))") // 1. create our final destination file (let‘s say myfile.zip) and setup a File Presenter for it // doing this we preserve access to the file until it‘s actually downloaded - let destinationFilePresenter = try SandboxFilePresenter(url: destinationURL, consumeUnbalancedStartAccessingResource: true, logger: log) { url in + let destinationFilePresenter = try BookmarkFilePresenter(url: destinationURL, consumeUnbalancedStartAccessingResource: true, logger: log) { url in try fm.createFile(atPath: url.path, contents: nil) ? url : { throw CocoaError(.fileWriteNoPermission, userInfo: [NSFilePathErrorKey: url.path]) }() @@ -353,45 +361,76 @@ final class WebKitDownloadTask: NSObject, ProgressReporting, @unchecked Sendable // 2. mark the file as hidden until it‘s downloaded to not to confuse user // and prevent from unintentional opening of the empty file - var resourceValues = URLResourceValues() - resourceValues.isHidden = true - try destinationURL.setResourceValues(resourceValues) - os_log(.debug, log: log, "🧙‍♂️ \"\(destinationURL.path)\" hidden, moving temp file from \"\(tempURL.path)\" to \"\(duckloadURL.path)\"") + try destinationURL.setFileHidden(true) + os_log(.debug, log: log, "🧙‍♂️ \"\(destinationURL.path)\" hidden") - // 3. then we move the temporary download file to the destination directory (myfile.zip.duckload) + // 3. then we move the temporary download file to the destination directory (myfile.duckload) // this is doable in sandboxed builds by using “Related Items” i.e. using a file URL with an extra // `.duckload` extension appended and “Primary Item” pointing to the sandbox-accessible destination URL // the `.duckload` document type is registered in the Info.plist with `NSIsRelatedItemType` flag // // - after the file is downloaded we‘ll replace the destination file with the `.duckload` file if fm.fileExists(atPath: duckloadURL.path) { - // remove the `.duckload` item if already exists + // `.duckload` already exists do { - try FilePresenter(url: duckloadURL, primaryItemURL: destinationURL).coordinateWrite(with: .forDeleting) { duckloadURL in - try fm.removeItem(at: duckloadURL) - } + try chooseAlternativeDuckloadFileNameOrRemove(&duckloadURL, destinationURL: destinationURL) } catch { // that‘s ok, we‘ll keep using the original temp file - os_log(.error, log: log, "❗️ could not remove \"\(duckloadURL.path)\" \(error)") + os_log(.error, log: log, "❗️ can‘t resolve duckload file exists: \"\(duckloadURL.path)\": \(error)") + duckloadURL = tempURL } } - // now move the temp file to `.duckload` instantiating a File Presenter with it - let tempFilePresenter = try SandboxFilePresenter(url: duckloadURL, primaryItemURL: destinationURL, logger: log) { [log] duckloadURL in - do { - try fm.moveItem(at: tempURL, to: duckloadURL) - } catch { - // fallback: move failed, keep the temp file in the original location - os_log(.error, log: log, "🙁 fallback with \(error), will use \(tempURL.path)") - Pixel.fire(.debug(event: .fileAccessRelatedItemFailed, error: error)) - return tempURL + + let tempFilePresenter = if duckloadURL == tempURL { + // we won‘t use a `.duckload` file for this download, the file will be left in the temp location instead + try BookmarkFilePresenter(url: duckloadURL, logger: log) + } else { + // now move the temp file to `.duckload` instantiating a File Presenter with it + try BookmarkFilePresenter(url: duckloadURL, primaryItemURL: destinationURL, logger: log) { [log] duckloadURL in + do { + try fm.moveItem(at: tempURL, to: duckloadURL) + return duckloadURL + } catch { + // fallback: move failed, keep the temp file in the original location + os_log(.error, log: log, "🙁 fallback with \(error), will use \(tempURL.path)") + Pixel.fire(.debug(event: .fileAccessRelatedItemFailed, error: error)) + return tempURL + } } - return duckloadURL } os_log(.debug, log: log, "🧙‍♂️ \"\(duckloadURL.path)\" (\"\(tempFilePresenter.url?.path ?? "")\") ready") return (tempFile: tempFilePresenter, destinationFile: destinationFilePresenter) } + private func chooseAlternativeDuckloadFileNameOrRemove(_ duckloadURL: inout URL, destinationURL: URL) throws { + let fm = FileManager() + // are we using the `.duckload` file for some other download (with different extension)? + if NSFileCoordinator.filePresenters.first(where: { $0.presentedItemURL?.resolvingSymlinksInPath() == duckloadURL.resolvingSymlinksInPath() }) != nil { + // if the downloads directory is writable without extra permission – try choosing another `.duckload` filename + if fm.isWritableFile(atPath: duckloadURL.deletingLastPathComponent().path) { + // append `.duckload` to the destination file name with extension + let destinationPathExtension = destinationURL.pathExtension + let pathExtension = destinationPathExtension.isEmpty ? Self.downloadExtension : destinationPathExtension + "." + Self.downloadExtension + duckloadURL = duckloadURL.deletingPathExtension().appendingPathExtension(pathExtension) + + // choose non-existent path + duckloadURL = try fm.withNonExistentUrl(for: duckloadURL, incrementingIndexIfExistsUpTo: 1000, pathExtension: pathExtension) { url in + try Data().write(to: url) + return url + } + } else { + // continue keeping the temp file in the temp dir + throw CocoaError(.fileWriteFileExists) + } + } + + os_log(.debug, log: log, "removing temp file \"\(duckloadURL.path)\"") + try FilePresenter(url: duckloadURL, primaryItemURL: destinationURL).coordinateWrite(with: .forDeleting) { duckloadURL in + try fm.removeItem(at: duckloadURL) + } + } + private nonisolated func reuseFilePresenters(tempFile: FilePresenter, destination: FilePresenter, tempURL: URL) async throws -> (tempFile: FilePresenter, destinationFile: FilePresenter) { // if the download is “resumed” as a new download (replacing the destination file) - // use the existing `.duckload` file and move the temp file in its place @@ -587,6 +626,7 @@ extension WebKitDownloadTask: WKDownloadDelegate { return nil } + self.selectedDestinationURL = destinationURL return await prepareChosenDestinationURL(destinationURL, fileType: suggestedFileType, cleanupStyle: cleanupStyle) } @@ -697,20 +737,7 @@ extension WebKitDownloadTask { override var description: String { guard Thread.isMainThread else { #if DEBUG - os_log(""" - - - ------------------------------------------------------------------------------------------------------ - BREAK: - ------------------------------------------------------------------------------------------------------ - - ❗️accessing WebKitDownloadTask.description from non-main thread - - Hit Continue (^⌘Y) to continue program execution - ------------------------------------------------------------------------------------------------------ - - """, type: .fault) - raise(SIGINT) + breakByRaisingSigInt("❗️accessing WebKitDownloadTask.description from non-main thread") #endif return "" } diff --git a/DuckDuckGo/FileDownload/Services/DownloadListCoordinator.swift b/DuckDuckGo/FileDownload/Services/DownloadListCoordinator.swift index 5e9c9e14c9..70cfda3f2d 100644 --- a/DuckDuckGo/FileDownload/Services/DownloadListCoordinator.swift +++ b/DuckDuckGo/FileDownload/Services/DownloadListCoordinator.swift @@ -152,9 +152,9 @@ final class DownloadListCoordinator { // locate destination file let destinationPresenterResult = Result { if let destinationFileBookmarkData = item.destinationFileBookmarkData { - try SandboxFilePresenter(fileBookmarkData: destinationFileBookmarkData, logger: log) + try BookmarkFilePresenter(fileBookmarkData: destinationFileBookmarkData, logger: log) } else if let destinationURL = item.destinationURL { - try SandboxFilePresenter(url: destinationURL, logger: log) + try BookmarkFilePresenter(url: destinationURL, logger: log) } else { nil } @@ -163,9 +163,9 @@ final class DownloadListCoordinator { // locate temp download file var tempFilePresenterResult = Result { if let tempFileBookmarkData = item.tempFileBookmarkData { - try SandboxFilePresenter(fileBookmarkData: tempFileBookmarkData, logger: log) + try BookmarkFilePresenter(fileBookmarkData: tempFileBookmarkData, logger: log) } else if let tempURL = item.tempURL { - try SandboxFilePresenter(url: tempURL, logger: log) + try BookmarkFilePresenter(url: tempURL, logger: log) } else { nil } @@ -223,10 +223,10 @@ final class DownloadListCoordinator { case .downloading(destination: let destination, tempFile: let tempFile): self.addItemIfNeededAndSubscribe(to: (destination, tempFile), for: item) case .downloaded(let destination): - let updatedItem = self.downloadTask(task, withId: item.identifier, completedWith: .finished) + let updatedItem = self.downloadTask(task, withOriginalItem: item, completedWith: .finished) self.subscribeToPresenters((destination: destination, tempFile: nil), of: updatedItem ?? item) case .failed(destination: let destination, tempFile: let tempFile, resumeData: _, error: let error): - let updatedItem = self.downloadTask(task, withId: item.identifier, completedWith: .failure(error)) + let updatedItem = self.downloadTask(task, withOriginalItem: item, completedWith: .failure(error)) self.subscribeToPresenters((destination: destination, tempFile: tempFile), of: updatedItem ?? item) } } @@ -250,7 +250,7 @@ final class DownloadListCoordinator { Publishers.CombineLatest( presenters.destination?.urlPublisher ?? Just(nil).eraseToAnyPublisher(), - (presenters.destination as? SandboxFilePresenter)?.fileBookmarkDataPublisher ?? Just(nil).eraseToAnyPublisher() + (presenters.destination as? BookmarkFilePresenter)?.fileBookmarkDataPublisher ?? Just(nil).eraseToAnyPublisher() ) .scan((oldURL: nil, newURL: nil, fileBookmarkData: nil)) { (oldURL: $0.newURL, newURL: $1.0, fileBookmarkData: $1.1) } .sink { [weak self] oldURL, newURL, fileBookmarkData in @@ -279,7 +279,7 @@ final class DownloadListCoordinator { Publishers.CombineLatest( presenters.tempFile?.urlPublisher ?? Just(nil).eraseToAnyPublisher(), - (presenters.tempFile as? SandboxFilePresenter)?.fileBookmarkDataPublisher ?? Just(nil).eraseToAnyPublisher() + (presenters.tempFile as? BookmarkFilePresenter)?.fileBookmarkDataPublisher ?? Just(nil).eraseToAnyPublisher() ) .scan((oldURL: nil, newURL: nil, fileBookmarkData: nil)) { (oldURL: $0.newURL, newURL: $1.0, fileBookmarkData: $1.1) } .sink { [weak self] oldURL, newURL, fileBookmarkData in @@ -341,19 +341,25 @@ final class DownloadListCoordinator { } @MainActor - private func downloadTask(_ task: WebKitDownloadTask, withId identifier: UUID, completedWith result: Subscribers.Completion) -> DownloadListItem? { - os_log(.debug, log: log, "coordinator: task did finish \(identifier) \(task) with .\(result)") + private func downloadTask(_ task: WebKitDownloadTask, withOriginalItem initialItem: DownloadListItem, completedWith result: Subscribers.Completion) -> DownloadListItem? { + os_log(.debug, log: log, "coordinator: task did finish \(initialItem.identifier) \(task) with .\(result)") self.downloadTaskCancellables[task] = nil - // item will be really updated (completed) only if it was added before in `addItemOrUpdateFilePresenter` (when state switched to .downloading) - // if it has failed without starting - it won‘t be added or updated here - return updateItem(withId: identifier) { item in + return updateItem(withId: initialItem.identifier) { item in if item?.isBurner ?? false { item = nil return } + if item == nil, + case .failure(let failure) = result, !failure.isCancelled, + let fileName = task.selectedDestinationURL?.lastPathComponent { + // add instantly failed downloads to the list (not user-cancelled) + item = initialItem + item?.fileName = fileName + } + item?.progress = nil if case .failure(let error) = result { item?.error = error diff --git a/DuckDuckGo/Preferences/Model/DownloadsPreferences.swift b/DuckDuckGo/Preferences/Model/DownloadsPreferences.swift index ce6962e4bf..e8e435c447 100644 --- a/DuckDuckGo/Preferences/Model/DownloadsPreferences.swift +++ b/DuckDuckGo/Preferences/Model/DownloadsPreferences.swift @@ -16,6 +16,7 @@ // limitations under the License. // +import Common import Foundation protocol DownloadsPreferencesPersistor { @@ -65,17 +66,19 @@ final class DownloadsPreferences: ObservableObject { static let shared = DownloadsPreferences(persistor: DownloadsPreferencesUserDefaultsPersistor()) - private func validatedDownloadLocation(_ location: String?) -> URL? { - if let selectedLocation = location, - let selectedLocationURL = URL(string: selectedLocation), - Self.isDownloadLocationValid(selectedLocationURL) { - return selectedLocationURL + private func validatedDownloadLocation(_ selectedLocation: URL?) -> URL? { + if let selectedLocation, Self.isDownloadLocationValid(selectedLocation) { + return selectedLocation } return nil } var effectiveDownloadLocation: URL? { - if let selectedLocationURL = alwaysRequestDownloadLocation ? validatedDownloadLocation(persistor.lastUsedCustomDownloadLocation) : validatedDownloadLocation(persistor.selectedDownloadLocation) { + if alwaysRequestDownloadLocation { + if let lastUsedCustomDownloadLocation = validatedDownloadLocation(persistor.lastUsedCustomDownloadLocation.flatMap(URL.init(string:))) { + return lastUsedCustomDownloadLocation + } + } else if let selectedLocationURL = validatedDownloadLocation(selectedDownloadLocation) { return selectedLocationURL } return Self.defaultDownloadLocation() @@ -94,37 +97,54 @@ final class DownloadsPreferences: ObservableObject { defer { objectWillChange.send() } - guard let newDownloadLocation = newValue else { - persistor.lastUsedCustomDownloadLocation = nil - return - } - if Self.isDownloadLocationValid(newDownloadLocation) { - persistor.lastUsedCustomDownloadLocation = newDownloadLocation.absoluteString - } + persistor.lastUsedCustomDownloadLocation = newValue?.absoluteString } } + private var selectedDownloadLocationController: SecurityScopedFileURLController? + var selectedDownloadLocation: URL? { get { - persistor.selectedDownloadLocation?.url + if let selectedDownloadLocation = selectedDownloadLocationController?.url { + return selectedDownloadLocation + } +#if APPSTORE + var isStale = false + if let bookmarkData = persistor.selectedDownloadLocation.flatMap({ Data(base64Encoded: $0) }), + let url = try? URL(resolvingBookmarkData: bookmarkData, options: .withSecurityScope, relativeTo: nil, bookmarkDataIsStale: &isStale) { + if isStale { + setSelectedDownloadLocation(url) // update bookmark data and selectedDownloadLocationController + } else { + selectedDownloadLocationController = SecurityScopedFileURLController(url: url, logger: OSLog.downloads) + } + return url + } +#endif + guard let url = persistor.selectedDownloadLocation.flatMap(URL.init(string:)), + url.isFileURL else { return nil } + return url.resolvingSymlinksInPath() } - set { defer { objectWillChange.send() } - guard let newDownloadLocation = newValue else { - persistor.selectedDownloadLocation = nil - return - } - if Self.isDownloadLocationValid(newDownloadLocation) { - persistor.selectedDownloadLocation = newDownloadLocation.absoluteString - } + setSelectedDownloadLocation(validatedDownloadLocation(newValue)) } } + private func setSelectedDownloadLocation(_ url: URL?) { + selectedDownloadLocationController = url.map { SecurityScopedFileURLController(url: $0, logger: OSLog.downloads) } + let locationString: String? +#if APPSTORE + locationString = (try? url?.bookmarkData(options: .withSecurityScope).base64EncodedString()) ?? url?.absoluteString +#else + locationString = url?.absoluteString +#endif + persistor.selectedDownloadLocation = locationString + } + var alwaysRequestDownloadLocation: Bool { get { persistor.alwaysRequestDownloadLocation diff --git a/DuckDuckGo/Preferences/View/PreferencesGeneralView.swift b/DuckDuckGo/Preferences/View/PreferencesGeneralView.swift index 521893ba2e..dc4da53438 100644 --- a/DuckDuckGo/Preferences/View/PreferencesGeneralView.swift +++ b/DuckDuckGo/Preferences/View/PreferencesGeneralView.swift @@ -108,15 +108,15 @@ extension Preferences { // MARK: Location PreferencePaneSubSection { Text(UserText.downloadsLocation).bold() + HStack { NSPathControlView(url: downloadsModel.selectedDownloadLocation) -#if !APPSTORE Button(UserText.downloadsChangeDirectory) { downloadsModel.presentDownloadDirectoryPanel() } -#endif } .disabled(downloadsModel.alwaysRequestDownloadLocation) + ToggleMenuItem(UserText.downloadsAlwaysAsk, isOn: $downloadsModel.alwaysRequestDownloadLocation) } diff --git a/DuckDuckGo/Tab/View/BrowserTabViewController.swift b/DuckDuckGo/Tab/View/BrowserTabViewController.swift index 539d7007ea..d43aa95fd9 100644 --- a/DuckDuckGo/Tab/View/BrowserTabViewController.swift +++ b/DuckDuckGo/Tab/View/BrowserTabViewController.swift @@ -974,13 +974,28 @@ extension BrowserTabViewController: TabDelegate { suggestedFilename: request.parameters.suggestedFilename, directoryURL: directoryURL) - savePanel.beginSheetModal(for: window) { [weak request] response in + savePanel.beginSheetModal(for: window) { [weak request, weak self] response in switch response { case .abort: // panel not closed by user but by a tab switching return case .OK: - guard let url = savePanel.url else { fallthrough } + guard let self, + let window = view.window, + let url = savePanel.url else { fallthrough } + + do { + // validate selected URL is writable + try FileManager.default.checkWritability(url) + } catch { + // hide the save panel + self.activeUserDialogCancellable = nil + NSAlert(error: error).beginSheetModal(for: window) { [weak self] _ in + guard let self, let request else { return } + self.activeUserDialogCancellable = showSavePanel(with: request) + } + return + } request?.submit( (url, savePanel.selectedFileType) ) default: request?.submit(nil) diff --git a/DuckDuckGo/Tab/View/WebView.swift b/DuckDuckGo/Tab/View/WebView.swift index bc578f6d19..249356909b 100644 --- a/DuckDuckGo/Tab/View/WebView.swift +++ b/DuckDuckGo/Tab/View/WebView.swift @@ -30,6 +30,7 @@ protocol WebViewInteractionEventsDelegate: AnyObject { func webView(_ webView: WebView, scrollWheel event: NSEvent) } +@objc(DuckDuckGo_WebView) final class WebView: WKWebView { weak var contextMenuDelegate: WebViewContextMenuDelegate? diff --git a/IntegrationTests/Downloads/DownloadsIntegrationTests.swift b/IntegrationTests/Downloads/DownloadsIntegrationTests.swift index 35e2024c05..d108921c11 100644 --- a/IntegrationTests/Downloads/DownloadsIntegrationTests.swift +++ b/IntegrationTests/Downloads/DownloadsIntegrationTests.swift @@ -70,9 +70,9 @@ class DownloadsIntegrationTests: XCTestCase { @MainActor func testWhenShouldDownloadResponse_downloadStarts() async throws { - let persistor = DownloadsPreferencesUserDefaultsPersistor() - persistor.alwaysRequestDownloadLocation = false - persistor.selectedDownloadLocation = FileManager.default.temporaryDirectory.absoluteString + let preferences = DownloadsPreferences.shared + preferences.alwaysRequestDownloadLocation = false + preferences.selectedDownloadLocation = FileManager.default.temporaryDirectory let downloadTaskFuture = FileDownloadManager.shared.downloadsPublisher.timeout(5).first().promise() let suffix = Int.random(in: 0.. URL { @@ -95,15 +94,17 @@ final class FilePresenterTests: XCTestCase { return app } - private func terminateApp(timeout: TimeInterval = 1) async { - let eTerminated = runningApp != nil ? expectation(description: "terminated") : nil + private func terminateApp(timeout: TimeInterval = 5, expectation: XCTestExpectation = XCTestExpectation(description: "terminated")) async { + if runningApp == nil { + expectation.fulfill() + } let c = runningApp?.publisher(for: \.isTerminated).filter { $0 }.sink { _ in - eTerminated?.fulfill() + expectation.fulfill() } post(.terminate) runningApp?.forceTerminate() - await fulfillment(of: eTerminated.map { [$0] } ?? [], timeout: timeout) + await fulfillment(of: [expectation], timeout: timeout) withExtendedLifetime(c) {} } @@ -111,7 +112,7 @@ final class FilePresenterTests: XCTestCase { DistributedNotificationCenter.default().post(name: .init(name.rawValue), object: object) } - private func fileReadPromise(timeout: TimeInterval = 5) -> Future { + private func fileReadPromise(timeout: TimeInterval = 5, file: StaticString = #file, line: UInt = #line) -> Future { Future { [unowned self] fulfill in onFileRead = { result in fulfill(.success(result)) @@ -124,13 +125,14 @@ final class FilePresenterTests: XCTestCase { self.onError = nil } } - .timeout(timeout) + .timeout(timeout, file: file, line: line) .first() .promise() } // MARK: - Test sandboxed file access #if APPSTORE && !CI + func testTool_run() async throws { // 1. make non-sandbox file let nonSandboxUrl = try makeNonSandboxFile() @@ -177,7 +179,7 @@ final class FilePresenterTests: XCTestCase { await terminateApp() runningApp = try await runHelperApp() - // 3. open the bookmark with SandboxFilePresenter + // 3. open the bookmark with BookmarkFilePresenter post(.openBookmarkWithFilePresenter, with: bookmark.base64EncodedString()) // 4. read the file @@ -188,7 +190,7 @@ final class FilePresenterTests: XCTestCase { XCTAssertEqual(result.data, testData.utf8String()) XCTAssertEqual(result.bookmark, bookmark) - // 5. close SandboxFilePresenter + // 5. close BookmarkFilePresenter let e = expectation(description: "access stopped") let c = DistributedNotificationCenter.default().publisher(for: SandboxTestNotification.stopAccessingSecurityScopedResourceCalled.name).sink { _ in e.fulfill() @@ -220,7 +222,7 @@ final class FilePresenterTests: XCTestCase { await terminateApp() runningApp = try await runHelperApp() - // 3. open the bookmark with SandboxFilePresenter + // 3. open the bookmark with BookmarkFilePresenter post(.openBookmarkWithFilePresenter, with: bookmark.base64EncodedString()) fileReadPromise = self.fileReadPromise() post(.openFile, with: nonSandboxUrl.path) @@ -290,7 +292,7 @@ final class FilePresenterTests: XCTestCase { await terminateApp() runningApp = try await runHelperApp() - // 3. open the bookmark with SandboxFilePresenter + // 3. open the bookmark with BookmarkFilePresenter post(.openBookmarkWithFilePresenter, with: bookmark.base64EncodedString()) fileReadPromise = self.fileReadPromise() @@ -316,7 +318,7 @@ final class FilePresenterTests: XCTestCase { await terminateApp() runningApp = try await runHelperApp() - // 3. open the bookmark with SandboxFilePresenter + // 3. open the bookmark with BookmarkFilePresenter post(.openBookmarkWithFilePresenter, with: bookmark.base64EncodedString()) fileReadPromise = self.fileReadPromise() post(.openFile, with: nonSandboxUrl.path) @@ -355,7 +357,7 @@ final class FilePresenterTests: XCTestCase { await terminateApp() runningApp = try await runHelperApp() - // 3. open the bookmark with SandboxFilePresenter + // 3. open the bookmark with BookmarkFilePresenter post(.openBookmarkWithFilePresenter, with: bookmark.base64EncodedString()) fileReadPromise = self.fileReadPromise() post(.openFile, with: nonSandboxUrl.path) @@ -374,7 +376,7 @@ final class FilePresenterTests: XCTestCase { } await fulfillment(of: [e], timeout: 5) - // 5. close SandboxFilePresenter + // 5. close BookmarkFilePresenter let eStopped = expectation(description: "access stopped") let c2 = DistributedNotificationCenter.default().publisher(for: SandboxTestNotification.stopAccessingSecurityScopedResourceCalled.name).sink { _ in eStopped.fulfill() @@ -406,13 +408,13 @@ final class FilePresenterTests: XCTestCase { await terminateApp() runningApp = try await runHelperApp() - // 3. open the bookmark with SandboxFilePresenter + // 3. open the bookmark with BookmarkFilePresenter post(.openBookmarkWithFilePresenter, with: bookmark.base64EncodedString()) fileReadPromise = self.fileReadPromise() post(.openFile, with: nonSandboxUrl.path) _=try await fileReadPromise.value - // 4. close SandboxFilePresenter + // 4. close BookmarkFilePresenter let eStopped = expectation(description: "access stopped") let c2 = DistributedNotificationCenter.default().publisher(for: SandboxTestNotification.stopAccessingSecurityScopedResourceCalled.name).sink { _ in eStopped.fulfill() @@ -456,7 +458,7 @@ final class FilePresenterTests: XCTestCase { await terminateApp() runningApp = try await runHelperApp() - // 3. open the bookmark with SandboxFilePresenter + // 3. open the bookmark with BookmarkFilePresenter post(.openBookmarkWithFilePresenter, with: bookmark.base64EncodedString()) fileReadPromise = self.fileReadPromise() post(.openFile, with: nonSandboxUrl.path) @@ -503,7 +505,7 @@ final class FilePresenterTests: XCTestCase { await terminateApp() runningApp = try await runHelperApp() - // 3. open the bookmark with SandboxFilePresenter + // 3. open the bookmark with BookmarkFilePresenter post(.openBookmarkWithFilePresenter, with: bookmark.base64EncodedString()) fileReadPromise = self.fileReadPromise() post(.openFile, with: nonSandboxUrl.path) @@ -543,7 +545,7 @@ final class FilePresenterTests: XCTestCase { await terminateApp() runningApp = try await runHelperApp() - // 3. open the bookmark with SandboxFilePresenter + // 3. open the bookmark with BookmarkFilePresenter post(.openBookmarkWithFilePresenter, with: bookmark1.base64EncodedString()) post(.openBookmarkWithFilePresenter, with: bookmark2.base64EncodedString()) fileReadPromise = self.fileReadPromise() @@ -571,7 +573,6 @@ final class FilePresenterTests: XCTestCase { // 5. close FilePresenter 1 (at the original URL) let e3 = expectation(description: "access stopped") let c2 = DistributedNotificationCenter.default().publisher(for: SandboxTestNotification.stopAccessingSecurityScopedResourceCalled.name).sink { n in - XCTAssertEqual(n.object as? String, nonSandboxUrl1.path) e3.fulfill() } post(.closeFilePresenter, with: nonSandboxUrl1.path) @@ -600,32 +601,6 @@ final class FilePresenterTests: XCTestCase { } } - func testWhenFilePresenterClosesFileOpenedByOS_fileAccessIsPreserved() async throws { - // 1. make non-sandbox file and open the file with the helper app - let nonSandboxUrl = try makeNonSandboxFile() - var fileReadPromise = self.fileReadPromise() - runningApp = try await runHelperApp(opening: nonSandboxUrl) - guard let bookmark = try await fileReadPromise.value.bookmark else { XCTFail("No bookmark"); return } - - // 2. open file presenter - post(.openBookmarkWithFilePresenter, with: bookmark.base64EncodedString()) - - // 3. close the file presenter - let e = expectation(description: "access stopped") - let c = DistributedNotificationCenter.default().publisher(for: SandboxTestNotification.stopAccessingSecurityScopedResourceCalled.name).sink { n in - XCTAssertEqual(n.object as? String, nonSandboxUrl.path) - e.fulfill() - } - post(.closeFilePresenter, with: nonSandboxUrl.path) - await fulfillment(of: [e], timeout: 1) - withExtendedLifetime(c) {} - - // 4. validate file can still be read - fileReadPromise = self.fileReadPromise() - post(.openFile, with: nonSandboxUrl.path) - let result = try await fileReadPromise.value - XCTAssertEqual(result.path, nonSandboxUrl.path) - } #endif // MARK: - Test non-sandboxed file access @@ -633,10 +608,10 @@ final class FilePresenterTests: XCTestCase { func testWhenSandboxFilePresenterIsOpen_itCanReadFile_accessIsNotStoppedWhenClosed_noSandbox() async throws { // 1. make non-sandbox file; create bookmark let nonSandboxUrl = try makeNonSandboxFile() - guard let bookmarkData = try SandboxFilePresenter(url: nonSandboxUrl).fileBookmarkData else { XCTFail("No bookmark"); return } + guard let bookmarkData = try BookmarkFilePresenter(url: nonSandboxUrl).fileBookmarkData else { XCTFail("No bookmark"); return } - // 2. open the bookmark with SandboxFilePresenter - var filePresenter: SandboxFilePresenter! = try SandboxFilePresenter(fileBookmarkData: bookmarkData) + // 2. open the bookmark with BookmarkFilePresenter + var filePresenter: BookmarkFilePresenter! = try BookmarkFilePresenter(fileBookmarkData: bookmarkData) // 3. validate var publishedUrl: URL? @@ -656,7 +631,7 @@ final class FilePresenterTests: XCTestCase { func testWhenFileIsRenamed_urlIsUpdated_noSandbox() async throws { // 1. make non-sandbox file let nonSandboxUrl = try makeNonSandboxFile() - let filePresenter = try SandboxFilePresenter(url: nonSandboxUrl) + let filePresenter = try BookmarkFilePresenter(url: nonSandboxUrl) // 4. rename the file let newUrl = nonSandboxUrl.deletingPathExtension().appendingPathExtension("1.txt") @@ -689,7 +664,7 @@ final class FilePresenterTests: XCTestCase { func testWhenFileIsRemoved_removalIsDetected_noSandbox() async throws { // 1. make non-sandbox file let nonSandboxUrl = try makeNonSandboxFile() - let filePresenter = try SandboxFilePresenter(url: nonSandboxUrl) + let filePresenter = try BookmarkFilePresenter(url: nonSandboxUrl) // 2. remove the file let e1 = expectation(description: "file presenter: file removed") @@ -716,8 +691,8 @@ final class FilePresenterTests: XCTestCase { let bookmarkData1 = try nonSandboxUrl1.bookmarkData(options: .withSecurityScope) let nonSandboxUrl2 = try makeNonSandboxFile() let bookmarkData2 = try nonSandboxUrl2.bookmarkData(options: .withSecurityScope) - let filePresenter1 = try SandboxFilePresenter(fileBookmarkData: bookmarkData1) - let filePresenter2 = try SandboxFilePresenter(fileBookmarkData: bookmarkData2) + let filePresenter1 = try BookmarkFilePresenter(fileBookmarkData: bookmarkData1) + let filePresenter2 = try BookmarkFilePresenter(fileBookmarkData: bookmarkData2) // 2. cross-rename the files let tempUrl = nonSandboxUrl1.appendingPathExtension("tmp") diff --git a/UnitTests/Preferences/DownloadsPreferencesTests.swift b/UnitTests/Preferences/DownloadsPreferencesTests.swift index 3213d7c0af..7c50d88ff5 100644 --- a/UnitTests/Preferences/DownloadsPreferencesTests.swift +++ b/UnitTests/Preferences/DownloadsPreferencesTests.swift @@ -142,7 +142,7 @@ class DownloadsPreferencesTests: XCTestCase { preferences.selectedDownloadLocation = invalidDownloadLocationURL - XCTAssertEqual(preferences.effectiveDownloadLocation, testDirectory) + XCTAssertEqual(preferences.effectiveDownloadLocation, DownloadsPreferences.defaultDownloadLocation()) } func testWhenGettingSelectedDownloadLocationAndSelectedLocationIsInaccessibleThenDefaultDownloadLocationIsReturned() { @@ -213,18 +213,15 @@ class DownloadsPreferencesTests: XCTestCase { XCTAssertNil(preferences.lastUsedCustomDownloadLocation) } - func testWhenInvalidLastUsedCustomDownloadLocationIsSet_oldValueIsPreserved() { + func testWhenInvalidLastUsedCustomDownloadLocationIsSet_lastUsedCustomLocationIsNil() { let testDirectory = createTemporaryTestDirectory() let persistor = DownloadsPreferencesPersistorMock(selectedDownloadLocation: nil) let preferences = DownloadsPreferences(persistor: persistor) - let valuesBeforeChange = persistor.values() preferences.lastUsedCustomDownloadLocation = testDirectory preferences.lastUsedCustomDownloadLocation = testDirectory.appendingPathComponent("non-existent-dir") - let valuesAfterChange = persistor.values() - XCTAssertEqual(valuesBeforeChange.difference(from: valuesAfterChange), ["\(\DownloadsPreferencesPersistorMock.lastUsedCustomDownloadLocation)".pathExtension]) - XCTAssertEqual(preferences.lastUsedCustomDownloadLocation, testDirectory) + XCTAssertNil(preferences.lastUsedCustomDownloadLocation) } func testWhenLastUsedCustomDownloadLocationIsReset_nilIsReturned() { diff --git a/sandbox-test-tool/SandboxTestTool.swift b/sandbox-test-tool/SandboxTestTool.swift index 2a800cfa18..1234426615 100644 --- a/sandbox-test-tool/SandboxTestTool.swift +++ b/sandbox-test-tool/SandboxTestTool.swift @@ -55,7 +55,11 @@ extension FileLogger: FilePresenterLogger { final class FileLogger { static let shared = FileLogger() - private init() {} + private init() { + if !FileManager.default.fileExists(atPath: fileURL.path) { + FileManager.default.createFile(atPath: fileURL.path, contents: nil) + } + } private let pid = ProcessInfo().processIdentifier @@ -176,7 +180,7 @@ final class SandboxTestToolAppDelegate: NSObject, NSApplicationDelegate { return } do { - let filePresenter = try SandboxFilePresenter(fileBookmarkData: bookmark, logger: logger) + let filePresenter = try BookmarkFilePresenter(fileBookmarkData: bookmark, logger: logger) guard let url = filePresenter.url else { throw NSError(domain: "SandboxTestTool", code: -1, userInfo: [NSLocalizedDescriptionKey: "FilePresenter URL is nil"]) } filePresenter.urlPublisher.dropFirst().sink { [unowned self] url in @@ -188,7 +192,7 @@ final class SandboxTestToolAppDelegate: NSObject, NSApplicationDelegate { self.filePresenters[url] = filePresenter logger.log("📗 openBookmarkWithFilePresenter done: \"\(filePresenter.url?.path ?? "")\"") } catch { - post(.error, with: error.encoded("could not open SandboxFilePresenter")) + post(.error, with: error.encoded("could not open BookmarkFilePresenter")) } } @@ -198,7 +202,6 @@ final class SandboxTestToolAppDelegate: NSObject, NSApplicationDelegate { return } logger.log("🌂 closeFilePresenter for \(path)") - let url = URL(fileURLWithPath: path) filePresenterCancellables[url] = nil filePresenters[url] = nil @@ -230,3 +233,31 @@ private extension Error { return String(data: json!, encoding: .utf8)! } } + +extension NSURL { + + private static var stopAccessingSecurityScopedResourceCallback: ((URL) -> Void)? + + private static let originalStopAccessingSecurityScopedResource = { + class_getInstanceMethod(NSURL.self, #selector(NSURL.stopAccessingSecurityScopedResource))! + }() + private static let swizzledStopAccessingSecurityScopedResource = { + class_getInstanceMethod(NSURL.self, #selector(NSURL.test_tool_stopAccessingSecurityScopedResource))! + }() + private static let swizzleStopAccessingSecurityScopedResourceOnce: Void = { + method_exchangeImplementations(originalStopAccessingSecurityScopedResource, swizzledStopAccessingSecurityScopedResource) + }() + + static func swizzleStopAccessingSecurityScopedResource(with stopAccessingSecurityScopedResourceCallback: ((URL) -> Void)?) { + _=swizzleStopAccessingSecurityScopedResourceOnce + self.stopAccessingSecurityScopedResourceCallback = stopAccessingSecurityScopedResourceCallback + } + + @objc private dynamic func test_tool_stopAccessingSecurityScopedResource() { + if let stopAccessingSecurityScopedResourceCallback = Self.stopAccessingSecurityScopedResourceCallback { + stopAccessingSecurityScopedResourceCallback(self as URL) + } + self.test_tool_stopAccessingSecurityScopedResource() // call original + } + +} From 404b0b04e9b3edf89337e968255dfb0ca7bb267d Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Wed, 10 Apr 2024 15:04:53 +0500 Subject: [PATCH 13/14] Add supported document types (#2581) Task/Issue URL: https://app.asana.com/0/1177771139624306/1201791966665400/f --- .../Common/Extensions/BundleExtension.swift | 10 + .../Common/Extensions/NSAlertExtension.swift | 9 - DuckDuckGo/Common/Localizables/UserText.swift | 2 - .../SecurityScopedFileURLController.swift | 3 + DuckDuckGo/Info.plist | 476 ++++++++++++++++-- .../View/AddressBarTextField.swift | 8 +- DuckDuckGo/Tab/Model/Tab.swift | 15 +- .../TabExtensions/DownloadsTabExtension.swift | 11 +- 8 files changed, 478 insertions(+), 56 deletions(-) diff --git a/DuckDuckGo/Common/Extensions/BundleExtension.swift b/DuckDuckGo/Common/Extensions/BundleExtension.swift index b3c7a629f5..28133debd8 100644 --- a/DuckDuckGo/Common/Extensions/BundleExtension.swift +++ b/DuckDuckGo/Common/Extensions/BundleExtension.swift @@ -26,6 +26,8 @@ extension Bundle { static let buildNumber = kCFBundleVersionKey as String static let versionNumber = "CFBundleShortVersionString" static let displayName = "CFBundleDisplayName" + static let documentTypes = "CFBundleDocumentTypes" + static let typeExtensions = "CFBundleTypeExtensions" static let vpnMenuAgentBundleId = "AGENT_BUNDLE_ID" static let vpnMenuAgentProductName = "AGENT_PRODUCT_NAME" @@ -115,6 +117,14 @@ extension Bundle { return path.hasPrefix(applicationsPath) } + var documentTypes: [[String: Any]] { + infoDictionary?[Keys.documentTypes] as? [[String: Any]] ?? [] + } + + var fileTypeExtensions: Set { + documentTypes.reduce(into: []) { $0.formUnion($1[Keys.typeExtensions] as? [String] ?? []) } + } + } enum BundleGroup { diff --git a/DuckDuckGo/Common/Extensions/NSAlertExtension.swift b/DuckDuckGo/Common/Extensions/NSAlertExtension.swift index 554ce896e5..a1f71cdeb4 100644 --- a/DuckDuckGo/Common/Extensions/NSAlertExtension.swift +++ b/DuckDuckGo/Common/Extensions/NSAlertExtension.swift @@ -137,15 +137,6 @@ extension NSAlert { return alert } - static func noAccessToSelectedFolder() -> NSAlert { - let alert = NSAlert() - alert.messageText = UserText.noAccessToSelectedFolderHeader - alert.informativeText = UserText.noAccessToSelectedFolder - alert.alertStyle = .warning - alert.addButton(withTitle: UserText.cancel) - return alert - } - static func disableEmailProtection() -> NSAlert { let alert = NSAlert() alert.messageText = UserText.disableEmailProtectionTitle diff --git a/DuckDuckGo/Common/Localizables/UserText.swift b/DuckDuckGo/Common/Localizables/UserText.swift index 4033037606..00a327d353 100644 --- a/DuckDuckGo/Common/Localizables/UserText.swift +++ b/DuckDuckGo/Common/Localizables/UserText.swift @@ -976,8 +976,6 @@ struct UserText { } } - static let noAccessToSelectedFolderHeader = NSLocalizedString("no.access.to.selected.folder.header", value: "DuckDuckGo needs permission to access selected folder", comment: "Header of the alert dialog informing user about failed download") - static let noAccessToSelectedFolder = NSLocalizedString("no.access.to.selected.folder", value: "Grant access to the location of download.", comment: "Alert presented to user if the app doesn't have rights to access selected folder") static let cannotOpenFileAlertHeader = NSLocalizedString("cannot.open.file.alert.header", value: "Cannot Open File", comment: "Header of the alert dialog informing user it is not possible to open the file") static let cannotOpenFileAlertInformative = NSLocalizedString("cannot.open.file.alert.informative", value: "The App Store version of DuckDuckGo can only access local files if you drag-and-drop them into a browser window.\n\n To navigate local files using the address bar, please download DuckDuckGo directly from https://duckduckgo.com/mac.", comment: "Informative of the alert dialog informing user it is not possible to open the file") diff --git a/DuckDuckGo/FileDownload/Model/SecurityScopedFileURLController.swift b/DuckDuckGo/FileDownload/Model/SecurityScopedFileURLController.swift index 7cdc7eecdf..bec94e02ac 100644 --- a/DuckDuckGo/FileDownload/Model/SecurityScopedFileURLController.swift +++ b/DuckDuckGo/FileDownload/Model/SecurityScopedFileURLController.swift @@ -152,6 +152,9 @@ extension NSURL { func hash(into hasher: inout Hasher) { hasher.combine(ObjectIdentifier(url)) } + static func == (lhs: Self, rhs: Self) -> Bool { + lhs.url === rhs.url + } } static var activeSecurityScopedUrlUsages: Set = [] #endif diff --git a/DuckDuckGo/Info.plist b/DuckDuckGo/Info.plist index 45285233da..095900fb99 100644 --- a/DuckDuckGo/Info.plist +++ b/DuckDuckGo/Info.plist @@ -9,44 +9,444 @@ CFBundleDevelopmentRegion $(DEVELOPMENT_LANGUAGE) CFBundleDocumentTypes - - - CFBundleTypeExtensions - - html - htm - shtml - xht - xhtml - - CFBundleTypeIconFile - document.icns - CFBundleTypeName - HTML Document - CFBundleTypeOSTypes - - HTML - - CFBundleTypeRole - Viewer - LSHandlerRank - Default - - - CFBundleTypeExtensions - - duckload - - CFBundleTypeName - Incomplete download - CFBundleTypeRole - Editor - NSIsRelatedItemType - - LSHandlerRank - Owner - - + + + CFBundleTypeExtensions + + html + htm + shtml + xht + xhtml + + CFBundleTypeIconFile + document.icns + CFBundleTypeIconSystemGenerated + YES + CFBundleTypeName + HTML Document + CFBundleTypeOSTypes + + HTML + + CFBundleTypeRole + Viewer + LSHandlerRank + Default + + + CFBundleTypeExtensions + + txt + text + log + + CFBundleTypeMIMETypes + + text/plain + + LSItemContentTypes + + public.text + + CFBundleTypeOSTypes + + TEXT + + CFBundleTypeIconFile + document.icns + CFBundleTypeIconSystemGenerated + YES + CFBundleTypeName + Text document + CFBundleTypeRole + Viewer + LSHandlerRank + Alternate + + + CFBundleTypeExtensions + + webarchive + + CFBundleTypeMIMETypes + + application/x-webarchive + + CFBundleTypeIconFile + document.icns + CFBundleTypeIconSystemGenerated + YES + CFBundleTypeName + Web archive + CFBundleTypeRole + Viewer + LSHandlerRank + Default + + + CFBundleTypeExtensions + + duckload + + CFBundleTypeName + Incomplete download + CFBundleTypeRole + Editor + NSIsRelatedItemType + + LSHandlerRank + Owner + + + CFBundleTypeExtensions + + pdf + + CFBundleTypeIconFile + document.icns + CFBundleTypeIconSystemGenerated + YES + CFBundleTypeName + PDF Document + CFBundleTypeMIMETypes + + application/pdf + + CFBundleTypeRole + Viewer + LSHandlerRank + Alternate + + + CFBundleTypeExtensions + + png + + CFBundleTypeMIMETypes + + image/png + + LSItemContentTypes + + public.png + + CFBundleTypeOSTypes + + PNGf + + CFBundleTypeIconFile + document.icns + CFBundleTypeName + PNG image + CFBundleTypeRole + Viewer + LSHandlerRank + Alternate + + + CFBundleTypeExtensions + + jpg + jpeg + jp2 + jpeg2 + + CFBundleTypeMIMETypes + + image/jpeg + image/jp2 + image/jpeg2000 + + CFBundleTypeOSTypes + + JPEG + jp2 + + LSItemContentTypes + + public.jpeg + public.jpeg-2000 + + CFBundleTypeIconFile + document.icns + CFBundleTypeIconSystemGenerated + YES + CFBundleTypeName + JPEG image + CFBundleTypeRole + Viewer + LSHandlerRank + Alternate + + + CFBundleTypeExtensions + + gif + giff + + CFBundleTypeMIMETypes + + image/gif + + CFBundleTypeOSTypes + + GIFf + + LSItemContentTypes + + com.compuserve.gif + + CFBundleTypeIconFile + document.icns + CFBundleTypeIconSystemGenerated + YES + CFBundleTypeName + GIF image + CFBundleTypeRole + Viewer + LSHandlerRank + Alternate + + + CFBundleTypeExtensions + + svg + + CFBundleTypeMIMETypes + + image/svg+xml + + LSItemContentTypes + + public.svg-image + + CFBundleTypeIconFile + document.icns + CFBundleTypeIconSystemGenerated + YES + CFBundleTypeName + SVG image + CFBundleTypeRole + Viewer + LSHandlerRank + Alternate + + + CFBundleTypeExtensions + + tiff + tif + + CFBundleTypeMIMETypes + + image/tiff + + CFBundleTypeOSTypes + + TIFF + + CFBundleTypeIconFile + document.icns + CFBundleTypeIconSystemGenerated + YES + CFBundleTypeName + TIFF image + CFBundleTypeRole + Viewer + LSHandlerRank + Alternate + + + CFBundleTypeExtensions + + ico + + CFBundleTypeMIMETypes + + image/x-icon + image/icon + image/ico + + CFBundleTypeOSTypes + + ICO + + CFBundleTypeIconFile + document.icns + CFBundleTypeIconSystemGenerated + YES + CFBundleTypeName + Windows icon + CFBundleTypeRole + Viewer + LSHandlerRank + Alternate + + + CFBundleTypeExtensions + + bmp + + CFBundleTypeMIMETypes + + image/bmp + image/x-bitmap + image/x-bmp + image/x-ms-bitmap + image/x-ms-bmp + + LSItemContentTypes + + com.microsoft.bmp + + CFBundleTypeName + BMP Image + CFBundleTypeIconFile + document.icns + CFBundleTypeIconSystemGenerated + YES + LSRoleHandlerScheme + Viewer + LSHandlerRank + Alternate + + + CFBundleTypeExtensions + + xml + rss + atom + + CFBundleTypeMIMETypes + + text/xml + application/xml + application/rss+xml + application/atom+xml + + CFBundleTypeIconFile + document.icns + CFBundleTypeIconSystemGenerated + YES + CFBundleTypeName + XML document + CFBundleTypeRole + Viewer + LSHandlerRank + Default + + + CFBundleTypeExtensions + + json + + CFBundleTypeMIMETypes + + text/json + application/json + + LSItemContentTypes + + public.json + + CFBundleTypeIconFile + document.icns + CFBundleTypeIconSystemGenerated + YES + CFBundleTypeName + JSON document + CFBundleTypeRole + Viewer + LSHandlerRank + Default + + + CFBundleTypeExtensions + + flac + m4a + mp3 + mpg + wav + aac + amr + mp4 + mpeg + + CFBundleTypeMIMETypes + + audio/flac + audio/x-flac + audio/m4a + audio/mp4 + audio/mp3 + audio/mpeg + audio/mpeg3 + audio/qcp + audio/qcelp + audio/vnd.qcelp + audio/vnd.qcp + audio/vnd.wave + audio/x-wav + audio/x-aac + audio/aac + audio/x-amr + audio/amr + audio/x-m4a + audio/x-mp3 + audio/x-mp4 + audio/x-mpeg + audio/x-mpeg3 + audio/x-mpg + + CFBundleTypeIconFile + document.icns + CFBundleTypeIconSystemGenerated + YES + CFBundleTypeName + Audio File + LSRoleHandlerScheme + Viewer + LSHandlerRank + Alternate + + + CFBundleTypeExtensions + + 3gp + avi + m4v + mov + mp4 + + CFBundleTypeMIMETypes + + video/3gp + video/3gpp + video/avi + video/x-msvideo + video/x-m4v + video/mp4 + video/x-quicktime + video/quicktime + + LSItemContentTypes + + public.movie + + CFBundleTypeIconFile + document.icns + CFBundleTypeIconSystemGenerated + YES + CFBundleTypeName + Video File + LSRoleHandlerScheme + Viewer + LSHandlerRank + Alternate + + CFBundleExecutable $(EXECUTABLE_NAME) CFBundleIdentifier diff --git a/DuckDuckGo/NavigationBar/View/AddressBarTextField.swift b/DuckDuckGo/NavigationBar/View/AddressBarTextField.swift index 84fbb30576..b68ce5a6ee 100644 --- a/DuckDuckGo/NavigationBar/View/AddressBarTextField.swift +++ b/DuckDuckGo/NavigationBar/View/AddressBarTextField.swift @@ -332,9 +332,10 @@ final class AddressBarTextField: NSTextField { } #if APPSTORE - if providedUrl.isFileURL, let window = self.window { - let alert = NSAlert.cannotOpenFileAlert() - alert.beginSheetModal(for: window) { response in + if providedUrl.isFileURL, !providedUrl.isWritableLocation(), // is sandbox extension available for the file? + let window = self.window { + + NSAlert.cannotOpenFileAlert().beginSheetModal(for: window) { response in switch response { case .alertSecondButtonReturn: WindowControllersManager.shared.show(url: URL.ddgLearnMore, source: .ui, newTab: false) @@ -344,6 +345,7 @@ final class AddressBarTextField: NSTextField { return } } + return } #endif diff --git a/DuckDuckGo/Tab/Model/Tab.swift b/DuckDuckGo/Tab/Model/Tab.swift index bfa43c24e3..bac1e48b47 100644 --- a/DuckDuckGo/Tab/Model/Tab.swift +++ b/DuckDuckGo/Tab/Model/Tab.swift @@ -1063,8 +1063,11 @@ protocol NewWindowPolicyDecisionMaker { let source = content.source if url.isFileURL { + // WebKit won‘t load local page‘s external resouces even with `allowingReadAccessTo` provided + // this could be fixed using a custom scheme handler loading local resources in future. + let readAccessScopeURL = url return webView.navigator(distributedNavigationDelegate: navigationDelegate) - .loadFileURL(url, allowingReadAccessTo: URL(fileURLWithPath: "/"), withExpectedNavigationType: source.navigationType) + .loadFileURL(url, allowingReadAccessTo: readAccessScopeURL, withExpectedNavigationType: source.navigationType) } var request = URLRequest(url: url, cachePolicy: source.cachePolicy) @@ -1122,7 +1125,12 @@ protocol NewWindowPolicyDecisionMaker { // only restore session from interactionStateData passed to Tab.init guard case .loadCachedFromTabContent(let interactionStateData) = self.interactionState else { return false } - if let url = content.urlForWebView, url.isFileURL { + switch content.urlForWebView { + case .some(let url) where url.isFileURL: +#if APPSTORE + guard url.isWritableLocation() else { fallthrough } +#endif + // request file system access before restoration webView.navigator(distributedNavigationDelegate: navigationDelegate) .loadFileURL(url, allowingReadAccessTo: url)? @@ -1131,7 +1139,8 @@ protocol NewWindowPolicyDecisionMaker { }, navigationDidFail: { [weak self] _, _ in self?.restoreInteractionState(with: interactionStateData) }) - } else { + + default: restoreInteractionState(with: interactionStateData) } diff --git a/DuckDuckGo/Tab/TabExtensions/DownloadsTabExtension.swift b/DuckDuckGo/Tab/TabExtensions/DownloadsTabExtension.swift index 2da642e537..23ce6e328e 100644 --- a/DuckDuckGo/Tab/TabExtensions/DownloadsTabExtension.swift +++ b/DuckDuckGo/Tab/TabExtensions/DownloadsTabExtension.swift @@ -182,7 +182,7 @@ extension DownloadsTabExtension: NavigationResponder { ?? navigationResponse.mainFrameNavigation?.navigationAction guard navigationResponse.httpResponse?.isSuccessful != false, // download non-http responses - !navigationResponse.canShowMIMEType || navigationResponse.shouldDownload + !responseCanShowMIMEType(navigationResponse) || navigationResponse.shouldDownload // if user pressed Opt+Enter in the Address bar to download from a URL || (navigationResponse.mainFrameNavigation?.redirectHistory.last ?? navigationResponse.mainFrameNavigation?.navigationAction)?.navigationType == .custom(.userRequestedPageDownload) else { @@ -199,6 +199,15 @@ extension DownloadsTabExtension: NavigationResponder { return .download } + private func responseCanShowMIMEType(_ response: NavigationResponse) -> Bool { + if response.canShowMIMEType { + return true + } else if response.url.isFileURL { + return Bundle.main.fileTypeExtensions.contains(response.url.pathExtension) + } + return false + } + @MainActor func navigationAction(_ navigationAction: NavigationAction, didBecome download: WebKitDownload) { enqueueDownload(download, withNavigationAction: navigationAction) From 44f0cce6035bf50a22fdb841fb86add94bd4dbf1 Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Wed, 10 Apr 2024 15:44:22 +0500 Subject: [PATCH 14/14] Disable directory download (#2585) Task/Issue URL: https://app.asana.com/0/1199230911884351/1201043708349615/f --- .../xcshareddata/swiftpm/Package.resolved | 2 +- .../Common/Extensions/URLExtension.swift | 7 ++ .../TabExtensions/DownloadsTabExtension.swift | 1 + .../Downloads/DownloadsIntegrationTests.swift | 94 +++++++++++++++++++ IntegrationTests/Tab/TabContentTests.swift | 4 +- 5 files changed, 105 insertions(+), 3 deletions(-) diff --git a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 12fabf86b4..a575d944d1 100644 --- a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -138,7 +138,7 @@ { "identity" : "swift-syntax", "kind" : "remoteSourceControl", - "location" : "https://github.com/apple/swift-syntax", + "location" : "https://github.com/apple/swift-syntax.git", "state" : { "revision" : "64889f0c732f210a935a0ad7cda38f77f876262d", "version" : "509.1.1" diff --git a/DuckDuckGo/Common/Extensions/URLExtension.swift b/DuckDuckGo/Common/Extensions/URLExtension.swift index b79720a9d5..3d95ae07f1 100644 --- a/DuckDuckGo/Common/Extensions/URLExtension.swift +++ b/DuckDuckGo/Common/Extensions/URLExtension.swift @@ -475,6 +475,13 @@ extension URL { } } + var isDirectory: Bool { + var isDirectory: ObjCBool = false + guard isFileURL, + FileManager.default.fileExists(atPath: path, isDirectory: &isDirectory) else { return false } + return isDirectory.boolValue + } + mutating func setFileHidden(_ hidden: Bool) throws { var resourceValues = URLResourceValues() resourceValues.isHidden = true diff --git a/DuckDuckGo/Tab/TabExtensions/DownloadsTabExtension.swift b/DuckDuckGo/Tab/TabExtensions/DownloadsTabExtension.swift index 23ce6e328e..1aae34f4c6 100644 --- a/DuckDuckGo/Tab/TabExtensions/DownloadsTabExtension.swift +++ b/DuckDuckGo/Tab/TabExtensions/DownloadsTabExtension.swift @@ -182,6 +182,7 @@ extension DownloadsTabExtension: NavigationResponder { ?? navigationResponse.mainFrameNavigation?.navigationAction guard navigationResponse.httpResponse?.isSuccessful != false, // download non-http responses + !navigationResponse.url.isDirectory, // don‘t download a local directory !responseCanShowMIMEType(navigationResponse) || navigationResponse.shouldDownload // if user pressed Opt+Enter in the Address bar to download from a URL || (navigationResponse.mainFrameNavigation?.redirectHistory.last ?? navigationResponse.mainFrameNavigation?.navigationAction)?.navigationType == .custom(.userRequestedPageDownload) diff --git a/IntegrationTests/Downloads/DownloadsIntegrationTests.swift b/IntegrationTests/Downloads/DownloadsIntegrationTests.swift index d108921c11..fbe3f5f557 100644 --- a/IntegrationTests/Downloads/DownloadsIntegrationTests.swift +++ b/IntegrationTests/Downloads/DownloadsIntegrationTests.swift @@ -91,6 +91,100 @@ class DownloadsIntegrationTests: XCTestCase { XCTAssertEqual(try? Data(contentsOf: fileUrl), data.html) } + @MainActor + func testWhenUnsupportedMimeType_downloadStarts() async throws { + let preferences = DownloadsPreferences.shared + preferences.alwaysRequestDownloadLocation = false + preferences.selectedDownloadLocation = FileManager.default.temporaryDirectory + + let downloadTaskFuture = FileDownloadManager.shared.downloadsPublisher.timeout(5).first().promise() + let suffix = Int.random(in: 0..