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)
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.
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: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.
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
A commit referencing this issue has landed:
Issue 7321 - Lower-case non-literal patterns for case-insensitive matching