rzo1 opened a new issue, #2087:
URL: https://github.com/apache/stormcrawler/issues/2087
## What happens
`HttpRobotRulesParser.setConf()` puts `http.content.limit` into the metadata
it uses for every robots.txt fetch, taking the value of
`http.robots.content.limit`, which ships as `-1`. In
`HttpProtocol.getProtocolOutput()` a metadata value of `http.content.limit`
overwrites the global limit unconditionally, so a robots.txt fetch is unlimited
even when the operator configured a finite limit for pages. The comment next to
the shipped default says the opposite: "default same as http.content.limit".
With no limit, `toByteArray()` reads until `Constants.MAX_ARRAY_SIZE`, which is
just under 2 GB, and it counts decompressed bytes because the compression
interceptor sits above it.
## Where
`core/src/main/java/org/apache/stormcrawler/protocol/HttpRobotRulesParser.java:62-64`,
`core/src/main/java/org/apache/stormcrawler/protocol/okhttp/HttpProtocol.java:400-407`
and `:481-484` on main, `core/src/main/resources/crawler-default.yaml:186`.
Config keys: `http.content.limit`, `http.robots.content.limit`.
```java
int robotsTxtContentLimit = ConfUtils.getInt(conf,
"http.robots.content.limit", -1);
fetchRobotsMd.addValue("http.content.limit",
Integer.toString(robotsTxtContentLimit));
```
```java
final String pageMaxContentStr =
metadata.getFirstValue("http.content.limit");
if (StringUtils.isNotBlank(pageMaxContentStr)) {
try {
pageMaxContent = Integer.parseInt(pageMaxContentStr);
} catch (NumberFormatException e) {
LOG.warn("Invalid http.content.limit in metadata: {}",
pageMaxContentStr);
}
}
```
## Why it matters
A robots.txt is fetched before the first page of every host, so the least
bounded read in the crawler is also the first one, and setting
`http.content.limit` does not restrain it. A server that answers with a
compressed body that expands to a large size makes a fetcher thread buffer it
in full, and the archetype topologies run with a 2 GB heap and up to 50 fetcher
threads. The effect is on the crawler's own workers, and the URL stays
scheduled, so the fetch is repeated after the worker restarts. The same
unlimited default applies to page bodies for anyone using the library default
of `-1`.
## Reproduction
Save as
`core/src/test/java/org/apache/stormcrawler/protocol/RobotsContentLimitTest.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.protocol;
import org.apache.storm.Config;
import org.apache.stormcrawler.Metadata;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
/** Checks the content limit used for the robots.txt fetch. */
class RobotsContentLimitTest {
/** Protocol stub recording the metadata it is called with. */
private static class RecordingProtocol implements Protocol {
Metadata seen;
@Override
public void configure(Config conf) {}
@Override
public ProtocolResponse getProtocolOutput(String url, Metadata
metadata) {
seen = metadata;
return new ProtocolResponse(new byte[0], 200, new Metadata());
}
@Override
public crawlercommons.robots.BaseRobotRules getRobotRules(String
url) {
return null;
}
@Override
public void cleanup() {}
}
@Test
void robotsFetchKeepsTheGlobalContentLimit() {
Config conf = new Config();
conf.put("http.agent.name", "this_is_only_a_test");
// operator sets a finite limit and does not touch
http.robots.content.limit
conf.put("http.content.limit", 65536);
RecordingProtocol protocol = new RecordingProtocol();
HttpRobotRulesParser parser = new HttpRobotRulesParser();
parser.setConf(conf);
parser.getRobotRulesSet(protocol, "http://limit.example.org/";);
String limit = protocol.seen.getFirstValue("http.content.limit");
Assertions.assertNotEquals(
"-1",
limit,
"the robots.txt fetch should not remove the configured
content limit");
}
}
```
Run it:
```
mvn -pl core test -Dtest=RobotsContentLimitTest
```
A `Protocol` stub records the metadata the parser passes in. The test
asserts the intended behaviour and fails on main.
```
[ERROR] RobotsContentLimitTest.robotsFetchKeepsTheGlobalContentLimit:64 the
robots.txt fetch should not remove the configured content limit ==> expected:
not equal but was: <-1>
```
## Suggested fix
In `HttpRobotRulesParser.setConf`, treat `http.robots.content.limit: -1` as
"inherit the global limit" and only write the key into `fetchRobotsMd` when a
robots specific limit is set. Give `http.robots.content.limit` a finite shipped
default; RFC 9309 asks for at least 500 KiB, so 524288 works and is already in
the file as a comment. In `HttpProtocol.getProtocolOutput`, ignore a metadata
value of `-1` when the global limit is finite, so per-URL metadata can tighten
the limit but not remove it. Consider a finite default for `http.content.limit`
as well; that changes what large pages look like to existing deployments and
belongs in 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]
