diff --git a/Package.resolved b/Package.resolved index df79e962f..227ddd730 100644 --- a/Package.resolved +++ b/Package.resolved @@ -50,8 +50,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/duckduckgo/privacy-dashboard", "state" : { - "revision" : "9de2b2aa317a48d3ee31116dc15b0feeb2cc9414", - "version" : "5.3.0" + "revision" : "53fd1a0f8d91fcf475d9220f810141007300dffd", + "version" : "7.1.1" } }, { @@ -77,8 +77,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/duckduckgo/sync_crypto", "state" : { - "revision" : "2ab6ab6f0f96b259c14c2de3fc948935fc16ac78", - "version" : "0.2.0" + "revision" : "0c8bf3c0e75591bc366407b9d7a73a9fcfc7736f", + "version" : "0.3.0" } }, { diff --git a/Package.swift b/Package.swift index 2aa41d3ba..3e72538cd 100644 --- a/Package.swift +++ b/Package.swift @@ -49,9 +49,9 @@ let package = Package( .package(url: "https://github.com/duckduckgo/duckduckgo-autofill.git", exact: "15.1.0"), .package(url: "https://github.com/duckduckgo/GRDB.swift.git", exact: "2.4.0"), .package(url: "https://github.com/duckduckgo/TrackerRadarKit", exact: "3.0.0"), - .package(url: "https://github.com/duckduckgo/sync_crypto", exact: "0.2.0"), + .package(url: "https://github.com/duckduckgo/sync_crypto", exact: "0.3.0"), .package(url: "https://github.com/gumob/PunycodeSwift.git", exact: "3.0.0"), - .package(url: "https://github.com/duckduckgo/privacy-dashboard", exact: "5.3.0"), + .package(url: "https://github.com/duckduckgo/privacy-dashboard", exact: "7.1.1"), .package(url: "https://github.com/duckduckgo/content-scope-scripts", exact: "6.28.0"), .package(url: "https://github.com/httpswift/swifter.git", exact: "1.5.0"), .package(url: "https://github.com/duckduckgo/bloom_cpp.git", exact: "3.0.0"), diff --git a/Sources/History/HistoryCoordinator.swift b/Sources/History/HistoryCoordinator.swift index e49280670..9017f1db8 100644 --- a/Sources/History/HistoryCoordinator.swift +++ b/Sources/History/HistoryCoordinator.swift @@ -44,6 +44,7 @@ public protocol HistoryCoordinating: AnyObject { func burnDomains(_ baseDomains: Set, tld: TLD, completion: @escaping (Set) -> Void) func burnVisits(_ visits: [Visit], completion: @escaping () -> Void) + func removeUrlEntry(_ url: URL, completion: ((Error?) -> Void)?) } /// Coordinates access to History. Uses its own queue with high qos for all operations. @@ -191,6 +192,20 @@ final public class HistoryCoordinator: HistoryCoordinating { } } + public enum EntryRemovalError: Error { + case notAvailable + } + + public func removeUrlEntry(_ url: URL, completion: ((Error?) -> Void)? = nil) { + guard let historyDictionary = historyDictionary else { return } + guard let entry = historyDictionary[url] else { + completion?(EntryRemovalError.notAvailable) + return + } + + removeEntries([entry], completionHandler: completion) + } + var cleaningDate: Date { .monthAgo } @objc private func cleanOld() { diff --git a/Sources/NetworkProtection/PacketTunnelProvider.swift b/Sources/NetworkProtection/PacketTunnelProvider.swift index fb133916c..db94999bf 100644 --- a/Sources/NetworkProtection/PacketTunnelProvider.swift +++ b/Sources/NetworkProtection/PacketTunnelProvider.swift @@ -42,6 +42,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { case rekeyAttempt(_ step: RekeyAttemptStep) case failureRecoveryAttempt(_ step: FailureRecoveryStep) case serverMigrationAttempt(_ step: ServerMigrationAttemptStep) + case malformedErrorDetected(_ error: Error) } public enum AttemptStep: CustomDebugStringConvertible { @@ -710,7 +711,13 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { Logger.networkProtection.log("🔴 Stopping VPN due to no auth token") await attemptShutdownDueToRevokedAccess() - throw error + // Check that the error is valid and able to be re-thrown to the OS before shutting the tunnel down + if let wrappedError = wrapped(error: error) { + providerEvents.fire(.malformedErrorDetected(error)) + throw wrappedError + } else { + throw error + } } do { @@ -737,7 +744,14 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { self.knownFailureStore.lastKnownFailure = KnownFailure(error) providerEvents.fire(.tunnelStartAttempt(.failure(error))) - throw error + + // Check that the error is valid and able to be re-thrown to the OS before shutting the tunnel down + if let wrappedError = wrapped(error: error) { + providerEvents.fire(.malformedErrorDetected(error)) + throw wrappedError + } else { + throw error + } } } @@ -1815,6 +1829,56 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { snoozeTimingStore.reset() } + // MARK: - Error Validation + + enum InvalidDiagnosticError: Error, CustomNSError { + case errorWithInvalidUnderlyingError(Error) + + var errorCode: Int { + switch self { + case .errorWithInvalidUnderlyingError(let error): + return (error as NSError).code + } + } + + var localizedDescription: String { + switch self { + case .errorWithInvalidUnderlyingError(let error): + return "Error '\(type(of: error))', message: \(error.localizedDescription)" + } + } + + var errorUserInfo: [String: Any] { + switch self { + case .errorWithInvalidUnderlyingError(let error): + let newError = NSError(domain: (error as NSError).domain, code: (error as NSError).code) + return [NSUnderlyingErrorKey: newError] + } + } + } + + /// Wraps an error instance in a new error type in cases where it is malformed; i.e., doesn't use an `NSError` instance for its underlying error, etc. + private func wrapped(error: Error) -> Error? { + if containsValidUnderlyingError(error) { + return nil + } else { + return InvalidDiagnosticError.errorWithInvalidUnderlyingError(error) + } + } + + private func containsValidUnderlyingError(_ error: Error) -> Bool { + let nsError = error as NSError + + if let underlyingError = nsError.userInfo[NSUnderlyingErrorKey] as? Error { + return containsValidUnderlyingError(underlyingError) + } else if nsError.userInfo[NSUnderlyingErrorKey] != nil { + // If `NSUnderlyingErrorKey` exists but is not an `Error`, return false + return false + } + + return true + } + } extension WireGuardAdapterError: LocalizedError, CustomDebugStringConvertible { diff --git a/Sources/Suggestions/Suggestion.swift b/Sources/Suggestions/Suggestion.swift index 3a50fdc33..d96cea05e 100644 --- a/Sources/Suggestions/Suggestion.swift +++ b/Sources/Suggestions/Suggestion.swift @@ -28,7 +28,7 @@ public enum Suggestion: Equatable { case openTab(title: String, url: URL) case unknown(value: String) - var url: URL? { + public var url: URL? { switch self { case .website(url: let url), .historyEntry(title: _, url: let url, allowedInTopHits: _), @@ -67,20 +67,26 @@ public enum Suggestion: Equatable { } } - var isOpenTab: Bool { + public var isOpenTab: Bool { if case .openTab = self { return true } return false } - var isBookmark: Bool { + public var isBookmark: Bool { if case .bookmark = self { return true } return false } + public var isHistoryEntry: Bool { + if case .historyEntry = self { + return true + } + return false + } } extension Suggestion { diff --git a/Sources/Suggestions/SuggestionResult.swift b/Sources/Suggestions/SuggestionResult.swift index af0997dea..1576ed48c 100644 --- a/Sources/Suggestions/SuggestionResult.swift +++ b/Sources/Suggestions/SuggestionResult.swift @@ -24,9 +24,9 @@ public struct SuggestionResult: Equatable { SuggestionResult(topHits: [], duckduckgoSuggestions: [], localSuggestions: []) } - private(set) public var topHits: [Suggestion] - private(set) public var duckduckgoSuggestions: [Suggestion] - private(set) public var localSuggestions: [Suggestion] + public let topHits: [Suggestion] + public let duckduckgoSuggestions: [Suggestion] + public let localSuggestions: [Suggestion] public init(topHits: [Suggestion], duckduckgoSuggestions: [Suggestion], diff --git a/Tests/HistoryTests/HistoryCoordinatorTests.swift b/Tests/HistoryTests/HistoryCoordinatorTests.swift index 98811e65d..c84622e00 100644 --- a/Tests/HistoryTests/HistoryCoordinatorTests.swift +++ b/Tests/HistoryTests/HistoryCoordinatorTests.swift @@ -316,6 +316,43 @@ class HistoryCoordinatorTests: XCTestCase { return bookmarksDatabase } + func testWhenRemoveUrlEntryCalledWithExistingUrl_ThenEntryIsRemovedAndNoError() { + let (historyStoringMock, historyCoordinator) = HistoryCoordinator.aHistoryCoordinator + + let url = URL(string: "https://duckduckgo.com")! + historyCoordinator.addVisit(of: url) + + XCTAssertTrue(historyCoordinator.history!.contains(where: { $0.url == url })) + + let removalExpectation = expectation(description: "Entry removed without error") + historyCoordinator.removeUrlEntry(url) { error in + XCTAssertNil(error, "Expected no error when removing an existing URL entry") + removalExpectation.fulfill() + } + + waitForExpectations(timeout: 1.0) + + XCTAssertFalse(historyCoordinator.history!.contains(where: { $0.url == url })) + XCTAssertTrue(historyStoringMock.removeEntriesCalled, "Expected removeEntries to be called") + XCTAssertEqual(historyStoringMock.removeEntriesArray.count, 1) + XCTAssertEqual(historyStoringMock.removeEntriesArray.first?.url, url) + } + + func testWhenRemoveUrlEntryCalledWithNonExistingUrl_ThenEntryRemovalFailsWithNotAvailableError() { + let (_, historyCoordinator) = HistoryCoordinator.aHistoryCoordinator + + let nonExistentUrl = URL(string: "https://nonexistent.com")! + + let removalExpectation = expectation(description: "Entry removal fails with notAvailable error") + historyCoordinator.removeUrlEntry(nonExistentUrl) { error in + XCTAssertNotNil(error, "Expected an error when removing a non-existent URL entry") + XCTAssertEqual(error as? HistoryCoordinator.EntryRemovalError, .notAvailable, "Expected notAvailable error") + removalExpectation.fulfill() + } + + waitForExpectations(timeout: 1.0) + } + } fileprivate extension HistoryCoordinator {