Common API design for IPv4 and IPv6

[Ribesg]

Hi, thanks for this library! I'm using it in Kotlin.

My main problem with this library is that the IPv4 and IPv6 parts are entirely separated, from an API point of view. There is no IpRange for example. It would be far easier to use, especially as I have to mostly write the same code twice.

I understand that it's a huge change, but it would nice if you could consider this for a v2.

Thanks, Ribesg

[jgonian]

Hi Ribesg,

Although I understand your use-case, the objective of the library was to have very simple API with strongly-typed guarantees and as such it was a deliberate design choice to use more specific type representations as opposed to have one generic type that can either represent IPv4 or IPv6.

Moreover, compared to the API of java.net.InetAddress that applies to both IPv4 and IPv6, the API of commons-ip-math can only be applied to addresses of the same family. Set operations or comparisons (sorting, overlaps, contains, etc) do not hold between IPv4 and IPv6. All these operations would have to throw some kind of exception (e.g. CannotPerformOperationException).

Having said that, it is left to the clients of the library to find ways of sharing code and one way I've seen in practice is to introduce a parameterized abstract class to keep the shared code (like shown below), although it is not the cleanest solution because of generics.

class MyClass<SingleIp extends AbstractIp<SingleIp, IpRange>, IpRange extends AbstractIpRange<SingleIp, IpRange>> {

        private final IpRange ipRange;

        public MyClass(IpRange ipRange) {
            this.ipRange = ipRange;
        }

        public String getAddressFamily() {
            if (ipRange instanceof Ipv4Range) {
                return "IPv4";
            } else {
                return "IPv6";
            }
        }
    }

I'm keeping this issue open for now as something to consider for a future version (pull requests welcome)

[Ribesg]

Moreover, compared to the API of java.net.InetAddress that applies to both IPv4 and IPv6, the API of commons-ip-math can only be applied to addresses of the same family. Set operations or comparisons (sorting, overlaps, contains, etc) do not hold between IPv4 and IPv6. All these operations would have to throw some kind of exception (e.g. CannotPerformOperationException).

This is exactly what I expected from such library, but I understand your point of view.

Another little flaw I found is that this lib is oriented a bit much to IP ranges, and does not really take the concept of subnets into account. I would expect such library to offer ways to get subnet address, broadcast address and some way to iterate over only assignable IPs in the subnet. Something like Ip.isAssignable(Subnet) or similar would be nice too. However, it's still easy to implement with the ranges provided with some quick calls to start and end, and next and previous on IPs, so it's not a big problem.

I don't think that I'll contribute to this project, my ideas are too different, I would rewrite mostly everything but the "final" classes :tongue:

Thanks again for this lib anyway!

[michael-newsrx]

We were also trying to mix ipv4 and ipv6 addresses together.

After doing some research, there is an RFC which describes an official way to encode ipv4 destinations as ipv6 addresses. Following this we are able to do comparisons and range checks treating both ipv4 and ipv6 addresses the same.

List<Ipv6Range> list = new ArrayList<>();
        for (String ipset : license.valid_ip_ranges.split(",")) {
            if (ipset.contains(":")){
                if (ipset.contains("/") || ipset.contains("-")) {
                    list.add(Ipv6Range.parse(ipset));
                } else {
                    list.add(Ipv6Range.parse(ipset + "-" + ipset));
                }
                continue;
            }
            if (ipset.contains("/") || ipset.contains("-")) {
                Ipv4Range ipv4Range = Ipv4Range.parse(ipset);
                list.add(Ipv6Range.parse("::FFFF:"+ipv4Range.start()+"-"+"::FFFF:"+ipv4Range.end()));
            } else {
                list.add(Ipv6Range.parse("::FFFF:"+ipset + "-::FFFF:" + ipset));
            }
        }
        Collections.sort(list, (a, b) -> {
            if (a.start().equals(b.start())) {
                return a.end().compareTo(b.end());
            }
            return a.start().compareTo(b.start());
        });

[jgonian]

Thanks for the input @michael-newsrx.

Indeed, this is something you can also do as the library supports parsing IPv6 address with embedded IPv4 addressed according to RFC 4291. However it is not only limited to IPv4-Compatible IPv6 Address (§2.5.5.1) and IPv4-Mapped IPv6 Address (§2.5.5.2) but can parse any valid address that is composed by an IPv6 and an IPv4 address e.g. ffce:abcd::192.168.1.0.

Thinking of it, it does make sense to add a helper method to support this use case (e.g. Ipv6.isIpv4MappedToIpv6Address(...) so I'll create another issue for it.

Finally, you can simplify your code by using one of the predefined Comparators for sorting the IPv6 addressed:

Collections.sort(list, StartAndSizeComparator.<Ipv6, Ipv6Range>get());
// or 
Collections.sort(list, SizeComparator.<Ipv6Range>get());

[michael-newsrx]

As we chose FFFF variant based on what information I could find at the time (using 0000 as not-recommended) and there exists the other 0000 variant, you might want to set the first utility function to use FFFF and add a second copy of the function where you can specify the 0000 or FFFF being applied. I can see where people might need access to both forms.

But I think it would be more useful to have casting methods (not util functions) on ipv4 and ipv6 addresses and ranges so that you can do:

... = ipv4address.asIpv6Legacy();
... = ipv4address.asIpv6Mapped();

... = ipv4range.asIpv6Legacy();
... = ipv4range.asIpv6Mapped();

boolean ... = ipv6address.isIpv6Legacy();
boolean ... = ipv6address.isIpv6Mapped();

boolean ... = ipv6range.isIpv6Legacy();
boolean ... = ipv6range.isIpv6Mapped();

... = ipv6address.asIpv4Address(); //null or ip4address, no exceptions - they can be expensive

... = ipv6range.asIpv4Range(); // null or ip4range, no exceptions - they can be expensive