Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#4450 closed change (fixed)

Prevent ElemHide filters from being created with an empty domain restriction

Reported by: kzar Assignee: kzar
Priority: Unknown Milestone:
Module: Core Keywords:
Cc: trev, sebastian Blocked By: #4501
Blocking: #524 Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29356018/

Description (last modified by kzar)

Background

Element hiding filters can optionally be restricted to a domain, for example example.com##selector. They can even be restricted to multiple domains for example example.com,anotherexample.com##selector.

Currently filters can be restricted to an empty string domain too, empty domains are simply ignored. Meaning there can be multiple different filters which apply the same selector to all domains unconditionally, which makes the elemHide logic and data structures more complex.

What to change

Modify lib/filterClasses.js so that if an element hiding filter is created with any empty string domain restrictions it results in an InvalidFilter instance. Some examples of bad filters that should now result in InvalidFilter:

  • ,##selector
  • ,,,##selector
  • ~,example.com##selector

Simplify lib/elemHide.js so that filterKeysBySelector becomes filterKeyBySelector.

Change History (7)

comment:1 Changed 2 years ago by kzar

I'm not sure about the best approach to take here since we currently don't parse the domains until they are accessed. Perhaps we should modify the ActiveFilter constructor to check for empty domains?

comment:2 Changed 2 years ago by kzar

  • Owner set to kzar

comment:3 Changed 2 years ago by kzar

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:4 Changed 2 years ago by abpbot

A commit referencing this issue has landed:
Issue 4450 - Prevent ElemHide filters with empty domains

comment:5 Changed 2 years ago by kzar

  • Description modified (diff)
  • Ready set
  • Resolution set to fixed
  • Status changed from reviewing to closed
  • Summary changed from Prevent filters from being created with an empty domain restriction to Prevent ElemHide filters from being created with an empty domain restriction

comment:6 Changed 2 years ago by trev

  • Blocking 524 added

comment:7 Changed 2 years ago by trev

  • Blocked By 4501 added
Note: See TracTickets for help on using tickets.