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