SiteMapParserBolt sniffs every document, parses sitemaps non-strict, and turns a failed sitemap parse into a terminal ERROR

[rzo1]

What happens

execute() looks for the sitemaps.org namespace in the first sitemap.offset.guess bytes of every document it receives and, if it finds it, treats the document as a sitemap. There is no content type test and no way to switch the sniffing off; the sibling FeedParserBolt has feed.sniffContent, defaulting to false, but SiteMapParserBolt has no equivalent and ignores the sitemap.sniffContent key that SiteMapParserBoltTest sets. The parser is built as new SiteMapParser(false), which turns off the crawler-commons cross submission check, so a sitemap on one host may list URLs on any other host and they are emitted DISCOVERED. Sitemap index children are emitted with isSitemap=true, which is in the default metadata.persist list, and if such a URL later turns out not to be a sitemap the bolt emits Status.ERROR.

Where

core/src/main/java/org/apache/stormcrawler/bolt/SiteMapParserBolt.java:106-120 (sniffing), :338 (parser construction), :137-151 (error emission), :232-240 (isSitemap=true on index children). Config keys: sitemap.offset.guess, sitemap.discovery, metadata.persist, and fetchInterval.error, which the archetype sets to -1 (archetype/src/main/resources/archetype-resources/crawler-conf.yaml:106).

        boolean looksLikeSitemap = sniff(content);
        // can force the mimetype as we know it is XML
        if (looksLikeSitemap) {
            ct = "application/xml";
        }

        parser = new SiteMapParser(false);

Why it matters

A crawled page decides how the pipeline treats it. Any HTML page that contains the namespace string early enough is reclassified as a sitemap, is never passed to the parser bolt, and is therefore never indexed. A sitemap can enrol URLs on hosts it has nothing to do with, and those entries skip parser.emitOutlinks.max.per.page and the robots meta tags that apply on the HTML path. Because isSitemap is persisted, the classification sticks: when such a URL is fetched and does not parse as a sitemap it becomes Status.ERROR, and with the archetype's fetchInterval.error: -1 it is never scheduled again. In an open crawl that lets a third party remove other people's URLs from the corpus; in a scoped crawl it is limited to hosts already in scope. internals.adoc:141 describes the bolt as parsing tuples that carry isSitemap=true, which does not mention the sniffing path.

Reproduction

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

/*
 * 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;

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);
        for (List<Object> t : emitted) {
            Assertions.assertNotEquals(Status.ERROR, t.get(2), "emitted as ERROR: " + t.get(0));
        }
    }
}

Run it:

mvn -pl core test -Dtest=SiteMapParserBoltCrossHostTest

All three tests assert the intended behaviour and fail on main.

[ERROR] Tests run: 3, Failures: 3, Errors: 0, Skipped: 0, Time elapsed: 0.745 s <<< FAILURE! -- in org.apache.stormcrawler.bolt.SiteMapParserBoltCrossHostTest
[ERROR]   SiteMapParserBoltCrossHostTest.crossSubmittedUrlsAreNotDiscovered:60 discovered a URL on another host: https://b.example/other-page ==> expected: <false> but was: <true>
[ERROR]   SiteMapParserBoltCrossHostTest.htmlMentioningTheSitemapNamespaceIsNotASitemap:80 HTML page classified as a sitemap ==> expected: <false> but was: <true>
[ERROR]   SiteMapParserBoltCrossHostTest.unparseableSitemapIsNotTerminalError:95 emitted as ERROR: https://a.example/page.html ==> expected: not equal but was: <ERROR>

The third case is driven by crawler-commons, which rejects an HTML body outright:

ERROR org.apache.stormcrawler.bolt.SiteMapParserBolt - Exception while parsing https://a.example/page.html: crawlercommons.sitemaps.UnknownFormatException: Failed to detect MediaType of sitemap 'https://a.example/page.html'

Suggested fix

Construct the parser with strict=true in SiteMapParserBolt.prepare and add a config key for operators who need the old behaviour. Add sitemap.sniffContent, defaulting to false, and read it in prepare; when sniffing is on, require a sitemap compatible content type as well as the namespace clue. In execute, on a parse failure of a document that was only marked as a sitemap through persisted metadata, drop the isSitemap key and emit Status.FETCHED or FETCH_ERROR rather than Status.ERROR, so the URL stays schedulable. Strict parsing and the sniffing default both change what an existing crawl discovers, so both need a release note.

[abhinav-phi]

Hi! I'd like to work on this.
Plan: I'll make SiteMapParserBolt sniff only sitemap-like URLs/content types instead of every document, parse sitemaps strictly, and handle failed sitemap parsing gracefully — with tests for each case.
/assign take