diff --git a/core/src/main/java/org/apache/stormcrawler/filtering/URLFilters.java b/core/src/main/java/org/apache/stormcrawler/filtering/URLFilters.java index 36e39c300..76c14a92e 100644 --- a/core/src/main/java/org/apache/stormcrawler/filtering/URLFilters.java +++ b/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); diff --git a/core/src/main/java/org/apache/stormcrawler/filtering/regex/FastURLFilter.java b/core/src/main/java/org/apache/stormcrawler/filtering/regex/FastURLFilter.java index aff263f4a..1a832c723 100644 --- a/core/src/main/java/org/apache/stormcrawler/filtering/regex/FastURLFilter.java +++ b/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(); diff --git a/core/src/test/java/org/apache/stormcrawler/filtering/URLFiltersExceptionTest.java b/core/src/test/java/org/apache/stormcrawler/filtering/URLFiltersExceptionTest.java new file mode 100644 index 000000000..16c3b1a3d --- /dev/null +++ b/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 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 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()); + } +} diff --git a/core/src/test/resources/urlfilters-throwing.json b/core/src/test/resources/urlfilters-throwing.json new file mode 100644 index 000000000..767a07ead --- /dev/null +++ b/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" + } + ] +}