Skip to content

Commit a2d6ad7

Browse files
cushonError Prone Team
cushon
authored and
Error Prone Team
committed
Add an Error Prone check to discourage APIs that convert a hostname to a single
IP address PiperOrigin-RevId: 552955605
1 parent 2ef7ec2 commit a2d6ad7

File tree

4 files changed

+320
-0
lines changed

4 files changed

+320
-0
lines changed

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

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
/*
2+
* Copyright 2023 The Error Prone Authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.google.errorprone.bugpatterns;
18+
19+
import static com.google.common.collect.Iterables.getOnlyElement;
20+
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
21+
import static com.google.errorprone.fixes.SuggestedFixes.qualifyType;
22+
import static com.google.errorprone.fixes.SuggestedFixes.renameMethodInvocation;
23+
import static com.google.errorprone.matchers.Description.NO_MATCH;
24+
import static com.google.errorprone.matchers.method.MethodMatchers.constructor;
25+
import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod;
26+
import static com.google.errorprone.util.ASTHelpers.constValue;
27+
28+
import com.google.common.collect.ImmutableSet;
29+
import com.google.errorprone.BugPattern;
30+
import com.google.errorprone.VisitorState;
31+
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
32+
import com.google.errorprone.bugpatterns.BugChecker.NewClassTreeMatcher;
33+
import com.google.errorprone.fixes.SuggestedFix;
34+
import com.google.errorprone.matchers.Description;
35+
import com.google.errorprone.matchers.Matcher;
36+
import com.google.errorprone.matchers.Matchers;
37+
import com.sun.source.tree.ExpressionTree;
38+
import com.sun.source.tree.MethodInvocationTree;
39+
import com.sun.source.tree.NewClassTree;
40+
import java.util.Objects;
41+
import java.util.function.Supplier;
42+
43+
/** A {@link BugChecker}; see the associated {@link BugPattern} annotation for details. */
44+
@BugPattern(
45+
summary =
46+
"Prefer InetAddress.getAllName to APIs that convert a hostname to a single IP address",
47+
severity = WARNING)
48+
public final class AddressSelection extends BugChecker
49+
implements NewClassTreeMatcher, MethodInvocationTreeMatcher {
50+
51+
private static final Matcher<ExpressionTree> CONSTRUCTORS =
52+
Matchers.anyOf(
53+
constructor().forClass("java.net.Socket").withParameters("java.lang.String", "int"),
54+
constructor()
55+
.forClass("java.net.InetSocketAddress")
56+
.withParameters("java.lang.String", "int"));
57+
private static final Matcher<ExpressionTree> METHODS =
58+
staticMethod()
59+
.onClass("java.net.InetAddress")
60+
.named("getByName")
61+
.withParameters("java.lang.String");
62+
63+
@Override
64+
public Description matchNewClass(NewClassTree tree, VisitorState state) {
65+
if (!CONSTRUCTORS.matches(tree, state)) {
66+
return NO_MATCH;
67+
}
68+
ExpressionTree argument = tree.getArguments().get(0);
69+
return handleMatch(
70+
argument,
71+
argument,
72+
() -> {
73+
SuggestedFix.Builder fix = SuggestedFix.builder();
74+
fix.replace(
75+
argument, qualifyType(state, fix, "java.net.InetAddress") + ".getLoopbackAddress()");
76+
return fix.build();
77+
});
78+
}
79+
80+
@Override
81+
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
82+
if (!METHODS.matches(tree, state)) {
83+
return NO_MATCH;
84+
}
85+
ExpressionTree argument = getOnlyElement(tree.getArguments());
86+
return handleMatch(
87+
argument,
88+
tree,
89+
() ->
90+
SuggestedFix.builder()
91+
.merge(renameMethodInvocation(tree, "getLoopbackAddress", state))
92+
.delete(argument)
93+
.build());
94+
}
95+
96+
private static final ImmutableSet<String> LOOPBACK = ImmutableSet.of("127.0.0.1", "::1");
97+
98+
private Description handleMatch(
99+
ExpressionTree argument, ExpressionTree replacement, Supplier<SuggestedFix> fix) {
100+
String value = constValue(argument, String.class);
101+
if (Objects.equals(value, "localhost")) {
102+
return NO_MATCH;
103+
}
104+
Description.Builder description = buildDescription(replacement);
105+
if (LOOPBACK.contains(value)) {
106+
description.addFix(fix.get());
107+
}
108+
return description.build();
109+
}
110+
}

‎core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import com.google.common.collect.Streams;
2323
import com.google.errorprone.BugCheckerInfo;
2424
import com.google.errorprone.bugpatterns.ASTHelpersSuggestions;
25+
import com.google.errorprone.bugpatterns.AddressSelection;
2526
import com.google.errorprone.bugpatterns.AlreadyChecked;
2627
import com.google.errorprone.bugpatterns.AlwaysThrows;
2728
import com.google.errorprone.bugpatterns.AmbiguousMethodReference;
@@ -817,6 +818,7 @@ public static ScannerSupplier warningChecks() {
817818
getSuppliers(
818819
// keep-sorted start
819820
ASTHelpersSuggestions.class,
821+
AddressSelection.class,
820822
AlmostJavadoc.class,
821823
AlreadyChecked.class,
822824
AmbiguousMethodReference.class,

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

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
/*
2+
* Copyright 2023 The Error Prone Authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.google.errorprone.bugpatterns;
18+
19+
import com.google.errorprone.BugCheckerRefactoringTestHelper;
20+
import com.google.errorprone.CompilationTestHelper;
21+
import org.junit.Test;
22+
import org.junit.runner.RunWith;
23+
import org.junit.runners.JUnit4;
24+
25+
@RunWith(JUnit4.class)
26+
public class AddressSelectionTest {
27+
28+
private final CompilationTestHelper compilationHelper =
29+
CompilationTestHelper.newInstance(AddressSelection.class, getClass());
30+
31+
private final BugCheckerRefactoringTestHelper refactoringTestHelper =
32+
BugCheckerRefactoringTestHelper.newInstance(AddressSelection.class, getClass());
33+
34+
@Test
35+
public void positive() {
36+
compilationHelper
37+
.addSourceLines(
38+
"Test.java",
39+
"import java.net.InetAddress;",
40+
"import java.net.InetSocketAddress;",
41+
"import java.net.Socket;",
42+
"class Test {",
43+
" void f() throws Exception{",
44+
" // BUG: Diagnostic contains:",
45+
" InetAddress.getByName(\"example.com\");",
46+
" // BUG: Diagnostic contains:",
47+
" new Socket(\"example.com\", 80);",
48+
" // BUG: Diagnostic contains:",
49+
" new InetSocketAddress(\"example.com\", 80);",
50+
" }",
51+
"}")
52+
.doTest();
53+
}
54+
55+
@Test
56+
public void negative() throws Exception {
57+
compilationHelper
58+
.addSourceLines(
59+
"Test.java",
60+
"import java.net.InetAddress;",
61+
"import java.net.InetSocketAddress;",
62+
"import java.net.Socket;",
63+
"class Test {",
64+
" void f() throws Exception{",
65+
" new Socket(InetAddress.getLoopbackAddress(), 80);",
66+
" InetAddress.getAllByName(\"example.com\");",
67+
" new InetSocketAddress(InetAddress.getLoopbackAddress(), 80);",
68+
" }",
69+
"}")
70+
.doTest();
71+
}
72+
73+
@Test
74+
public void negativeLocalhost() throws Exception {
75+
compilationHelper
76+
.addSourceLines(
77+
"Test.java",
78+
"import java.net.InetAddress;",
79+
"import java.net.InetSocketAddress;",
80+
"import java.net.Socket;",
81+
"class Test {",
82+
" void f() throws Exception{",
83+
" new Socket(\"localhost\", 80);",
84+
" InetAddress.getByName(\"localhost\");",
85+
" new InetSocketAddress(\"localhost\", 80);",
86+
" }",
87+
"}")
88+
.doTest();
89+
}
90+
91+
@Test
92+
public void refactor() throws Exception {
93+
refactoringTestHelper
94+
.addInputLines(
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(\"127.0.0.1\", 80);",
102+
" InetAddress.getByName(\"127.0.0.1\");",
103+
" new InetSocketAddress(\"127.0.0.1\", 80);",
104+
" new Socket(\"::1\", 80);",
105+
" InetAddress.getByName(\"::1\");",
106+
" new InetSocketAddress(\"::1\", 80);",
107+
" }",
108+
"}")
109+
.addOutputLines(
110+
"Test.java",
111+
"import java.net.InetAddress;",
112+
"import java.net.InetSocketAddress;",
113+
"import java.net.Socket;",
114+
"class Test {",
115+
" void f() throws Exception{",
116+
" new Socket(InetAddress.getLoopbackAddress(), 80);",
117+
" InetAddress.getLoopbackAddress();",
118+
" new InetSocketAddress(InetAddress.getLoopbackAddress(), 80);",
119+
" new Socket(InetAddress.getLoopbackAddress(), 80);",
120+
" InetAddress.getLoopbackAddress();",
121+
" new InetSocketAddress(InetAddress.getLoopbackAddress(), 80);",
122+
" }",
123+
"}")
124+
.doTest();
125+
}
126+
}

‎docs/bugpattern/AddressSelection.md

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
Avoid APIs that convert a hostname to a single IP address:
2+
3+
* [`java.net.Socket(String,int)`](https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/net/Socket.html#%3Cinit%3E\(java.lang.String,int,boolean\))
4+
* [`java.net.InetSocketAddress(String,int)`](https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/net/InetSocketAddress.html#%3Cinit%3E\(java.lang.String,int\))
5+
* [`java.net.InetAddress.html#getByName(String)`](https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/net/InetAddress.html#getByName\(java.lang.String\))
6+
7+
Depending on the value of the
8+
[`-Djava.net.preferIPv6Addresses=true`](https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/net/doc-files/net-properties.html)
9+
system property, those APIs will return an IPv4 or IPv6 address. If a client
10+
only has IPv4 connectivity, it will fail to connect with
11+
`-Djava.net.preferIPv6Addresses=true`. If a client only has IPv6 connectivity,
12+
it will fail to connect with `-Djava.net.preferIPv6Addresses=false`.
13+
14+
The preferred alternative is for clients to consider all addresses returned by
15+
[`java.net.InetAddress.html#getAllByName(String)`](https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/net/InetAddress.html#getAllByName\(java.lang.String\)),
16+
and try to connect to each one until a successful connection is made.
17+
18+
TIP: To resolve a loopback address, prefer `InetAddress.getLoopbackAddress()`
19+
over hard-coding an IPv4 or IPv6 loopback address with
20+
`InetAddress.getByName("127.0.0.1")` or `InetAddress.getByName("::1")`.
21+
22+
This is, prefer this:
23+
24+
```java
25+
Socket doConnect(String hostname, int port) throws IOException {
26+
IOException exception = null;
27+
for (InetAddress address : InetAddress.getAllByName(hostname)) {
28+
try {
29+
return new Socket(address, port);
30+
} catch (IOException e) {
31+
if (exception == null) {
32+
exception = e;
33+
} else {
34+
exception.addSuppressed(e);
35+
}
36+
}
37+
}
38+
throw exception;
39+
}
40+
```
41+
42+
```java
43+
Socket doConnect(String hostname, int port) throws IOException {
44+
IOException exception = null;
45+
for (InetAddress address : InetAddress.getAllByName(hostname)) {
46+
try {
47+
Socket s = new Socket();
48+
s.connect(new InetSocketAddress(address, port));
49+
return s;
50+
} catch (IOException e) {
51+
if (exception == null) {
52+
exception = e;
53+
} else {
54+
exception.addSuppressed(e);
55+
}
56+
}
57+
}
58+
throw exception;
59+
}
60+
```
61+
62+
instead of this:
63+
64+
```java
65+
Socket doConnect(String hostname, int port) throws IOException {
66+
return new Socket(hostname, port);
67+
}
68+
```
69+
70+
```java
71+
void doConnect(String hostname, int port) throws IOException {
72+
Socket s = new Socket();
73+
s.connect(new InetSocketAddress(hostname, port));
74+
}
75+
```
76+
77+
```java
78+
void doConnect(String hostname, int port) throws IOException {
79+
Socket s = new Socket();
80+
s.connect(new InetSocketAddress(InetAddress.getByName(hostname), port));
81+
}
82+
```

0 commit comments

Comments
 (0)