Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: lib/abp2blocklist.js

Issue 29467585: Issue 4327 - Prevent filters with no hostname from blocking documents (Closed) Base URL: https://hg.adblockplus.org/abp2blocklist
Patch Set: Add comments Created July 8, 2017, 11:28 a.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | test/abp2blocklist.js » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: lib/abp2blocklist.js
===================================================================
--- a/lib/abp2blocklist.js
+++ b/lib/abp2blocklist.js
@@ -271,20 +271,32 @@
parseDomains(filter.domains, included, excluded);
if (exceptionDomains)
excluded = excluded.concat(exceptionDomains);
if (withResourceTypes)
{
- trigger["resource-type"] = getResourceTypes(filter);
+ let resourceTypes = getResourceTypes(filter);
- if (trigger["resource-type"].length == 0)
+ // Content blocker rules can't differentiate between sub-document requests
+ // (iframes) and top-level document requests. To avoid too many false
+ // positives, we prevent rules with no hostname part from blocking document
+ // requests.
+ //
+ // Once Safari 11 becomes our minimum supported version, we could change
+ // our approach here to use the new "unless-top-url" property instead.
+ if (filter instanceof filterClasses.BlockingFilter && !parsed.hostname)
+ resourceTypes = resourceTypes.filter(type => type != "document");
+
+ if (resourceTypes.length == 0)
return;
+
+ trigger["resource-type"] = resourceTypes;
}
if (filter.thirdParty != null)
trigger["load-type"] = [filter.thirdParty ? "third-party" : "first-party"];
if (included.length > 0)
{
trigger["if-domain"] = [];
@@ -310,18 +322,25 @@
}
}
}
else if (excluded.length > 0)
{
trigger["unless-domain"] = excluded.map(name => "*" + name);
}
else if (filter instanceof filterClasses.BlockingFilter &&
- filter.contentType & typeMap.SUBDOCUMENT)
+ filter.contentType & typeMap.SUBDOCUMENT && parsed.hostname)
{
+ // Rules with a hostname part are still allowed to block document requests,
+ // but we add an exception for top-level documents.
+ //
+ // Note that we can only do this if there's no "unless-domain" property for
+ // now. This also only works in Safari 11 onwards, while older versions
+ // simply ignore this property. Once Safari 11 becomes our minimum
+ // supported version, we can merge "unless-domain" into "unless-top-url".
trigger["unless-top-url"] = [trigger["url-filter"]];
if (trigger["url-filter-is-case-sensitive"])
trigger["top-url-filter-is-case-sensitive"] = true;
}
rules.push({trigger: trigger, action: {type: action}});
}
« no previous file with comments | « no previous file | test/abp2blocklist.js » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld