The robots.txt fetch replaces the configured http.content.limit with no limit at all

[rzo1]

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.

int robotsTxtContentLimit = ConfUtils.getInt(conf, "http.robots.content.limit", -1);
fetchRobotsMd.addValue("http.content.limit", Integer.toString(robotsTxtContentLimit));

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.

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

[abhinav-phi]

Hi! I'd like to take this up.
Plan: I'll keep the configured http.content.limit in effect for the robots.txt fetch instead of replacing it with no limit, and add a test asserting the limit applies.
/assign take