Repository: netty/netty
Issue: IpV4Subnet.contains() always returns true for single-address subnets (3.10.4)
Labels: defect

I may have this wrong (since I'm fairly new to network programming) but my understanding is that a /32 CIDR block represents a single IP address, so I would expect an IpV4Subnet instance with this range to only match that exact IP - but, for example

new IpV4Subnet("1.1.1.1/32").contains("10.0.16.64")

both return true (at least in the version I tested, netty-3.10.4) The same is true for an IpV4Subnet instance constructed with mask notation (eg 1.1.1.1/255.255.255.255).


Issue comments:
checking...

Verified that the issue not exists in 4.x. Looking into 3.10.x now.

Hi @normanmaurer I would like to know if there are any plans to fix this soon as I would like to see 3.10.6.Final released as soon as it is humanly possible.

I had no time yet... If you want help submit a PR 👍

> Am 11.04.2016 um 15:30 schrieb Guido Medina notifications@github.com:
> 
> Hi @normanmaurer I would like to know if there are any plans to fix this soon as I would like to see 3.10.6.Final released as soon as it is humanly possible.
> 
> —
> You are receiving this because you were mentioned.
> Reply to this email directly or view it on GitHub

Fixed by https://github.com/netty/netty/pull/5451


Pull request review comments:
This is a copy & paste from `4.0` branch, I noticed it preferred direct access vs `for-each byte` which creates an iterator.

This is the actual bug fix, to not return true when `mask == -1` which is also a copy & paste from `4.0` branch.


Patch that resolved the issue:
diff --git a/src/main/java/org/jboss/netty/handler/ipfilter/IpV4Subnet.java b/src/main/java/org/jboss/netty/handler/ipfilter/IpV4Subnet.java
index 09dfdf2bb25..51d30964c04 100644
--- a/src/main/java/org/jboss/netty/handler/ipfilter/IpV4Subnet.java
+++ b/src/main/java/org/jboss/netty/handler/ipfilter/IpV4Subnet.java
@@ -15,8 +15,6 @@
  */
 package org.jboss.netty.handler.ipfilter;
 
-import org.jboss.netty.logging.InternalLogger;
-import org.jboss.netty.logging.InternalLoggerFactory;
 import org.jboss.netty.util.internal.StringUtil;
 
 import java.net.InetAddress;
@@ -44,13 +42,8 @@
  */
 public class IpV4Subnet implements IpSet, Comparable<IpV4Subnet> {
 
-    private static final InternalLogger logger =
-        InternalLoggerFactory.getInstance(IpV4Subnet.class);
-
     private static final int SUBNET_MASK = 0x80000000;
 
-    private static final int BYTE_ADDRESS_MASK = 0xFF;
-
     private InetAddress inetAddress;
 
     private int subnet;
@@ -142,13 +135,13 @@ private void setNetId(String netId) throws UnknownHostException {
      * @return the integer representation
      */
     private static int toInt(InetAddress inetAddress1) {
-        byte[] address = inetAddress1.getAddress();
-        int net = 0;
-        for (byte addres : address) {
-            net <<= 8;
-            net |= addres & BYTE_ADDRESS_MASK;
-        }
-        return net;
+        byte[] octets = inetAddress1.getAddress();
+        assert octets.length == 4;
+
+        return (octets[0] & 0xff) << 24 |
+                (octets[1] & 0xff) << 16 |
+                (octets[2] & 0xff) << 8 |
+                octets[3] & 0xff;
     }
 
     /** Sets the BaseAdress of the Subnet. */
@@ -210,10 +203,6 @@ public boolean contains(String ipAddr) throws UnknownHostException {
      *         set network.
      */
     public boolean contains(InetAddress inetAddress1) {
-        if (mask == -1) {
-            // ANY
-            return true;
-        }
         return (toInt(inetAddress1) & mask) == subnet;
     }
 


Test patch: