Fix network protection controller leak

Core/ContentBlockerStoreConstants.swift

@@ -22,5 +22,6 @@ import Foundation
 public struct ContentBlockerStoreConstants {
 
     public static let groupName = "\(Global.groupIdPrefix).contentblocker"
+    public static let configurationGroupName = "\(Global.groupIdPrefix).app-configuration"
 
 }

Core/DailyPixel.swift

@@ -43,7 +43,7 @@ public final class DailyPixel {
 
     }
 
-    private static let storage: UserDefaults = UserDefaults(suiteName: Constant.dailyPixelStorageIdentifier)!
+    public static let storage: UserDefaults = UserDefaults(suiteName: Constant.dailyPixelStorageIdentifier)!
 
     /// Sends a given Pixel once per day.
     /// This is useful in situations where pixels receive spikes in volume, as the daily pixel can be used to determine how many users are actually affected.

Core/EtagStorage.swift

@@ -31,7 +31,7 @@ public protocol BlockerListETagStorage {
 
 public struct UserDefaultsETagStorage: BlockerListETagStorage {
 
-    private let defaults = UserDefaults(suiteName: "com.duckduckgo.blocker-list.etags")
+    private let defaults = UserDefaults(suiteName: "\(Global.groupIdPrefix).app-configuration")
 
     public init() { }
 

Core/FileStore.swift

@@ -22,16 +22,30 @@ import Configuration
 
 public class FileStore {
 
-    private let groupIdentifier: String = ContentBlockerStoreConstants.groupName
+    private let groupIdentifier: String = ContentBlockerStoreConstants.configurationGroupName
 
     public init() { }
 
     public func persist(_ data: Data, for configuration: Configuration) throws {
-        do {
-            try data.write(to: persistenceLocation(for: configuration), options: .atomic)
-        } catch {
-            Pixel.fire(pixel: .fileStoreWriteFailed, error: error, withAdditionalParameters: ["config": configuration.rawValue])
-            throw error
+        let file = persistenceLocation(for: configuration)
+        var coordinatorError: NSError?
+        var writeError: Error?
+
+        NSFileCoordinator().coordinate(writingItemAt: file, options: .forReplacing, error: &coordinatorError) { fileUrl in
+            do {
+                try data.write(to: fileUrl, options: .atomic)
+            } catch {
+                Pixel.fire(pixel: .fileStoreWriteFailed, error: error, withAdditionalParameters: ["config": configuration.rawValue])
+                writeError = error
+            }
+        }
+
+        if let writeError {
+            throw writeError
+        }
+        if let coordinatorError {
+            Pixel.fire(pixel: .fileStoreCoordinatorFailed, error: coordinatorError, withAdditionalParameters: ["config": configuration.rawValue])
+            throw coordinatorError
         }
     }
 
@@ -55,22 +69,33 @@ public class FileStore {
     }
 
     public func loadAsData(for configuration: Configuration) -> Data? {
-        do {
-            return try Data(contentsOf: persistenceLocation(for: configuration))
-        } catch {
-            let nserror = error as NSError
-            if nserror.domain != NSCocoaErrorDomain || nserror.code != NSFileReadNoSuchFileError {
-                Pixel.fire(pixel: .trackerDataCouldNotBeLoaded, error: error)
+        let file = persistenceLocation(for: configuration)
+        var data: Data?
+        var coordinatorError: NSError?
+
+        NSFileCoordinator().coordinate(readingItemAt: file, error: &coordinatorError) { fileUrl in
+            do {
+                data = try Data(contentsOf: fileUrl)
+            } catch {
+                let nserror = error as NSError
+                if nserror.domain != NSCocoaErrorDomain || nserror.code != NSFileReadNoSuchFileError {
+                    Pixel.fire(pixel: .trackerDataCouldNotBeLoaded, error: error)
+                }
             }
-            return nil
         }
+
+        if let coordinatorError {
+            Pixel.fire(pixel: .fileStoreCoordinatorFailed, error: coordinatorError, withAdditionalParameters: ["config": configuration.rawValue])
+        }
+
+        return data
     }
 
     func hasData(for configuration: Configuration) -> Bool {
         FileManager.default.fileExists(atPath: persistenceLocation(for: configuration).path)
     }
 
-    func persistenceLocation(for configuration: Configuration) -> URL {
+    public func persistenceLocation(for configuration: Configuration) -> URL {
         let path = FileManager.default.containerURL(forSecurityApplicationGroupIdentifier: groupIdentifier)
         return path!.appendingPathComponent(configuration.storeKey)
     }

Core/PixelEvent.swift

@@ -449,6 +449,9 @@ extension Pixel {
         case networkProtectionVPNConfigurationRemoved
         case networkProtectionVPNConfigurationRemovalFailed
 
+        case networkProtectionConfigurationInvalidPayload(configuration: Configuration)
+        case networkProtectionConfigurationPixelTest
+
         // MARK: remote messaging pixels
 
         case remoteMessageShown
@@ -482,6 +485,7 @@ extension Pixel {
         case trackerDataReloadFailed
         case trackerDataCouldNotBeLoaded
         case fileStoreWriteFailed
+        case fileStoreCoordinatorFailed
         case privacyConfigurationReloadFailed
         case privacyConfigurationParseFailed
         case privacyConfigurationCouldNotBeLoaded
@@ -1234,6 +1238,9 @@ extension Pixel.Event {
         case .networkProtectionVPNConfigurationRemoved: return "m_netp_vpn_configuration_removed"
         case .networkProtectionVPNConfigurationRemovalFailed: return "m_netp_vpn_configuration_removal_failed"
 
+        case .networkProtectionConfigurationInvalidPayload(let config): return "m_netp_vpn_configuration_\(config.rawValue)_invalid_payload"
+        case .networkProtectionConfigurationPixelTest: return "m_netp_vpn_configuration_pixel_test"
+
             // MARK: remote messaging pixels
 
         case .remoteMessageShown: return "m_remote_message_shown"
@@ -1269,6 +1276,7 @@ extension Pixel.Event {
         case .trackerDataReloadFailed: return "m_d_tds_r"
         case .trackerDataCouldNotBeLoaded: return "m_d_tds_l"
         case .fileStoreWriteFailed: return "m_d_fswf"
+        case .fileStoreCoordinatorFailed: return "m_d_configuration_file_coordinator_error"
         case .privacyConfigurationReloadFailed: return "m_d_pc_r"
         case .privacyConfigurationParseFailed: return "m_d_pc_p"
         case .privacyConfigurationCouldNotBeLoaded: return "m_d_pc_l"

Core/UserDefaults+NetworkProtection.swift

@@ -27,6 +27,14 @@ public extension UserDefaults {
         }
         return defaults
     }
+
+    static var configurationGroupDefaults: UserDefaults {
+        let suiteName = ContentBlockerStoreConstants.configurationGroupName
+        guard let defaults = UserDefaults(suiteName: suiteName) else {
+            fatalError("Failed to create configuration UserDefaults")
+        }
+        return defaults
+    }
 }
 
 public enum NetworkProtectionUserDefaultKeys {