Skip to content

Handle malformed content-blocking domain metadata#662

Open
KirtiRamchandani wants to merge 1 commit into
brave:masterfrom
KirtiRamchandani:fix/content-blocking-domain-panics
Open

Handle malformed content-blocking domain metadata#662
KirtiRamchandani wants to merge 1 commit into
brave:masterfrom
KirtiRamchandani:fix/content-blocking-domain-panics

Conversation

@KirtiRamchandani

Copy link
Copy Markdown

Problem

  • Issue Review all potential panics in the entire codebase. #432 tracks panic cleanup in library code. One content-blocking conversion path still used unwrap() while rebuilding domain= options from raw_line.
  • If a NetworkFilter has domain metadata but an inconsistent or malformed debug raw_line, conversion can unwind even though TryFrom<NetworkFilter> already returns Result.

Root cause

  • The content-blocking conversion assumed opt_domains/opt_not_domains always implied a $domain= option in raw_line.
  • It also unwrapped IDNA normalization for non-ASCII domains while building Safari content-blocking domain lists.

Solution

  • Return the existing FromNotSupported error when domain metadata cannot be reconstructed from raw_line.
  • Return the existing RuleContainsNonASCII error when IDNA normalization fails.
  • Add a regression test for domain metadata with an unparseable options section.

Tests run

  • cargo fmt --all -- --check
  • cargo check -p adblock
  • cargo check -p adblock --features content-blocking
  • cargo test -p adblock --features content-blocking domain_metadata_without_parseable_options_is_unsupported -- --nocapture
  • cargo test -p adblock --features content-blocking content_blocking -- --nocapture
  • cargo test -p adblock --lib --features content-blocking
  • cargo test -p adblock --lib
  • cargo clippy -p adblock --features content-blocking -- -D warnings
  • git diff --check

Related: #432

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant