Opened 3 months ago

Closed 3 months ago

Last modified 2 months ago

#7321 closed change (fixed)

Lower-case non-literal patterns for case-insensitive matching

Reported by: mjethani Assignee: mjethani
Priority: P2 Milestone:
Module: Core Keywords:
Cc: Blocked By:
Blocking: #7000 Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/30021563/

Description (last modified by mjethani)

Background

In #7052 and #7209 we stopped using regular expressions for literal patterns. As part of this, we started converting case-insensitive literal patterns to lower case, then comparing such patterns with the lower-case version of the request URL.

ticket:7318#comment:1 (Sebastian) suggested that we might benefit from lower-casing the pattern for non-literal patterns as well (those that are eventually converted to RegExp objects). In my tests, a case-sensitive RegExp object is indeed significantly faster.

What to change

In the RegExpFilter constructor in lib/filterClasses.js, convert the pattern to lower case unless the $match-case flag is set. In matchesLocation(), lower-case the request URL first thing unless the $match-case flag is set.

Hints for testers

See that regular expression filters and filters with patterns containing special characters (esp. somewhere in the middle) work correctly, with and without the $match-case option. The filters /FoO/ and FoO^BaR should both block the URL https://example.com/foo/bar (each one on its own, not together). If the $match-case option is added to these filters, they should not block the URL. If the $~match-case option is added instead, they should block the URL.

Change History (7)

comment:1 Changed 3 months ago by mjethani

  • Blocking 7000 added

comment:2 Changed 3 months ago by mjethani

  • Owner set to mjethani
  • Review URL(s) modified (diff)

comment:3 Changed 3 months ago by mjethani

  • Description modified (diff)

comment:4 Changed 3 months ago by abpbot

comment:5 Changed 3 months ago by mjethani

  • Description modified (diff)
  • Resolution set to fixed
  • Status changed from new to closed

comment:6 Changed 3 months ago by mjethani

  • Priority changed from Unknown to P2
  • Ready set

comment:7 Changed 2 months ago by mjethani

  • Component changed from Unknown to Core
Note: See TracTickets for help on using tickets.