garydgregory commented on a change in pull request #41: [NET-405] Support for 
IPv6 in SubnetUtils
URL: https://github.com/apache/commons-net/pull/41#discussion_r324955428
 
 

 ##########
 File path: src/main/java/org/apache/commons/net/util/SubnetUtils.java
 ##########
 @@ -16,348 +16,344 @@
  */
 package org.apache.commons.net.util;
 
-import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
 /**
- * A class that performs some subnet calculations given a network address and 
a subnet mask.
- * @see "http://www.faqs.org/rfcs/rfc1519.html";
+ * This class that performs some subnet calculations given IP address in 
CIDR-notation.
+ * <p>For IPv4 address subnet, especially Classless Inter-Domain Routing 
(CIDR),
+ * refer to <a href="https://tools.ietf.org/html/rfc4632";>RFC4632</a>.</p>
+ * <p>For IPv6 address subnet, refer to <a 
href="https://tools.ietf.org/html/rfc4291#section-2.3";>
+ * Section 2.3 of RFC 4291</a>.</p>
+ *
  * @since 2.0
  */
-public class SubnetUtils {
-
-    private static final String IP_ADDRESS = 
"(\\d{1,3})\\.(\\d{1,3})\\.(\\d{1,3})\\.(\\d{1,3})";
-    private static final String SLASH_FORMAT = IP_ADDRESS + "/(\\d{1,2})"; // 
0 -> 32
-    private static final Pattern addressPattern = Pattern.compile(IP_ADDRESS);
-    private static final Pattern cidrPattern = Pattern.compile(SLASH_FORMAT);
-    private static final int NBITS = 32;
-
-    private final int netmask;
-    private final int address;
-    private final int network;
-    private final int broadcast;
-
-    /** Whether the broadcast/network address are included in host count */
-    private boolean inclusiveHostCount = false;
-
+public class SubnetUtils
+{
+
+    private static final String IPV4_ADDRESS = 
"(\\d{1,3}\\.){3}\\d{1,3}/\\d{1,2}";
+    private static final String IPV6_ADDRESS = 
"(([0-9a-f]{1,4}:){7}[0-9a-f]{1,4}|"
+                                               + "([0-9a-f]{1,4}:){1,7}:|"
+                                               + 
"([0-9a-f]{1,4}:){1,6}:[0-9a-f]{1,4}|"
+                                               + 
"([0-9a-f]{1,4}:){1,5}(:[0-9a-f]{1,4}){1,2}|"
+                                               + 
"([0-9a-f]{1,4}:){1,4}(:[0-9a-f]{1,4}){1,3}|"
+                                               + 
"([0-9a-f]{1,4}:){1,3}(:[0-9a-f]{1,4}){1,4}|"
+                                               + 
"([0-9a-f]{1,4}:){1,2}(:[0-9a-f]{1,4}){1,5}|"
+                                               + 
"[0-9a-f]{1,4}:((:[0-9a-f]{1,4}){1,6})|"
+                                               + 
":((:[0-9a-f]{1,4}){1,7}|:))/\\d{1,3}";
+
+    private final SubnetInfo subnetInfo;
 
     /**
-     * Constructor that takes a CIDR-notation string, e.g. "192.168.0.1/16"
-     * @param cidrNotation A CIDR-notation string, e.g. "192.168.0.1/16"
+     * Constructor that creates subnet summary information based on the 
provided IPv4 or IPv6 address in CIDR-notation,
+     * e.g. "192.168.0.1/16" or "2001:db8:0:0:0:ff00:42:8329/46"
+     * <p>
+     * NOTE: IPv6 address does NOT allow to omit consecutive sections of zeros 
in the current version.
+     *
+     * @param cidrNotation IPv4 or IPv6 address, e.g. "192.168.0.1/16" or 
"2001:db8:0:0:0:ff00:42:8329/46"
      * @throws IllegalArgumentException if the parameter is invalid,
-     * i.e. does not match n.n.n.n/m where n=1-3 decimal digits, m = 1-2 
decimal digits in range 0-32
+     * e.g. does not match either n.n.n.n/m where n = 1-3 decimal digits, m = 
1-2 decimal digits in range 0-32; or
+     * n:n:n:n:n:n:n:n/m n = 1-4 hexadecimal digits, m = 1-3 decimal digits in 
range 0-128.
      */
-    public SubnetUtils(String cidrNotation) {
-      Matcher matcher = cidrPattern.matcher(cidrNotation);
-
-      if (matcher.matches()) {
-          this.address = matchAddress(matcher);
-
-          /* Create a binary netmask from the number of bits specification /x 
*/
-
-          int trailingZeroes = NBITS - 
rangeCheck(Integer.parseInt(matcher.group(5)), 0, NBITS);
-          /*
-           * An IPv4 netmask consists of 32 bits, a contiguous sequence 
-           * of the specified number of ones followed by all zeros.
-           * So, it can be obtained by shifting an unsigned integer (32 bits) 
to the left by
-           * the number of trailing zeros which is (32 - the # bits 
specification).
-           * Note that there is no unsigned left shift operator, so we have to 
use
-           * a long to ensure that the left-most bit is shifted out correctly.
-           */
-          this.netmask = (int) (0x0FFFFFFFFL << trailingZeroes );
-
-          /* Calculate base network address */
-          this.network = (address & netmask);
-
-          /* Calculate broadcast address */
-          this.broadcast = network | ~(netmask);
-      } else {
-          throw new IllegalArgumentException("Could not parse [" + 
cidrNotation + "]");
-      }
+    public SubnetUtils(String cidrNotation)
+    {
+        subnetInfo = getByCIDRNotation(cidrNotation);
     }
 
     /**
-     * Constructor that takes a dotted decimal address and a dotted decimal 
mask.
-     * @param address An IP address, e.g. "192.168.0.1"
-     * @param mask A dotted decimal netmask e.g. "255.255.0.0"
+     * Constructor that creates IPv4 subnet summary information, given a 
dotted decimal address and mask.
+     *
+     * @param address an IP address, e.g. "192.168.0.1"
+     * @param mask a dotted decimal netmask e.g. "255.255.0.0"
      * @throws IllegalArgumentException if the address or mask is invalid,
-     * i.e. does not match n.n.n.n where n=1-3 decimal digits and the mask is 
not all zeros
+     * e.g. the address does not match n.n.n.n where n=1-3 decimal digits, or
+     * the mask does not match n.n.n.n which n={0, 128, 192, 224, 240, 248, 
252, 254, 255} and after the 0-field, it is all zeros.
      */
-    public SubnetUtils(String address, String mask) {
-        this.address = toInteger(address);
-        this.netmask = toInteger(mask);
-
-        if ((this.netmask & -this.netmask) - 1 != ~this.netmask) {
-            throw new IllegalArgumentException("Could not parse [" + mask + 
"]");
-        }
-
-        /* Calculate base network address */
-        this.network = (this.address & this.netmask);
-
-        /* Calculate broadcast address */
-        this.broadcast = this.network | ~(this.netmask);
+    public SubnetUtils(String address, String mask)
+    {
+        subnetInfo = new IP4Subnet(address, mask);
     }
 
-
     /**
-     * Returns <code>true</code> if the return value of {@link 
SubnetInfo#getAddressCount()}
+     * Returns {@code true} if the return value of {@link 
SubnetInfo#getAddressCountLong() getAddressCountLong}
      * includes the network and broadcast addresses.
+     *
+     * @return {@code true} if the host count includes the network and 
broadcast addresses
      * @since 2.2
-     * @return true if the host count includes the network and broadcast 
addresses
      */
-    public boolean isInclusiveHostCount() {
-        return inclusiveHostCount;
+    public boolean isInclusiveHostCount()
+    {
+        return subnetInfo.isInclusiveHostCount();
     }
 
     /**
-     * Set to <code>true</code> if you want the return value of {@link 
SubnetInfo#getAddressCount()}
+     * Set to {@code true} if you want the return value of {@link 
SubnetInfo#getAddressCountLong() getAddressCountLong}
      * to include the network and broadcast addresses.
-     * @param inclusiveHostCount true if network and broadcast addresses are 
to be included
+     *
+     * @param inclusiveHostCount {@code true} if network and broadcast 
addresses are to be included
      * @since 2.2
      */
-    public void setInclusiveHostCount(boolean inclusiveHostCount) {
-        this.inclusiveHostCount = inclusiveHostCount;
+    public void setInclusiveHostCount(boolean inclusiveHostCount)
+    {
+        subnetInfo.setInclusiveHostCount(inclusiveHostCount);
     }
 
-
+    /**
+     * Creates subnet summary information based on the provided IPv4 or IPv6 
address in CIDR-notation,
+     * e.g. "192.168.0.1/16" or "2001:db8:0:0:0:ff00:42:8329/46"
+     * <p>
+     * NOTE: IPv6 address does NOT allow to omit consecutive sections of zeros 
in the current version.
+     *
+     * @param cidrNotation IPv4 or IPv6 address
+     * @return a {@link SubnetInfo SubnetInfo} object created from the IP 
address.
+     * @since 3.7
+     */
+    public static SubnetInfo getByCIDRNotation(String cidrNotation)
+    {
+        if (Pattern.matches(IPV4_ADDRESS, cidrNotation))
+        {
+            return new IP4Subnet(cidrNotation);
+        } else if (Pattern.matches(IPV6_ADDRESS, cidrNotation))
+        {
+            return new IP6Subnet(cidrNotation);
+        } else
+        {
+            throw new IllegalArgumentException("Could not parse [" + 
cidrNotation + "]");
+        }
+    }
 
     /**
-     * Convenience container for subnet summary information.
+     * Creates IPv4 subnet summary information, given a dotted decimal address 
and mask.
      *
+     * @param address an IP address, e.g. "192.168.0.1"
+     * @param mask a dotted decimal netmask e.g. "255.255.0.0"
+     * @return an {@link IP4Subnet} object generated based on {@code address} 
and {@code mask}.
+     * @throws IllegalArgumentException if the address or mask is invalid,
+     * e.g. the address does not match n.n.n.n where n=1-3 decimal digits, or
+     * the mask does not match n.n.n.n which n={0, 128, 192, 224, 240, 248, 
252, 254, 255} and after the 0-field, it is all zeros.
+     * @since 3.7
      */
-    public final class SubnetInfo {
-        /* Mask to convert unsigned int to a long (i.e. keep 32 bits) */
-        private static final long UNSIGNED_INT_MASK = 0x0FFFFFFFFL;
+    public static IP4Subnet getByMask(String address, String mask)
+    {
+        return new IP4Subnet(address, mask);
+    }
 
-        private SubnetInfo() {}
+    /**
+     * Convenience container for subnet summary information.
+     */
+    public static class SubnetInfo
+    {
 
-        // long versions of the values (as unsigned int) which are more 
suitable for range checking
-        private long networkLong()  { return network &  UNSIGNED_INT_MASK; }
 
 Review comment:
   Deleting `private` methods does not break binary (or source) compatibility. 
Delete away... ;-)

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to