Copilot commented on code in PR #832:
URL:
https://github.com/apache/rocketmq-dashboard/pull/832#discussion_r3702707919
##########
server/src/main/java/org/apache/rocketmq/studio/cluster/proxy/ProxyAddressService.java:
##########
@@ -25,11 +25,18 @@
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
@Slf4j
@Service
public class ProxyAddressService {
+ private static final Pattern PROXY_ADDR_PATTERN =
+
Pattern.compile("^(\\[[0-9a-fA-F:.]+]|[A-Za-z0-9._-]+):(\\d{1,5})$");
Review Comment:
The IPv6 portion of the regex (`\\[[0-9a-fA-F:.]+]`) is overly permissive
and will accept strings that are not valid IPv6 literals (e.g., `[....]:8081`,
`[::::]:8081`, or `[1.2.3.4]:8081`). This conflicts with the PR goal of
validating address *format* before persisting. Consider validating bracketed
hosts by parsing the IPv6 literal (e.g., via `InetAddress` and requiring an
`Inet6Address`) or tightening the pattern to reject clearly-non-IPv6 bracket
contents.
##########
server/src/test/java/org/apache/rocketmq/studio/cluster/proxy/ProxyAddressServiceTest.java:
##########
@@ -53,6 +55,33 @@ void addProxyAddrShouldRejectBlankAddress() {
.satisfies(ex -> assertThat(((BusinessException)
ex).getCode()).isEqualTo(400));
}
+ @Test
+ void addProxyAddrShouldAcceptBracketedIpv6Address() {
+ proxyAddressService.addProxyAddr(" [::1]:8081 ");
+
+ ProxyHomeVO home = proxyAddressService.getHomePage();
+ assertThat(home.getProxyAddrList()).containsExactly("127.0.0.1:8081",
"[::1]:8081");
Review Comment:
This test is brittle because it couples to the service’s default address
list and its ordering (`containsExactly`). If the default seed address changes
or the list ordering changes, this test will fail even if IPv6 acceptance still
works. Prefer asserting that the list contains the new value (and optionally
still contains the default) without assuming exact ordering/contents, or
explicitly reset/initialize the service state for the test.
##########
server/src/main/java/org/apache/rocketmq/studio/cluster/proxy/ProxyAddressService.java:
##########
@@ -64,6 +71,15 @@ private String normalizeProxyAddr(String proxyAddr, String
fieldName) {
if (proxyAddr == null || proxyAddr.trim().isEmpty()) {
throw new BusinessException(400, fieldName + " is required");
}
- return proxyAddr.trim();
+ String normalized = proxyAddr.trim();
+ Matcher matcher = PROXY_ADDR_PATTERN.matcher(normalized);
+ if (!matcher.matches()) {
+ throw new BusinessException(400, fieldName + " must be in
host:port or [ipv6]:port format");
Review Comment:
The error message doesn’t include the invalid value (or a safely
truncated/sanitized representation), which can make debugging difficult for API
consumers. Consider including the rejected input (after trimming) or a more
specific reason (e.g., missing port vs. invalid characters) while keeping the
response safe to echo.
--
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]
