Issue 5549 - Implement missing error handlings for custom filter lists

background.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present 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
@@ skipping 53 matching lines @@
         if (parts.length == 2 && parts[0] in data)
           data[parts[0]] = decodeURIComponent(parts[1]);
       }
     }
   }
 
   let params = {
     blockedURLs: "",
     filterlistsReinitialized: false,
     addSubscription: false,
-    filterError: false,
     downloadStatus: "synchronize_ok",
     showNotificationUI: false,
     showPageOptions: false
   };
   updateFromURL(params);
 
   let modules = {};
   window.require = function(module)
   {
     return modules[module];
@@ skipping 221 matching lines @@
           }
         }
       }
     }
   };
 
   function Filter(text)
   {
     this.text = text;
     this.disabled = false;
-  }
+    if (Filter.elemhideRegExp.test(text))
+      this.selector = RegExp.$3;
+  }
+  Filter.elemhideRegExp = /^([^/*|@"!]*?)#([@?])?#(.+)$/;
   Filter.fromText = (text) => new Filter(text);
 
   function BlockingFilter()
   {
   }
 
   function RegExpFilter()
   {
   }
   RegExpFilter.typeMap = Object.create(null);
 
   modules.filterClasses = {
     BlockingFilter,
     Filter,
     RegExpFilter
   };
 
+  const isValidCSSSelector = selector =>
+  {
+    if (!selector)
+      return true;
+    try
+    {
+      document.documentElement.matches(selector);
+      return true;
+    }
+    catch (error)
+    {
+      return false;
+    }
+  };
+
   modules.filterValidation =
   {
+    // to test failing filters
+    // use one or more bad CSS selectors
+    // or start the line with a [
     parseFilter(text)
     {
-      if (params.filterError)
-        return {error: "Invalid filter"};
-      return {filter: modules.filterClasses.Filter.fromText(text)};
+      let filter = null;
+      if (text)
+      {
+        if (text[0] == "[")
+          return {error: {reason: "Unexpected filter list header"}};
+
+        filter = modules.filterClasses.Filter.fromText(text);
+
+        if (!isValidCSSSelector(filter.selector))
+        {
+          return {error: {reason: "Invalid CSS selector"}};
+        }
+      }
+
+      return {filter};
     },
     parseFilters(text)
     {
-      if (params.filterError)

[saroyanm, 2018/01/29 11:44:48]

Suggestion: copy pasting with the slight modification of current method ->
https://hg.adblockplus.org/adblockpluschrome/file/tip/lib/filterValidation.js...


Can help us making better test, ex.: Testing 5+ and less than 5 lines.

[a.giammarchi, 2018/01/29 17:36:32]

not sure I understand this one. What do you mean?
what would be the best way for doing that? the amount of errors through the
queryString?

[saroyanm, 2018/01/30 12:03:44]

We will try to use part of filterValidation in order to validate the filters and
we might get rid of "params.filterError".

-        return {errors: [{reason: "Invalid filter", lineno: 1}]};
-      return {
-        filters: text.split("\n")
-          .filter((filter) => !!filter)
-          .map(modules.filterClasses.Filter.fromText),
-        errors: []
-      };
+      let lines = text.split("\n");
+      let filters = [];
+      let errors = [];
+
+      for (let i = 0; i < lines.length; i++)
+      {
+        let {filter, error} = this.parseFilter(lines[i]);
+
+        if (filter)
+          filters.push(filter);
+
+        if (error)
+        {
+          error.lineno = i + 1;
+          errors.push(error);
+        }
+      }
+
+      return {filters, errors};
     }
   };
 
   modules.synchronizer = {
     Synchronizer: {
       _downloading: false,
       execute(subscription, manual)
       {
         modules.synchronizer.Synchronizer._downloading = true;
         modules.filterNotifier.FilterNotifier.emit(
@@ skipping 250 matching lines @@
       },
       filter: {
         text: "||example.com/some-annoying-popup$popup",
         whitelisted: false,
         userDefined: true,
         subscription: null
       }
     });
   });
 }());