rzo1 opened a new issue, #2089:
URL: https://github.com/apache/stormcrawler/issues/2089
## What happens
`HttpProtocol.configure()` builds one list of request headers that includes
the `Authorization` header derived from `http.basicauth.user` and
`http.basicauth.password`, plus every entry of `http.custom.headers`.
`getProtocolOutput()` applies that list to each request, including robots.txt
fetches, with no reference to the host being fetched. There is no key to bind a
credential to an origin. The behaviour is described in `extending.adoc:407`,
which points at metadata-driven headers as the per-site alternative.
## Where
`core/src/main/java/org/apache/stormcrawler/protocol/okhttp/HttpProtocol.java:216-229`
and `:368-372` on main. Config keys: `http.basicauth.user`,
`http.basicauth.password`, `http.custom.headers`.
```java
final Builder rb = new Request.Builder().url(url);
customRequestHeaders.forEach(
(k) -> {
rb.header(k.getKey(), k.getValue());
});
```
## Why it matters
The keys exist for crawling a site that requires authentication, and a
crawler follows outlinks, so the moment a crawled page links off the
authenticated site the credential goes to the linked host. Pages on an internal
site are often editable by their users, so the outlink need not be one the
operator chose. The alternative named in the documentation works, but the
simple configuration that the reference table describes has no scoping at all,
and nothing warns about it.
## Reproduction
Save as
`core/src/test/java/org/apache/stormcrawler/protocol/OkHttpBasicAuthScopeTest.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 jakarta.servlet.http.HttpServletResponse;
import java.io.IOException;
import java.io.OutputStream;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.List;
import org.apache.storm.Config;
import org.apache.stormcrawler.Metadata;
import org.eclipse.jetty.server.Handler;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.handler.AbstractHandler;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
/** Records which requests carry the configured Authorization header. */
class OkHttpBasicAuthScopeTest extends AbstractProtocolTest {
static final List<String> authorizationSeen = new ArrayList<>();
@Override
protected Handler[] getHandlers() {
return new Handler[] {
new AbstractHandler() {
@Override
public void handle(
String target,
Request baseRequest,
jakarta.servlet.http.HttpServletRequest request,
HttpServletResponse response)
throws IOException {
baseRequest.setHandled(true);
authorizationSeen.add(String.valueOf(request.getHeader("Authorization")));
response.setStatus(HttpServletResponse.SC_OK);
response.setContentType("text/html");
final byte[] content =
"hello".getBytes(StandardCharsets.UTF_8);
response.setContentLength(content.length);
try (OutputStream out = response.getOutputStream()) {
out.write(content);
}
}
}
};
}
@Test
void credentialsGoToEveryHost() throws Exception {
authorizationSeen.clear();
Config conf = new Config();
conf.put("http.agent.name", "this_is_only_a_test");
conf.put("http.basicauth.user", "wikiuser");
conf.put("http.basicauth.password", "wikipass");
org.apache.stormcrawler.protocol.okhttp.HttpProtocol protocol =
new org.apache.stormcrawler.protocol.okhttp.HttpProtocol();
protocol.configure(conf);
// the credentials were configured for some other site; this fetch
is an
// outlink to an unrelated server and the robots.txt that precedes it
protocol.getProtocolOutput(
"http://127.0.0.1:"; + HTTP_PORT + "/robots.txt", new
Metadata());
protocol.getProtocolOutput("http://127.0.0.1:"; + HTTP_PORT +
"/page.html", new Metadata());
protocol.cleanup();
Assertions.assertEquals(2, authorizationSeen.size());
// documents the current behaviour: there is no way to bind the
header to
// an origin, so every request carries it. Both values should be
"null"
// once the credentials are scoped to the site they belong to.
for (String seen : authorizationSeen) {
Assertions.assertTrue(
seen.startsWith("Basic "),
"expected the Authorization header on every request, got
" + seen);
}
}
}
```
Run it:
```
mvn -pl core test -Dtest=OkHttpBasicAuthScopeTest
```
A local Jetty server records the `Authorization` header of each request. The
test passes on main and documents the present behaviour, because there is no
origin binding to assert against yet.
```
[INFO] Running org.apache.stormcrawler.protocol.OkHttpBasicAuthScopeTest
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.493
s -- in org.apache.stormcrawler.protocol.OkHttpBasicAuthScopeTest
```
Both requests, the robots.txt fetch and the page fetch, carry `Basic ...` to
a server the credentials were never meant for.
## Suggested fix
Add a host list alongside the credentials, for example
`http.basicauth.hosts`, and attach the `Authorization` header in
`getProtocolOutput` only when the request host matches. Do the same for
`http.custom.headers`, either with a per-entry host or by moving
credential-bearing headers to a separate key that takes a host. Requiring the
host is a behaviour change for existing configurations, so either land it in a
major release or accept an unset list as "everywhere" for one release with a
WARN at startup.
--
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]
