Copilot commented on code in PR #1509:
URL: https://github.com/apache/ratis/pull/1509#discussion_r3539964917
##########
ratis-shell/src/main/java/org/apache/ratis/shell/cli/CliUtils.java:
##########
@@ -164,11 +165,7 @@ public static void checkReply(RaftClientReply reply,
Supplier<String> message, P
/** Parse the given string as a {@link InetSocketAddress}. */
public static InetSocketAddress parseInetSocketAddress(String address) {
try {
- final String[] hostPortPair = address.split(":");
- if (hostPortPair.length < 2) {
- throw new IllegalArgumentException("Unexpected address format
<HOST:PORT>.");
- }
- return new InetSocketAddress(hostPortPair[0],
Integer.parseInt(hostPortPair[1]));
+ return NetUtils.createSocketAddr(address);
} catch (Exception e) {
throw new IllegalArgumentException("Failed to parse the server address
parameter \"" + address + "\".", e);
}
Review Comment:
`parseInetSocketAddress` now delegates to `NetUtils.createSocketAddr`, which
accepts URL-style inputs containing a scheme (e.g. `http://host:port`). Ratis
shell options/messages describe addresses as `<HOST:PORT>`, so accepting
schemes can silently mask invalid user input.
Consider explicitly rejecting inputs containing `://` before parsing, while
still using `NetUtils.createSocketAddr` for IPv6 bracket support.
##########
ratis-examples/src/main/java/org/apache/ratis/examples/common/SubCommandBase.java:
##########
@@ -41,26 +41,63 @@ public abstract class SubCommandBase {
private String peers;
public static RaftPeer[] parsePeers(String peers) {
- return Stream.of(peers.split(",")).map(address -> {
- String[] addressParts = address.split(":");
- if (addressParts.length < 3) {
- throw new IllegalArgumentException(
- "Raft peer " + address + " is not a legitimate format. "
- + "(format:
name:host:port:dataStreamPort:clientPort:adminPort)");
+ return
Stream.of(peers.split(",")).map(SubCommandBase::parsePeer).toArray(RaftPeer[]::new);
+ }
+
+ /**
+ * Parse a single peer definition in the format
+ * {@code name:host:port:dataStreamPort:clientPort:adminPort}, where the
trailing
+ * ports are optional. The host may be an IPv6 literal enclosed in brackets,
+ * e.g. {@code n0:[::1]:9000:9001:9002:9003}.
+ */
+ private static RaftPeer parsePeer(String address) {
+ final int idEnd = address.indexOf(':');
+ if (idEnd < 0) {
+ throw illegalFormat(address);
+ }
+ final String id = address.substring(0, idEnd);
+ final String hostAndPorts = address.substring(idEnd + 1);
+
+ // Separate the host from the port list, honoring IPv6 bracketed literals.
+ final String host;
+ final String portList;
+ if (hostAndPorts.startsWith("[")) {
+ final int bracketEnd = hostAndPorts.indexOf("]:");
+ if (bracketEnd < 0) {
+ throw illegalFormat(address);
}
- RaftPeer.Builder builder = RaftPeer.newBuilder();
- builder.setId(addressParts[0]).setAddress(addressParts[1] + ":" +
addressParts[2]);
- if (addressParts.length >= 4) {
- builder.setDataStreamAddress(addressParts[1] + ":" + addressParts[3]);
- if (addressParts.length >= 5) {
- builder.setClientAddress(addressParts[1] + ":" + addressParts[4]);
- if (addressParts.length >= 6) {
- builder.setAdminAddress(addressParts[1] + ":" + addressParts[5]);
- }
- }
+ host = hostAndPorts.substring(0, bracketEnd + 1); // include the closing
']'
+ portList = hostAndPorts.substring(bracketEnd + 2);
+ } else {
+ final int hostEnd = hostAndPorts.indexOf(':');
+ if (hostEnd < 0) {
+ throw illegalFormat(address);
}
- return builder.build();
- }).toArray(RaftPeer[]::new);
+ host = hostAndPorts.substring(0, hostEnd);
+ portList = hostAndPorts.substring(hostEnd + 1);
+ }
+
+ final String[] ports = portList.split(":");
+ if (ports[0].isEmpty()) {
+ throw illegalFormat(address);
+ }
+ final RaftPeer.Builder builder = RaftPeer.newBuilder();
+ builder.setId(id).setAddress(host + ":" + ports[0]);
+ if (ports.length >= 2) {
+ builder.setDataStreamAddress(host + ":" + ports[1]);
+ }
+ if (ports.length >= 3) {
+ builder.setClientAddress(host + ":" + ports[2]);
+ }
+ if (ports.length >= 4) {
+ builder.setAdminAddress(host + ":" + ports[3]);
+ }
Review Comment:
`parsePeer` only checks that the first port is non-empty. If a user supplies
an empty optional port (e.g. `n0:host:6000::6002` or `n0:[::1]:6000:`), the
code will build invalid addresses like `host:` which will fail later and be
harder to diagnose.
Reject empty values for any provided port segment (up to the supported 4
ports).
--
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]
