Skip to content

Commit f6ee80e

Browse files
committed
fix: Address bind address review feedback
- Use bracket notation for IPv6 hosts in startup log - Add isLoopbackAddress() assertion to loopback bind test - Remove redundant shouldBindToWildcardWhenBindAddressIsExplicitlyNull test - Update bindAddress() Javadoc to remove test-intent leak - Convert loopback test to use HttpClient instead of HttpURLConnection
1 parent 78ed3a4 commit f6ee80e

2 files changed

Lines changed: 24 additions & 25 deletions

File tree

src/main/java/com/retailsvc/http/OpenApiServer.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,9 +122,11 @@ record HandlerConfig(
122122

123123
this.shutdownTimeoutSeconds = shutdownTimeoutSeconds;
124124

125+
String host = httpServer.getAddress().getHostString();
126+
String displayHost = host.contains(":") ? "[" + host + "]" : host;
125127
LOG.info(
126128
"Server started ({}:{}) in {}ms",
127-
httpServer.getAddress().getHostString(),
129+
displayHost,
128130
httpServer.getAddress().getPort(),
129131
System.currentTimeMillis() - t0);
130132
}
@@ -134,8 +136,8 @@ public int listenPort() {
134136
}
135137

136138
/**
137-
* Returns the actual address the server is bound to, including any wildcard resolution by the
138-
* underlying {@link HttpServer}. Useful for verifying loopback restriction.
139+
* Returns the local address the server is bound to. For a wildcard-bound server this is the
140+
* wildcard address; for a loopback-bound server this is the loopback address.
139141
*/
140142
public InetAddress bindAddress() {
141143
return httpServer.getAddress().getAddress();

src/test/java/com/retailsvc/http/OpenApiServerTest.java

Lines changed: 19 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package com.retailsvc.http;
22

3+
import static java.net.http.HttpClient.Version.HTTP_1_1;
34
import static java.util.Collections.emptyMap;
5+
import static java.util.concurrent.Executors.newVirtualThreadPerTaskExecutor;
46
import static org.assertj.core.api.Assertions.assertThat;
57
import static org.assertj.core.api.Assertions.assertThatThrownBy;
68
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
@@ -10,6 +12,9 @@
1012
import java.net.HttpURLConnection;
1113
import java.net.InetAddress;
1214
import java.net.URI;
15+
import java.net.http.HttpClient;
16+
import java.net.http.HttpRequest;
17+
import java.net.http.HttpResponse;
1318
import java.util.List;
1419
import java.util.Map;
1520
import org.junit.jupiter.api.Test;
@@ -59,23 +64,28 @@ void testExceptionIsThrownOnInvalidHttpPort() {
5964
}
6065

6166
@Test
62-
void shouldBindOnlyToLoopbackWhenBindAddressIsLoopback() throws IOException {
67+
void shouldBindOnlyToLoopbackWhenBindAddressIsLoopback() throws Exception {
6368
try (var server =
6469
OpenApiServer.builder()
6570
.spec(testSpec())
6671
.handlers(emptyMap())
6772
.port(0)
6873
.bindAddress(InetAddress.getLoopbackAddress())
6974
.build()) {
75+
assertThat(server.bindAddress().isLoopbackAddress()).isTrue();
7076
int port = server.listenPort();
71-
HttpURLConnection conn =
72-
(HttpURLConnection)
73-
URI.create("http://127.0.0.1:" + port + "/api/missing").toURL().openConnection();
74-
try {
75-
assertThat(conn.getResponseCode()).isEqualTo(HttpURLConnection.HTTP_NOT_FOUND);
76-
} finally {
77-
conn.disconnect();
78-
}
77+
HttpClient client =
78+
HttpClient.newBuilder()
79+
.executor(newVirtualThreadPerTaskExecutor())
80+
.version(HTTP_1_1)
81+
.build();
82+
HttpRequest request =
83+
HttpRequest.newBuilder()
84+
.uri(URI.create("http://127.0.0.1:" + port + "/api/missing"))
85+
.GET()
86+
.build();
87+
HttpResponse<Void> response = client.send(request, HttpResponse.BodyHandlers.discarding());
88+
assertThat(response.statusCode()).isEqualTo(HttpURLConnection.HTTP_NOT_FOUND);
7989
}
8090
}
8191

@@ -87,19 +97,6 @@ void shouldBindToWildcardWhenBindAddressIsUnset() throws IOException {
8797
}
8898
}
8999

90-
@Test
91-
void shouldBindToWildcardWhenBindAddressIsExplicitlyNull() throws IOException {
92-
try (var server =
93-
OpenApiServer.builder()
94-
.spec(testSpec())
95-
.handlers(emptyMap())
96-
.port(0)
97-
.bindAddress(null)
98-
.build()) {
99-
assertThat(server.bindAddress().isAnyLocalAddress()).isTrue();
100-
}
101-
}
102-
103100
private Spec testSpec() {
104101
Map<String, Object> raw =
105102
Map.of(

0 commit comments

Comments
 (0)