Opened on 10/13/2018 at 08:54:11 PM
Closed on 10/15/2018 at 11:36:29 PM
Last modified on 10/17/2018 at 02:51:17 PM
#7043 closed change (fixed)
Accept whitelist filters with blank CSPs
| Reported by: | mjethani | Assignee: | mjethani |
|---|---|---|---|
| Priority: | P1 | Milestone: | |
| Module: | Core | Keywords: | |
| Cc: | jsonesen, kzar, amrmak | Blocked By: | |
| Blocking: | #7047 | Platform: | Unknown / Cross platform |
| Ready: | yes | Confidential: | no |
| Tester: | Ross | Verified working: | yes |
| Review URL(s): | |||
Description (last modified by mjethani)
Background
In #6871 we stopped accepting filters with blank CSPs. But there are such filters in the anti-circumvention list, they just happen to be whitelist filters. In the current version development build of Adblock Plus these filters are being considered invalid because of the change for #6871.
It makes sense to allow blank CSPs in whitelist filters, because the value itself has no meaning (the fact that the filter matches the URL and has the $csp option is enough for whitelisting the URL).
What to change
In lib/filterClasses.js reject blank CSPs only if it is a blocking filter, not if it is whitelist filter.
Hints for testers
Make sure that the behavior expected in #6871 still works for blocking filters.
For whitelist filters, after this change, the $csp option should be able to have a blank value. In order to verify this, write two filters ||example.com^$csp=script-src 'none' and @@||example.com^$csp and see that inline scripts are still allowed on the page. You could use the following HTML to test this:
<!DOCTYPE html>
<h1>Test</h1>
<script>
console.log("Hello from inline script");
</script>
If the second filter is being accepted as valid, then the page should print Hello from inline script to the console. Also verify that removing the second filter then prevents the inline script from executing.
Attachments (0)
Change History (13)
comment:1 Changed on 10/13/2018 at 08:55:06 PM by mjethani
comment:2 Changed on 10/13/2018 at 08:55:32 PM by mjethani
- Cc kzar added
comment:3 Changed on 10/15/2018 at 09:27:24 AM by amrmak
- Cc amrmak added
comment:4 Changed on 10/15/2018 at 02:04:11 PM by mjethani
- Priority changed from P2 to P1
Actually, this is a regression since the last release, it should be a P1.
Jon, are you available to work on the patch? If not, I can look into it.
comment:5 Changed on 10/15/2018 at 04:32:52 PM by jsonesen
I can do it, thanks for assigning it I'll get right to it today
comment:7 Changed on 10/15/2018 at 09:59:03 PM by mjethani
- Owner changed from jsonesen to mjethani
comment:8 Changed on 10/15/2018 at 10:21:21 PM by mjethani
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:9 Changed on 10/15/2018 at 11:27:49 PM by abpbot
A commit referencing this issue has landed:
Issue 7043 - Accept whitelist filters with blank CSPs
comment:10 Changed on 10/15/2018 at 11:36:29 PM by mjethani
- Description modified (diff)
- Resolution set to fixed
- Status changed from reviewing to closed
comment:11 Changed on 10/15/2018 at 11:42:03 PM by mjethani
- Blocking 7047 added
comment:12 Changed on 10/16/2018 at 11:01:51 AM by abpbot
A commit referencing this issue has landed:
Issue 7043 - Accept whitelist filters with blank CSPs
comment:13 Changed on 10/17/2018 at 02:51:17 PM by Ross
- Tester changed from Unknown to Ross
- Verified working set
Fixed. Whitelist filters can accept a blank CSP.
ABP 3.3.2.2175
Firefox 62 / 51 / Windows 10
Chrome 69 / 49 / Windows 10
Opera 56 / 36 / Windows 10
Jon, I have assigned this to you.