URLFilters: treat an exception from a filter as a rejection

core/src/main/java/org/apache/stormcrawler/filtering/URLFilters.java

@@ -26,6 +26,7 @@
 import java.net.URL;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.atomic.AtomicLong;
 import org.apache.commons.cli.CommandLine;
 import org.apache.commons.cli.CommandLineParser;
 import org.apache.commons.cli.DefaultParser;
@@ -56,10 +57,20 @@ public class URLFilters extends URLFilter implements JSONResource {
 
     private URLFilter[] filters;
 
+    private final AtomicLong filteredOut = new AtomicLong();
+
     private URLFilters() {
         filters = new URLFilters[0];
     }
 
+    /**
+     * Number of URLs rejected because a filter in the chain threw an exception. A rejected URL is
+     * the safe verdict: a chain which throws must not widen what the crawl accepts.
+     */
+    public long getExceptionsCount() {
+        return filteredOut.get();
+    }
+
     /**
      * Loads the filters from a JSON configuration file.
      *
@@ -113,18 +124,23 @@ public void loadJSONResources(InputStream inputStream)
             @Nullable Metadata sourceMetadata,
             @NotNull String urlToFilter) {
         String normalizedUrl = urlToFilter;
-        try {
-            for (URLFilter filter : filters) {
-                long start = System.currentTimeMillis();
+        for (URLFilter filter : filters) {
+            long start = System.currentTimeMillis();
+            try {
                 normalizedUrl = filter.filter(sourceUrl, sourceMetadata, normalizedUrl);
-                long end = System.currentTimeMillis();
-                LOG.debug("URLFilter {} took {} msec", filter.getClass().getName(), end - start);
-                if (normalizedUrl == null) {
-                    break;
-                }
+            } catch (Exception e) {
+                // a filter which throws must not disable the filters after it:
+                // treat the URL as rejected, the same verdict a broken chain
+                // must not be allowed to widen
+                LOG.error("URL filter {} threw exception", filter.getClass().getName(), e);
+                filteredOut.incrementAndGet();
+                return null;
+            }
+            long end = System.currentTimeMillis();
+            LOG.debug("URLFilter {} took {} msec", filter.getClass().getName(), end - start);
+            if (normalizedUrl == null) {
+                break;
             }
-        } catch (Exception e) {
-            LOG.error("URL filtering threw exception", e);
         }
         return normalizedUrl;
     }
@@ -189,25 +205,28 @@ public static void main(String[] args) throws ParseException {
         try {
             URLFilters filters = new URLFilters(conf, configFile);
             String normalizedUrl = inputUrl;
-            try {
-                for (URLFilter filter : filters.filters) {
-                    long start = System.currentTimeMillis();
+            for (URLFilter filter : filters.filters) {
+                long start = System.currentTimeMillis();
+                try {
                     normalizedUrl =
                             filter.filter(URLUtil.toURL(sourceUrl), new Metadata(), normalizedUrl);
-                    long end = System.currentTimeMillis();
-                    System.out.println(
-                            "\t["
-                                    + filter.getClass().getName()
-                                    + "] "
-                                    + (end - start)
-                                    + "msec => "
-                                    + normalizedUrl);
-                    if (normalizedUrl == null) {
-                        break;
-                    }
+                } catch (Exception e) {
+                    LOG.error("URL filter {} threw exception", filter.getClass().getName(), e);
+                    System.err.println(
+                            "\t[" + filter.getClass().getName() + "] threw " + e + " => rejected");
+                    normalizedUrl = null;
+                }
+                long end = System.currentTimeMillis();
+                System.out.println(
+                        "\t["
+                                + filter.getClass().getName()
+                                + "] "
+                                + (end - start)
+                                + "msec => "
+                                + normalizedUrl);
+                if (normalizedUrl == null) {
+                    break;
                 }
-            } catch (Exception e) {
-                LOG.error("URL filtering threw exception", e);
             }
         } catch (IOException e) {
             LOG.error("Failed to initialize URLFilters", e);

core/src/main/java/org/apache/stormcrawler/filtering/regex/FastURLFilter.java

@@ -348,7 +348,10 @@ public Rule(String line) {
             }
             // no match?
             if (type == null) {
-                return;
+                // a rule without a type would throw when evaluated; fail at
+                // load time instead, where the misconfiguration belongs
+                throw new IllegalArgumentException(
+                        "FastURLFilter rule does not start with a known type: " + line);
             }
 
             String patternString = line.substring(offset).trim();

core/src/test/java/org/apache/stormcrawler/filtering/URLFiltersExceptionTest.java

@@ -0,0 +1,76 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.stormcrawler.filtering;
+
+import java.io.IOException;
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.util.HashMap;
+import java.util.Map;
+import org.apache.stormcrawler.Metadata;
+import org.apache.stormcrawler.util.URLUtil;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+/** Behaviour of the filter chain when one of its filters throws. */
+class URLFiltersExceptionTest {
+
+    /** Stands in for any filter that throws at evaluation time. */
+    public static class ThrowingURLFilter extends URLFilter {
+        @Override
+        public @Nullable String filter(
+                @Nullable URL sourceUrl,
+                @Nullable Metadata sourceMetadata,
+                @NotNull String urlToFilter) {
+            throw new NullPointerException("filter blew up");
+        }
+    }
+
+    /** Stands in for an exclusion rule placed after it, such as the private-range regexes. */
+    public static class RejectEverythingURLFilter extends URLFilter {
+        @Override
+        public @Nullable String filter(
+                @Nullable URL sourceUrl,
+                @Nullable Metadata sourceMetadata,
+                @NotNull String urlToFilter) {
+            return null;
+        }
+    }
+
+    @Test
+    void urlIsRejectedWhenAnEarlierFilterThrows() throws IOException, MalformedURLException {
+        Map<String, Object> conf = new HashMap<>();
+        URLFilters filters = new URLFilters(conf, "urlfilters-throwing.json");
+        URL source = URLUtil.toURL("http://www.example.com/index.html");
+        Assertions.assertNull(
+                filters.filter(source, new Metadata(), "http://www.example.com/outlink.html"),
+                "a filter that throws must reject the URL, not let it through");
+        Assertions.assertEquals(1, filters.getExceptionsCount());
+    }
+
+    @Test
+    void rejectionShortensTheChain() throws IOException, MalformedURLException {
+        Map<String, Object> conf = new HashMap<>();
+        URLFilters filters = new URLFilters(conf, "urlfilters-throwing.json");
+        URL source = URLUtil.toURL("http://www.example.com/index.html");
+        Assertions.assertNull(filters.filter(source, new Metadata(), "http://www.example.com/"));
+        Assertions.assertEquals(1, filters.getExceptionsCount());
+    }
+}

core/src/test/resources/urlfilters-throwing.json

@@ -0,0 +1,12 @@
+{
+	"org.apache.stormcrawler.filtering.URLFilters": [
+		{
+			"class": "org.apache.stormcrawler.filtering.URLFiltersExceptionTest$ThrowingURLFilter",
+			"name": "ThrowingURLFilter"
+		},
+		{
+			"class": "org.apache.stormcrawler.filtering.URLFiltersExceptionTest$RejectEverythingURLFilter",
+			"name": "RejectEverythingURLFilter"
+		}
+	]
+}