Malware protection 1: rename PhishingDetection to MaliciousSiteProtection #1091
Conversation
|
|
||
| extension APIClient { | ||
|
|
||
| public struct ChangeSetResponse<T: Codable & Hashable>: Codable, Equatable { |
| // If nothing found, hit the API to get matches | ||
| if await checkApiMatches(canonicalHost: canonicalHost, canonicalUrl: canonicalUrl) { | ||
| return true | ||
| public func evaluate(_ url: URL) async -> ThreatKind? { |
There was a problem hiding this comment.
I think this is fine for now, but soon this will result in two API requests when we introduce malware protections. It also doesn't allow for independent feature flagging. We should discuss if/how we want to do this on Monday in our call. Doesn't block this PR though.
| @@ -58,8 +58,7 @@ public class PhishingDetectionDataProvider: PhishingDetectionDataProviding { | |||
| let filterSetData = try loadData(from: embeddedFilterSetURL, expectedSHA: embeddedFilterSetDataSHA) | |||
| return try JSONDecoder().decode(Set<Filter>.self, from: filterSetData) | |||
| } catch { | |||
| Logger.phishingDetectionDataProvider.error("🔴 Error: SHA mismatch for filterSet JSON file. Expected \(self.embeddedFilterSetDataSHA)") | |||
| return [] | |||
| fatalError("🔴 Error: SHA mismatch for filterSet JSON file. Expected \(self.embeddedFilterSetDataSHA)") | |||
There was a problem hiding this comment.
I know we do this for privacy configs, but I'm not a fan of crashing the app in this instance. However, ideally this would never make it past internal testing so I'm okay with it.
There was a problem hiding this comment.
Reaching the catch block means the app bundle corruption (same is valid for a missing Storyboard), and considering the Let it crash discussion this makes impossible to run the app further and we must crash here.
Plus, it happens early in the app lifetime during smoke testing that makes it easily noticeable and not affecting the production built.
| public init() { | ||
| let dataStoreDirectory: URL | ||
| do { | ||
| dataStoreDirectory = try FileManager.default.url(for: .applicationSupportDirectory, in: .userDomainMask, appropriateFor: nil, create: true) |
There was a problem hiding this comment.
Again, doesn't affect this PR but just making notes for myself: in the next PR we should try to align with @alessandroboron's TD for storage locations: https://app.asana.com/0/481882893211075/1207273224076495/f
There was a problem hiding this comment.
this part is not modified from the original implementation: to be update in later PRs to match the config files storage location
mallexxx
changed the title
Please review the release process for BrowserServicesKit here.
Required:
Task/Issue URL: https://app.asana.com/0/1202406491309510/1208033567421351/f
iOS PR:
macOS PR: duckduckgo/macos-browser#3588
What kind of version bump will this require?: Major
Description:
PhishingDetectiontoMaliciousSiteProtectionSteps to test this PR: