rzo1 commented on code in PR #2077:
URL: https://github.com/apache/stormcrawler/pull/2077#discussion_r3879546577
##########
core/src/main/java/org/apache/stormcrawler/protocol/HttpRobotRulesParser.java:
##########
@@ -149,7 +170,18 @@ public BaseRobotRules getRobotRulesSet(Protocol http, URL
url) {
String redirection =
response.getMetadata().getFirstValue(HttpHeaders.LOCATION);
LOG.debug("Redirected from {} to {}", redir, redirection);
if (StringUtils.isNotBlank(redirection)) {
- redir = URLUtil.resolveUrl(redir, redirection);
+ URL target = URLUtil.resolveUrl(redir, redirection);
+ if (!followRedirect(redir, target)) {
+ LOG.debug(
+ "Robots for {} redirected to {} which is not
fetched "
+ + "(not on the same host as {})",
+ url,
+ target,
+ redir);
+ // handled like any other response which does not
provide rules
+ break;
Review Comment:
Agreed, and decided explicitly rather than by fall-through. A refused
redirect now sets `cacheRule = false`, so it lands in the error cache like a
429 or a 5xx instead of the success cache, and the hosts on the chain get
nothing stored under their keys either. Default is fail-closed: no rules for
that host, so nothing is crawled there until the redirect is resolved.
`http.robots.redirect.refused.allow: true` restores allow-all for anyone who
wants it. It is logged at warn with the target, why it was not followed and
which setting changes it, so the two outcomes are distinguishable from a
genuinely absent robots.txt.
##########
core/src/test/java/org/apache/stormcrawler/protocol/HttpRobotRulesParserRedirectTest.java:
##########
@@ -79,6 +79,8 @@ void setUp() {
mockServer7.start();
mockServer8.start();
conf.put("http.agent.name", "this_is_only_a_test");
+ // the redirect chains tested below point at other ports of the same
host
+ conf.put("http.robots.redirect.crosshost.allow", true);
Review Comment:
Added. `HttpRobotRulesParserRedirectTest` now has
`testRedirectOnTheSameAuthorityIsFollowedByDefault` and
`testRedirectToAnotherAuthorityIsNotFollowedByDefault`, both on the default
conf, plus `testHeadersNotSentToAnUnfollowedRedirectTarget`. The same-host http
-> https case is covered at unit level in
`HttpRobotRulesParserRedirectTargetTest` (upgrade, explicit `:443`, explicit
same port, downgrade, other host, other port) rather than end to end — core has
no keystore test setup, and a genuine upgrade compares `http:h:80` against
`https:h:443`, which the wiremock harness cannot represent.
--
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]
