Opened on 03/01/2019 at 10:58:32 AM

Closed on 03/02/2019 at 12:43:08 PM

Last modified on 07/25/2019 at 07:42:07 PM

#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):

Description (last modified by mjethani)


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 (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.

Attachments (0)

Change History (8)

comment:1 Changed on 03/01/2019 at 10:58:45 AM by mjethani

  • Blocking 7000 added

comment:2 Changed on 03/01/2019 at 11:00:30 AM by mjethani

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

comment:3 Changed on 03/01/2019 at 11:03:56 AM by mjethani

  • Description modified (diff)

comment:4 Changed on 03/02/2019 at 09:30:12 AM by abpbot

comment:5 Changed on 03/02/2019 at 12:43:08 PM by mjethani

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

comment:6 Changed on 03/02/2019 at 12:43:26 PM by mjethani

  • Priority changed from Unknown to P2
  • Ready set

comment:7 Changed on 03/07/2019 at 11:31:08 AM by mjethani

  • Component changed from Unknown to Core

comment:8 Changed on 07/25/2019 at 07:42:07 PM by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Done. Working as described.

Microsoft Edge 44.17763.1.0 / Windows 10 1809

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

Add Comment

Modify Ticket

Change Properties
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from mjethani.
Note: See TracTickets for help on using tickets.