Issue 3671 - Split out contentBlockerList API

abp2blocklist.js

+/*
+ * This file is part of Adblock Plus <https://adblockplus.org/>,
+ * Copyright (C) 2006-2016 Eyeo GmbH
+ *
+ * Adblock Plus is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * Adblock Plus is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
 "use strict";
 
 let readline = require("readline");
-
-// Hack to force `require("url")` to return our URL module instead of the
-// one included with Node.js! We manually load it here and then tweak the cache.
-// (After loading our code we must clear it again so that Node.js libraries will
-// be given the URL module that they expect.)
-require("url.js");
-require.cache["url"] = require.cache[require.resolve("url.js")];

[Sebastian Noack, 2016/02/21 20:34:44]

I'd rather rename our url module, here and in Adblock Plus, rather than going
with this hack.

[kzar, 2016/02/22 12:28:05]

OK I've renamed the module urlHelpers and removed the hack. (Open to suggestions
if you have a better idea for the name, I just made that up on the spot.)

[Sebastian Noack, 2016/02/22 17:35:27]

I might have got an even a better idea. How about splitting out url.geBaseDomain
(and url.isThirdParty), in the extension, into a separate module called tldjs
and renaming getBaseDomain to getDomain. That way we wouldn't need any
abstraction here at all. What do you think?

[kzar, 2016/02/22 18:09:28]

Sounds good, done.

-
 let Filter = require("filterClasses").Filter;
-let contentBlockerLists = require("./lib/contentBlockerLists.js");
-
-delete require.cache["url"];
+let ContentBlockerList = require("./lib/abp2blocklist.js").ContentBlockerList;
 
 var rl = readline.createInterface({input: process.stdin, terminal: false});
-var filters = [];
+var blockerList = new ContentBlockerList();
 
 rl.on("line", line =>
 {
-  if (line.charAt(0) != "[")

[Sebastian Noack, 2016/02/21 20:34:44]

What's about empty lines? It seems that we didn't handle those before either.
But wouldn't an empty line result result into a filter that would block
everything?

[kzar, 2016/02/22 12:28:05]

It appears that contentBlockerLists.js logic was handling that case somehow but
I can't see exactly why. Anyway I think you're right, better to check here
explicitly. Done.

[Sebastian Noack, 2016/02/22 17:35:27]

OK, I looked into it and figured that this case was already handled by following
check in the original code:

  if (filter instanceof filterClasses.RegExpFilter && !filter.regexpSource)
    return;

This check was meant to exclude raw regular expression filters. But it also
bails if the filter has been parsed from and empty string, in which case
regexpSource is "" rather than null.

I think that check should be changed to |filter.regexpSource == null|. While
empty lines should be explicitly ignored here.

Also what is if a line in a filter list contains only whitespace characters? I
didn't look into it yet, but we should make sure that we treat that case the
same as when parsing filter lists in Adblock Plus.

[kzar, 2016/02/22 18:09:28]

Done.

As for lines with only white-space, we're currently passing them to
Filter.fromText, which I guess is all that Adblock Plus does. I tried it out and
here's what a rule generated from a filter with a fitertext of "  " looks like:

{
  "trigger": {
    "url-filter": "^https?://.*  ",
    "resource-type": [
      "image",
      "style-sheet",
      "script",
      "font",
      "media",
      "raw",
      "document"
    ]
  },
  "action": {
    "type": "block"
  }
}

I'm not entirely sure if we want abp2blocklist to skip over lines with only
whitespace or treat them as any other line. I guess probably the former, as it's
more likely a mistake than anything else.

[Sebastian Noack, 2016/02/22 18:20:26]

Yeah, I already figured that myself. The point is we want abp2blocklist behave
exactly like Adblock Plus here. Can you try out what happens to empty lines and
lines who only have whitespace when Adblock Plus parses filter subscriptions?

[kzar, 2016/02/22 19:46:01]

OK add the custom subscription of http://static.kzar.co.uk/dave.txt and then run
this in the background console:
require("subscriptionClasses").Subscription.knownSubscriptions["http://static.kzar.co.uk/dave.txt"].filters.map(function(f)
{ return f.text; }).join("\n")

It turns out that the lines with just whitespace are stripped, so I've updated
abp2blocklist.js to do the same.

-    filters.push(Filter.fromText(line));
+  if (/^\s*[^\[\s]/.test(line))
+    blockerList.addFilter(Filter.fromText(line));
 });
 
 rl.on("close", () =>
 {
-  console.log(JSON.stringify(contentBlockerLists.convertFilters(filters),
+  console.log(JSON.stringify(blockerList.generateRules(),
                              null, "\t"));
 });