SiteMapParserBolt: strict parsing, opt-in sniffing, non-terminal parse failure

core/src/main/java/org/apache/stormcrawler/bolt/SiteMapParserBolt.java

@@ -86,6 +86,21 @@ public class SiteMapParserBolt extends StatusEmitterBolt {
 
     private int maxOffsetGuess = 300;
 
+    /**
+     * Whether a document without the {@code isSitemap} key is classified as a sitemap by searching
+     * the first bytes for the sitemaps.org namespace. Any page that carries the namespace string
+     * early enough is reclassified as a sitemap and never reaches the parser bolt, so this defaults
+     * to false, like {@code feed.sniffContent} does for feeds.
+     */
+    private boolean sniffContent = false;
+
+    /**
+     * Whether the parser rejects documents which are not well formed sitemaps. Strict parsing keeps
+     * an ordinary HTML page that mentions the sitemap namespace from being parsed leniently into
+     * half a sitemap.
+     */
+    private boolean strict = true;
+
     private Consumer<Number> averagedMetrics;
 
     /** Delay in minutes used for scheduling sub-sitemaps. */
@@ -103,22 +118,19 @@ public void execute(Tuple tuple) {
 
         LOG.debug("Processing {}", url);
 
-        boolean looksLikeSitemap = sniff(content);
-        // can force the mimetype as we know it is XML
-        if (looksLikeSitemap) {
+        String isSitemap = metadata.getFirstValue(isSitemapKey);
+
+        // only sniff when the operator asked for it: a page deciding how the
+        // pipeline treats it must not depend on a string in its body, and a
+        // sniffed document also needs a sitemap compatible content type
+        if (isSitemap == null && sniffContent && sniffsAsSitemap(ct, content)) {
+            LOG.info("{} detected as sitemap based on content and content type", url);
             ct = "application/xml";
+            isSitemap = "true";
         }
 
-        String isSitemap = metadata.getFirstValue(isSitemapKey);
-
         boolean treatAsSitemap = Boolean.parseBoolean(isSitemap);
 
-        // doesn't have the key and want to rely on the clue
-        if (isSitemap == null && looksLikeSitemap) {
-            LOG.info("{} detected as sitemap based on content", url);
-            treatAsSitemap = true;
-        }
-
         // decided that it is not a sitemap file
         if (!treatAsSitemap) {
             LOG.debug("Not a sitemap {}", url);
@@ -140,12 +152,21 @@ public void execute(Tuple tuple) {
             // exception while parsing the sitemap
             String errorMessage = "Exception while parsing " + url + ": " + e;
             LOG.error(errorMessage);
-            // send to status stream in case another component wants to update
-            // its status
+            /*
+             * A document which does not parse as a sitemap is most likely an
+             * ordinary page whose persisted metadata carried isSitemap=true.
+             * Dropping the marking and emitting it as FETCH_ERROR keeps it
+             * schedulable: a terminal ERROR would remove it from the crawl for
+             * good when fetchInterval.error is negative, which lets whoever
+             * controls the content remove URLs from the corpus. The document
+             * goes on to the parser bolt on its next fetch, like any other
+             * page.
+             */
+            metadata.remove(isSitemapKey);
             metadata.setValue(Constants.STATUS_ERROR_SOURCE, "sitemap parsing");
             metadata.setValue(Constants.STATUS_ERROR_MESSAGE, errorMessage);
             collector.emit(
-                    Constants.StatusStreamName, tuple, new Values(url, metadata, Status.ERROR));
+                    Constants.StatusStreamName, tuple, new Values(url, metadata, Status.FETCH_ERROR));
             collector.ack(tuple);
             return;
         }
@@ -335,7 +356,9 @@ public void parseExtensionAttributes(SiteMapURL url, Metadata metadata) {
     public void prepare(
             Map<String, Object> stormConf, TopologyContext context, OutputCollector collector) {
         super.prepare(stormConf, context, collector);
-        parser = new SiteMapParser(false);
+        strict = ConfUtils.getBoolean(stormConf, "sitemap.strict", true);
+        parser = new SiteMapParser(strict);
+        sniffContent = ConfUtils.getBoolean(stormConf, "sitemap.sniffContent", false);
         filterHoursSinceModified =
                 ConfUtils.getInt(stormConf, "sitemap.filter.hours.since.modified", -1);
         parseFilters = ParseFilters.fromConf(stormConf);
@@ -364,8 +387,23 @@ public void declareOutputFields(OutputFieldsDeclarer declarer) {
 
     /**
      * Examines the first bytes of the content for a clue of whether this document is a sitemap,
-     * based on namespaces. Works for XML and non-compressed documents only.
+     * based on namespaces. Works for XML and non-compressed documents only. Used only when
+     * {@code sitemap.sniffContent} is enabled. A content type which rules a sitemap out (a page
+     * served as HTML) stops the sniffing; an absent or generic one lets it proceed, since the
+     * parser guesses the type of the document anyway.
      */
+    private boolean sniffsAsSitemap(String contentType, byte[] content) {
+        if (StringUtils.isNotBlank(contentType)) {
+            String ctLower = contentType.toLowerCase(Locale.ROOT);
+            if (!ctLower.contains("xml")
+                    && !ctLower.contains("text/plain")
+                    && !ctLower.contains("octet-stream")) {
+                return false;
+            }
+        }
+        return sniff(content);
+    }
+
     private boolean sniff(byte[] content) {
         byte[] beginning = content;
         if (content.length > maxOffsetGuess && maxOffsetGuess > 0) {

core/src/main/resources/crawler-default.yaml

@@ -275,6 +275,20 @@ config:
   # filters URLs in sitemaps based on their modified Date (if any)
   sitemap.filter.hours.since.modified: -1
 
+  # whether a document without the isSitemap key is classified as a sitemap
+  # by searching the first bytes of its content for the sitemaps.org
+  # namespace. Off by default: any page carrying the namespace string early
+  # enough would be reclassified as a sitemap and never reach the parser
+  # bolt. When enabled, a content type which rules a sitemap out (a page
+  # served as HTML) stops the sniffing.
+  sitemap.sniffContent: false
+
+  # whether the sitemap parser rejects documents which are not well formed
+  # sitemaps. Strict parsing also discards URLs a sitemap lists on hosts
+  # other than its own, and keeps an ordinary HTML page which mentions the
+  # sitemap namespace from being parsed leniently into half a sitemap.
+  sitemap.strict: true
+
   # staggered scheduling of sitemaps
   sitemap.schedule.delay: -1
 

core/src/test/java/org/apache/stormcrawler/bolt/SiteMapParserBoltCrossHostTest.java

@@ -0,0 +1,128 @@
+/*
+ * 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.bolt;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.List;
+import org.apache.stormcrawler.Constants;
+import org.apache.stormcrawler.Metadata;
+import org.apache.stormcrawler.parse.ParsingTester;
+import org.apache.stormcrawler.persistence.Status;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+/**
+ * A page decides how the pipeline treats it only through its own metadata: content sniffing must
+ * not promote an ordinary HTML page to a sitemap, a sitemap must not enrol URLs on other hosts,
+ * and a sitemap marking that no longer parses must not make the URL unschedulable.
+ */
+class SiteMapParserBoltCrossHostTest extends ParsingTester {
+
+    @BeforeEach
+    void setupParserBolt() {
+        bolt = new SiteMapParserBolt();
+        setupParserBolt(bolt);
+    }
+
+    private static byte[] xml(String body) {
+        return body.getBytes(StandardCharsets.UTF_8);
+    }
+
+    /** A sitemap may only list URLs below its own location. */
+    @Test
+    void crossSubmittedUrlsAreNotDiscovered() throws IOException {
+        prepareParserBolt("test.parsefilters.json");
+        Metadata metadata = new Metadata();
+        metadata.setValue(SiteMapParserBolt.isSitemapKey, "true");
+        parse(
+                "https://a.example/sitemap.xml",
+                xml(
+                        "<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
+                                + "<urlset xmlns=\"http://www.sitemaps.org/schemas/sitemap/0.9\">"
+                                + "<url><loc>https://a.example/own-page</loc></url>"
+                                + "<url><loc>https://b.example/other-page</loc></url>"
+                                + "</urlset>"),
+                metadata);
+        List<List<Object>> emitted = output.getEmitted(Constants.StatusStreamName);
+        for (List<Object> t : emitted) {
+            Assertions.assertFalse(
+                    t.get(0).toString().startsWith("https://b.example/"),
+                    "discovered a URL on another host: " + t.get(0));
+        }
+    }
+
+    /** Content sniffing must not promote an ordinary HTML page to a sitemap. */
+    @Test
+    void htmlMentioningTheSitemapNamespaceIsNotASitemap() throws IOException {
+        prepareParserBolt("test.parsefilters.json");
+        Metadata metadata = new Metadata();
+        parse(
+                "https://a.example/page.html",
+                xml(
+                        "<html><body><a href=\"http://www.sitemaps.org/schemas/sitemap/0.9\">"
+                                + "sitemaps</a>"
+                                + "<urlset xmlns=\"http://www.sitemaps.org/schemas/sitemap/0.9\">"
+                                + "<url><loc>https://b.example/other-page</loc></url></urlset>"
+                                + "</body></html>"),
+                metadata);
+        Assertions.assertEquals(
+                "false",
+                metadata.getFirstValue(SiteMapParserBolt.isSitemapKey),
+                "HTML page classified as a sitemap");
+    }
+
+    /** A page carrying isSitemap=true that does not parse must stay fetchable. */
+    @Test
+    void unparseableSitemapIsNotTerminalError() throws IOException {
+        prepareParserBolt("test.parsefilters.json");
+        Metadata metadata = new Metadata();
+        metadata.setValue(SiteMapParserBolt.isSitemapKey, "true");
+        parse("https://a.example/page.html", xml("<html><body>hello</body></html>"), metadata);
+        List<List<Object>> emitted = output.getEmitted(Constants.StatusStreamName);
+        Assertions.assertFalse(emitted.isEmpty());
+        for (List<Object> t : emitted) {
+            if (t.get(0).toString().equals("https://a.example/page.html")) {
+                Assertions.assertNotEquals(Status.ERROR, t.get(2), "emitted as ERROR: " + t.get(0));
+                Assertions.assertEquals(Status.FETCH_ERROR, t.get(2));
+            }
+        }
+    }
+
+    /** With sniffing enabled, an XML content type and the namespace still need to agree. */
+    @Test
+    void sniffingRequiresSitemapCompatibleContentType() throws IOException {
+        prepareParserBolt("test.parsefilters.json");
+        Metadata metadata = new Metadata();
+        metadata.setValue("Content-Type".toLowerCase(), "text/html");
+        parse(
+                "https://a.example/page.html",
+                xml(
+                        "<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
+                                + "<urlset xmlns=\"http://www.sitemaps.org/schemas/sitemap/0.9\">"
+                                + "<url><loc>https://b.example/other-page</loc></url>"
+                                + "</urlset>"),
+                metadata);
+        // the document was passed on to the parser bolt, not consumed as a sitemap
+        Assertions.assertEquals(
+                "false",
+                metadata.getFirstValue(SiteMapParserBolt.isSitemapKey),
+                "HTML content type sniffed into a sitemap");
+    }
+}

core/src/test/resources/stormcrawler.sitemap.extensions.all.xml

@@ -30,7 +30,7 @@ under the License.
 <!-- created with Free Online Sitemap Generator www.xml-sitemaps.com -->
 
     <url>
-        <loc>http://www.example.com/</loc>
+        <loc>https://stormcrawler.apache.org/with-image.html</loc>
         <lastmod>2012-12-05T10:59:04+00:00</lastmod>
         <changefreq>monthly</changefreq>
         <priority>1.00</priority>

core/src/test/resources/stormcrawler.sitemap.extensions.image.xml

@@ -26,7 +26,7 @@ under the License.
 <!-- created with Free Online Sitemap Generator www.xml-sitemaps.com -->
 
     <url>
-        <loc>http://www.example.com/</loc>
+        <loc>https://stormcrawler.apache.org/with-image.html</loc>
         <lastmod>2012-12-05T10:59:04+00:00</lastmod>
         <changefreq>monthly</changefreq>
         <priority>1.00</priority>