Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Facing problem with IPAddressString contains(IPAddressString other) method #13

Closed
vamsikrishnanekkalapudi opened this issue Dec 17, 2018 · 19 comments

Comments

@vamsikrishnanekkalapudi

Hello @seancfoley,
I am using contains method to check whether one IPAddressString contains other,

Examples,
192.168.1.0/24 contains 192.168.1.0/12.(true)
192.168.1.0/24 contains 192.168.2.0/24.(false)
192.168.1.1-10 contains 192.168.1.5-7.(true)
192.168.1.5-7 contains 192.168.1.1-10.(false)
192.168.. contains 192.168.1..(true)
192.168.1.
contains 192.168...(false)
192.168.1.0/24 contains 192.168.1.1-10.(true)
192.168.1.0/24 contains 192.168.2.1-10.(false)
192.168.1.* contains 192.168.1.0/24.(true)

All the above cases are working fine,but some of the below cases are failing,

  • Case-1
IIPAddressString ipAddressStringSubnetError1 = new IPAddressString( "10.162.155.1-51" );
        IPAddressString ipAddressStringSubnetError2 = new IPAddressString( "10.162.155.1-255" );
        System.out.println( String.format( "%s contains %s %s", ipAddressStringSubnetError2,
                ipAddressStringSubnetError1, ipAddressStringSubnetError2.contains( ipAddressStringSubnetError1 ) ) );

Output
10.162.155.1-255 contains 10.162.155.1-51 false

It should return true but it's returning false.But if ipAddressStringSubnetError1 range is 10.162.155.2-51 then it returns true.The problem only occurs when both ranges start with same address(10.162.155.1).

NOTE: Same problem is there for IPV6 also.

  • Case-2
IPAddressString ipAddressStringSubnetError3 =
                new IPAddressString( "2001:0db8:85a3:0000:0000:8a2e:0370:7334/120" );
        IPAddressString ipAddressStringSubnetError4 =
                new IPAddressString( "2001:0db8:85a3:0000:0000:8a2e:0370:7334/128" );
        System.out.println( String.format( "%s contains %s %s", ipAddressStringSubnetError3,
                ipAddressStringSubnetError4, ipAddressStringSubnetError3.contains( ipAddressStringSubnetError4 ) ) );

        System.out.println( String.format( "%s contains %s %s", ipAddressStringSubnetError4,
                ipAddressStringSubnetError3, ipAddressStringSubnetError4.contains( ipAddressStringSubnetError3 ) ) );

Output

2001:0db8:85a3:0000:0000:8a2e:0370:7334/120 contains 2001:0db8:85a3:0000:0000:8a2e:0370:7334/128 true

2001:0db8:85a3:0000:0000:8a2e:0370:7334/128 contains 2001:0db8:85a3:0000:0000:8a2e:0370:7334/120 true

Subnet Calculator

IP Address:	2001:0db8:85a3:0000:0000:8a2e:0370:7334/120
Full IP Address:	2001:0db8:85a3:0000:0000:8a2e:0370:7334
Total IP Addresses:	256
Network:	2001:0db8:85a3:0000:0000:8a2e:0370:7300
IP Range:	2001:0db8:85a3:0000:0000:8a2e:0370:7300 - 2001:0db8:85a3:0000:0000:8a2e:0370:73ff


IP Address:	2001:0db8:85a3:0000:0000:8a2e:0370:7334/128
Full IP Address:	2001:0db8:85a3:0000:0000:8a2e:0370:7334
Total IP Addresses:	1
Network:	2001:0db8:85a3:0000:0000:8a2e:0370:7334
IP Range:	2001:0db8:85a3:0000:0000:8a2e:0370:7334 - 2001:0db8:85a3:0000:0000:8a2e:0370:7334

In the case2 both are returning true, but 2001:0db8:85a3:0000:0000:8a2e:0370:7334/128 contains 2001:0db8:85a3:0000:0000:8a2e:0370:7334/120 should return false.

Note:
Same problem is there with IPV4 address also when address not end with zero.

-Case-3

IPAddressString ipAddressStringSubnetError5 = new IPAddressString( "192.13.1.0/25" );
        IPAddressString ipAddressStringSubnetError6 = new IPAddressString( "192.13.1.1-255" );
        System.out.println( String.format( "%s contains %s %s", ipAddressStringSubnetError5,
                ipAddressStringSubnetError6, ipAddressStringSubnetError5.contains( ipAddressStringSubnetError6 ) ) );
        System.out.println( String.format( "%s contains %s %s", ipAddressStringSubnetError6,
                ipAddressStringSubnetError5, ipAddressStringSubnetError6.contains( ipAddressStringSubnetError5 ) ) );

Output

192.13.1.0/25 contains 192.13.1.1-255 false
192.13.1.1-255 contains 192.13.1.0/25 false

In case3 i think 192.13.1.1-255 contains 192.13.1.0/25 should return true.

Thanks,
Vamsi.

@seancfoley
Copy link
Owner

seancfoley commented Dec 18, 2018

For the moment taking a brief look at case 3:
192.13.1.0/25 is equivalent to 192.13.1.0-127 so 192.13.1.1-255 contains 192.13.1.0/25 should be false, because 192.13.1.1-255 does not contain the address 192.13.1.0.

However, the address 192.13.1.0 has a zero host (when used with prefix length 25) so it is generally not used as an address, and is generally used to denote the entire subnet. So this may be the source of contention here. There are a few methods, like iterators, that allow you to exclude the zero hosts, although the contains method is not one of those methods, so the address with zero host is considered when assessing containment in this case. I will think about whether it makes sense to add a contains method that does not consider the zero host.

I will take a look at case 1 and case 2 as well.

@hannesrohde
Copy link

hannesrohde commented Dec 27, 2018

Hi Sean,

I am also having issues with the "contains" method, not sure if related:

The following JUnit test fails although it should succeed as far as I understand IPv6:

IPAddressString range = new IPAddressString("2001:db8::/120");
IPAddressString host = new IPAddressString("2001:db8::1");
assertTrue(range.contains(host));

@seancfoley
Copy link
Owner

Yes, that example shows a bug, looking into it.

@seancfoley
Copy link
Owner

Case-1 showing a bug as well.

The IPAddressString.contains method is intended as a high-performance option over IPAddress.contains, so that you can check containment without having to create address objects. It seems to have a couple of glitches.

As a work-around, just call IPAddressString.getAddress(), check for null on the result to avoid malformed addresses, and then use IPAddress.contains() to check containment.

Working on the fixes.

@seancfoley
Copy link
Owner

I've fixed the issues identified in this bug. Those fixes are included in version 5.0.1 just released. If you find any others not working as expected, please let me know.

@hannesrohde
Copy link

Sean, thanks alot for the quick fix!
I will integrate the new version into my project and report back if anything looks suspicious :)

@vamsikrishnanekkalapudi
Copy link
Author

Thanks for fixes.
Case 1 and 2 are working fine.

Is there any way to handle case3,

IPAddressString ipAddressStringSubnetError5 = new IPAddressString( "192.13.1.0/25" );
IPAddressString ipAddressStringSubnetError6 = new IPAddressString( "192.13.1.1-255" );
System.out.println( String.format( "%s contains %s %s", ipAddressStringSubnetError6,
ipAddressStringSubnetError5, ipAddressStringSubnetError6.contains( ipAddressStringSubnetError5 ) ) );

Output
192.13.1.1-255 contains 192.13.1.0/25 false

And few more doubts,
IPAddressString ipAddressStringSubnetError5 = new IPAddressString( "192.13.1.1/24" );
System.out.println( ipAddressStringSubnetError5.getAddress().getNonZeroHostCount() );
Output
1
Eventhough subnet address is non zero host, is there any way to get the output 255 in the above case?

IPAddressString ipAddressStringSubnetError5 = new IPAddressString( "www.google.com" );
System.out.println( ipAddressStringSubnetError5.isValid() );

In the case of host names, isValid method is returning false.Does IPAddressString supports host names?

@seancfoley
Copy link
Owner

seancfoley commented Jan 7, 2019

@vamsikrishnanekkalapudi
I looked at case 3 previously and it is not a bug. 192.13.1.0/25 is equivalent to 192.13.1.0-127
So 192.13.1.1-255 does not contain 192.13.1.0-127, because of the address 192.13.1.0.
192.13.1.0-255 does contain 192.13.1.0-127.

If you wanted to test without the zero host involved, it looks to me like the subtract method would help:
IPAddress addr = new IPAddressString("192.13.1.0/25").getAddress();
IPAddress addr2 = new IPAddressString("192.13.1.1-255").getAddress();
IPAddress subtracted[] = addr.subtract(addr.getLower());
boolean result = addr2.contains(subtracted[0]);
System.out.println(addr2 + " contains" + subtracted[0] + ": " + result);

Output is
192.13.1.1-255 contains192.13.1.1-127: true

As I said before, I thought about whether it makes sense to add a contains method that does not consider the zero host. Since it turns out that there were a couple of real bugs here in the other examples, I decided that was the issue at hand rather than adding new functionality. I can give it some more thought, whether adding functionality that ignored zero hosts would add value or not. Perhaps a IPAddress.removeZeroHosts() method would make sense, which is a bit more straightforward than using the subtract method like I did above. Is there a scenario you have that explains why you ended up with this test?

As for your other concerns:

new IPAddressString( "192.13.1.1/24" ).getAddress().getNonZeroHostCount() is 1, that is correct and is not a bug.
That is because the host is 1, it is not 0. When you have a non-zero host it is just a single address. If it were 192.13.1.0/24 it would be the whole block of 256 addresses. If you want to convert that single address 192.13.1.1/24 to the containing prefix block, just use toPrefixBlock():

new IPAddressString( "192.13.1.1/24" ).getAddress().toPrefixBlock().getNonZeroHostCount() is 255.

IPAddressString does not support host names, only addresses. There is a HostName class for host names, and in fact that class allows addresses as well. It also has a method called isValid.

@vamsikrishnanekkalapudi
Copy link
Author

Is there a scenario you have that explains why you ended up with this test?
In my use case i have to remove duplicate entries.
In my case this is 192.13.1.0/25 the duplicate of 192.13.1.1-255 .So i have to remove 192.13.1.0/25 entry.

@SLYJason
Copy link

Hi @seancfoley
I updated dependency to 5.0.1 but still seeing this issue. Have a subnet 74.58.224.1/22 and IP 74.58.227.104, however the contains() still failed.

IPAddressString ipAddressStringFromConfig = new IPAddressString("74.58.224.1/22");
IPAddressString warehouseIpAdressString = new IPAddressString("74.58.227.104");
try {
    IPAddress ipAddressFromConfig = ipAddressStringFromConfig.toAddress();
    IPAddress warehouseIpAdress = warehouseIpAdressString.toAddress();
    System.out.println(ipAddressFromConfig.contains(warehouseIpAdress));
} catch (Exception e) {

}

It prints out false. Can you please take a look?

@seancfoley
Copy link
Owner

seancfoley commented Jan 11, 2019

@SLYJason Hello, that's not a bug. If the address has a zero host it represents the subnet. If it is non-zero host then it is a single address. So 74.58.224.1/22 is a single address. If you want to be sure to get the subnet use toPrefixBlock()

IPAddressString ipAddressStringFromConfig = new IPAddressString("74.58.224.1/22");
IPAddressString warehouseIpAdressString = new IPAddressString("74.58.227.104");
IPAddress ipAddressFromConfig = ipAddressStringFromConfig.getAddress();
IPAddress warehouseIpAdress = warehouseIpAdressString.getAddress();
System.out.println(ipAddressFromConfig.contains(warehouseIpAdress));
System.out.println(new IPAddressString("74.58.224.0/22").getAddress().contains(warehouseIpAdress));
System.out.println(ipAddressFromConfig.toPrefixBlock().contains(warehouseIpAdress));

Output:

false
true
true

@seancfoley
Copy link
Owner

Also, you can use toBinaryString() to look at the bits:

IPAddressString ipAddressStringFromConfig = new IPAddressString("74.58.224.1/22");
IPAddress ipAddressFromConfig = ipAddressStringFromConfig.getAddress();
System.out.println(ipAddressFromConfig.toBinaryString());

Output

01001010001110101110000000000001

As you can see the host is not zero.

@SLYJason
Copy link

Hi, I quite don't understand why you said 74.58.224.1/22 is a single address. I actually used other libraries to test whether 74.58.224.1/22 contains 74.58.227.104 and all is passed:

// Use Apache Commons Net.
@Test
public void subNetTestCase1() {
 Assert.assertTrue(new SubnetUtils("74.58.224.1/22").getInfo().isInRange("74.58.227.104"));
}

// Use Spring Security.
@Test
public void subNetTestCase2() {
 IpAddressMatcher ipAddressMatcher = new IpAddressMatcher("74.58.224.1/22");
 Assert.assertTrue(ipAddressMatcher.matches("74.58.227.104"));
}

But you library is not working in this case. I referred from this: https://stackoverflow.com/questions/577363/does-anyone-know-a-java-component-to-check-if-ip-address-is-from-particular-netw

@seancfoley
Copy link
Owner

seancfoley commented Jan 11, 2019

74.58.224.1/22 is the address 74.58.224.1 associated with the prefix length 22. That is what it means, nothing else.

It is not atypical for people to assume that when you see something like a.b.c.d/x and the host bits are all zero in a.b.c.d for prefix length x, then it is not atypical for them to be actually talking about the subnet, which is all addresses like a.b.c.d with the same prefix of length x.

So in this case, when the host bits are all 0, then it's 74.58.224.0/22. This library interprets that particular all-zero host address to mean any address that has the same first 22 bits as 74.58.224.0/22. But when the host bits are not zero, it is ambiguous as to what you are talking about with the string 74.58.224.1/22, it is basically a single address with a prefix length. When this library sees that, it considers it to be just the single address 74.58.224.1 having a prefix length of 22.

If other libraries behave differently, that's fine, it doesn't really mean anything at all. In fact, this library used to behave differently in previous versions. But it's not wrong for different libraries to have different behaviour.

As you can see, with those libraries you are using classes that already infer that they are concerned only with the subnet associated with 74.58.224.1/22, that's why it's called SubnetUtils or IPAddressMatcher rather than just Address. So those classes in those other libraries are completely ignoring the 1 in 74.58.224.1/22, because all they care about is the subnet. IPAddress is different in that the same class can be used for either addresses or subnets. So the 1 matters with this library, it is not ignored. And I would tell you that this can be extremely valuable that the same class can be used for either one. If you want the same behaviour as those other classes, then as I said, just call toPrefixBlock().

As for that stackoverflow answer, all those addresses with prefix lengths have hosts of zero, hence they are treated as the subnet and not as an individual address, which is what I said above.

That is how this library is designed. So it is not correct for you to be saying it is not working. It is doing exactly what it is designed to be doing.

@SLYJason
Copy link

Thanks for the detailed explanation, I gotcha it now.

@daniftodi
Copy link

daniftodi commented Jan 25, 2019

@seancfoley I'm using version 5.0.1 and new IPAddressString("5.62.62.0/23").contains(new IPAddressString("5.62.64.1")) return true, it should actually return false.

5.62.62.0 - 5.62.63.255

Even this returns true: new IPAddressString("5.62.62.0/23").contains(new IPAddressString("5.62.68.1")), probably somewhere you are comparing one bit less.

new IPAddressString("5.62.62.0/23").contains(new IPAddressString("5.62.78.1")) - returns false, which is correct.

@seancfoley
Copy link
Owner

seancfoley commented Jan 26, 2019

@daniftodi yes, good catch. It is a glitch with prefixes /7 /15 /23 /31 for ipv4 (ie 1 bit short of segment boundaries).

As a work-around, use IPAddressString.getAddress() or IPAddressString.toAddress()

boolean result1 = new IPAddressString("5.62.62.0/23").getAddress().contains(new IPAddressString("5.62.64.1").getAddress());
boolean result2 = new IPAddressString("5.62.62.0/23").getAddress().contains(new IPAddressString("5.62.68.1").getAddress());

This bug does not affect other prefix lengths.

@seancfoley seancfoley reopened this Jan 26, 2019
@seancfoley
Copy link
Owner

seancfoley commented Feb 3, 2019

@daniftodi The issue you identified is fixed in release 5.0.2

@seancfoley
Copy link
Owner

seancfoley commented Feb 3, 2019

@vamsikrishnanekkalapudi In version 5.0.2 I've added the method containsNonZeroHosts for the issue you identified:

System.out.println(new IPAddressString("192.13.1.1-255").getAddress().containsNonZeroHosts(new IPAddressString("192.13.1.0/24").getAddress()));

Output:

true

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants