Opened 9 months ago

Closed 9 months ago

Last modified 4 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: Ross Verified working: yes
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 (8)

comment:1 Changed 9 months ago by mjethani

  • Blocking 7000 added

comment:2 Changed 9 months ago by mjethani

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

comment:3 Changed 9 months ago by mjethani

  • Description modified (diff)

comment:4 Changed 9 months ago by abpbot

comment:5 Changed 9 months ago by mjethani

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

comment:6 Changed 9 months ago by mjethani

  • Priority changed from Unknown to P2
  • Ready set

comment:7 Changed 9 months ago by mjethani

  • Component changed from Unknown to Core

comment:8 Changed 4 months ago by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Done. Working as described.

ABP 0.9.15.2340
Microsoft Edge 44.17763.1.0 / Windows 10 1809

ABP 3.5.2.2340
Chrome 49.0.2623.75 / Windows 10 1809
Chrome 75.0.3770.142 / Windows 10 1809
Opera 36.0.2130.65 / Windows 10 1809
Opera 62.0.3331.72 / Windows 10 1809
Firefox 51.0 / Windows 10 1809
Firefox 68.0 / Windows 10 1809
Firefox Mobile 68.0 / Android 7.2.2

Note: See TracTickets for help on using tickets.