gnodet-bot commented on code in PR #13496:
URL: https://github.com/apache/maven/pull/13496#discussion_r4269243047
##########
compat/maven-compat/src/main/java/org/apache/maven/repository/DefaultMirrorSelector.java:
##########
@@ -149,8 +154,29 @@ static boolean isExternalRepo(ArtifactRepository
originalRepository) {
return !(isLocal(m.group(5)) || "file".equalsIgnoreCase(m.group(1)));
}
+ /**
+ * Checks whether a host names the local machine: {@code localhost}, an
IPv4 address of the loopback
+ * range {@code 127.0.0.0/8} or an IPv6 loopback address in brackets. The
names are not resolved.
+ *
+ * @param host the host of a repository URL, may be {@code null}
+ * @return true if the host is local
+ */
private static boolean isLocal(String host) {
- return "localhost".equals(host) || "127.0.0.1".equals(host);
+ if (host == null) {
+ return false;
+ }
+ if ("localhost".equals(host) || IPV4_LOOPBACK.matcher(host).matches())
{
+ return true;
+ }
+ if (host.startsWith("[") && host.endsWith("]")) {
+ try {
+ // an IP literal is parsed without any name lookup
+ return InetAddress.getByName(host).isLoopbackAddress();
Review Comment:
⚠️ **Edge case:** `InetAddress.getByName(host).isLoopbackAddress()` returns
`false` for IPv4-mapped IPv6 addresses like `[::ffff:127.0.0.1]` — the JDK only
recognises `::1` as the IPv6 loopback. Such a URL would be treated as external.
Given RFC 4291 §2.5.5 semantics, this is an obscure case, but worth documenting
or handling explicitly.
##########
compat/maven-compat/src/test/java/org/apache/maven/repository/DefaultMirrorSelectorTest.java:
##########
@@ -65,6 +65,34 @@ void testExternalRepoClassification() {
assertFalse(DefaultMirrorSelector.isExternalRepo(repo("http://127.0.0.1/m2/";)));
}
+ @Test
+ void testLoopbackAddressesAreLocal() {
+ for (String url : new String[] {
+ "http://127.0.0.2/m2/";,
+ "http://127.255.255.254:8080/m2/";,
+ "http://[::1]/m2/";,
+ "http://[::1]:8080/m2/";,
+ "http://[0:0:0:0:0:0:0:1]:8080/m2/";
+ }) {
+ assertFalse(DefaultMirrorSelector.isExternalRepo(repo(url)), url);
+ assertFalse(DefaultMirrorSelector.isExternalHttpRepo(repo(url)),
url);
+ }
+ }
+
+ @Test
+ void testOtherAddressesAreExternal() {
+ for (String url : new String[] {
+ "http://128.0.0.1/m2/";,
+ "http://127.0.0.1.example.com/m2/";,
+ "http://127.example.com/m2/";,
+ "http://[2001:db8::1]/m2/";,
+ "http://[2001:db8::1]:8080/m2/";
Review Comment:
💡 **Suggestion:** Consider adding `"http://[::ffff:7f00:1]/m2/"` (or
`[::ffff:127.0.0.1]`) to document the current behaviour for IPv4-mapped
loopback addresses — either as an expected-local case (if the code is updated)
or as an acknowledged external case with a comment explaining why.
--
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]
