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

Support for IPv6 as shown #40

Closed
spragucm opened this issue May 7, 2020 · 3 comments
Closed

Support for IPv6 as shown #40

spragucm opened this issue May 7, 2020 · 3 comments

Comments

@spragucm
Copy link

spragucm commented May 7, 2020

Hello! Great library and thanks for your effort :)

I found it through the following SO answer:
https://stackoverflow.com/questions/577363/how-to-check-if-an-ip-address-is-from-a-particular-network-netmask-in-java/41482123#41482123

I was writing some unit tests against your answer and @omid's answer and found that your subnet test failed in the IPv6 example while his IpAddressMatcher passed.

Am I missing something?

Omid's Matcher:

import org.springframework.security.web.util.matcher.IpAddressMatcher;
...

private void checkIpMatch() {
    matches("192.168.2.1", "192.168.2.1"); // true
    matches("192.168.2.1", "192.168.2.0/32"); // false
    matches("192.168.2.5", "192.168.2.0/24"); // true
    matches("92.168.2.1", "fe80:0:0:0:0:0:c0a8:1/120"); // false
    matches("fe80:0:0:0:0:0:c0a8:11", "fe80:0:0:0:0:0:c0a8:1/120"); // true
    matches("fe80:0:0:0:0:0:c0a8:11", "fe80:0:0:0:0:0:c0a8:1/128"); // false
    matches("fe80:0:0:0:0:0:c0a8:11", "192.168.2.0/32"); // false
}

private boolean matches(String ip, String subnet) {
    IpAddressMatcher ipAddressMatcher = new IpAddressMatcher(subnet);
    return ipAddressMatcher.matches(ip);
}

My Unit Tests (in Kotlin):

enum class IPSUB (val ip: String, val subNet: String, val isOnSubNet: Boolean) {
        A("192.168.2.1", "192.168.2.1", true),
        B("192.168.2.1", "192.168.2.0/32", false),
        C("192.168.2.5", "192.168.2.0/24", true),
        D("92.168.2.1", "fe80:0:0:0:0:0:c0a8:1/120", false),
        E("fe80:0:0:0:0:0:c0a8:11", "fe80:0:0:0:0:0:c0a8:1/120", true),
        F("fe80:0:0:0:0:0:c0a8:11", "fe80:0:0:0:0:0:c0a8:1/128", false),
        G("fe80:0:0:0:0:0:c0a8:11", "192.168.2.0/32", false),
        H("10.10.20.3", "10.10.20.0/30", true),
        I("10.10.20.5", "10.10.20.0/30", false),
        J("1::1", "1::/64", true),
        K("2::1", "1::/64", false),
        L("1::4:5", "1::3-4:5-6", true),
        M("2::", "1-2::/64", true),
        N("foo", "bar", false)
    }

@Test
    fun isIpOnSubnetsWithIpStringTest() {
        IPSUB.values().forEach {
            val ip = IPAddressString(it.ip)
            val sub = IPAddressString(it.subNet)
            println("IP:$ip Sub:$sub onSub:${sub.contains(ip)} expect:${it.isOnSubNet}")
            //Assertions.assertEquals(it.isOnSubNet, sub.contains(ip))
        }
    }
@omid
Copy link

omid commented May 7, 2020

Not every @omid is the @omid ;-D

@seancfoley
Copy link
Owner

seancfoley commented May 8, 2020

This is really the same as the previous issue, issue #39.

The IPAddress library uses the same types for both subnets and individual addresses. Other libraries do not do this, they typically have one type for an address, another type for a subnet.

In practice, using the same type can be really useful (I was motivated to do this myself for my own use-cases at first). However, there is one aspect to that which confuses some people at first. There is a basic convention regarding how to specify CIDR block subnets vs addresses, which also matches the same convention typically used by network admins. If you specify an address string with a zero host, it is considered to be a subnet, otherwise it is considered to be a single address.

So, in your example, the difference is this pair:
"fe80:0:0:0:0:0:c0a8:11", "fe80:0:0:0:0:0:c0a8:1/120"

The other library says the second contains the first, mine says it does not.

In my library, the second one is not a subnet, it is the same as "fe80:0:0:0:0:0:c0a8:1", it just happens to have a prefix length associated, and since the two addresses are not the same address, false is the result. It is not a subnet because of the 1 at the end. The host, the last 8 bits, is not 0. And in fact, a network admin would say the same, "fe80:0:0:0:0:0:c0a8:1/120" is not a subnet. Why would someone stick the 1 at the end if it does not need to be there? However, in other libraries which have different types for addresses and subnets, when feeding that string into the subnet type it will of course try to convert it to a subnet, that is the only thing it can be in that context.

Anyway, all that being said...

If you want to be sure to get the subnet, use the toPrefixBlock() method. toPrefixBlock() converts an address with a prefix length to the associated prefix block subnet. Otherwise, to get the subnet, your address must have a zero host (ie the bits after the prefix must be zero).

contains("fe80:0:0:0:0:0:c0a8:11", "fe80:0:0:0:0:0:c0a8:1/120"); // true, with this method, second is considered a subnet, not an address
contains("fe80:0:0:0:0:0:c0a8:11", "fe80:0:0:0:0:0:c0a8:0/120"); // true
containsFromString("fe80:0:0:0:0:0:c0a8:11", "fe80:0:0:0:0:0:c0a8:1/120"); // false, second is not a subnet, host is not zero
containsFromString("fe80:0:0:0:0:0:c0a8:11", "fe80:0:0:0:0:0:c0a8:0/120"); // true, second is a subnet because host is zero

static void containsFromString(String address, String network) {
    IPAddressString one = new IPAddressString(network);
    IPAddressString two = new IPAddressString(address);
    System.out.println(one +  " string contains " + two + " " + one.contains(two));
}
	
static void contains(String address, String network) {
    IPAddressString one = new IPAddressString(network);
    IPAddressString two = new IPAddressString(address);
    System.out.println(one +  " contains " + two + " " + one.getAddress().toPrefixBlock().contains(two.getAddress()));
}

See here or the previous issue for more details.

@spragucm
Copy link
Author

spragucm commented May 9, 2020

Thanks for the quick and in-depth response. I really appreciate it.

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

3 participants