From 77e58abf392aefcb5f83bb58d40da8ca6ec4abe7 Mon Sep 17 00:00:00 2001 From: StephenHeaps <5314553+StephenHeaps@users.noreply.github.com> Date: Tue, 3 Jun 2025 18:25:17 -0400 Subject: [PATCH] fix(privacy): Re-compile engines from cached combined rules when failing to deserialize cached engine on iOS (#29369) * Fix incorrect [AdblockEngine initWithSerializedData:error:] error flow not returning nil as documentation states. * Fix incorrect [AdblockEngine initWithRules:error:] error flow not returning nil as documentation states. * Add debug menu option for corrupting adblock DAT cache. * Fallback to loading cached plaintext (combined) rule list when failing to load or deserialize from DAT. * Cleanup cached serialized engine file or combined rules file when failing to compile from the cache. --- .../Debug/AdBlock/AdBlockDebugView.swift | 111 ++++++++++++++++ .../AdBlock/AdBlockEngineManager.swift | 125 +++++++++++++++--- .../AdBlock/AdBlockGroupsManager.swift | 16 +-- .../AdBlock/GroupedAdBlockEngine.swift | 8 +- .../GroupedAdBlockEngineTests.swift | 9 +- .../api/brave_shields/adblock_engine.mm | 7 + 6 files changed, 234 insertions(+), 42 deletions(-) diff --git a/ios/brave-ios/Sources/Brave/Frontend/Settings/Debug/AdBlock/AdBlockDebugView.swift b/ios/brave-ios/Sources/Brave/Frontend/Settings/Debug/AdBlock/AdBlockDebugView.swift index 06c45d3209a..e8c3ee1e50a 100644 --- a/ios/brave-ios/Sources/Brave/Frontend/Settings/Debug/AdBlock/AdBlockDebugView.swift +++ b/ios/brave-ios/Sources/Brave/Frontend/Settings/Debug/AdBlock/AdBlockDebugView.swift @@ -3,6 +3,8 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. +import BraveShared +import Strings import SwiftUI import WebKit @@ -10,6 +12,7 @@ struct AdBlockDebugView: View { var body: some View { Form { CompileContentBlockersSectionView() + CorruptCacheSectionView() } } } @@ -183,3 +186,111 @@ private struct CompileContentBlockersSectionView: View { } } } + +private struct CorruptCacheSectionView: View { + enum CacheResult: Error, Identifiable { + case success + case noCache + case failedStandard + case failedAggressive + + var id: String { + return message + } + + var title: String { + switch self { + case .success: + return "Success" + case .noCache, .failedStandard, .failedAggressive: + return "Failure" + } + } + + var message: String { + switch self { + case .success: + return "Successfully corrupted adblock cache." + case .noCache: + return "Cache unavailable." + case .failedStandard: + return "Failed to corrupt standard cache." + case .failedAggressive: + return "Failed to corrupt aggressive cache." + } + } + } + @State private var corruptCacheResult: CacheResult? + + var body: some View { + Section { + Button(action: corruptAdblockDATCache) { + Text("Corrupt Adblock Engine DAT Caches") + } + } + .alert(item: $corruptCacheResult) { corruptCacheResult in + Alert( + title: Text(corruptCacheResult.title), + message: Text(corruptCacheResult.message), + dismissButton: .default(Text(Strings.OKString)) + ) + } + } + + private func corruptAdblockDATCache() { + Task { + guard + let folderURL = try? AsyncFileManager.default.url( + for: .cachesDirectory, + in: .userDomainMask + ) + else { + self.corruptCacheResult = .noCache + return + } + + let standardCacheFolderURL = + folderURL + .appendingPathComponent("engines", conformingTo: .folder) + .appendingPathComponent("standard", conformingTo: .folder) + if await !corruptListDATFile(in: standardCacheFolderURL) { + self.corruptCacheResult = .failedStandard + return + } + + let aggressiveCacheFolderURL = + folderURL + .appendingPathComponent("engines", conformingTo: .folder) + .appendingPathComponent("aggressive", conformingTo: .folder) + if await !corruptListDATFile(in: aggressiveCacheFolderURL) { + self.corruptCacheResult = .failedAggressive + return + } + return self.corruptCacheResult = .success + } + } + + private func corruptListDATFile(in directory: URL) async -> Bool { + guard await AsyncFileManager.default.fileExists(atPath: directory.path) else { + return false + } + let cachedDATFile = directory.appendingPathComponent("list.dat", conformingTo: .data) + guard await AsyncFileManager.default.fileExists(atPath: cachedDATFile.path) else { + // if file doesn't exist, we can't corrupt it. + return false + } + guard let content = await AsyncFileManager.default.contents(atPath: cachedDATFile.path), + var corruptedData = UUID().uuidString.data(using: .utf8) + else { + return false + } + // prefix UUID string to existing data to corrupt it + corruptedData.append(content) + // remove the cached DAT file + try? await AsyncFileManager.default.removeItem(atPath: cachedDATFile.path) + // 'corrupt' by replacing with corrupted data format + await AsyncFileManager.default.createFile(atPath: cachedDATFile.path, contents: corruptedData) + + return true + } +} diff --git a/ios/brave-ios/Sources/Brave/WebFilters/AdBlock/AdBlockEngineManager.swift b/ios/brave-ios/Sources/Brave/WebFilters/AdBlock/AdBlockEngineManager.swift index 2f347064264..c86c68a40b5 100644 --- a/ios/brave-ios/Sources/Brave/WebFilters/AdBlock/AdBlockEngineManager.swift +++ b/ios/brave-ios/Sources/Brave/WebFilters/AdBlock/AdBlockEngineManager.swift @@ -129,16 +129,95 @@ import os return compilableInfos != engine.group.infos } - /// Load the engine from cache so it can be ready during launch - func loadFromCache(resourcesInfo: GroupedAdBlockEngine.ResourcesInfo?) async -> Bool { - guard let createdCacheFolderURL else { return false } - do { - guard let cachedGroupInfo = await loadCachedInfo(cacheFolderURL: createdCacheFolderURL) else { - return false + /// Load the engine from cache so it can be ready during launch. First attempts to load from + /// cached DAT, if that fails we compile from the cached TXT file. + /// - parameter resourcesInfo: The `ResourcesInfo` model to use after compiling the engine. + /// - returns: success / failure status of loading the engine from cache. + @discardableResult func loadFromCache( + resourcesInfo: GroupedAdBlockEngine.ResourcesInfo? + ) async -> Bool { + guard let createdCacheFolderURL else { + // if the cached folder does not exist, the cache does not exist. + return false + } + let startFromDAT = ContinuousClock().now + // First try loading from cached DAT file (more performant that plaintext cache) + let cachedSerializedEngineURL = cachedEngineSerializedURL(in: createdCacheFolderURL) + if let cachedGroupInfoFromDAT = await loadCachedInfo( + cacheFolderURL: createdCacheFolderURL, + cachedEngineURL: cachedSerializedEngineURL + ), + let groupedEngine = await compileEngine( + cachedGroupInfo: cachedGroupInfoFromDAT, + resourcesInfo: resourcesInfo + ) + { + self.set(engine: groupedEngine, start: startFromDAT) + return true + } else { + // Failed to find or load cached DAT file, or failed to compile engine from cached DAT. + // Cleanup the cached DAT as we can't compile it. + try? await AsyncFileManager.default.removeItem(atPath: cachedSerializedEngineURL.path) + } + + switch self.engineType { + case .standard: + ContentBlockerManager.log.debug( + """ + Failed to load `\(self.cacheFolderName)` engine from DAT after (\(ContinuousClock().now.formatted(since: startFromDAT))). \ + Attempting to load from TXT. + """ + ) + case .aggressive: + ContentBlockerManager.log.debug( + """ + Failed to load `\(self.cacheFolderName)` engine from DAT after (\(ContinuousClock().now.formatted(since: startFromDAT))). + """ + ) + } + + // Next try loading from cached TXT file for standard engine. + if self.engineType == .standard { + // New timer to accurately measure load of cached txt & engine. + let startFromTXT = ContinuousClock().now + // Previously we never waited for the aggressive engines to be ready, + // only attempt early recompile for standard engine as this is slower + // that creating AdblockEngine from serialized DAT. + let cachedCombinedRulesURL = cachedCombinedRulesURL(in: createdCacheFolderURL) + if let cachedGroupInfoFromTXT = await loadCachedInfo( + cacheFolderURL: createdCacheFolderURL, + cachedEngineURL: cachedCombinedRulesURL + ), + let groupedEngine = await compileEngine( + cachedGroupInfo: cachedGroupInfoFromTXT, + resourcesInfo: resourcesInfo + ) + { + self.set(engine: groupedEngine, start: startFromTXT) + // Force caching of engine now that we've re-compiled it from TXT + await cache(engine: groupedEngine) + return true + } else { + // Failed to find or load cached TXT file, or failed to compile engine from cached TXT. + // Cleanup the cached combined rules TXT as we can't compile it. + try? await AsyncFileManager.default.removeItem(atPath: cachedCombinedRulesURL.path) } - let start = ContinuousClock().now - let engineType = self.engineType - let groupedEngine = try await Task.detached(priority: .high) { + } // else engine is aggressive, compile later + + // Failed to load or compile engine using both DAT and TXT. + return false + } + + /// Compiles a `GroupedAdBlockEngine` from the given cached `FilterListGroup`. + /// If successful, will setup engine to use `ResourcesInfo`. + /// If unsuccessful in compiling the engine, returns nil. + private func compileEngine( + cachedGroupInfo: GroupedAdBlockEngine.FilterListGroup, + resourcesInfo: GroupedAdBlockEngine.ResourcesInfo? + ) async -> GroupedAdBlockEngine? { + let engineType = self.engineType + do { + let groupedAdBlockEngine = try await Task.detached(priority: .high) { let engine = try GroupedAdBlockEngine.compile( group: cachedGroupInfo, type: engineType @@ -150,15 +229,12 @@ import os return engine }.value - - self.set(engine: groupedEngine, start: start) - return true + return groupedAdBlockEngine } catch { ContentBlockerManager.log.error( "Failed to load engine from cache for `\(self.cacheFolderName)`: \(String(describing: error))" ) - - return false + return nil } } @@ -400,8 +476,7 @@ import os // 4. Return a group containing info on this new file return GroupedAdBlockEngine.FilterListGroup( infos: compiledInfos, - localFileURL: fileURL, - fileType: .text + localFileURL: fileURL ) }.value } @@ -458,10 +533,21 @@ import os } } + /// Given the `cacheFolderURL` for the engine, returns the serialized engine DAT file url + private func cachedEngineSerializedURL(in cacheFolderURL: URL) -> URL { + cacheFolderURL.appendingPathComponent("list.dat", conformingTo: .data) + } + + /// Given the `cacheFolderURL` for the engine, returns the combined rules TXT file url + private func cachedCombinedRulesURL(in cacheFolderURL: URL) -> URL { + cacheFolderURL.appendingPathComponent("list.txt", conformingTo: .text) + } + + /// Loads & decode the `FilterListGroup` from the cache for this engine. nonisolated private func loadCachedInfo( - cacheFolderURL: URL + cacheFolderURL: URL, + cachedEngineURL: URL ) async -> GroupedAdBlockEngine.FilterListGroup? { - let cachedEngineURL = cacheFolderURL.appendingPathComponent("list.dat", conformingTo: .data) guard await AsyncFileManager.default.fileExists(atPath: cachedEngineURL.path) else { return nil } @@ -478,8 +564,7 @@ import os return GroupedAdBlockEngine.FilterListGroup( infos: cachedInfo.infos, - localFileURL: cachedEngineURL, - fileType: cachedInfo.fileType + localFileURL: cachedEngineURL ) } catch { ContentBlockerManager.log.error( diff --git a/ios/brave-ios/Sources/Brave/WebFilters/AdBlock/AdBlockGroupsManager.swift b/ios/brave-ios/Sources/Brave/WebFilters/AdBlock/AdBlockGroupsManager.swift index 7c3955a2145..72eb0caa7de 100644 --- a/ios/brave-ios/Sources/Brave/WebFilters/AdBlock/AdBlockGroupsManager.swift +++ b/ios/brave-ios/Sources/Brave/WebFilters/AdBlock/AdBlockGroupsManager.swift @@ -146,21 +146,7 @@ import os /// will load from legacy storage (using text files) if the dat file is unavailable. private func loadEngineFromCache(for engineType: GroupedAdBlockEngine.EngineType) async { let manager = getManager(for: engineType) - - if await !manager.loadFromCache(resourcesInfo: self.resourcesInfo) { - // This migration will add ~24s on an iPhone 8 (~8s on an iPhone 14) - // Even though its a one time thing, let's skip it. - // We never waited for the aggressive engines to be ready before anyways - guard engineType == .standard else { return } - for fileInfo in sourceProvider.legacyCacheFiles(for: engineType) { - manager.add(fileInfo: fileInfo) - } - - await manager.compileAvailableEnginesIfNeeded( - for: sourceProvider.enabledSources, - resourcesInfo: self.resourcesInfo - ) - } + await manager.loadFromCache(resourcesInfo: self.resourcesInfo) } /// Inform this manager of updates to the resources so our engines can be updated diff --git a/ios/brave-ios/Sources/Brave/WebFilters/AdBlock/GroupedAdBlockEngine.swift b/ios/brave-ios/Sources/Brave/WebFilters/AdBlock/GroupedAdBlockEngine.swift index ac05b41618c..62f7203a3c0 100644 --- a/ios/brave-ios/Sources/Brave/WebFilters/AdBlock/GroupedAdBlockEngine.swift +++ b/ios/brave-ios/Sources/Brave/WebFilters/AdBlock/GroupedAdBlockEngine.swift @@ -85,7 +85,13 @@ public actor GroupedAdBlockEngine { public struct FilterListGroup: Hashable, Equatable { let infos: [FilterListInfo] let localFileURL: URL - let fileType: GroupedAdBlockEngine.FileType + var fileType: GroupedAdBlockEngine.FileType { + if localFileURL.lastPathComponent.hasSuffix(".txt") { + return .text + } else { + return .data + } + } public func makeDebugDescription(for engineType: GroupedAdBlockEngine.EngineType) -> String { return infos.enumerated() diff --git a/ios/brave-ios/Tests/ClientTests/Web Filters/GroupedAdBlockEngineTests.swift b/ios/brave-ios/Tests/ClientTests/Web Filters/GroupedAdBlockEngineTests.swift index 1ef0ef2fd8b..9426f53a084 100644 --- a/ios/brave-ios/Tests/ClientTests/Web Filters/GroupedAdBlockEngineTests.swift +++ b/ios/brave-ios/Tests/ClientTests/Web Filters/GroupedAdBlockEngineTests.swift @@ -75,8 +75,7 @@ final class GroupedAdBlockEngineTests: XCTestCase { engine: engine!, group: GroupedAdBlockEngine.FilterListGroup( infos: [filterListInfo], - localFileURL: localFileURL, - fileType: .text + localFileURL: localFileURL ), type: .standard ) @@ -107,8 +106,7 @@ final class GroupedAdBlockEngineTests: XCTestCase { ) let group = GroupedAdBlockEngine.FilterListGroup( infos: [textFilterListInfo], - localFileURL: localFileURL, - fileType: .text + localFileURL: localFileURL ) let expectation = expectation(description: "Compiled engine resources") @@ -187,8 +185,7 @@ final class GroupedAdBlockEngineTests: XCTestCase { let group = GroupedAdBlockEngine.FilterListGroup( infos: [filterListInfo], - localFileURL: sampleFilterListURL, - fileType: .text + localFileURL: sampleFilterListURL ) do { diff --git a/ios/browser/api/brave_shields/adblock_engine.mm b/ios/browser/api/brave_shields/adblock_engine.mm index 0da73eb5fbd..dd0b5cb6ac7 100644 --- a/ios/browser/api/brave_shields/adblock_engine.mm +++ b/ios/browser/api/brave_shields/adblock_engine.mm @@ -91,7 +91,13 @@ class AdblockEngineBox final { if (error) { *error = [[self class] adblockErrorForKind:result.result_kind message:result.error_message]; + } else { + *error = [[self class] + adblockErrorForKind:adblock::ResultKind::AdblockError + message: + "Unknown error initializing engine with rules"]; } + return nil; } } } @@ -105,6 +111,7 @@ class AdblockEngineBox final { *error = [[self class] adblockErrorForKind:adblock::ResultKind::AdblockError message:"Failed to deserialize data"]; + return nil; } } }