Opened on 06/16/2017 at 05:17:40 PM
Closed on 07/13/2017 at 09:57:13 AM
Last modified on 08/02/2017 at 06:30:08 AM
#5325 closed change (fixed)
[abp2blocklist] Add proper support for separator characters
Reported by: | mjethani | Assignee: | mjethani |
---|---|---|---|
Priority: | P4 | Milestone: | |
Module: | Platform | Keywords: | abp2blocklist, Acceptable Ads |
Cc: | kzar, sebastian, mario | Blocked By: | |
Blocking: | #5464 | Platform: | Unknown / Cross platform |
Ready: | yes | Confidential: | no |
Tester: | Unknown | Verified working: | no |
Review URL(s): |
Description (last modified by mjethani)
Background
Currently abp2blocklist will convert example.co^ into example\.co and example.co^hello into example\.co.hello. This will match example.co as well as example.com, which is wrong. Based on the filters documentation, the correct patterns to generate are example\.co([^-_.%A-Za-z0-9].*)?$ and example\.co[^-_.%A-Za-z0-9]hello respectively.
What to change
Modify the parseFilterRegexpSource function to generate the correct pattern.
Attachments (0)
Change History (12)
comment:2 Changed on 06/16/2017 at 05:22:36 PM by mjethani
- Cc kzar sebastian mario added
comment:4 Changed on 07/07/2017 at 12:33:04 PM by kzar
comment:5 Changed on 07/08/2017 at 06:06:19 AM by mjethani
The . must not be escaped inside square brackets.
comment:6 Changed on 07/08/2017 at 06:07:26 AM by mjethani
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:7 Changed on 07/08/2017 at 06:09:21 AM by mjethani
- Description modified (diff)
I found that we can deal with the - after all. I've updated the description accordingly.
comment:9 Changed on 07/11/2017 at 04:16:56 PM by kzar
- Priority changed from Unknown to P4
- Ready set
comment:10 Changed on 07/13/2017 at 09:55:01 AM by abpbot
A commit referencing this issue has landed:
Issue 5325 - Add support for separator characters
comment:11 Changed on 07/13/2017 at 09:57:13 AM by mjethani
- Resolution set to fixed
- Status changed from reviewing to closed
comment:12 Changed on 08/02/2017 at 06:30:08 AM by mjethani
- Blocking 5464 added
I think those regexp examples might be wrong, doesn't the . need to be escaped? E.g. example\.co([^\.%A-Za-z0-9_].*)?$