From 1d7190ea01b09290277f8a70ddd32bd2360ebc73 Mon Sep 17 00:00:00 2001 From: abhinav-phi Date: Sun, 6 Sep 2026 04:01:59 +0530 Subject: [PATCH] URLFilters: treat an exception from a filter as a rejection (#2084) The try/catch wrapped the whole chain: when a filter threw, the catch logged and fell through to the value the last successful filter had produced, so callers saw the URL as accepted and every filter after the one that threw was skipped - including the regex exclusions placed last in the archetype chain. The try/catch now sits inside the loop, logs which filter threw and returns null: a chain which throws must not widen what the crawl accepts. main() applies the same verdict, and a counter records how often it happens. FastURLFilter.Rule no longer accepts a rule line without a recognised type; it failed at evaluation time with an NPE and now fails at load time instead. --- .../stormcrawler/filtering/URLFilters.java | 71 ++++++++++------- .../filtering/regex/FastURLFilter.java | 5 +- .../filtering/URLFiltersExceptionTest.java | 76 +++++++++++++++++++ .../test/resources/urlfilters-throwing.json | 12 +++ 4 files changed, 137 insertions(+), 27 deletions(-) create mode 100644 core/src/test/java/org/apache/stormcrawler/filtering/URLFiltersExceptionTest.java create mode 100644 core/src/test/resources/urlfilters-throwing.json 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" + } + ] +}