Skip to content

Commit 44b6552

Browse files
eamonnmcmanusError Prone Team
authored and
Error Prone Team
committed
Don't complain about literal IP addresses in AddressSelection.
A correct literal IP address will always resolve to the same `InetAddress` value so there is no point in suggesting the use of `getAllByName`. Also fix a typo: `getAllName`. PiperOrigin-RevId: 555506842
1 parent 1c1f0f8 commit 44b6552

File tree

2 files changed

+48
-7
lines changed

2 files changed

+48
-7
lines changed

core/src/main/java/com/google/errorprone/bugpatterns/AddressSelection.java

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod;
2626
import static com.google.errorprone.util.ASTHelpers.constValue;
2727

28+
import com.google.common.base.CharMatcher;
2829
import com.google.common.collect.ImmutableSet;
2930
import com.google.errorprone.BugPattern;
3031
import com.google.errorprone.VisitorState;
@@ -37,13 +38,12 @@
3738
import com.sun.source.tree.ExpressionTree;
3839
import com.sun.source.tree.MethodInvocationTree;
3940
import com.sun.source.tree.NewClassTree;
40-
import java.util.Objects;
4141
import java.util.function.Supplier;
4242

4343
/** A {@link BugChecker}; see the associated {@link BugPattern} annotation for details. */
4444
@BugPattern(
4545
summary =
46-
"Prefer InetAddress.getAllName to APIs that convert a hostname to a single IP address",
46+
"Prefer InetAddress.getAllByName to APIs that convert a hostname to a single IP address",
4747
severity = WARNING)
4848
public final class AddressSelection extends BugChecker
4949
implements NewClassTreeMatcher, MethodInvocationTreeMatcher {
@@ -98,13 +98,36 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
9898
private Description handleMatch(
9999
ExpressionTree argument, ExpressionTree replacement, Supplier<SuggestedFix> fix) {
100100
String value = constValue(argument, String.class);
101-
if (Objects.equals(value, "localhost")) {
101+
// If it's a numeric loopback address, suggest using the method for that.
102+
if (LOOPBACK.contains(value)) {
103+
return describeMatch(replacement, fix.get());
104+
}
105+
// If it isn't a constant, or it's "localhost", or it looks like a numeric IP address, then
106+
// we don't say anything.
107+
if (value == null || value.equals("localhost") || isNumericIp(value)) {
102108
return NO_MATCH;
103109
}
104-
Description.Builder description = buildDescription(replacement);
105-
if (LOOPBACK.contains(value)) {
106-
description.addFix(fix.get());
110+
// Otherwise flag it but don't suggest a fix.
111+
return describeMatch(replacement);
112+
}
113+
114+
/**
115+
* Returns true if this string looks like it might be a numeric IP address. The matching here is
116+
* very approximate. We want every numeric IP address to return true, but it's OK if some strings
117+
* return true even though they are not actually valid numeric IP addresses. Actually parsing
118+
* numeric IPv6 addresses in all their glory is more than we need here.
119+
*/
120+
private static boolean isNumericIp(String value) {
121+
if (value.isEmpty()) {
122+
return false;
123+
}
124+
if (value.contains(":")) {
125+
return true; // Every numeric IPv6 address contains a colon and no non-numeric hostname does.
107126
}
108-
return description.build();
127+
return ASCII_DIGIT.matches(value.charAt(0))
128+
&& ASCII_DIGIT.matches(value.charAt(value.length() - 1));
129+
// Every numeric IPv4 address begins and ends with an ASCII digit.
109130
}
131+
132+
private static final CharMatcher ASCII_DIGIT = CharMatcher.inRange('0', '9');
110133
}

core/src/test/java/com/google/errorprone/bugpatterns/AddressSelectionTest.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,24 @@ public void negativeLocalhost() throws Exception {
8888
.doTest();
8989
}
9090

91+
@Test
92+
public void negativeNumeric() throws Exception {
93+
compilationHelper
94+
.addSourceLines(
95+
"Test.java",
96+
"import java.net.InetAddress;",
97+
"import java.net.InetSocketAddress;",
98+
"import java.net.Socket;",
99+
"class Test {",
100+
" void f() throws Exception {",
101+
" new Socket(\"1.2.3.4\", 80);",
102+
" InetAddress.getByName(\"2001:db8:85a3:8d3:1319:8a2e:370:7348\");",
103+
" new InetSocketAddress(\"::ffff:192.0.2.128\", 80);",
104+
" }",
105+
"}")
106+
.doTest();
107+
}
108+
91109
@Test
92110
public void refactor() throws Exception {
93111
refactoringTestHelper

0 commit comments

Comments
 (0)