abhinav-phi commented on code in PR #2122:
URL: https://github.com/apache/stormcrawler/pull/2122#discussion_r3996891837
##########
core/src/main/java/org/apache/stormcrawler/util/URLUtil.java:
##########
@@ -253,6 +254,31 @@ public static String getHost(String url) {
}
}
+ /**
+ * Returns the host in the form the HTTP client will connect to it:
percent-escapes decoded,
+ * lowercased and without a trailing dot. Host strings which only differ
in escaping or case
+ * reach the same server, so politeness queues and robots.txt caches must
key on the same
+ * value, otherwise one server is fetched under several queue ids and its
robots.txt is
+ * downloaded once per spelling.
+ *
+ * @param url The url to check.
+ * @return String The canonical host for the url, or null if the url is
not well formed or has
+ * no host.
+ */
+ public static String getCanonicalHost(URL url) {
+ String host = url.getHost();
+ if (host == null) {
+ return null;
+ }
+ // okhttp percent-decodes the host when it parses the URL; do the same
+ // so keys derived from the URL string agree with what it connects to
+ String decoded = URLDecoder.decode(host, StandardCharsets.UTF_8);
Review Comment:
Fixed in bbb945e8 — `URLDecoder` is gone. `percentDecodeHost` decodes only
`%XX` pairs, never maps `+` to space, and keeps malformed escapes like `%zz` as
literal characters, so a hostile host string cannot throw (the javadoc states
the contract). `URLUtilNormaliseHostTest` pins the escaping, idempotence and
multi-byte cases.
##########
core/src/main/java/org/apache/stormcrawler/util/URLUtil.java:
##########
@@ -253,6 +254,31 @@ public static String getHost(String url) {
}
}
+ /**
+ * Returns the host in the form the HTTP client will connect to it:
percent-escapes decoded,
Review Comment:
Reworded in bbb945e8 as suggested: the javadoc now says "the form of the
host used to key politeness queues and the robots.txt cache: what okhttp
connects to, with the root label normalised away".
`HostAliasCacheKeyTest.okhttpCollapsesHostAliases` keeps the trailing-dot
divergence documented.
##########
core/src/main/java/org/apache/stormcrawler/bolt/FetcherBolt.java:
##########
@@ -186,7 +186,7 @@ public static FetchItem create(URL u, String url, Tuple t,
String queueMode) {
key = u.getHost();
}
} else {
- key = u.getHost();
+ key = URLUtil.getCanonicalHost(u);
Review Comment:
Fixed in bbb945e8 — `canonicalHost` is computed once at the top of the
method (FetcherBolt.java:177) and used in all three branches: `byIP` →
`getByName(canonicalHost)`, `byDomain` → `getPLD(canonicalHost)`, `byHost` →
the key itself.
##########
core/src/main/java/org/apache/stormcrawler/bolt/SimpleFetcherBolt.java:
##########
@@ -628,7 +628,7 @@ private String getPolitenessKey(URL u) {
key = u.getHost();
}
} else {
- key = u.getHost();
+ key = URLUtil.getCanonicalHost(u);
Review Comment:
Fixed in bbb945e8 — same here: canonicalHost is computed once at the top
(SimpleFetcherBolt.java:617) and used in all three modes.
##########
core/src/main/java/org/apache/stormcrawler/util/URLUtil.java:
##########
@@ -253,6 +254,134 @@ public static String getHost(String url) {
}
}
+ /**
+ * Returns the form of the host used to key politeness queues and the
robots.txt cache: what
+ * okhttp connects to, with the root label normalised away. Host strings
which only differ in
+ * escaping or case reach the same server, so both spellings must end up
under one key,
+ * otherwise one server is fetched under several queue ids and its
robots.txt is downloaded once
+ * per spelling.
+ *
+ * @param url The url to check.
+ * @return String The canonical host for the url, or null if the url is
not well formed or has
+ * no host.
+ */
+ public static String getCanonicalHost(URL url) {
+ String host = url.getHost();
+ if (host == null) {
+ return null;
+ }
+ // okhttp percent-decodes the host when it parses the URL; do the same
+ // so keys derived from the URL string agree with what it connects to.
+ // The decoder never throws: crawled content is hostile input, and a
+ // malformed escape falls back to the raw spelling rather than blowing
+ // up the caller
+ String decoded = percentDecodeHost(host);
+ if (decoded.endsWith(".")) {
+ decoded = decoded.substring(0, decoded.length() - 1);
+ }
+ return decoded.toLowerCase(Locale.ROOT);
+ }
+
+ /**
+ * Percent-decodes a host string, leaving {@code +} alone and keeping
malformed escapes as
+ * literal characters. Unlike {@link URLDecoder#decode}, this never throws.
+ */
+ private static String percentDecodeHost(String host) {
+ if (!host.contains("%")) {
+ return host;
+ }
+ StringBuilder sb = new StringBuilder(host.length());
+ for (int i = 0; i < host.length(); i++) {
+ char c = host.charAt(i);
+ if (c == '%' && i + 2 < host.length()) {
+ int hi = Character.digit(host.charAt(i + 1), 16);
+ int lo = Character.digit(host.charAt(i + 2), 16);
+ if (hi != -1 && lo != -1) {
+ sb.append((char) ((hi << 4) | lo));
Review Comment:
Good catch — fixed in 57f76f7c: the decoded octets are collected and
interpreted as one UTF-8 sequence, so `%C3%BC` becomes `ü`, not `ü`. Pinned by
`URLUtilNormaliseHostTest.multiByteEscapeDecodesAsUtf8`; a host normalised via
`status.updater.normalise.hosts` can no longer resolve to a different host than
its queue key.
--
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]
