rzo1 opened a new issue, #2083:
URL: https://github.com/apache/stormcrawler/issues/2083
## 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`).
```java
boolean looksLikeSitemap = sniff(content);
// can force the mimetype as we know it is XML
if (looksLikeSitemap) {
ct = "application/xml";
}
```
```java
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`.
```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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
