From d94895fc004b99f304b6d876f4ecd2fff3c2021f Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Tue, 2 Apr 2024 15:16:05 +0600 Subject: [PATCH 01/14] Allow choosing downloads location in App Store builds --- .../Model/DownloadsPreferences.swift | 71 +++++++++++++------ .../View/PreferencesGeneralView.swift | 4 +- 2 files changed, 52 insertions(+), 23 deletions(-) diff --git a/DuckDuckGo/Preferences/Model/DownloadsPreferences.swift b/DuckDuckGo/Preferences/Model/DownloadsPreferences.swift index ce6962e4bf..5e23fe3ffd 100644 --- a/DuckDuckGo/Preferences/Model/DownloadsPreferences.swift +++ b/DuckDuckGo/Preferences/Model/DownloadsPreferences.swift @@ -65,17 +65,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,36 +96,63 @@ 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 _selectedDownloadLocation: URL? + private var isUsingSecurityScopedResource = false var selectedDownloadLocation: URL? { get { - persistor.selectedDownloadLocation?.url + if let selectedDownloadLocation = _selectedDownloadLocation { + 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 + } + if !isUsingSecurityScopedResource { + isUsingSecurityScopedResource = url.startAccessingSecurityScopedResource() + } + + _selectedDownloadLocation = url + return url + } +#endif + guard let url = persistor.selectedDownloadLocation.flatMap(URL.init(string:)), + url.isFileURL else { return nil } + return url } - set { defer { objectWillChange.send() } - guard let newDownloadLocation = newValue else { - persistor.selectedDownloadLocation = nil - return - } - if Self.isDownloadLocationValid(newDownloadLocation) { - persistor.selectedDownloadLocation = newDownloadLocation.absoluteString + if isUsingSecurityScopedResource, + let newValue, _selectedDownloadLocation /* oldValue */ != newValue { + selectedDownloadLocation?.stopAccessingSecurityScopedResource() } + // the setter is called for already selected directory, + // so consume the unbalanced startAccessingSecurityScopedResource + isUsingSecurityScopedResource = true + + setSelectedDownloadLocation(validatedDownloadLocation(newValue)) } } + private func setSelectedDownloadLocation(_ url: URL?) { + _selectedDownloadLocation = url + 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 { diff --git a/DuckDuckGo/Preferences/View/PreferencesGeneralView.swift b/DuckDuckGo/Preferences/View/PreferencesGeneralView.swift index 591961ee75..1773291e66 100644 --- a/DuckDuckGo/Preferences/View/PreferencesGeneralView.swift +++ b/DuckDuckGo/Preferences/View/PreferencesGeneralView.swift @@ -106,15 +106,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) } From 419a7c224bc021db5f8ec55411329303fd4d68b5 Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Wed, 3 Apr 2024 13:36:45 +0600 Subject: [PATCH 02/14] fix tests --- UnitTests/Preferences/DownloadsPreferencesTests.swift | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/UnitTests/Preferences/DownloadsPreferencesTests.swift b/UnitTests/Preferences/DownloadsPreferencesTests.swift index 3213d7c0af..0653e74e24 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() { @@ -222,9 +222,7 @@ class DownloadsPreferencesTests: XCTestCase { 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() { From 176892c106bec6edd244b48b4b31c422a3ece77c Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Wed, 3 Apr 2024 15:47:21 +0600 Subject: [PATCH 03/14] File Presenter fixes --- .../FileDownload/Model/FilePresenter.swift | 68 ++++++++++++------- 1 file changed, 42 insertions(+), 26 deletions(-) diff --git a/DuckDuckGo/FileDownload/Model/FilePresenter.swift b/DuckDuckGo/FileDownload/Model/FilePresenter.swift index be8d94e8a6..c911e3170c 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 @@ -56,13 +55,15 @@ internal class FilePresenter { final let presentedItemOperationQueue: OperationQueue fileprivate final weak var delegate: FilePresenterDelegate? + final var shouldBeRemovedTwice = false init(presentedItemOperationQueue: OperationQueue) { self.presentedItemOperationQueue = presentedItemOperationQueue } + 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) {} @@ -100,9 +101,11 @@ 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) { + self.primaryPresentedItemURL = primaryPresentedItemURL + super.init(presentedItemOperationQueue: presentedItemOperationQueue) } } @@ -113,8 +116,6 @@ internal class FilePresenter { fileprivate let logger: any FilePresenterLogger - let primaryPresentedItemURL: URL? - private var _url: URL? final var url: URL? { lock.withLock { @@ -139,35 +140,46 @@ internal class FilePresenter { init(url: URL, primaryItemURL: URL? = nil, logger: FilePresenterLogger = OSLog.disabled, createIfNeededCallback: ((URL) throws -> URL)? = nil) throws { self._url = url - self.primaryPresentedItemURL = primaryItemURL self.logger = logger - let innerPresenter: DelegatingFilePresenter - if primaryItemURL != nil { - innerPresenter = DelegatingRelatedFilePresenter(presentedItemOperationQueue: FilePresenter.presentedItemOperationQueue) + do { + try setupInnerPresenter(for: url, primaryItemURL: primaryItemURL, createIfNeededCallback: createIfNeededCallback) + logger.log("🗄️ added file presenter for \"\(url.path)\"\(primaryItemURL != nil ? " primary item: \"\(primaryItemURL!.path)\"" : "")") + } catch { + removeFilePresenter() + throw error + } + } + + private func setupInnerPresenter(for url: URL, primaryItemURL: URL?, createIfNeededCallback: ((URL) throws -> URL)?) throws { + var innerPresenter = if let primaryItemURL { + DelegatingRelatedFilePresenter(primaryPresentedItemURL: primaryItemURL, presentedItemOperationQueue: FilePresenter.presentedItemOperationQueue) } else { - innerPresenter = DelegatingFilePresenter(presentedItemOperationQueue: FilePresenter.presentedItemOperationQueue) + DelegatingFilePresenter(presentedItemOperationQueue: FilePresenter.presentedItemOperationQueue) } - self.innerPresenter = innerPresenter innerPresenter.delegate = self + self.innerPresenter = innerPresenter + + // even though we will call `addFilePresenter` again for a non-existent file, + // we still must call this `addFilePresenter` to get access to the secondary item + // (when primaryPresentedItemURL is provided). 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 - NSFileCoordinator.removeFilePresenter(innerPresenter) - NSFileCoordinator.addFilePresenter(innerPresenter) - - } else { + guard let createFile = createIfNeededCallback else { throw CocoaError(.fileReadNoSuchFile, userInfo: [NSFilePathErrorKey: url.path]) } + logger.log("🗄️💥 creating file for presenter at \"\(url.path)\"") + self._url = try coordinateWrite(at: url, using: createFile) + // add the File Presenter once again after the file was created. + // otherwise, file move events may not be received (looks like an API bug). + NSFileCoordinator.addFilePresenter(innerPresenter) + // need to balance sandbox extension release for secondary item presenters added twice + innerPresenter.shouldBeRemovedTwice = (primaryItemURL != nil) + } + try coordinateRead(at: url, with: .withoutChanges) { url in + addFSODispatchSource(for: url) } - addFSODispatchSource(for: url) - - logger.log("🗄️ added file presenter for \"\(url.path)\"\(primaryPresentedItemURL != nil ? " primary item: \"\(primaryPresentedItemURL!.path)\"" : "")") } private func addFSODispatchSource(for url: URL) { @@ -215,7 +227,9 @@ internal class FilePresenter { private func removeFilePresenter() { if let innerPresenter { logger.log("🗄️ removing file presenter for \"\(url?.path ?? "")\"") - NSFileCoordinator.removeFilePresenter(innerPresenter) + for i in 1...(innerPresenter.shouldBeRemovedTwice ? 2 : 1) { // if secondary item presenter was added twice - remove it twice + NSFileCoordinator.removeFilePresenter(innerPresenter) + } self.innerPresenter = nil } } @@ -227,6 +241,8 @@ internal class FilePresenter { } deinit { + // innerPresenter delegate won‘t be available at this point, so set the final url here to remove the presenter + innerPresenter?.fallbackPresentedItemURL = _url removeFilePresenter() } From 593904b0cbacd7335f0dcc6a14e16cc1bc9fd3d9 Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Wed, 3 Apr 2024 16:30:47 +0600 Subject: [PATCH 04/14] fix tests --- .../FileDownload/Model/FilePresenter.swift | 4 ++-- .../Downloads/DownloadsIntegrationTests.swift | 18 +++++++++--------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/DuckDuckGo/FileDownload/Model/FilePresenter.swift b/DuckDuckGo/FileDownload/Model/FilePresenter.swift index c911e3170c..ab320e638b 100644 --- a/DuckDuckGo/FileDownload/Model/FilePresenter.swift +++ b/DuckDuckGo/FileDownload/Model/FilePresenter.swift @@ -152,7 +152,7 @@ internal class FilePresenter { } private func setupInnerPresenter(for url: URL, primaryItemURL: URL?, createIfNeededCallback: ((URL) throws -> URL)?) throws { - var innerPresenter = if let primaryItemURL { + let innerPresenter = if let primaryItemURL { DelegatingRelatedFilePresenter(primaryPresentedItemURL: primaryItemURL, presentedItemOperationQueue: FilePresenter.presentedItemOperationQueue) } else { DelegatingFilePresenter(presentedItemOperationQueue: FilePresenter.presentedItemOperationQueue) @@ -227,7 +227,7 @@ internal class FilePresenter { private func removeFilePresenter() { if let innerPresenter { logger.log("🗄️ removing file presenter for \"\(url?.path ?? "")\"") - for i in 1...(innerPresenter.shouldBeRemovedTwice ? 2 : 1) { // if secondary item presenter was added twice - remove it twice + for _ in 1...(innerPresenter.shouldBeRemovedTwice ? 2 : 1) { // if secondary item presenter was added twice - remove it twice NSFileCoordinator.removeFilePresenter(innerPresenter) } self.innerPresenter = nil diff --git a/IntegrationTests/Downloads/DownloadsIntegrationTests.swift b/IntegrationTests/Downloads/DownloadsIntegrationTests.swift index 35e2024c05..c1ded9f47c 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(persistor: DownloadsPreferencesUserDefaultsPersistor()) + preferences.alwaysRequestDownloadLocation = false + preferences.selectedDownloadLocation = FileManager.default.temporaryDirectory let downloadTaskFuture = FileDownloadManager.shared.downloadsPublisher.timeout(5).first().promise() let suffix = Int.random(in: 0.. Date: Wed, 3 Apr 2024 16:47:45 +0600 Subject: [PATCH 05/14] fix tests --- IntegrationTests/Downloads/DownloadsIntegrationTests.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/IntegrationTests/Downloads/DownloadsIntegrationTests.swift b/IntegrationTests/Downloads/DownloadsIntegrationTests.swift index c1ded9f47c..d108921c11 100644 --- a/IntegrationTests/Downloads/DownloadsIntegrationTests.swift +++ b/IntegrationTests/Downloads/DownloadsIntegrationTests.swift @@ -70,7 +70,7 @@ class DownloadsIntegrationTests: XCTestCase { @MainActor func testWhenShouldDownloadResponse_downloadStarts() async throws { - let preferences = DownloadsPreferences(persistor: DownloadsPreferencesUserDefaultsPersistor()) + let preferences = DownloadsPreferences.shared preferences.alwaysRequestDownloadLocation = false preferences.selectedDownloadLocation = FileManager.default.temporaryDirectory @@ -235,7 +235,7 @@ class DownloadsIntegrationTests: XCTestCase { @MainActor func testWhenNavigationActionIsData_downloadStarts() async throws { - let preferences = DownloadsPreferences(persistor: DownloadsPreferencesUserDefaultsPersistor()) + let preferences = DownloadsPreferences.shared preferences.alwaysRequestDownloadLocation = false preferences.selectedDownloadLocation = FileManager.default.temporaryDirectory @@ -268,7 +268,7 @@ class DownloadsIntegrationTests: XCTestCase { @MainActor func testWhenNavigationActionIsBlob_downloadStarts() async throws { - let preferences = DownloadsPreferences(persistor: DownloadsPreferencesUserDefaultsPersistor()) + let preferences = DownloadsPreferences.shared preferences.alwaysRequestDownloadLocation = false preferences.selectedDownloadLocation = FileManager.default.temporaryDirectory From 34544679d869c456d9237551bf4c2d438be71dac Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Wed, 3 Apr 2024 16:59:45 +0600 Subject: [PATCH 06/14] fix warning --- UnitTests/Preferences/DownloadsPreferencesTests.swift | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/UnitTests/Preferences/DownloadsPreferencesTests.swift b/UnitTests/Preferences/DownloadsPreferencesTests.swift index 0653e74e24..7c50d88ff5 100644 --- a/UnitTests/Preferences/DownloadsPreferencesTests.swift +++ b/UnitTests/Preferences/DownloadsPreferencesTests.swift @@ -213,12 +213,11 @@ 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") From 6865cc7de26dab8340eee8398d0f0bca75165bb9 Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Fri, 5 Apr 2024 20:04:00 +0600 Subject: [PATCH 07/14] minor tests adjustment --- UnitTests/FileDownload/FilePresenterTests.swift | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/UnitTests/FileDownload/FilePresenterTests.swift b/UnitTests/FileDownload/FilePresenterTests.swift index fb0390c09d..003267d039 100644 --- a/UnitTests/FileDownload/FilePresenterTests.swift +++ b/UnitTests/FileDownload/FilePresenterTests.swift @@ -95,15 +95,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 +113,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 +126,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() From 993c8db9cf38a23182a91e9a8f8b048f3023e8e4 Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Mon, 8 Apr 2024 21:11:27 +0600 Subject: [PATCH 08/14] Correctly handle downloads sandbox extensions lifetime --- Configuration/App/DuckDuckGo.xcconfig | 8 +- Configuration/AppStore.xcconfig | 8 +- DuckDuckGo.xcodeproj/project.pbxproj | 22 ++ .../Extensions/FileManagerExtension.swift | 12 + .../Common/Extensions/URLExtension.swift | 44 ++++ .../FileDownload/Model/FilePresenter.swift | 236 ++++++++++-------- .../Model/NSURL+sandboxExtensionRetainCount.m | 40 +++ .../SecurityScopedFileURLController.swift | 179 +++++++++++++ .../Model/WebKitDownloadTask.swift | 44 ++-- .../Services/DownloadListCoordinator.swift | 12 +- DuckDuckGo/Tab/View/WebView.swift | 1 + .../FileDownload/FilePresenterTests.swift | 66 ++--- sandbox-test-tool/SandboxTestTool.swift | 39 ++- 13 files changed, 514 insertions(+), 197 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 710327265e..c878535495 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 3269bcf0a5..d0e5b5a699 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -3058,6 +3058,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 */; }; @@ -3198,6 +3204,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 */; }; @@ -4633,6 +4642,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 = ""; }; @@ -6671,6 +6682,8 @@ B6C0B23826E742610031CB7F /* FileDownloadError.swift */, 856C98DE257014BD00A22F1F /* FileDownloadManager.swift */, B6E6B9E22BA1F5F1008AA7E1 /* FilePresenter.swift */, + B6ABD0C92BC03F610000EB69 /* SecurityScopedFileURLController.swift */, + B6ABD0CD2BC042CE0000EB69 /* NSURL+sandboxExtensionRetainCount.m */, B6A924D82664C72D001A28CA /* WebKitDownloadTask.swift */, B6CC266B2BAD9CD800F53F8D /* FileProgressPresenter.swift */, ); @@ -10706,6 +10719,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 */, @@ -10851,6 +10865,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 */, @@ -12002,6 +12017,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 */, @@ -12026,6 +12042,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 */, @@ -12868,6 +12885,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 */, @@ -12987,6 +13005,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 */, @@ -13348,7 +13367,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/FileDownload/Model/FilePresenter.swift b/DuckDuckGo/FileDownload/Model/FilePresenter.swift index ab320e638b..1770935af2 100644 --- a/DuckDuckGo/FileDownload/Model/FilePresenter.swift +++ b/DuckDuckGo/FileDownload/Model/FilePresenter.swift @@ -55,10 +55,10 @@ internal class FilePresenter { final let presentedItemOperationQueue: OperationQueue fileprivate final weak var delegate: FilePresenterDelegate? - final var shouldBeRemovedTwice = false - init(presentedItemOperationQueue: OperationQueue) { + init(presentedItemOperationQueue: OperationQueue, delegate: FilePresenterDelegate) { self.presentedItemOperationQueue = presentedItemOperationQueue + self.delegate = delegate } final var fallbackPresentedItemURL: URL? @@ -73,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) @@ -88,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) @@ -103,66 +100,103 @@ internal class FilePresenter { let primaryPresentedItemURL: URL? - init(primaryPresentedItemURL: URL?, presentedItemOperationQueue: OperationQueue) { + init(primaryPresentedItemURL: URL?, presentedItemOperationQueue: OperationQueue, delegate: FilePresenterDelegate) { self.primaryPresentedItemURL = primaryPresentedItemURL - super.init(presentedItemOperationQueue: presentedItemOperationQueue) + 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 - 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 + /// - 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 + + 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)\"\(primaryItemURL != nil ? " primary item: \"\(primaryItemURL!.path)\"" : "")") + logger.log("🗄️ added file presenter for \"\(url.path) primary item: \"\(primaryItemURL.path)\"") } catch { - removeFilePresenter() + 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) + DelegatingRelatedFilePresenter(primaryPresentedItemURL: primaryItemURL, presentedItemOperationQueue: FilePresenter.presentedItemOperationQueue, delegate: self) } else { - DelegatingFilePresenter(presentedItemOperationQueue: FilePresenter.presentedItemOperationQueue) + DelegatingFilePresenter(presentedItemOperationQueue: FilePresenter.presentedItemOperationQueue, delegate: self) } - innerPresenter.delegate = self - self.innerPresenter = innerPresenter + self.innerPresenters = [innerPresenter] - // even though we will call `addFilePresenter` again for a non-existent file, - // we still must call this `addFilePresenter` to get access to the secondary item - // (when primaryPresentedItemURL is provided). NSFileCoordinator.addFilePresenter(innerPresenter) if !FileManager.default.fileExists(atPath: url.path) { @@ -170,13 +204,29 @@ internal class FilePresenter { throw CocoaError(.fileReadNoSuchFile, userInfo: [NSFilePathErrorKey: url.path]) } logger.log("🗄️💥 creating file for presenter at \"\(url.path)\"") - self._url = try coordinateWrite(at: url, using: createFile) - // add the File Presenter once again after the file was created. - // otherwise, file move events may not be received (looks like an API bug). - NSFileCoordinator.addFilePresenter(innerPresenter) - // need to balance sandbox extension release for secondary item presenters added twice - innerPresenter.shouldBeRemovedTwice = (primaryItemURL != nil) + // 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) + } + } + // 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) } + try coordinateRead(at: url, with: .withoutChanges) { url in addFSODispatchSource(for: url) } @@ -199,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 ? @@ -224,29 +274,36 @@ internal class FilePresenter { dispatchSource.resume() } - private func removeFilePresenter() { - if let innerPresenter { - logger.log("🗄️ removing file presenter for \"\(url?.path ?? "")\"") - for _ in 1...(innerPresenter.shouldBeRemovedTwice ? 2 : 1) { // if secondary item presenter was added twice - remove it twice - NSFileCoordinator.removeFilePresenter(innerPresenter) + 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 } - self.innerPresenter = nil + logger.log("🗄️ removing file presenter \(idx) for \"\(innerPresenter.presentedItemURL?.path ?? "")\"") + NSFileCoordinator.removeFilePresenter(innerPresenter) } + 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 { - // innerPresenter delegate won‘t be available at this point, so set the final url here to remove the presenter - innerPresenter?.fallbackPresentedItemURL = _url - removeFilePresenter() + removeFilePresenters() } } + extension FilePresenter: FilePresenterDelegate { func presentedItemDidMove(to newURL: URL) { @@ -256,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 { @@ -270,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? { @@ -287,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 { @@ -321,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 @@ -362,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() @@ -393,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) } @@ -441,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 a39f173fa1..ba36304cd7 100644 --- a/DuckDuckGo/FileDownload/Model/WebKitDownloadTask.swift +++ b/DuckDuckGo/FileDownload/Model/WebKitDownloadTask.swift @@ -226,6 +226,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 +340,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,30 +360,30 @@ 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) } } 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 + let tempFilePresenter = try BookmarkFilePresenter(url: duckloadURL, primaryItemURL: destinationURL, logger: log) { [log] duckloadURL in + guard duckloadURL != tempURL else { return tempURL } do { try fm.moveItem(at: tempURL, to: duckloadURL) } catch { @@ -697,20 +704,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 a7d0b3e1d3..326b964d5f 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 } @@ -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 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/UnitTests/FileDownload/FilePresenterTests.swift b/UnitTests/FileDownload/FilePresenterTests.swift index 003267d039..52a36f568e 100644 --- a/UnitTests/FileDownload/FilePresenterTests.swift +++ b/UnitTests/FileDownload/FilePresenterTests.swift @@ -64,7 +64,6 @@ final class FilePresenterTests: XCTestCase { cancellables.removeAll() onError = nil onFileRead = nil - NSURL.swizzleStopAccessingSecurityScopedResource(with: nil) } private func makeNonSandboxFile() throws -> URL { @@ -180,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 @@ -191,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() @@ -223,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) @@ -293,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() @@ -319,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) @@ -358,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) @@ -377,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() @@ -409,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() @@ -459,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) @@ -506,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) @@ -546,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() @@ -574,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) @@ -603,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 @@ -636,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? @@ -659,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") @@ -692,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") @@ -719,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/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 f40844ded7045f625c00b8e013bd67aafa9e9f46 Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Tue, 9 Apr 2024 13:37:25 +0600 Subject: [PATCH 09/14] Add supported document types --- .../Common/Extensions/BundleExtension.swift | 10 + .../Common/Extensions/NSAlertExtension.swift | 9 - DuckDuckGo/Common/Localizables/UserText.swift | 2 - DuckDuckGo/Info.plist | 476 ++++++++++++++++-- .../View/AddressBarTextField.swift | 8 +- DuckDuckGo/Tab/Model/Tab.swift | 19 +- .../TabExtensions/DownloadsTabExtension.swift | 11 +- 7 files changed, 479 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 49a5fb4d84..2f96b6fe0f 100644 --- a/DuckDuckGo/Common/Localizables/UserText.swift +++ b/DuckDuckGo/Common/Localizables/UserText.swift @@ -973,8 +973,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/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 6fd7bffd3a..2f0edd324d 100644 --- a/DuckDuckGo/Tab/Model/Tab.swift +++ b/DuckDuckGo/Tab/Model/Tab.swift @@ -1077,8 +1077,15 @@ protocol NewWindowPolicyDecisionMaker { let source = content.source if url.isFileURL { +#if APPSTORE + // grant access to a containing folder when a sandbox extension available for it (e.g. Downloads) + // only access the file when no access to the containing folder + let readAccessScopeURL = url.deletingLastPathComponent().isWritableLocation() ? url.deletingLastPathComponent() : url +#else + let readAccessScopeURL = url.deletingLastPathComponent() +#endif 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) @@ -1136,7 +1143,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)? @@ -1145,7 +1157,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..24829786ea 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 + !responseaCanShowMIMEType(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 responseaCanShowMIMEType(_ 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 8957215cf4f5b28f496a2929912f66bfb966096b Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Tue, 9 Apr 2024 18:41:30 +0600 Subject: [PATCH 10/14] Disable directory download --- DuckDuckGo/Tab/TabExtensions/DownloadsTabExtension.swift | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/DuckDuckGo/Tab/TabExtensions/DownloadsTabExtension.swift b/DuckDuckGo/Tab/TabExtensions/DownloadsTabExtension.swift index 24829786ea..f13d3cb99c 100644 --- a/DuckDuckGo/Tab/TabExtensions/DownloadsTabExtension.swift +++ b/DuckDuckGo/Tab/TabExtensions/DownloadsTabExtension.swift @@ -196,6 +196,14 @@ extension DownloadsTabExtension: NavigationResponder { return .cancel } + var isDirectory: ObjCBool = false + if navigationResponse.url.isFileURL, + FileManager.default.fileExists(atPath: navigationResponse.url.path, isDirectory: &isDirectory), + isDirectory.boolValue == true { + // don‘t download a directory + return .next + } + return .download } From bd1153e1f057a6af1e1a6969f6dc14ff53fbbc3f Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Wed, 10 Apr 2024 11:34:06 +0600 Subject: [PATCH 11/14] move isDirectory check to URL extension --- DuckDuckGo/Common/Extensions/URLExtension.swift | 7 +++++++ DuckDuckGo/Tab/TabExtensions/DownloadsTabExtension.swift | 9 +-------- 2 files changed, 8 insertions(+), 8 deletions(-) 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 f13d3cb99c..c1c5554bce 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 !responseaCanShowMIMEType(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) @@ -196,14 +197,6 @@ extension DownloadsTabExtension: NavigationResponder { return .cancel } - var isDirectory: ObjCBool = false - if navigationResponse.url.isFileURL, - FileManager.default.fileExists(atPath: navigationResponse.url.path, isDirectory: &isDirectory), - isDirectory.boolValue == true { - // don‘t download a directory - return .next - } - return .download } From 6530cb5b6d2a01da659317ec8aa2d7975d912464 Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Wed, 10 Apr 2024 11:34:22 +0600 Subject: [PATCH 12/14] add integration tests --- .../Downloads/DownloadsIntegrationTests.swift | 96 +++++++++++++++++++ IntegrationTests/Tab/TabContentTests.swift | 4 +- 2 files changed, 98 insertions(+), 2 deletions(-) diff --git a/IntegrationTests/Downloads/DownloadsIntegrationTests.swift b/IntegrationTests/Downloads/DownloadsIntegrationTests.swift index d108921c11..06f7d8d667 100644 --- a/IntegrationTests/Downloads/DownloadsIntegrationTests.swift +++ b/IntegrationTests/Downloads/DownloadsIntegrationTests.swift @@ -91,6 +91,102 @@ 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.. Date: Tue, 9 Apr 2024 15:49:28 +0600 Subject: [PATCH 13/14] move breakByRaisingSigInt code to the main module --- DuckDuckGo/Common/Logging/Logging.swift | 42 +++++++++++++++++++++++++ 1 file changed, 42 insertions(+) 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 From d905db19229b62892793e37d0b4e39e76c1a12bb Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Wed, 10 Apr 2024 11:47:36 +0600 Subject: [PATCH 14/14] fix linter issues --- IntegrationTests/Downloads/DownloadsIntegrationTests.swift | 2 -- 1 file changed, 2 deletions(-) diff --git a/IntegrationTests/Downloads/DownloadsIntegrationTests.swift b/IntegrationTests/Downloads/DownloadsIntegrationTests.swift index 06f7d8d667..fbe3f5f557 100644 --- a/IntegrationTests/Downloads/DownloadsIntegrationTests.swift +++ b/IntegrationTests/Downloads/DownloadsIntegrationTests.swift @@ -161,7 +161,6 @@ class DownloadsIntegrationTests: XCTestCase { preferences.selectedDownloadLocation = FileManager.default.temporaryDirectory let tempFileURL = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) - let downloadTaskFuture = FileDownloadManager.shared.downloadsPublisher.timeout(5).first().promise() let tab = tabViewModel.tab let loadingResult = await tab.setUrl(tempFileURL, source: .link)?.result @@ -178,7 +177,6 @@ class DownloadsIntegrationTests: XCTestCase { let dirURL = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) try FileManager.default.createDirectory(at: dirURL, withIntermediateDirectories: true) - let downloadTaskFuture = FileDownloadManager.shared.downloadsPublisher.timeout(5).first().promise() let tab = tabViewModel.tab let loadingResult = await tab.setUrl(dirURL, source: .link)?.result