Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 3.2.1
    • Fix Version/s: 4.0.0
    • Component/s: Framework
    • Labels:
      None
    • Environment:
      ZooKeeper 3.5.1-alpha

      Description

      I've noticed an issue with Curator 3.2.1 which relates to the fix from CURATOR-345 (also reported by me).

      When we would reconnect after losing connection to Zookeeper (due to network issues), our services would always have the wrong connection string, and never manage to reconnect to the Zookeeper cluster. Assuming that 10.1.2.3 is our zookeeper server, and we have two scenarios (with different zoo.cfg files) we were seeing the following results when a reconnection was established:

      Scenario 1: ClientCnxn - Opening socket connection to server 0.0.0.0/0.0.0.0:2181.
      Scenario 2: ClientCnxn - Opening socket connection to server 10.1.2.3/10.1.2.3:2888.

      Obviously these are both undesirable connection strings, as both are wrong. The issue arises in the EnsembleTracker.processConfigData() when we reconnect to Zookeeper. The config coming from zookeeper is in the format:

      server.<positive id> = <address1>:<port1>:<port2>[:role];[<client port address>:]<client port>

      As we can see, both [:role] and [<client port address>:] are optional. Hence, the following string is perfectly valid:

      server.1=10.1.2.3:2888:3888:participant;2181

      When Zookeeper sends this, it defaults the clientAddress to 0.0.0.0, so we retrieve the following value in EnsembleTracker:

      server.1=10.1.2.3:2888:3888:participant;0.0.0.0:2181

      The resulting connection string, therefore, turns in to 0.0.0.0:2181 instead of 10.1.2.3:2181, and Curator creates a new ZooKeeper to connect to that IP - which obviously never works.

      In the second scenario, our connection string looks a bit different. It is wrong according to the docs, but is valid:

      server.1=10.1.2.3:2888:3888:participant

      Now, this is missing the client port and address. That means that the resulting string from the EnsembleTracker is 10.1.2.3:2888 - which isn't desired. Including the port would just lead to the above scenario.

      From what I can see, the EnsembleTracker.configToConnectionString() method is the issue here:

      InetSocketAddress address = Objects.firstNonNull(server.clientAddr, server.addr);
                  sb.append(address.getAddress().getHostAddress()).append(":").append(address.getPort());
      

      In the above cases, both the server.Addr and server.clientAddr values are wrong. We also prefer the value of clientAddr for some reason, which doesn't look right to me (given that it can be 0.0.0.0 or 127.0.0.1).

      It seems to me that Curator should use server.Addr.getHostAddress() with server.clientAddr.getPort(). When the clientAddr is missing, however, I'm not sure what should be done.

        Issue Links

          Activity

          Hide
          randgalt Jordan Zimmerman added a comment -

          Hi Alex Rankin - we don't accept patch files. Please open a Pull Request on Github. See here: https://cwiki.apache.org/confluence/display/CURATOR/Submitting+Pull+Requests

          Show
          randgalt Jordan Zimmerman added a comment - Hi Alex Rankin - we don't accept patch files. Please open a Pull Request on Github. See here: https://cwiki.apache.org/confluence/display/CURATOR/Submitting+Pull+Requests
          Hide
          Kenco Alex Rankin added a comment -

          I've attached a patch (second one was fixing inconsistent formatting) of a proposed update, along with some test cases. These tests only cover the configToConnectionString() method though, and not the processConfigData() method.

          The changes do the following:

          • Print the properties in plaintext, instead of a byte array (I believe there is an issue for this already - it wasn't applied in 3.2.1).
          • If the properties are emtpy, we ignore.
          • If not, we get the connectionString:
            • We check the server.clientAddr is not null (which can happen). If it is, we skip that server. There should probably be a log warning here, but none of the loggers are static and I didn't want to change that, since it seems to be the standard.
            • If not, we check if it's a wildcard address (0.0.0.0).
            • If it is, we use the server.addr.
            • If it isn't, we use the server.clientAddr.
          • If the returned connectionString's length is 0, then we log a warning, as the received configuration was invalid (no client ports given).
          • If the returned connectionString's length is more than 0, we set the newConfig and the connectionString.
          Show
          Kenco Alex Rankin added a comment - I've attached a patch (second one was fixing inconsistent formatting) of a proposed update, along with some test cases. These tests only cover the configToConnectionString() method though, and not the processConfigData() method. The changes do the following: Print the properties in plaintext, instead of a byte array (I believe there is an issue for this already - it wasn't applied in 3.2.1). If the properties are emtpy, we ignore. If not, we get the connectionString: We check the server.clientAddr is not null (which can happen). If it is, we skip that server. There should probably be a log warning here, but none of the loggers are static and I didn't want to change that, since it seems to be the standard. If not, we check if it's a wildcard address (0.0.0.0). If it is, we use the server.addr. If it isn't, we use the server.clientAddr. If the returned connectionString's length is 0, then we log a warning, as the received configuration was invalid (no client ports given). If the returned connectionString's length is more than 0, we set the newConfig and the connectionString.
          Hide
          Kenco Alex Rankin added a comment - - edited

          Jordan Zimmerman - Ah, sorry. I must have got confused with the Zookeeper board. I'll open a pull request then.

          *Edit:* Master seems to be significantly different from the tagged 3.2.1 and 3.3.0 releases, so I'm not sure how to proceed. Don't have a huge amount of time, so I'll try look when I can. I can provide more details on the solution though if anyone picks it up.

          Show
          Kenco Alex Rankin added a comment - - edited Jordan Zimmerman - Ah, sorry. I must have got confused with the Zookeeper board. I'll open a pull request then. * Edit: * Master seems to be significantly different from the tagged 3.2.1 and 3.3.0 releases, so I'm not sure how to proceed. Don't have a huge amount of time, so I'll try look when I can. I can provide more details on the solution though if anyone picks it up.
          Hide
          randgalt Jordan Zimmerman added a comment - - edited

          Master seems to be significantly different

          We have two active branches. "master" corresponds to Curator 2.x and "CURATOR-3.0" corresponds to Curator 3.x. In general, all patches should be against master. We then merge master into CURATOR-3.0. For a change such as this, work off of CURATOR-3.0 only.

          Show
          randgalt Jordan Zimmerman added a comment - - edited Master seems to be significantly different We have two active branches. "master" corresponds to Curator 2.x and " CURATOR-3 .0" corresponds to Curator 3.x. In general, all patches should be against master. We then merge master into CURATOR-3 .0. For a change such as this, work off of CURATOR-3 .0 only.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user Vile2539 opened a pull request:

          https://github.com/apache/curator/pull/205

          CURATOR-392 Fixing connection string construction for wildcard client addresses

          The client address in Zookeeper can be a wildcard (0.0.0.0), and if this is used by Curator, it will attempt to create a new connection to 0.0.0.0 instead of the correct server address. It is also possible that no client address or port is set, which should be counted as invalid by Curator, since the appropriate information isn't contained in the data sent.

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/Vile2539/curator CURATOR-392

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/curator/pull/205.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 #205


          commit 9b73d11bc03c6e37567e237e96c36cdf93e4ff66
          Author: Kenco <davelister@gmail.com>
          Date: 2017-03-13T20:51:42Z

          Fixing connection string construction to ignore wildcard client addresses


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user Vile2539 opened a pull request: https://github.com/apache/curator/pull/205 CURATOR-392 Fixing connection string construction for wildcard client addresses The client address in Zookeeper can be a wildcard (0.0.0.0), and if this is used by Curator, it will attempt to create a new connection to 0.0.0.0 instead of the correct server address. It is also possible that no client address or port is set, which should be counted as invalid by Curator, since the appropriate information isn't contained in the data sent. You can merge this pull request into a Git repository by running: $ git pull https://github.com/Vile2539/curator CURATOR-392 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/curator/pull/205.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 #205 commit 9b73d11bc03c6e37567e237e96c36cdf93e4ff66 Author: Kenco <davelister@gmail.com> Date: 2017-03-13T20:51:42Z Fixing connection string construction to ignore wildcard client addresses
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user hanm commented on a diff in the pull request:

          https://github.com/apache/curator/pull/205#discussion_r126865680

          — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/EnsembleTracker.java —
          @@ -166,12 +166,26 @@ public static String configToConnectionString(QuorumVerifier data) throws Except
          StringBuilder sb = new StringBuilder();
          for ( QuorumPeer.QuorumServer server : data.getAllMembers().values() )
          {
          + if ( server.clientAddr == null )
          + {
          + // Invalid client address configuration in zoo.cfg
          + continue;
          — End diff –

          The spec of server address is server.<positive id> = <address1>:<port1>:<port2>[:role];[<client port address>:]<client port>, and if server.clientAddr is null then it means the client port address is not present. The client port address is optional, so the address is still a valid address.

          Show
          githubbot ASF GitHub Bot added a comment - Github user hanm commented on a diff in the pull request: https://github.com/apache/curator/pull/205#discussion_r126865680 — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/EnsembleTracker.java — @@ -166,12 +166,26 @@ public static String configToConnectionString(QuorumVerifier data) throws Except StringBuilder sb = new StringBuilder(); for ( QuorumPeer.QuorumServer server : data.getAllMembers().values() ) { + if ( server.clientAddr == null ) + { + // Invalid client address configuration in zoo.cfg + continue; — End diff – The spec of server address is server.<positive id> = <address1>:<port1>:<port2> [:role] ; [<client port address>:] <client port>, and if server.clientAddr is null then it means the client port address is not present. The client port address is optional, so the address is still a valid address.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user hanm commented on a diff in the pull request:

          https://github.com/apache/curator/pull/205#discussion_r126865752

          — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/EnsembleTracker.java —
          @@ -166,12 +166,26 @@ public static String configToConnectionString(QuorumVerifier data) throws Except
          StringBuilder sb = new StringBuilder();
          for ( QuorumPeer.QuorumServer server : data.getAllMembers().values() )
          {
          + if ( server.clientAddr == null )
          +

          { + // Invalid client address configuration in zoo.cfg + continue; + }

          if ( sb.length() != 0 )

          { sb.append(","); }
          • InetSocketAddress address = Objects.firstNonNull(server.clientAddr, server.addr);
          • sb.append(address.getAddress().getHostAddress()).append(":").append(address.getPort());
            + InetAddress wildcardAddress = new InetSocketAddress(0).getAddress();
              • End diff –

          I think you can just use server.addr which is guaranteed not null, and then use getHostAddress to get the hostAddress.

          Show
          githubbot ASF GitHub Bot added a comment - Github user hanm commented on a diff in the pull request: https://github.com/apache/curator/pull/205#discussion_r126865752 — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/EnsembleTracker.java — @@ -166,12 +166,26 @@ public static String configToConnectionString(QuorumVerifier data) throws Except StringBuilder sb = new StringBuilder(); for ( QuorumPeer.QuorumServer server : data.getAllMembers().values() ) { + if ( server.clientAddr == null ) + { + // Invalid client address configuration in zoo.cfg + continue; + } if ( sb.length() != 0 ) { sb.append(","); } InetSocketAddress address = Objects.firstNonNull(server.clientAddr, server.addr); sb.append(address.getAddress().getHostAddress()).append(":").append(address.getPort()); + InetAddress wildcardAddress = new InetSocketAddress(0).getAddress(); End diff – I think you can just use server.addr which is guaranteed not null, and then use getHostAddress to get the hostAddress.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Vile2539 commented on the issue:

          https://github.com/apache/curator/pull/205

          It's been about 4 months, so I'm a bit fuzzy on the exact details.

          > The spec of server address is server. = ::[:role];[:], and if server.clientAddr is null then it means the client port address is not present. The client port address is optional, so the address is still a valid address.

          The problem is that EnsembleTracker is getting the connection string for itself to Zookeeper. If it receives a config without a valid client port, then it can't create a new connection string to that client. Since Zookeeper isn't sending the clientPort configured outside of that string (in this case), we can't use any of the connection string.

          > I think you can just use server.addr which is guaranteed not null, and then use getHostAddress to get the hostAddress.

          I'm not entirely sure about that. We want to use the server.clientAddr ideally - unless it's a wildcard, in which case we use the server.addr. It's not about one of them being null, but the clientAddr should be favoured over the server.addr (for example - we might have clientAddr set to localhost to accept only local connections).

          Show
          githubbot ASF GitHub Bot added a comment - Github user Vile2539 commented on the issue: https://github.com/apache/curator/pull/205 It's been about 4 months, so I'm a bit fuzzy on the exact details. > The spec of server address is server. = :: [:role] ; [:] , and if server.clientAddr is null then it means the client port address is not present. The client port address is optional, so the address is still a valid address. The problem is that EnsembleTracker is getting the connection string for itself to Zookeeper. If it receives a config without a valid client port, then it can't create a new connection string to that client. Since Zookeeper isn't sending the clientPort configured outside of that string (in this case), we can't use any of the connection string. > I think you can just use server.addr which is guaranteed not null, and then use getHostAddress to get the hostAddress. I'm not entirely sure about that. We want to use the server.clientAddr ideally - unless it's a wildcard, in which case we use the server.addr. It's not about one of them being null, but the clientAddr should be favoured over the server.addr (for example - we might have clientAddr set to localhost to accept only local connections).
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Randgalt commented on a diff in the pull request:

          https://github.com/apache/curator/pull/205#discussion_r127054869

          — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/EnsembleTracker.java —
          @@ -166,12 +166,26 @@ public static String configToConnectionString(QuorumVerifier data) throws Except
          StringBuilder sb = new StringBuilder();
          for ( QuorumPeer.QuorumServer server : data.getAllMembers().values() )
          {
          + if ( server.clientAddr == null )
          +

          { + // Invalid client address configuration in zoo.cfg + continue; + }

          if ( sb.length() != 0 )

          { sb.append(","); }
          • InetSocketAddress address = Objects.firstNonNull(server.clientAddr, server.addr);
          • sb.append(address.getAddress().getHostAddress()).append(":").append(address.getPort());
            + InetAddress wildcardAddress = new InetSocketAddress(0).getAddress();
            + String hostAddress;
            + if ( wildcardAddress.equals(server.clientAddr.getAddress()) )
            + { + hostAddress = server.addr.getAddress().getHostAddress(); + }

            + else
            +

            { + hostAddress = server.clientAddr.getAddress().getHostAddress(); + }

            + sb.append(hostAddress).append(":").append(server.clientAddr.getPort());

              • End diff –

          Isn't calling `getHostAddress()` really expensive? Is this necessary?

          Show
          githubbot ASF GitHub Bot added a comment - Github user Randgalt commented on a diff in the pull request: https://github.com/apache/curator/pull/205#discussion_r127054869 — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/EnsembleTracker.java — @@ -166,12 +166,26 @@ public static String configToConnectionString(QuorumVerifier data) throws Except StringBuilder sb = new StringBuilder(); for ( QuorumPeer.QuorumServer server : data.getAllMembers().values() ) { + if ( server.clientAddr == null ) + { + // Invalid client address configuration in zoo.cfg + continue; + } if ( sb.length() != 0 ) { sb.append(","); } InetSocketAddress address = Objects.firstNonNull(server.clientAddr, server.addr); sb.append(address.getAddress().getHostAddress()).append(":").append(address.getPort()); + InetAddress wildcardAddress = new InetSocketAddress(0).getAddress(); + String hostAddress; + if ( wildcardAddress.equals(server.clientAddr.getAddress()) ) + { + hostAddress = server.addr.getAddress().getHostAddress(); + } + else + { + hostAddress = server.clientAddr.getAddress().getHostAddress(); + } + sb.append(hostAddress).append(":").append(server.clientAddr.getPort()); End diff – Isn't calling `getHostAddress()` really expensive? Is this necessary?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Randgalt commented on a diff in the pull request:

          https://github.com/apache/curator/pull/205#discussion_r127055979

          — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/EnsembleTracker.java —
          @@ -166,12 +166,26 @@ public static String configToConnectionString(QuorumVerifier data) throws Except
          StringBuilder sb = new StringBuilder();
          for ( QuorumPeer.QuorumServer server : data.getAllMembers().values() )
          {
          + if ( server.clientAddr == null )
          +

          { + // Invalid client address configuration in zoo.cfg + continue; + }

          if ( sb.length() != 0 )

          { sb.append(","); }
          • InetSocketAddress address = Objects.firstNonNull(server.clientAddr, server.addr);
          • sb.append(address.getAddress().getHostAddress()).append(":").append(address.getPort());
            + InetAddress wildcardAddress = new InetSocketAddress(0).getAddress();
              • End diff –

          So, @hanm you're saying the original code is correct?

          Show
          githubbot ASF GitHub Bot added a comment - Github user Randgalt commented on a diff in the pull request: https://github.com/apache/curator/pull/205#discussion_r127055979 — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/EnsembleTracker.java — @@ -166,12 +166,26 @@ public static String configToConnectionString(QuorumVerifier data) throws Except StringBuilder sb = new StringBuilder(); for ( QuorumPeer.QuorumServer server : data.getAllMembers().values() ) { + if ( server.clientAddr == null ) + { + // Invalid client address configuration in zoo.cfg + continue; + } if ( sb.length() != 0 ) { sb.append(","); } InetSocketAddress address = Objects.firstNonNull(server.clientAddr, server.addr); sb.append(address.getAddress().getHostAddress()).append(":").append(address.getPort()); + InetAddress wildcardAddress = new InetSocketAddress(0).getAddress(); End diff – So, @hanm you're saying the original code is correct?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Vile2539 commented on a diff in the pull request:

          https://github.com/apache/curator/pull/205#discussion_r127058434

          — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/EnsembleTracker.java —
          @@ -166,12 +166,26 @@ public static String configToConnectionString(QuorumVerifier data) throws Except
          StringBuilder sb = new StringBuilder();
          for ( QuorumPeer.QuorumServer server : data.getAllMembers().values() )
          {
          + if ( server.clientAddr == null )
          +

          { + // Invalid client address configuration in zoo.cfg + continue; + }

          if ( sb.length() != 0 )

          { sb.append(","); }
          • InetSocketAddress address = Objects.firstNonNull(server.clientAddr, server.addr);
          • sb.append(address.getAddress().getHostAddress()).append(":").append(address.getPort());
            + InetAddress wildcardAddress = new InetSocketAddress(0).getAddress();
            + String hostAddress;
            + if ( wildcardAddress.equals(server.clientAddr.getAddress()) )
            + { + hostAddress = server.addr.getAddress().getHostAddress(); + }

            + else
            +

            { + hostAddress = server.clientAddr.getAddress().getHostAddress(); + }

            + sb.append(hostAddress).append(":").append(server.clientAddr.getPort());

              • End diff –

          I don't believe getHostAddress() involves any DNS lookups, hostname resolution, etc. It just gets the String representation of the byte array on the InetAddressHolder. I may be mistaken about this though.

          Show
          githubbot ASF GitHub Bot added a comment - Github user Vile2539 commented on a diff in the pull request: https://github.com/apache/curator/pull/205#discussion_r127058434 — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/EnsembleTracker.java — @@ -166,12 +166,26 @@ public static String configToConnectionString(QuorumVerifier data) throws Except StringBuilder sb = new StringBuilder(); for ( QuorumPeer.QuorumServer server : data.getAllMembers().values() ) { + if ( server.clientAddr == null ) + { + // Invalid client address configuration in zoo.cfg + continue; + } if ( sb.length() != 0 ) { sb.append(","); } InetSocketAddress address = Objects.firstNonNull(server.clientAddr, server.addr); sb.append(address.getAddress().getHostAddress()).append(":").append(address.getPort()); + InetAddress wildcardAddress = new InetSocketAddress(0).getAddress(); + String hostAddress; + if ( wildcardAddress.equals(server.clientAddr.getAddress()) ) + { + hostAddress = server.addr.getAddress().getHostAddress(); + } + else + { + hostAddress = server.clientAddr.getAddress().getHostAddress(); + } + sb.append(hostAddress).append(":").append(server.clientAddr.getPort()); End diff – I don't believe getHostAddress() involves any DNS lookups, hostname resolution, etc. It just gets the String representation of the byte array on the InetAddressHolder. I may be mistaken about this though.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Randgalt commented on a diff in the pull request:

          https://github.com/apache/curator/pull/205#discussion_r127061717

          — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/EnsembleTracker.java —
          @@ -166,12 +166,26 @@ public static String configToConnectionString(QuorumVerifier data) throws Except
          StringBuilder sb = new StringBuilder();
          for ( QuorumPeer.QuorumServer server : data.getAllMembers().values() )
          {
          + if ( server.clientAddr == null )
          +

          { + // Invalid client address configuration in zoo.cfg + continue; + }

          if ( sb.length() != 0 )

          { sb.append(","); }
          • InetSocketAddress address = Objects.firstNonNull(server.clientAddr, server.addr);
          • sb.append(address.getAddress().getHostAddress()).append(":").append(address.getPort());
            + InetAddress wildcardAddress = new InetSocketAddress(0).getAddress();
            + String hostAddress;
            + if ( wildcardAddress.equals(server.clientAddr.getAddress()) )
            + { + hostAddress = server.addr.getAddress().getHostAddress(); + }

            + else
            +

            { + hostAddress = server.clientAddr.getAddress().getHostAddress(); + }

            + sb.append(hostAddress).append(":").append(server.clientAddr.getPort());

              • End diff –

          @Vile2539 you're right

          Show
          githubbot ASF GitHub Bot added a comment - Github user Randgalt commented on a diff in the pull request: https://github.com/apache/curator/pull/205#discussion_r127061717 — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/EnsembleTracker.java — @@ -166,12 +166,26 @@ public static String configToConnectionString(QuorumVerifier data) throws Except StringBuilder sb = new StringBuilder(); for ( QuorumPeer.QuorumServer server : data.getAllMembers().values() ) { + if ( server.clientAddr == null ) + { + // Invalid client address configuration in zoo.cfg + continue; + } if ( sb.length() != 0 ) { sb.append(","); } InetSocketAddress address = Objects.firstNonNull(server.clientAddr, server.addr); sb.append(address.getAddress().getHostAddress()).append(":").append(address.getPort()); + InetAddress wildcardAddress = new InetSocketAddress(0).getAddress(); + String hostAddress; + if ( wildcardAddress.equals(server.clientAddr.getAddress()) ) + { + hostAddress = server.addr.getAddress().getHostAddress(); + } + else + { + hostAddress = server.clientAddr.getAddress().getHostAddress(); + } + sb.append(hostAddress).append(":").append(server.clientAddr.getPort()); End diff – @Vile2539 you're right
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user hanm commented on a diff in the pull request:

          https://github.com/apache/curator/pull/205#discussion_r127122870

          — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/EnsembleTracker.java —
          @@ -166,12 +166,26 @@ public static String configToConnectionString(QuorumVerifier data) throws Except
          StringBuilder sb = new StringBuilder();
          for ( QuorumPeer.QuorumServer server : data.getAllMembers().values() )
          {
          + if ( server.clientAddr == null )
          +

          { + // Invalid client address configuration in zoo.cfg + continue; + }

          if ( sb.length() != 0 )

          { sb.append(","); }
          • InetSocketAddress address = Objects.firstNonNull(server.clientAddr, server.addr);
          • sb.append(address.getAddress().getHostAddress()).append(":").append(address.getPort());
            + InetAddress wildcardAddress = new InetSocketAddress(0).getAddress();
              • End diff –

          I checked spec again, and I agree with what @Vile2539 commented - we should try clientAddr first and then fall back if necessary. So the code change in this pull request here looks good to me.

          Show
          githubbot ASF GitHub Bot added a comment - Github user hanm commented on a diff in the pull request: https://github.com/apache/curator/pull/205#discussion_r127122870 — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/EnsembleTracker.java — @@ -166,12 +166,26 @@ public static String configToConnectionString(QuorumVerifier data) throws Except StringBuilder sb = new StringBuilder(); for ( QuorumPeer.QuorumServer server : data.getAllMembers().values() ) { + if ( server.clientAddr == null ) + { + // Invalid client address configuration in zoo.cfg + continue; + } if ( sb.length() != 0 ) { sb.append(","); } InetSocketAddress address = Objects.firstNonNull(server.clientAddr, server.addr); sb.append(address.getAddress().getHostAddress()).append(":").append(address.getPort()); + InetAddress wildcardAddress = new InetSocketAddress(0).getAddress(); End diff – I checked spec again, and I agree with what @Vile2539 commented - we should try clientAddr first and then fall back if necessary. So the code change in this pull request here looks good to me.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user hanm commented on a diff in the pull request:

          https://github.com/apache/curator/pull/205#discussion_r127124498

          — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/EnsembleTracker.java —
          @@ -166,12 +166,26 @@ public static String configToConnectionString(QuorumVerifier data) throws Except
          StringBuilder sb = new StringBuilder();
          for ( QuorumPeer.QuorumServer server : data.getAllMembers().values() )
          {
          + if ( server.clientAddr == null )
          + {
          + // Invalid client address configuration in zoo.cfg
          + continue;
          — End diff –

          I see what you mean - if the client_port_address is not present ZK will not initialize server.clientAddress, which is where you extract the client port. This is not a problem for ZK itself because it has the knowledge of clientPort of a quorum peer which is not publicly accessible.

          I think QuorumSever can provide APIs so you guys can use the APIs to get the client / server address / port rather than relying on the clientAddr etc fields. Before that happens it's fine to skip as there is no way to extract the client port (and I think that only happens when clientAddress is not explicitly present in the config string - cases like server.1= 125.23.63.23:1234:1235;1236)

          Show
          githubbot ASF GitHub Bot added a comment - Github user hanm commented on a diff in the pull request: https://github.com/apache/curator/pull/205#discussion_r127124498 — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/EnsembleTracker.java — @@ -166,12 +166,26 @@ public static String configToConnectionString(QuorumVerifier data) throws Except StringBuilder sb = new StringBuilder(); for ( QuorumPeer.QuorumServer server : data.getAllMembers().values() ) { + if ( server.clientAddr == null ) + { + // Invalid client address configuration in zoo.cfg + continue; — End diff – I see what you mean - if the client_port_address is not present ZK will not initialize server.clientAddress, which is where you extract the client port. This is not a problem for ZK itself because it has the knowledge of clientPort of a quorum peer which is not publicly accessible. I think QuorumSever can provide APIs so you guys can use the APIs to get the client / server address / port rather than relying on the clientAddr etc fields. Before that happens it's fine to skip as there is no way to extract the client port (and I think that only happens when clientAddress is not explicitly present in the config string - cases like server.1= 125.23.63.23:1234:1235;1236)
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user lvfangmin commented on a diff in the pull request:

          https://github.com/apache/curator/pull/205#discussion_r127384661

          — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/EnsembleTracker.java —
          @@ -166,12 +166,26 @@ public static String configToConnectionString(QuorumVerifier data) throws Except
          StringBuilder sb = new StringBuilder();
          for ( QuorumPeer.QuorumServer server : data.getAllMembers().values() )
          {
          + if ( server.clientAddr == null )
          +

          { + // Invalid client address configuration in zoo.cfg + continue; + }

          if ( sb.length() != 0 )

          { sb.append(","); }
          • InetSocketAddress address = Objects.firstNonNull(server.clientAddr, server.addr);
          • sb.append(address.getAddress().getHostAddress()).append(":").append(address.getPort());
            + InetAddress wildcardAddress = new InetSocketAddress(0).getAddress();
            + String hostAddress;
            + if ( wildcardAddress.equals(server.clientAddr.getAddress()) )
              • End diff –

          If the clientAddr is in IPv6 format, will this equal? Or we should also check the Inet6Address, would be great if you can add a test case with IPv6 wildcard.

          Show
          githubbot ASF GitHub Bot added a comment - Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/curator/pull/205#discussion_r127384661 — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/EnsembleTracker.java — @@ -166,12 +166,26 @@ public static String configToConnectionString(QuorumVerifier data) throws Except StringBuilder sb = new StringBuilder(); for ( QuorumPeer.QuorumServer server : data.getAllMembers().values() ) { + if ( server.clientAddr == null ) + { + // Invalid client address configuration in zoo.cfg + continue; + } if ( sb.length() != 0 ) { sb.append(","); } InetSocketAddress address = Objects.firstNonNull(server.clientAddr, server.addr); sb.append(address.getAddress().getHostAddress()).append(":").append(address.getPort()); + InetAddress wildcardAddress = new InetSocketAddress(0).getAddress(); + String hostAddress; + if ( wildcardAddress.equals(server.clientAddr.getAddress()) ) End diff – If the clientAddr is in IPv6 format, will this equal? Or we should also check the Inet6Address, would be great if you can add a test case with IPv6 wildcard.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user lvfangmin commented on a diff in the pull request:

          https://github.com/apache/curator/pull/205#discussion_r127384585

          — Diff: curator-framework/src/test/java/org/apache/curator/framework/imps/TestReconfiguration.java —
          @@ -313,6 +313,38 @@ public void testNewMembers() throws Exception
          }
          }

          + @Test
          + public void testConfigToConnectionStringNormal() throws Exception
          +

          { + String config = "server.1=10.1.2.3:2888:3888:participant;10.2.3.4:2181"; + String configString = EnsembleTracker.configToConnectionString(toQuorumVerifier(config.getBytes())); + Assert.assertEquals("10.2.3.4:2181", configString); + }

          +
          + @Test
          + public void testConfigToConnectionStringNoClientAddr() throws Exception
          +

          { + String config = "server.1=10.1.2.3:2888:3888:participant;2181"; + String configString = EnsembleTracker.configToConnectionString(toQuorumVerifier(config.getBytes())); + Assert.assertEquals("10.1.2.3:2181", configString); + }

          +
          + @Test
          + public void testConfigToConnectionStringWildcardClientAddr() throws Exception
          + {
          + String config = "server.1=10.1.2.3:2888:3888:participant;0.0.0.0:2181";
          — End diff –

          Please also add a test case for IPv6 wildcard:

          server.1=[1010:0001:0002:0003:0004:0005:0006:0007]:2888:3888:participant;[::0]:2181

          Show
          githubbot ASF GitHub Bot added a comment - Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/curator/pull/205#discussion_r127384585 — Diff: curator-framework/src/test/java/org/apache/curator/framework/imps/TestReconfiguration.java — @@ -313,6 +313,38 @@ public void testNewMembers() throws Exception } } + @Test + public void testConfigToConnectionStringNormal() throws Exception + { + String config = "server.1=10.1.2.3:2888:3888:participant;10.2.3.4:2181"; + String configString = EnsembleTracker.configToConnectionString(toQuorumVerifier(config.getBytes())); + Assert.assertEquals("10.2.3.4:2181", configString); + } + + @Test + public void testConfigToConnectionStringNoClientAddr() throws Exception + { + String config = "server.1=10.1.2.3:2888:3888:participant;2181"; + String configString = EnsembleTracker.configToConnectionString(toQuorumVerifier(config.getBytes())); + Assert.assertEquals("10.1.2.3:2181", configString); + } + + @Test + public void testConfigToConnectionStringWildcardClientAddr() throws Exception + { + String config = "server.1=10.1.2.3:2888:3888:participant;0.0.0.0:2181"; — End diff – Please also add a test case for IPv6 wildcard: server.1= [1010:0001:0002:0003:0004:0005:0006:0007] :2888:3888:participant; [::0] :2181
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Vile2539 commented on a diff in the pull request:

          https://github.com/apache/curator/pull/205#discussion_r127615377

          — Diff: curator-framework/src/test/java/org/apache/curator/framework/imps/TestReconfiguration.java —
          @@ -313,6 +313,38 @@ public void testNewMembers() throws Exception
          }
          }

          + @Test
          + public void testConfigToConnectionStringNormal() throws Exception
          +

          { + String config = "server.1=10.1.2.3:2888:3888:participant;10.2.3.4:2181"; + String configString = EnsembleTracker.configToConnectionString(toQuorumVerifier(config.getBytes())); + Assert.assertEquals("10.2.3.4:2181", configString); + }

          +
          + @Test
          + public void testConfigToConnectionStringNoClientAddr() throws Exception
          +

          { + String config = "server.1=10.1.2.3:2888:3888:participant;2181"; + String configString = EnsembleTracker.configToConnectionString(toQuorumVerifier(config.getBytes())); + Assert.assertEquals("10.1.2.3:2181", configString); + }

          +
          + @Test
          + public void testConfigToConnectionStringWildcardClientAddr() throws Exception
          + {
          + String config = "server.1=10.1.2.3:2888:3888:participant;0.0.0.0:2181";
          — End diff –

          It doesn't look like Zookeeper supports IPv6 addresses in 3.5.1-alpha (which is what Curator seems to be using). This appears to be fixed in Zookeeper 3.5.2 and 3.6.0:

          https://issues.apache.org/jira/browse/ZOOKEEPER-1460

          Show
          githubbot ASF GitHub Bot added a comment - Github user Vile2539 commented on a diff in the pull request: https://github.com/apache/curator/pull/205#discussion_r127615377 — Diff: curator-framework/src/test/java/org/apache/curator/framework/imps/TestReconfiguration.java — @@ -313,6 +313,38 @@ public void testNewMembers() throws Exception } } + @Test + public void testConfigToConnectionStringNormal() throws Exception + { + String config = "server.1=10.1.2.3:2888:3888:participant;10.2.3.4:2181"; + String configString = EnsembleTracker.configToConnectionString(toQuorumVerifier(config.getBytes())); + Assert.assertEquals("10.2.3.4:2181", configString); + } + + @Test + public void testConfigToConnectionStringNoClientAddr() throws Exception + { + String config = "server.1=10.1.2.3:2888:3888:participant;2181"; + String configString = EnsembleTracker.configToConnectionString(toQuorumVerifier(config.getBytes())); + Assert.assertEquals("10.1.2.3:2181", configString); + } + + @Test + public void testConfigToConnectionStringWildcardClientAddr() throws Exception + { + String config = "server.1=10.1.2.3:2888:3888:participant;0.0.0.0:2181"; — End diff – It doesn't look like Zookeeper supports IPv6 addresses in 3.5.1-alpha (which is what Curator seems to be using). This appears to be fixed in Zookeeper 3.5.2 and 3.6.0: https://issues.apache.org/jira/browse/ZOOKEEPER-1460
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Randgalt commented on a diff in the pull request:

          https://github.com/apache/curator/pull/205#discussion_r127615418

          — Diff: curator-framework/src/test/java/org/apache/curator/framework/imps/TestReconfiguration.java —
          @@ -313,6 +313,38 @@ public void testNewMembers() throws Exception
          }
          }

          + @Test
          + public void testConfigToConnectionStringNormal() throws Exception
          +

          { + String config = "server.1=10.1.2.3:2888:3888:participant;10.2.3.4:2181"; + String configString = EnsembleTracker.configToConnectionString(toQuorumVerifier(config.getBytes())); + Assert.assertEquals("10.2.3.4:2181", configString); + }

          +
          + @Test
          + public void testConfigToConnectionStringNoClientAddr() throws Exception
          +

          { + String config = "server.1=10.1.2.3:2888:3888:participant;2181"; + String configString = EnsembleTracker.configToConnectionString(toQuorumVerifier(config.getBytes())); + Assert.assertEquals("10.1.2.3:2181", configString); + }

          +
          + @Test
          + public void testConfigToConnectionStringWildcardClientAddr() throws Exception
          + {
          + String config = "server.1=10.1.2.3:2888:3888:participant;0.0.0.0:2181";
          — End diff –

          Curator will be at 3.5.3-beta for the next release. That has change since this PR has started. feel free to increment the ZK version in the main pom.

          Show
          githubbot ASF GitHub Bot added a comment - Github user Randgalt commented on a diff in the pull request: https://github.com/apache/curator/pull/205#discussion_r127615418 — Diff: curator-framework/src/test/java/org/apache/curator/framework/imps/TestReconfiguration.java — @@ -313,6 +313,38 @@ public void testNewMembers() throws Exception } } + @Test + public void testConfigToConnectionStringNormal() throws Exception + { + String config = "server.1=10.1.2.3:2888:3888:participant;10.2.3.4:2181"; + String configString = EnsembleTracker.configToConnectionString(toQuorumVerifier(config.getBytes())); + Assert.assertEquals("10.2.3.4:2181", configString); + } + + @Test + public void testConfigToConnectionStringNoClientAddr() throws Exception + { + String config = "server.1=10.1.2.3:2888:3888:participant;2181"; + String configString = EnsembleTracker.configToConnectionString(toQuorumVerifier(config.getBytes())); + Assert.assertEquals("10.1.2.3:2181", configString); + } + + @Test + public void testConfigToConnectionStringWildcardClientAddr() throws Exception + { + String config = "server.1=10.1.2.3:2888:3888:participant;0.0.0.0:2181"; — End diff – Curator will be at 3.5.3-beta for the next release. That has change since this PR has started. feel free to increment the ZK version in the main pom.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Vile2539 commented on a diff in the pull request:

          https://github.com/apache/curator/pull/205#discussion_r127615903

          — Diff: curator-framework/src/test/java/org/apache/curator/framework/imps/TestReconfiguration.java —
          @@ -313,6 +313,38 @@ public void testNewMembers() throws Exception
          }
          }

          + @Test
          + public void testConfigToConnectionStringNormal() throws Exception
          +

          { + String config = "server.1=10.1.2.3:2888:3888:participant;10.2.3.4:2181"; + String configString = EnsembleTracker.configToConnectionString(toQuorumVerifier(config.getBytes())); + Assert.assertEquals("10.2.3.4:2181", configString); + }

          +
          + @Test
          + public void testConfigToConnectionStringNoClientAddr() throws Exception
          +

          { + String config = "server.1=10.1.2.3:2888:3888:participant;2181"; + String configString = EnsembleTracker.configToConnectionString(toQuorumVerifier(config.getBytes())); + Assert.assertEquals("10.1.2.3:2181", configString); + }

          +
          + @Test
          + public void testConfigToConnectionStringWildcardClientAddr() throws Exception
          + {
          + String config = "server.1=10.1.2.3:2888:3888:participant;0.0.0.0:2181";
          — End diff –

          I seem to be getting a NPE with 3.5.3-beta when attempting to construct the TestingZooKeeperServer:

          java.lang.NullPointerException
          at org.apache.zookeeper.server.quorum.QuorumPeerMain.runFromConfig(QuorumPeerMain.java:158)
          at org.apache.curator.test.TestingZooKeeperServer$1.run(TestingZooKeeperServer.java:150)
          at java.lang.Thread.run(Thread.java:745)

          It looks like is a known issue, but I don't understand the fix version of 3.4.0 when it occurred on 3.5.3-beta only - I assume that should be 3.5.4-beta:

          https://issues.apache.org/jira/browse/CURATOR-409

          I'm not really sure how I should proceed here. Updating the pom would break a lot of the tests, and I can't add IPv6 tests without the updated Zookeeper version supporting IPv6.

          Show
          githubbot ASF GitHub Bot added a comment - Github user Vile2539 commented on a diff in the pull request: https://github.com/apache/curator/pull/205#discussion_r127615903 — Diff: curator-framework/src/test/java/org/apache/curator/framework/imps/TestReconfiguration.java — @@ -313,6 +313,38 @@ public void testNewMembers() throws Exception } } + @Test + public void testConfigToConnectionStringNormal() throws Exception + { + String config = "server.1=10.1.2.3:2888:3888:participant;10.2.3.4:2181"; + String configString = EnsembleTracker.configToConnectionString(toQuorumVerifier(config.getBytes())); + Assert.assertEquals("10.2.3.4:2181", configString); + } + + @Test + public void testConfigToConnectionStringNoClientAddr() throws Exception + { + String config = "server.1=10.1.2.3:2888:3888:participant;2181"; + String configString = EnsembleTracker.configToConnectionString(toQuorumVerifier(config.getBytes())); + Assert.assertEquals("10.1.2.3:2181", configString); + } + + @Test + public void testConfigToConnectionStringWildcardClientAddr() throws Exception + { + String config = "server.1=10.1.2.3:2888:3888:participant;0.0.0.0:2181"; — End diff – I seem to be getting a NPE with 3.5.3-beta when attempting to construct the TestingZooKeeperServer: java.lang.NullPointerException at org.apache.zookeeper.server.quorum.QuorumPeerMain.runFromConfig(QuorumPeerMain.java:158) at org.apache.curator.test.TestingZooKeeperServer$1.run(TestingZooKeeperServer.java:150) at java.lang.Thread.run(Thread.java:745) It looks like is a known issue, but I don't understand the fix version of 3.4.0 when it occurred on 3.5.3-beta only - I assume that should be 3.5.4-beta: https://issues.apache.org/jira/browse/CURATOR-409 I'm not really sure how I should proceed here. Updating the pom would break a lot of the tests, and I can't add IPv6 tests without the updated Zookeeper version supporting IPv6.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Randgalt commented on a diff in the pull request:

          https://github.com/apache/curator/pull/205#discussion_r127618052

          — Diff: curator-framework/src/test/java/org/apache/curator/framework/imps/TestReconfiguration.java —
          @@ -313,6 +313,38 @@ public void testNewMembers() throws Exception
          }
          }

          + @Test
          + public void testConfigToConnectionStringNormal() throws Exception
          +

          { + String config = "server.1=10.1.2.3:2888:3888:participant;10.2.3.4:2181"; + String configString = EnsembleTracker.configToConnectionString(toQuorumVerifier(config.getBytes())); + Assert.assertEquals("10.2.3.4:2181", configString); + }

          +
          + @Test
          + public void testConfigToConnectionStringNoClientAddr() throws Exception
          +

          { + String config = "server.1=10.1.2.3:2888:3888:participant;2181"; + String configString = EnsembleTracker.configToConnectionString(toQuorumVerifier(config.getBytes())); + Assert.assertEquals("10.1.2.3:2181", configString); + }

          +
          + @Test
          + public void testConfigToConnectionStringWildcardClientAddr() throws Exception
          + {
          + String config = "server.1=10.1.2.3:2888:3888:participant;0.0.0.0:2181";
          — End diff –

          @Vile2539 CURATOR-3.0 branch has been discontinued. "master" is now what used to be CURATOR-3.0

          Show
          githubbot ASF GitHub Bot added a comment - Github user Randgalt commented on a diff in the pull request: https://github.com/apache/curator/pull/205#discussion_r127618052 — Diff: curator-framework/src/test/java/org/apache/curator/framework/imps/TestReconfiguration.java — @@ -313,6 +313,38 @@ public void testNewMembers() throws Exception } } + @Test + public void testConfigToConnectionStringNormal() throws Exception + { + String config = "server.1=10.1.2.3:2888:3888:participant;10.2.3.4:2181"; + String configString = EnsembleTracker.configToConnectionString(toQuorumVerifier(config.getBytes())); + Assert.assertEquals("10.2.3.4:2181", configString); + } + + @Test + public void testConfigToConnectionStringNoClientAddr() throws Exception + { + String config = "server.1=10.1.2.3:2888:3888:participant;2181"; + String configString = EnsembleTracker.configToConnectionString(toQuorumVerifier(config.getBytes())); + Assert.assertEquals("10.1.2.3:2181", configString); + } + + @Test + public void testConfigToConnectionStringWildcardClientAddr() throws Exception + { + String config = "server.1=10.1.2.3:2888:3888:participant;0.0.0.0:2181"; — End diff – @Vile2539 CURATOR-3 .0 branch has been discontinued. "master" is now what used to be CURATOR-3 .0
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Vile2539 commented on a diff in the pull request:

          https://github.com/apache/curator/pull/205#discussion_r127640820

          — Diff: curator-framework/src/test/java/org/apache/curator/framework/imps/TestReconfiguration.java —
          @@ -313,6 +313,38 @@ public void testNewMembers() throws Exception
          }
          }

          + @Test
          + public void testConfigToConnectionStringNormal() throws Exception
          +

          { + String config = "server.1=10.1.2.3:2888:3888:participant;10.2.3.4:2181"; + String configString = EnsembleTracker.configToConnectionString(toQuorumVerifier(config.getBytes())); + Assert.assertEquals("10.2.3.4:2181", configString); + }

          +
          + @Test
          + public void testConfigToConnectionStringNoClientAddr() throws Exception
          +

          { + String config = "server.1=10.1.2.3:2888:3888:participant;2181"; + String configString = EnsembleTracker.configToConnectionString(toQuorumVerifier(config.getBytes())); + Assert.assertEquals("10.1.2.3:2181", configString); + }

          +
          + @Test
          + public void testConfigToConnectionStringWildcardClientAddr() throws Exception
          + {
          + String config = "server.1=10.1.2.3:2888:3888:participant;0.0.0.0:2181";
          — End diff –

          @Randgalt Ahh right. I've changed the base branch now and I'll have a look into the tests after work.

          Show
          githubbot ASF GitHub Bot added a comment - Github user Vile2539 commented on a diff in the pull request: https://github.com/apache/curator/pull/205#discussion_r127640820 — Diff: curator-framework/src/test/java/org/apache/curator/framework/imps/TestReconfiguration.java — @@ -313,6 +313,38 @@ public void testNewMembers() throws Exception } } + @Test + public void testConfigToConnectionStringNormal() throws Exception + { + String config = "server.1=10.1.2.3:2888:3888:participant;10.2.3.4:2181"; + String configString = EnsembleTracker.configToConnectionString(toQuorumVerifier(config.getBytes())); + Assert.assertEquals("10.2.3.4:2181", configString); + } + + @Test + public void testConfigToConnectionStringNoClientAddr() throws Exception + { + String config = "server.1=10.1.2.3:2888:3888:participant;2181"; + String configString = EnsembleTracker.configToConnectionString(toQuorumVerifier(config.getBytes())); + Assert.assertEquals("10.1.2.3:2181", configString); + } + + @Test + public void testConfigToConnectionStringWildcardClientAddr() throws Exception + { + String config = "server.1=10.1.2.3:2888:3888:participant;0.0.0.0:2181"; — End diff – @Randgalt Ahh right. I've changed the base branch now and I'll have a look into the tests after work.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Vile2539 commented on the issue:

          https://github.com/apache/curator/pull/205

          @Randgalt Updated pull request base to master.

          @lvfangmin Changed the wildcard check to use the [isAnyLocalAddress\(\)](https://docs.oracle.com/javase/7/docs/api/java/net/InetAddress.html#isAnyLocalAddress()) method of InetAddress, which works for IPv4 and IPv6:

          > Utility routine to check if the InetAddress in a wildcard address. (sic)

          Also added additional tests to make sure that both IPv4 and IPv6 wildcards are recognised, that both work together, and that regular IPv6 addresses work.

          Let me know if there's anything else you would like added.

          Show
          githubbot ASF GitHub Bot added a comment - Github user Vile2539 commented on the issue: https://github.com/apache/curator/pull/205 @Randgalt Updated pull request base to master. @lvfangmin Changed the wildcard check to use the [isAnyLocalAddress\(\)] ( https://docs.oracle.com/javase/7/docs/api/java/net/InetAddress.html#isAnyLocalAddress( )) method of InetAddress, which works for IPv4 and IPv6: > Utility routine to check if the InetAddress in a wildcard address. (sic) Also added additional tests to make sure that both IPv4 and IPv6 wildcards are recognised, that both work together, and that regular IPv6 addresses work. Let me know if there's anything else you would like added.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Randgalt commented on the issue:

          https://github.com/apache/curator/pull/205

          thanks @Vile2539 - I'll run the tests and, assuming they pass, I'll merge. Anyone have any last objections to merging? @hanm or @lvfangmin ?

          Show
          githubbot ASF GitHub Bot added a comment - Github user Randgalt commented on the issue: https://github.com/apache/curator/pull/205 thanks @Vile2539 - I'll run the tests and, assuming they pass, I'll merge. Anyone have any last objections to merging? @hanm or @lvfangmin ?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/curator/pull/205

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/curator/pull/205

            People

            • Assignee:
              Unassigned
              Reporter:
              Kenco Alex Rankin
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development