rzo1 opened a new issue, #2100:
URL: https://github.com/apache/stormcrawler/issues/2100
## What happens
`http.content.limit` bounds the bytes the fetcher stores, and on the normal
path the compression interceptor decodes `Content-Encoding` before the counter
runs. A sitemap served as a gzip file, an `.xml.gz` body with no
`Content-Encoding`, is stored and trimmed while still compressed.
`parseSiteMap` hands those bytes to crawler-commons 1.6, which decompresses
them with nothing on the StormCrawler side limiting the output, and no
effective cap arrives from the library either: the test below stores 54,547
bytes and gets 2,989,004 bytes and 20,001 emitted URLs back. Separately, line
102 reads the content type as
`metadata.getFirstValue(HttpHeaders.CONTENT_TYPE)`, without the
`protocol.md.prefix` the fetcher applies and without matching the lowercased
header name, so `ct` is always null in a shipped topology and the parser always
guesses.
## Where
`core/src/main/java/org/apache/stormcrawler/bolt/SiteMapParserBolt.java:102`
and `:183-195`. Config keys: `http.content.limit` (`65536` in the archetype),
`protocol.md.prefix` (`protocol.` in `crawler-default.yaml:239`).
```java
String ct = metadata.getFirstValue(HttpHeaders.CONTENT_TYPE);
```
```java
if (StringUtils.isBlank(contentType) ||
contentType.contains("octet-stream")) {
siteMap = parser.parseSiteMap(content, url1);
} else {
siteMap = parser.parseSiteMap(contentType, content, url1);
}
```
`JSoupParserBolt.java:230` shows the intended form of that read, using the
configured prefix.
## Why it matters
An operator who sets `http.content.limit` expects it to bound what a single
document can cost, and on the gzip sitemap path it does not: the ratio between
the stored bytes and what the parser bolt allocates is limited only by gzip, up
to about 1000 to 1. Several parser executors expanding such documents at the
same time push a worker toward memory exhaustion, and after a restart the same
URLs are still queued. The dead content type read is milder: it means the
declared type is never used, so every sitemap is identified by guessing, and
any future logic that branches on `ct` would be dead too.
## Reproduction
Save as
`core/src/test/java/org/apache/stormcrawler/bolt/SiteMapParserBoltGzipTest.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.ByteArrayOutputStream;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.HashMap;
import java.util.Map;
import java.util.zip.GZIPOutputStream;
import org.apache.stormcrawler.Constants;
import org.apache.stormcrawler.Metadata;
import org.apache.stormcrawler.parse.ParsingTester;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
/**
* http.content.limit bounds what the fetcher stores. A sitemap shipped as a
.xml.gz file is stored
* compressed, so the bolt expands it without any bound.
*/
class SiteMapParserBoltGzipTest extends ParsingTester {
/** archetype value of http.content.limit */
private static final int CONTENT_LIMIT = 65536;
private static final int URLS = 20000;
@BeforeEach
void setupParserBolt() {
bolt = new SiteMapParserBolt();
setupParserBolt(bolt);
}
private static byte[] gzippedSitemap() throws IOException {
StringBuilder sb = new StringBuilder();
sb.append("<?xml version=\"1.0\" encoding=\"UTF-8\"?>");
sb.append("<urlset
xmlns=\"http://www.sitemaps.org/schemas/sitemap/0.9\";>");
for (int i = 0; i < URLS; i++) {
sb.append("<url><loc>https://a.example/page-";).append(i).append("</loc></url>");
}
// padding, as a large sitemap would carry
sb.append("<!--");
for (int i = 0; i < 200_000; i++) {
sb.append("aaaaaaaaaa");
}
sb.append("-->");
sb.append("</urlset>");
byte[] plain = sb.toString().getBytes(StandardCharsets.UTF_8);
ByteArrayOutputStream baos = new ByteArrayOutputStream();
try (GZIPOutputStream gz = new GZIPOutputStream(baos)) {
gz.write(plain);
}
System.out.println(
"sitemap: " + plain.length + " bytes uncompressed, " +
baos.size() + " gzipped");
return baos.toByteArray();
}
/**
* Documents the current behaviour. The stored document is well under
http.content.limit, the
* expansion is two orders of magnitude above it, and every entry is
emitted. Expected
* behaviour: expansion beyond the configured content limit is refused
and the document is
* treated as a parse error.
*/
@Test
void gzippedSitemapIsExpandedWithoutAnyBound() throws IOException {
Map<String, Object> config = new HashMap<>();
config.put("http.content.limit", CONTENT_LIMIT);
prepareParserBolt("test.parsefilters.json", config);
byte[] content = gzippedSitemap();
Assertions.assertTrue(
content.length < CONTENT_LIMIT,
"stored document is " + content.length + " bytes, within the
content limit");
Metadata metadata = new Metadata();
metadata.setValue(SiteMapParserBolt.isSitemapKey, "true");
parse("https://a.example/sitemap.xml.gz";, content, metadata);
int emitted = output.getEmitted(Constants.StatusStreamName).size();
System.out.println("emitted status tuples: " + emitted);
Assertions.assertEquals(URLS + 1, emitted);
}
}
```
Run it:
```
mvn -pl core test -Dtest=SiteMapParserBoltGzipTest
```
This one passes on main and documents the present behaviour, because the
intended bound does not exist yet to assert against; the comment in the test
states what should happen. The stored document is 54,547 bytes, inside the
archetype value, and expands to 2,989,004 bytes with every entry emitted. The
ratio here is limited only by how much padding the test builds.
```
sitemap: 2989004 bytes uncompressed, 54547 gzipped
emitted status tuples: 20001
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0
```
## Suggested fix
In `SiteMapParserBolt.parseSiteMap`, detect the gzip magic bytes and
decompress into a buffer capped at `http.content.limit`, or at a dedicated
`sitemap.max.expanded.size` if operators need a larger bound for sitemaps than
for pages, and treat an overflow as a parse failure rather than passing the
content on. Pass the decompressed bytes to crawler-commons. Also change line
102 to `metadata.getFirstValue(HttpHeaders.CONTENT_TYPE, protocolMDprefix)`
with the prefix read from `protocol.md.prefix` in `prepare`, and match the
header name case the protocols produce. Fixing the content type read changes
which parse path is taken for documents that declare a type, so it is worth 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]
