Fix possible compilation loop in Content Blocking (#90)

* Prevent compilation loops in case current TDS is same as fallback oneand error occurs

* Remove unneeded var

* Fix linter warning

Sources/BrowserServicesKit/ContentBlocking/ContentBlockerRulesSourceManager.swift

@@ -71,6 +71,14 @@ public class ContentBlockerRulesSourceModel: ContentBlockerRulesSourceIdentifier
  Manages sources that are used to compile Content Blocking Rules, handles possible broken state by filtering out sources that are potentially corrupted.
  */
 public class ContentBlockerRulesSourceManager {
+
+    public class RulesSourceBreakageInfo {
+
+        public internal(set) var tdsIdentifier: String?
+        public internal(set) var tempListIdentifier: String?
+        public internal(set) var allowListIdentifier: String?
+        public internal(set) var unprotectedSitesIdentifier: String?
+    }
 
     /**
      Data source for all of the exception info used during compilation.
@@ -82,7 +90,7 @@ public class ContentBlockerRulesSourceManager {
     /**
      Identifiers of sources that have caused compilation process to fail.
      */
-    public private(set) var brokenSources: ContentBlockerRulesSourceIdentifiers?
+    public private(set) var brokenSources: RulesSourceBreakageInfo?
     public private(set) var fallbackTDSFailure = false
 
     private let errorReporting: EventMapping<ContentBlockerDebugEvents>?
@@ -156,33 +164,48 @@ public class ContentBlockerRulesSourceManager {
 
         return result
     }
-
+    
     /**
      Process information about last failed compilation in order to update `brokenSources` state.
      */
     func compilationFailed(for input: ContentBlockerRulesSourceIdentifiers, with error: Error) {
+        guard let brokenSources = brokenSources else {
+            let brokenSources = RulesSourceBreakageInfo()
+            self.brokenSources = brokenSources
+            compilationFailed(for: input, with: error, brokenSources: brokenSources)
+            return
+        }
+
+        compilationFailed(for: input, with: error, brokenSources: brokenSources)
+    }
+
+    /**
+     Process information about last failed compilation in order to update `brokenSources` state.
+     */
+    private func compilationFailed(for input: ContentBlockerRulesSourceIdentifiers,
+                                   with error: Error,
+                                   brokenSources: RulesSourceBreakageInfo) {
         if input.tdsIdentifier != rulesList.fallbackTrackerData.etag {
             // We failed compilation for non-embedded TDS, marking it as broken.
-            brokenSources = ContentBlockerRulesSourceIdentifiers(name: rulesList.name,
-                                                                 tdsIdentfier: input.tdsIdentifier)
+            brokenSources.tdsIdentifier = input.tdsIdentifier
             errorReporting?.fire(.contentBlockingTDSCompilationFailed,
                                  scope: input.name,
                                  error: error,
                                  parameters: [ContentBlockerDebugEvents.Parameters.etag: input.tdsIdentifier])
         } else if input.tempListIdentifier != nil {
-            brokenSources?.tempListIdentifier = input.tempListIdentifier
+            brokenSources.tempListIdentifier = input.tempListIdentifier
             errorReporting?.fire(.contentBlockingTempListCompilationFailed,
                                  scope: input.name,
                                  error: error,
                                  parameters: [ContentBlockerDebugEvents.Parameters.etag: input.tempListIdentifier ?? "empty"])
         } else if input.allowListIdentifier != nil {
-            brokenSources?.allowListIdentifier = input.allowListIdentifier
+            brokenSources.allowListIdentifier = input.allowListIdentifier
             errorReporting?.fire(.contentBlockingAllowListCompilationFailed,
                                  scope: input.name,
                                  error: error,
                                  parameters: [ContentBlockerDebugEvents.Parameters.etag: input.allowListIdentifier ?? "empty"])
         } else if input.unprotectedSitesIdentifier != nil {
-            brokenSources?.unprotectedSitesIdentifier = input.unprotectedSitesIdentifier
+            brokenSources.unprotectedSitesIdentifier = input.unprotectedSitesIdentifier
             errorReporting?.fire(.contentBlockingUnpSitesCompilationFailed,
                                  scope: input.name,
                                  error: error)

Tests/BrowserServicesKitTests/ContentBlocker/ContentBlockerRulesManagerTests.swift

@@ -264,7 +264,7 @@ class ContentBlockerRulesManagerLoadingTests: ContentBlockerRulesManagerTests {
 
                 errorExp.fulfill()
             default:
-                XCTFail()
+                XCTFail("Unexpected event received: \(event)")
             }
         }
 
@@ -601,6 +601,82 @@ class ContentBlockerRulesManagerLoadingTests: ContentBlockerRulesManagerTests {
                                                      allowListEtag: nil,
                                                      unprotectedSitesHash: nil))
     }
+
+    func test_CurrentTDSEqualToFallbackTDS_ValidTempList_ValidAllowList_BrokenUnprotectedSites() {
+
+        let mockRulesSource = MockSimpleContentBlockerRulesListsSource(trackerData: Self.fakeEmbeddedDataSet,
+                                                                       embeddedTrackerData: Self.fakeEmbeddedDataSet)
+        let mockExceptionsSource = MockContentBlockerRulesExceptionsSource()
+        mockExceptionsSource.tempListEtag = Self.makeEtag()
+        mockExceptionsSource.tempList = validTempSites
+        mockExceptionsSource.allowListEtag = Self.makeEtag()
+        mockExceptionsSource.allowList = validAllowList
+        mockExceptionsSource.unprotectedSites = ["broken site Ltd. . 😉.com"]
+
+        XCTAssertEqual(mockRulesSource.trackerData?.etag, mockRulesSource.embeddedTrackerData.etag)
+
+        let exp = expectation(description: "Rules Compiled")
+        rulesUpdateListener.onRulesUpdated = { _ in
+            exp.fulfill()
+        }
+
+        let errorExp = expectation(description: "Error reported")
+        errorExp.expectedFulfillmentCount = 4
+        var errorEvents = [ContentBlockerDebugEvents]()
+        let errorHandler = EventMapping<ContentBlockerDebugEvents>.init { event, scope, _, params, _ in
+            switch event {
+            case .contentBlockingTempListCompilationFailed,
+                    .contentBlockingAllowListCompilationFailed,
+                    .contentBlockingUnpSitesCompilationFailed:
+                XCTAssertEqual(scope, DefaultContentBlockerRulesListsSource.Constants.trackerDataSetRulesListName)
+                errorEvents.append(event)
+                errorExp.fulfill()
+            case .contentBlockingCompilationTime:
+                XCTAssertNil(scope)
+                XCTAssertNotNil(params?["compilationTime"])
+
+                errorExp.fulfill()
+            default:
+                XCTFail("Unexpected \(event)")
+            }
+        }
+
+        let cbrm = ContentBlockerRulesManager(rulesSource: mockRulesSource,
+                                              exceptionsSource: mockExceptionsSource,
+                                              updateListener: rulesUpdateListener,
+                                              errorReporting: errorHandler,
+                                              logger: .disabled)
+
+        wait(for: [exp, errorExp], timeout: 15.0)
+
+        XCTAssertEqual(Set(errorEvents), Set([.contentBlockingTempListCompilationFailed,
+                                              .contentBlockingAllowListCompilationFailed,
+                                              .contentBlockingUnpSitesCompilationFailed]))
+
+        XCTAssertNotNil(cbrm.currentRules.first?.etag)
+        XCTAssertEqual(cbrm.currentRules.first?.etag, mockRulesSource.embeddedTrackerData.etag)
+
+        XCTAssertNil(cbrm.sourceManagers[mockRulesSource.rukeListName]?.brokenSources?.tdsIdentifier)
+
+        XCTAssertNotNil(cbrm.sourceManagers[mockRulesSource.rukeListName]?.brokenSources?.tempListIdentifier)
+        XCTAssertEqual(cbrm.sourceManagers[mockRulesSource.rukeListName]?.brokenSources?.tempListIdentifier,
+                       mockExceptionsSource.tempListEtag)
+
+        XCTAssertNotNil(cbrm.sourceManagers[mockRulesSource.rukeListName]?.brokenSources?.allowListIdentifier)
+        XCTAssertEqual(cbrm.sourceManagers[mockRulesSource.rukeListName]?.brokenSources?.allowListIdentifier,
+                       mockExceptionsSource.allowListEtag)
+
+        XCTAssertNotNil(cbrm.sourceManagers[mockRulesSource.rukeListName]?.brokenSources?.unprotectedSitesIdentifier)
+        XCTAssertEqual(cbrm.sourceManagers[mockRulesSource.rukeListName]?.brokenSources?.unprotectedSitesIdentifier,
+                       mockExceptionsSource.unprotectedSitesHash)
+
+        XCTAssertEqual(cbrm.currentRules.first?.identifier,
+                       ContentBlockerRulesIdentifier(name: DefaultContentBlockerRulesListsSource.Constants.trackerDataSetRulesListName,
+                                                     tdsEtag: mockRulesSource.embeddedTrackerData.etag,
+                                                     tempListEtag: nil,
+                                                     allowListEtag: nil,
+                                                     unprotectedSitesHash: nil))
+    }
 }
 
 class ContentBlockerRulesManagerUpdatingTests: ContentBlockerRulesManagerTests {