You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by troodes <gi...@git.apache.org> on 2018/03/13 19:48:59 UTC
[GitHub] zookeeper pull request #489: ZOOKEEPER-2989: IPv6 literal address causes pro...
GitHub user troodes opened a pull request:
https://github.com/apache/zookeeper/pull/489
ZOOKEEPER-2989: IPv6 literal address causes problems for Quorum members
Handle tokenizing of "address:port" when address is an IPv6 literal address.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/troodes/zookeeper ZOOKEEPER-2989
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zookeeper/pull/489.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #489
----
commit 9e31f3448b6d5a950ea70ae715f26a8a0ca456d0
Author: Rick Trudeau <ri...@...>
Date: 2018-03-13T19:41:43Z
ZOOKEEPER-2989: IPv6 literal address causes problems for Quorum members
----
---
[GitHub] zookeeper pull request #489: ZOOKEEPER-2989: IPv6 literal address causes pro...
Posted by maoling <gi...@git.apache.org>.
Github user maoling commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/489#discussion_r194247374
--- Diff: src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java ---
@@ -222,15 +222,15 @@ static public InitialMessage parse(Long protocolVersion, DataInputStream din)
num_read, remaining, sid);
}
- // FIXME: IPv6 is not supported. Using something like Guava's HostAndPort
- // parser would be good.
String addr = new String(b);
- String[] host_port = addr.split(":");
-
- if (host_port.length != 2) {
- throw new InitialMessageException("Badly formed address: %s", addr);
+ int pos = addr.lastIndexOf(":");
+
+ if (pos == -1 || pos + 1 == (addr.length() )) {
--- End diff --
1. "I think the better way to do this is updating the initiateConnection function to pass in IPv6 literal address "[%s]:%s" -----pardon me.I don't get your idea.
2."here we can use QuorumPeer.splitWithLeadingHostname to check and get IPv6 host and port."-----IMHO,we should have a common class in the org.apache.zookeeper.server.util package which just like (HostAndPort#getHostAndPortFromBracketedHost)[https://github.com/google/guava/blob/master/guava/src/com/google/common/net/HostAndPort.java] does,then QuorumPeer.splitWithLeadingHostname , QuorumCnxManager.InitialMessage.parse and others can reuse it.
---
[GitHub] zookeeper issue #489: ZOOKEEPER-2989: IPv6 literal address causes problems f...
Posted by maoling <gi...@git.apache.org>.
Github user maoling commented on the issue:
https://github.com/apache/zookeeper/pull/489
@troodes can we move on ?
---
[GitHub] zookeeper issue #489: ZOOKEEPER-2989: IPv6 literal address causes problems f...
Posted by troodes <gi...@git.apache.org>.
Github user troodes commented on the issue:
https://github.com/apache/zookeeper/pull/489
Yes thanks @maoling , greatly appreciated.
---
[GitHub] zookeeper pull request #489: ZOOKEEPER-2989: IPv6 literal address causes pro...
Posted by lvfangmin <gi...@git.apache.org>.
Github user lvfangmin commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/489#discussion_r194216407
--- Diff: src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java ---
@@ -222,15 +222,15 @@ static public InitialMessage parse(Long protocolVersion, DataInputStream din)
num_read, remaining, sid);
}
- // FIXME: IPv6 is not supported. Using something like Guava's HostAndPort
- // parser would be good.
String addr = new String(b);
- String[] host_port = addr.split(":");
-
- if (host_port.length != 2) {
- throw new InitialMessageException("Badly formed address: %s", addr);
+ int pos = addr.lastIndexOf(":");
+
+ if (pos == -1 || pos + 1 == (addr.length() )) {
--- End diff --
I think the better way to do this is updating the initiateConnection function to pass in IPv6 literal address "[%s]:%s", and here we can use QuorumPeer.splitWithLeadingHostname to check and get IPv6 host and port.
---
[GitHub] zookeeper pull request #489: ZOOKEEPER-2989: IPv6 literal address causes pro...
Posted by lvfangmin <gi...@git.apache.org>.
Github user lvfangmin commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/489#discussion_r195968969
--- Diff: src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java ---
@@ -222,15 +222,15 @@ static public InitialMessage parse(Long protocolVersion, DataInputStream din)
num_read, remaining, sid);
}
- // FIXME: IPv6 is not supported. Using something like Guava's HostAndPort
- // parser would be good.
String addr = new String(b);
- String[] host_port = addr.split(":");
-
- if (host_port.length != 2) {
- throw new InitialMessageException("Badly formed address: %s", addr);
+ int pos = addr.lastIndexOf(":");
+
+ if (pos == -1 || pos + 1 == (addr.length() )) {
--- End diff --
@maoling I was suggesting to change the ip address format passed in from din to be [xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx]:xxxx instead of the one without '[]', and reuse the splitWithLeadingHostname code.
---
[GitHub] zookeeper issue #489: ZOOKEEPER-2989: IPv6 literal address causes problems f...
Posted by phunt <gi...@git.apache.org>.
Github user phunt commented on the issue:
https://github.com/apache/zookeeper/pull/489
Can someone else take this on? Also it would be good to have some IPv6 tests included as well.
---
[GitHub] zookeeper issue #489: ZOOKEEPER-2989: IPv6 literal address causes problems f...
Posted by maoling <gi...@git.apache.org>.
Github user maoling commented on the issue:
https://github.com/apache/zookeeper/pull/489
@phunt
I will pick it up if @troodes feel free or no reaction within three days
---