From d037fa8806c42c9f0f14575ebeb36fa6353dc57e Mon Sep 17 00:00:00 2001 From: abhinav-phi Date: Sun, 6 Sep 2026 04:32:37 +0530 Subject: [PATCH] SiteMapParserBolt: strict parsing, opt-in sniffing, non-terminal parse failure (#2083) - the parser is now built in strict mode (sitemap.strict, default true): crawler-commons then discards URLs a sitemap lists on hosts other than its own, so a sitemap cannot enrol URLs on hosts it has nothing to do with, and an HTML page mentioning the sitemap namespace is not parsed leniently into half a sitemap - content sniffing moves behind sitemap.sniffContent (default false, like feed.sniffContent for feeds): a page carrying the namespace string in its first bytes was reclassified as a sitemap, never reached the parser bolt and was never indexed. The key the existing test already set but the bolt ignored is now honoured; a content type which rules a sitemap out (a page served as HTML) stops the sniffing - a document marked as a sitemap through persisted metadata whose body does not parse is emitted as FETCH_ERROR with the isSitemap key dropped, instead of a terminal ERROR: with the archetype's fetchInterval.error of -1, an ERROR removed the URL from the crawl for good, letting whoever controls the content decide what stays in the corpus. On its next fetch the document goes to the parser bolt like any other page The image and all-extensions test sitemaps listed their first URL on www.example.com, which strict mode now correctly excludes; that entry moved under the sitemap's own host. --- .../stormcrawler/bolt/SiteMapParserBolt.java | 70 +++++++--- core/src/main/resources/crawler-default.yaml | 14 ++ .../bolt/SiteMapParserBoltCrossHostTest.java | 128 ++++++++++++++++++ .../stormcrawler.sitemap.extensions.all.xml | 2 +- .../stormcrawler.sitemap.extensions.image.xml | 2 +- 5 files changed, 198 insertions(+), 18 deletions(-) create mode 100644 core/src/test/java/org/apache/stormcrawler/bolt/SiteMapParserBoltCrossHostTest.java diff --git a/core/src/main/java/org/apache/stormcrawler/bolt/SiteMapParserBolt.java b/core/src/main/java/org/apache/stormcrawler/bolt/SiteMapParserBolt.java index 66bea2afe..b53f909b8 100644 --- a/core/src/main/java/org/apache/stormcrawler/bolt/SiteMapParserBolt.java +++ b/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 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 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) { diff --git a/core/src/main/resources/crawler-default.yaml b/core/src/main/resources/crawler-default.yaml index 6945b2c4c..9f90cd1be 100644 --- a/core/src/main/resources/crawler-default.yaml +++ b/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 diff --git a/core/src/test/java/org/apache/stormcrawler/bolt/SiteMapParserBoltCrossHostTest.java b/core/src/test/java/org/apache/stormcrawler/bolt/SiteMapParserBoltCrossHostTest.java new file mode 100644 index 000000000..2ba5bc5da --- /dev/null +++ b/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( + "" + + "" + + "https://a.example/own-page" + + "https://b.example/other-page" + + ""), + metadata); + List> emitted = output.getEmitted(Constants.StatusStreamName); + for (List 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( + "" + + "sitemaps" + + "" + + "https://b.example/other-page" + + ""), + 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("hello"), metadata); + List> emitted = output.getEmitted(Constants.StatusStreamName); + Assertions.assertFalse(emitted.isEmpty()); + for (List 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( + "" + + "" + + "https://b.example/other-page" + + ""), + 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"); + } +} diff --git a/core/src/test/resources/stormcrawler.sitemap.extensions.all.xml b/core/src/test/resources/stormcrawler.sitemap.extensions.all.xml index f832f8fe6..8eed072fc 100644 --- a/core/src/test/resources/stormcrawler.sitemap.extensions.all.xml +++ b/core/src/test/resources/stormcrawler.sitemap.extensions.all.xml @@ -30,7 +30,7 @@ under the License. - http://www.example.com/ + https://stormcrawler.apache.org/with-image.html 2012-12-05T10:59:04+00:00 monthly 1.00 diff --git a/core/src/test/resources/stormcrawler.sitemap.extensions.image.xml b/core/src/test/resources/stormcrawler.sitemap.extensions.image.xml index ceb5bf5e2..45a4572ee 100644 --- a/core/src/test/resources/stormcrawler.sitemap.extensions.image.xml +++ b/core/src/test/resources/stormcrawler.sitemap.extensions.image.xml @@ -26,7 +26,7 @@ under the License. - http://www.example.com/ + https://stormcrawler.apache.org/with-image.html 2012-12-05T10:59:04+00:00 monthly 1.00