brave / brave-ios

Brave iOS Browser
https://brave.com
Mozilla Public License 2.0
1.7k stars 440 forks source link

Use list_catalog.json file in iOS #8729

Closed cuba closed 9 months ago

cuba commented 9 months ago

Summary of Changes

This pull request fixes https://github.com/brave/brave-browser/issues/35210

MUST Be merged in response to https://github.com/brave/brave-core/pull/21883

This actually has a number of benefits:

  1. All lists are now filter lists. so no additional code if we add new hidden lists such as first party list or iOS only lists
  2. Will very likely use this mechanism for any launch only lists too to increase launch performance (but this is not yet done and no such list exists yet)
  3. Brings us one step closer to how desktop functions.

Submitter Checklist:

Test Plan:

Screenshots:

Reviewer Checklist:

github-actions[bot] commented 9 months ago

[puLL-Merge] - brave/brave-ios@8729

Description

This pull request introduces a set of updates primarily focused on improving the ContentBlockerManager logging, the handling of filter lists, and integrating signpost IDs for performance logging. It includes the refinement of data models and the implementation of guard checks across various functionalities to ensure a smoother and more reliable runtime behavior.

Changes ### Changes #### BVC+WKNavigationDelegate.swift - **Addition of signpost IDs and logging:** - Added signpost ID generation and performance logging for various methods within `decidePolicyFor` navigation actions. - This allows for better performance measurement and debugging. - **Code cleanup and minor refactorings:** - Removed unnecessary whitespace and improved code readability. #### LaunchHelper.swift - **Enhanced logging during the launch phase:** - Introduced debug logging for loading blocking data and loaded blocking launch data for better insight during startup. #### UserScriptType.swift - **Syntax correction:** - Fixed a missing closing parenthesis in the `gpc` case's debug description. #### FilterListsView.swift - **UI adjustments for filter lists:** - Modified the filter list view to hide certain filter lists based on their properties, improving UI clarity. #### ContentBlockerManager.swift - **Logging and performance measurement:** - Introduced signpost ID usage for the cleanup of invalid rule lists, aiding in performance analysis. #### CustomFilterListStorage.swift - **Extension for enabled sources:** - Added a new extension to provide source representations for all enabled custom filter lists. #### FilterList.swift - **Data model adjustments:** - Refinements in the filter list structure, including default behavior adjustments and removing redundant code. #### FilterListCustomURLDownloader.swift - **Downloader adjustments:** - Removed unnecessary do-catch block and improved structure for fetching resources.

Security Hotspots

  1. FilterListResourceDownloader.swift:

    • Risk: The method compileFilterListEngineIfNeeded directly checks for file existence which could potentially introduce race conditions if files are modified or removed between the check and usage - Mitigation: Ensure atomicity or use more robust mechanisms to handle resource integrity.
  2. CachedAdBlockEngine.swift:

    • Risk: Direct usage of FileManager to check for file existence within cosmeticFilterModel. Similar to above, this introduces possible race conditions - Mitigation: Implement safer file handling practices.
  3. CustomFilterListStorage.swift (enabledSources extension):

    • Risk: Usage of compactMap on filter lists with optional safety checks may skip error handling for unexpected nil values, leading to skipped filter lists without notice - Mitigation: Consider explicit handling or logging of nil cases to ensure all expected filter lists are loaded without silently failing.
cuba commented 9 months ago

moved to brave-core