apache / apache/stormcrawler

Gzipped sitemap files are expanded with no size bound, and SiteMapParserBolt reads Content-Type under a key that is never present

Open
#2,100 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
995
Forks
292
Avg merge
2d 49m
Merged PRs (30d)
62

Description

## 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("");
sb.append("");
for (int i = 0; i < URLS; i++) {
sb.append("https://a.example/page-").append(i).append("");
}
// padding, as a large sitemap would carry
sb.append("");
sb.append("");
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 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.

Contributor guide

Open the contributing guide

Research direction

Start with core/src/main/java/org/apache/stormcrawler/bolt/SiteMapParserBolt.java, especially parseSiteMap and the metadata read near lines 102 and 183-195; compare the content-type lookup with JSoupParserBolt.java:230. Run the supplied core/src/test/java/org/apache/stormcrawler/bolt/SiteMapParserBoltGzipTest.java command to reproduce the expansion. Done means gzip sitemap expansion is bounded by the configured limit and oversized input is treated as a parse failure, while the configured content type is read correctly.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
backend, distributed-systems
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.