From 3ad6a075a12ddb5e5ab147c73e12515294345687 Mon Sep 17 00:00:00 2001 From: Siyao Meng <50227127+smengcl@users.noreply.github.com> Date: Tue, 7 Jul 2026 01:26:26 -0700 Subject: [PATCH 1/3] RATIS-2592. Ratis CLI and example peer-address parsers fail on IPv6 literal addresses --- .../ratis/examples/common/SubCommandBase.java | 73 ++++++++++++++----- .../org/apache/ratis/shell/cli/CliUtils.java | 7 +- .../cli/sh/local/RaftMetaConfCommand.java | 4 +- 3 files changed, 60 insertions(+), 24 deletions(-) diff --git a/ratis-examples/src/main/java/org/apache/ratis/examples/common/SubCommandBase.java b/ratis-examples/src/main/java/org/apache/ratis/examples/common/SubCommandBase.java index d650a5cfe8..8e82bd637c 100644 --- a/ratis-examples/src/main/java/org/apache/ratis/examples/common/SubCommandBase.java +++ b/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]); + } + return builder.build(); + } + + private static IllegalArgumentException illegalFormat(String address) { + return new IllegalArgumentException("Raft peer " + address + " is not a legitimate format. " + + "(format: name:host:port:dataStreamPort:clientPort:adminPort)"); } public RaftPeer[] getPeers() { diff --git a/ratis-shell/src/main/java/org/apache/ratis/shell/cli/CliUtils.java b/ratis-shell/src/main/java/org/apache/ratis/shell/cli/CliUtils.java index 1cecc665c6..df5bac11fb 100644 --- a/ratis-shell/src/main/java/org/apache/ratis/shell/cli/CliUtils.java +++ b/ratis-shell/src/main/java/org/apache/ratis/shell/cli/CliUtils.java @@ -24,6 +24,7 @@ import org.apache.ratis.protocol.RaftPeer; import org.apache.ratis.protocol.RaftPeerId; import org.apache.ratis.protocol.exceptions.RaftException; +import org.apache.ratis.util.NetUtils; import org.apache.ratis.util.function.CheckedFunction; import java.io.IOException; @@ -164,11 +165,7 @@ public static void checkReply(RaftClientReply reply, Supplier 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 ."); - } - 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); } diff --git a/ratis-shell/src/main/java/org/apache/ratis/shell/cli/sh/local/RaftMetaConfCommand.java b/ratis-shell/src/main/java/org/apache/ratis/shell/cli/sh/local/RaftMetaConfCommand.java index a63b659375..fd1ef78e6a 100644 --- a/ratis-shell/src/main/java/org/apache/ratis/shell/cli/sh/local/RaftMetaConfCommand.java +++ b/ratis-shell/src/main/java/org/apache/ratis/shell/cli/sh/local/RaftMetaConfCommand.java @@ -29,6 +29,7 @@ import org.apache.ratis.shell.cli.sh.command.AbstractCommand; import org.apache.ratis.shell.cli.sh.command.Context; import org.apache.ratis.thirdparty.com.google.protobuf.ByteString; +import org.apache.ratis.util.NetUtils; import java.io.IOException; import java.io.InputStream; @@ -91,7 +92,8 @@ public int run(CommandLine cl) throws IOException { } InetSocketAddress inetSocketAddress = CliUtils.parseInetSocketAddress( peerIdWithAddressArray[peerIdWithAddressArray.length - 1]); - String addressString = inetSocketAddress.getHostString() + ":" + inetSocketAddress.getPort(); + // Use address2String so that IPv6 literals are surrounded with '[', ']'. + String addressString = NetUtils.address2String(inetSocketAddress); if (addresses.contains(addressString)) { printf("Found duplicated address: %s. Please make sure the address of peer have no duplicated value.", addressString); From e494c5fa6bb6ece86ab6f042130969f0dfb18325 Mon Sep 17 00:00:00 2001 From: Siyao Meng <50227127+smengcl@users.noreply.github.com> Date: Tue, 7 Jul 2026 01:26:42 -0700 Subject: [PATCH 2/3] Add tests --- .../ratis/examples/common/TestSubCommand.java | 31 ++++++++++ .../apache/ratis/shell/cli/TestCliUtils.java | 58 +++++++++++++++++++ .../cli/sh/LocalCommandIntegrationTest.java | 35 +++++++++++ 3 files changed, 124 insertions(+) create mode 100644 ratis-test/src/test/java/org/apache/ratis/shell/cli/TestCliUtils.java diff --git a/ratis-examples/src/test/java/org/apache/ratis/examples/common/TestSubCommand.java b/ratis-examples/src/test/java/org/apache/ratis/examples/common/TestSubCommand.java index c47dbb82a1..6855e5ee3f 100644 --- a/ratis-examples/src/test/java/org/apache/ratis/examples/common/TestSubCommand.java +++ b/ratis-examples/src/test/java/org/apache/ratis/examples/common/TestSubCommand.java @@ -21,7 +21,9 @@ import java.util.Collection; import java.util.Collections; +import org.apache.ratis.protocol.RaftPeer; import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; @@ -38,4 +40,33 @@ public void testParsePeers(String peers) { () -> SubCommandBase.parsePeers(peers)); } + @Test + public void testParseIpv4Peer() { + final RaftPeer[] peers = SubCommandBase.parsePeers("n0:127.0.0.1:6000:6001:6002:6003"); + Assertions.assertEquals(1, peers.length); + Assertions.assertEquals("n0", peers[0].getId().toString()); + Assertions.assertEquals("127.0.0.1:6000", peers[0].getAddress()); + Assertions.assertEquals("127.0.0.1:6001", peers[0].getDataStreamAddress()); + Assertions.assertEquals("127.0.0.1:6002", peers[0].getClientAddress()); + Assertions.assertEquals("127.0.0.1:6003", peers[0].getAdminAddress()); + } + + @Test + public void testParseIpv6Peer() { + final RaftPeer[] peers = SubCommandBase.parsePeers("n0:[::1]:6000:6001:6002:6003"); + Assertions.assertEquals(1, peers.length); + Assertions.assertEquals("n0", peers[0].getId().toString()); + Assertions.assertEquals("[::1]:6000", peers[0].getAddress()); + Assertions.assertEquals("[::1]:6001", peers[0].getDataStreamAddress()); + Assertions.assertEquals("[::1]:6002", peers[0].getClientAddress()); + Assertions.assertEquals("[::1]:6003", peers[0].getAdminAddress()); + } + + @Test + public void testParseMixedPeers() { + final RaftPeer[] peers = SubCommandBase.parsePeers("n0:[::1]:6000,n1:127.0.0.1:6001"); + Assertions.assertEquals(2, peers.length); + Assertions.assertEquals("[::1]:6000", peers[0].getAddress()); + Assertions.assertEquals("127.0.0.1:6001", peers[1].getAddress()); + } } diff --git a/ratis-test/src/test/java/org/apache/ratis/shell/cli/TestCliUtils.java b/ratis-test/src/test/java/org/apache/ratis/shell/cli/TestCliUtils.java new file mode 100644 index 0000000000..d22bbd1c02 --- /dev/null +++ b/ratis-test/src/test/java/org/apache/ratis/shell/cli/TestCliUtils.java @@ -0,0 +1,58 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.ratis.shell.cli; + +import org.apache.ratis.protocol.RaftPeer; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.net.Inet4Address; +import java.net.Inet6Address; +import java.net.InetSocketAddress; +import java.util.List; + +public class TestCliUtils { + + @Test + public void testParseIpv4Address() { + final InetSocketAddress addr = CliUtils.parseInetSocketAddress("127.0.0.1:6000"); + Assertions.assertEquals(6000, addr.getPort()); + Assertions.assertTrue(addr.getAddress() instanceof Inet4Address); + } + + @Test + public void testParseIpv6Address() { + final InetSocketAddress addr = CliUtils.parseInetSocketAddress("[::1]:6000"); + Assertions.assertEquals(6000, addr.getPort()); + Assertions.assertTrue(addr.getAddress() instanceof Inet6Address); + } + + @Test + public void testParseIpv6Peers() { + final List peers = CliUtils.parseRaftPeers("[::1]:6000,[::1]:6001"); + Assertions.assertEquals(2, peers.size()); + Assertions.assertEquals(6000, CliUtils.parseInetSocketAddress(peers.get(0).getAddress()).getPort()); + Assertions.assertEquals(6001, CliUtils.parseInetSocketAddress(peers.get(1).getAddress()).getPort()); + } + + @Test + public void testParseMissingPort() { + Assertions.assertThrows(IllegalArgumentException.class, + () -> CliUtils.parseInetSocketAddress("127.0.0.1")); + } +} diff --git a/ratis-test/src/test/java/org/apache/ratis/shell/cli/sh/LocalCommandIntegrationTest.java b/ratis-test/src/test/java/org/apache/ratis/shell/cli/sh/LocalCommandIntegrationTest.java index afc13837c5..b4c1cd4ccf 100644 --- a/ratis-test/src/test/java/org/apache/ratis/shell/cli/sh/LocalCommandIntegrationTest.java +++ b/ratis-test/src/test/java/org/apache/ratis/shell/cli/sh/LocalCommandIntegrationTest.java @@ -22,6 +22,7 @@ import org.apache.ratis.proto.RaftProtos.RaftConfigurationProto; import org.apache.ratis.proto.RaftProtos.RaftPeerProto; import org.apache.ratis.thirdparty.com.google.protobuf.ByteString; +import org.apache.ratis.util.NetUtils; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -29,6 +30,8 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.net.Inet6Address; +import java.net.InetSocketAddress; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; @@ -121,6 +124,38 @@ public void testRunMethod(@TempDir Path tempDir) throws Exception { } + @Test + public void testRunMethodWithIpv6(@TempDir Path tempDir) throws Exception { + final int index = 1; + generateRaftConf(tempDir.resolve(RAFT_META_CONF), index); + + // IPv6 literals must be enclosed in brackets, e.g. [2001:db8::1]:9872. + final String peersListStr = "peer1_ID|[2001:db8::1]:9872,peer2_ID|[2001:db8::2]:9873"; + StringPrintStream out = new StringPrintStream(); + RatisShell shell = new RatisShell(out.getPrintStream()); + int ret = shell.run("local", "raftMetaConf", "-peers", peersListStr, "-path", tempDir.toString()); + Assertions.assertEquals(0, ret); + + final List peers; + try (InputStream in = Files.newInputStream(tempDir.resolve(NEW_RAFT_META_CONF))) { + peers = LogEntryProto.newBuilder().mergeFrom(in).build() + .getConfigurationEntry().getPeersList(); + } + Assertions.assertEquals(2, peers.size()); + + // Each written address must stay bracketed and round-trip to an IPv6 address with the same port. + final int[] expectedPorts = {9872, 9873}; + for (int i = 0; i < peers.size(); i++) { + final String address = peers.get(i).getAddress(); + Assertions.assertTrue(address.startsWith("[") && address.contains("]:"), + () -> "IPv6 address is not bracketed: " + address); + final InetSocketAddress parsed = NetUtils.createSocketAddr(address); + Assertions.assertTrue(parsed.getAddress() instanceof Inet6Address, + () -> "Not an IPv6 address: " + address); + Assertions.assertEquals(expectedPorts[i], parsed.getPort()); + } + } + private void generateRaftConf(Path path, int index) throws IOException { Map map = new HashMap<>(); map.put("peer1_ID", "host1:9872"); From c6bc1ccbfac3564a57038c954d35eda6864c2c59 Mon Sep 17 00:00:00 2001 From: Siyao Meng <50227127+smengcl@users.noreply.github.com> Date: Tue, 7 Jul 2026 15:23:26 -0700 Subject: [PATCH 3/3] Address copilot comments --- .../apache/ratis/examples/common/SubCommandBase.java | 10 +++++++--- .../apache/ratis/examples/common/TestSubCommand.java | 12 ++++++++++++ .../java/org/apache/ratis/shell/cli/CliUtils.java | 5 +++++ .../org/apache/ratis/shell/cli/TestCliUtils.java | 6 ++++++ 4 files changed, 30 insertions(+), 3 deletions(-) diff --git a/ratis-examples/src/main/java/org/apache/ratis/examples/common/SubCommandBase.java b/ratis-examples/src/main/java/org/apache/ratis/examples/common/SubCommandBase.java index 8e82bd637c..0755cbd8ba 100644 --- a/ratis-examples/src/main/java/org/apache/ratis/examples/common/SubCommandBase.java +++ b/ratis-examples/src/main/java/org/apache/ratis/examples/common/SubCommandBase.java @@ -77,9 +77,13 @@ private static RaftPeer parsePeer(String address) { portList = hostAndPorts.substring(hostEnd + 1); } - final String[] ports = portList.split(":"); - if (ports[0].isEmpty()) { - throw illegalFormat(address); + // Use -1 limit so trailing/embedded empty segments (e.g. "6000:" or "6000::6002") + // are preserved and can be rejected rather than silently building "host:". + final String[] ports = portList.split(":", -1); + for (String port : ports) { + if (port.isEmpty()) { + throw illegalFormat(address); + } } final RaftPeer.Builder builder = RaftPeer.newBuilder(); builder.setId(id).setAddress(host + ":" + ports[0]); diff --git a/ratis-examples/src/test/java/org/apache/ratis/examples/common/TestSubCommand.java b/ratis-examples/src/test/java/org/apache/ratis/examples/common/TestSubCommand.java index 6855e5ee3f..59806b71b7 100644 --- a/ratis-examples/src/test/java/org/apache/ratis/examples/common/TestSubCommand.java +++ b/ratis-examples/src/test/java/org/apache/ratis/examples/common/TestSubCommand.java @@ -69,4 +69,16 @@ public void testParseMixedPeers() { Assertions.assertEquals("[::1]:6000", peers[0].getAddress()); Assertions.assertEquals("127.0.0.1:6001", peers[1].getAddress()); } + + @Test + public void testParseEmptyEmbeddedPortRejected() { + Assertions.assertThrows(IllegalArgumentException.class, + () -> SubCommandBase.parsePeers("n0:127.0.0.1:6000::6002")); + } + + @Test + public void testParseEmptyTrailingPortRejected() { + Assertions.assertThrows(IllegalArgumentException.class, + () -> SubCommandBase.parsePeers("n0:[::1]:6000:")); + } } diff --git a/ratis-shell/src/main/java/org/apache/ratis/shell/cli/CliUtils.java b/ratis-shell/src/main/java/org/apache/ratis/shell/cli/CliUtils.java index df5bac11fb..a4a30ae776 100644 --- a/ratis-shell/src/main/java/org/apache/ratis/shell/cli/CliUtils.java +++ b/ratis-shell/src/main/java/org/apache/ratis/shell/cli/CliUtils.java @@ -165,6 +165,11 @@ public static void checkReply(RaftClientReply reply, Supplier message, P /** Parse the given string as a {@link InetSocketAddress}. */ public static InetSocketAddress parseInetSocketAddress(String address) { try { + // NetUtils.createSocketAddr also accepts scheme://host:port; the shell only + // expects , so reject a scheme to avoid masking invalid input. + if (address.contains("://")) { + throw new IllegalArgumentException("Unexpected scheme in \"" + address + "\"; expected format ."); + } return NetUtils.createSocketAddr(address); } catch (Exception e) { throw new IllegalArgumentException("Failed to parse the server address parameter \"" + address + "\".", e); diff --git a/ratis-test/src/test/java/org/apache/ratis/shell/cli/TestCliUtils.java b/ratis-test/src/test/java/org/apache/ratis/shell/cli/TestCliUtils.java index d22bbd1c02..151914b64a 100644 --- a/ratis-test/src/test/java/org/apache/ratis/shell/cli/TestCliUtils.java +++ b/ratis-test/src/test/java/org/apache/ratis/shell/cli/TestCliUtils.java @@ -55,4 +55,10 @@ public void testParseMissingPort() { Assertions.assertThrows(IllegalArgumentException.class, () -> CliUtils.parseInetSocketAddress("127.0.0.1")); } + + @Test + public void testParseRejectsScheme() { + Assertions.assertThrows(IllegalArgumentException.class, + () -> CliUtils.parseInetSocketAddress("http://127.0.0.1:6000")); + } }