rzo1 opened a new issue, #2090:
URL: https://github.com/apache/stormcrawler/issues/2090
## What happens
`FetcherBolt` merges every response header into the tuple metadata under the
protocol prefix, lowercased. The okhttp protocol reads the `set-header`
directive from that same prefixed namespace. A server that returns a header
literally named `set-header` therefore produces the metadata key
`protocol.set-header`, which is exactly the key `addHeadersToRequest` uses to
add arbitrary headers to an outgoing request. The merge already strips
`robots.crawl.delay` for the same reason, so the collision class is known, but
`set-header` is not stripped. The other request shaping keys (`http.post.json`,
`http.method.head`, `http.content.limit`, `http.proxy*`) are read without the
prefix and are not reachable this way.
## Where
`core/src/main/java/org/apache/stormcrawler/bolt/FetcherBolt.java:802`, with
the existing strip four lines below at `:806`:
```java
mergedMetadata.putAll(response.getMetadata(),
protocolMetadataPrefix);
// Only the locally parsed robots.txt value may populate
this control signal.
// A colliding protocol prefix/header must not pace an
unrelated queue.
mergedMetadata.remove(Constants.ROBOTS_CRAWL_DELAY_KEY);
```
`core/src/main/java/org/apache/stormcrawler/protocol/okhttp/HttpProtocol.java:299`:
```java
final String[] headerStrings = md.getValues(SET_HEADER_BY_REQUEST,
protocolMetadataPrefix);
```
Config keys involved: `protocol.md.prefix`, `metadata.persist`,
`metadata.transfer`.
## Why it matters
The value only becomes live on a later fetch if the operator persists or
transfers it, and `metadata.transfer` is empty in every shipped configuration.
It is not an exotic setup, though: internals.adoc tells operators to persist
`protocol.` keys such as `protocol.etag` and `protocol.set-cookie`, and both
lists accept wildcards, so a `protocol.*` entry carries the forged key along
with the intended ones. The result is that a crawled site chooses headers sent
on subsequent requests, including requests to other hosts when the key is
transferred to outlinks. Request shaping should come from configuration, not
from a response body or header.
## Reproduction
Save as
`core/src/test/java/org/apache/stormcrawler/protocol/okhttp/ProtocolSetHeaderFromResponseTest.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.okhttp;
import okhttp3.Request;
import org.apache.storm.Config;
import org.apache.stormcrawler.Metadata;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
/**
* Documents the current handling of a response header named "set-header".
The header merge in
* FetcherBolt writes response headers under the protocol metadata prefix,
which is the same place
* addHeadersToRequest reads the set-header directive from. The value should
not become a request
* header on a later fetch.
*/
class ProtocolSetHeaderFromResponseTest {
private static Config protocolConfig() {
Config conf = new Config();
conf.put("http.agent.name", "test");
conf.put("http.agent.version", "1.0");
conf.put("http.agent.description", "test");
conf.put("http.agent.url", "http://test.example.com";);
conf.put("http.agent.email", "[email protected]");
conf.put("protocol.md.prefix", "protocol.");
return conf;
}
@Test
void responseHeaderNamedSetHeaderBecomesARequestHeader() {
HttpProtocol protocol = new HttpProtocol();
protocol.configure(protocolConfig());
// what a server returned, lowercased and merged the way FetcherBolt
does it
Metadata fromResponse = new Metadata();
fromResponse.addValue("set-header", "X-Forged=1");
Metadata carried = new Metadata();
carried.putAll(fromResponse, "protocol.");
Assertions.assertEquals("X-Forged=1",
carried.getFirstValue("protocol.set-header"));
Request.Builder rb = new
Request.Builder().url("https://example.com/";);
protocol.addHeadersToRequest(rb, carried);
Request request = rb.build();
// current behaviour: the server-supplied value is sent on the next
request.
// It should be ignored: only operator configuration may set request
headers.
Assertions.assertEquals("1", request.header("X-Forged"));
}
}
```
Run it:
```
mvn -pl core test -Dtest=ProtocolSetHeaderFromResponseTest
```
It asserts the present behaviour and passes on main, with a comment saying
what should happen instead; the fix can land either in the bolt or in the
protocol, so a failing test would prejudge where.
```
[INFO] Running
org.apache.stormcrawler.protocol.okhttp.ProtocolSetHeaderFromResponseTest
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.243
s -- in
org.apache.stormcrawler.protocol.okhttp.ProtocolSetHeaderFromResponseTest
```
The test builds metadata the way the bolt does (`putAll(responseMetadata,
"protocol.")`), calls `addHeadersToRequest`, and finds `X-Forged: 1` on the
request.
## Suggested fix
Strip `set-header` from the merged metadata in `FetcherBolt.fetch` right
after the `putAll` with the protocol prefix, next to the existing
`robots.crawl.delay` removal. Beyond that one key, treat this as a class:
reserve a namespace for directives that shape a request and have
`MetadataTransfer` refuse to transfer or persist keys in it unless the operator
names them one by one, so that a wildcard cannot pick them up. Operators who
rely on a `protocol.*` wildcard today will see fewer keys carried, which
belongs in the release notes.
--
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]
