IPAddress IPv6 usability


#1

I just made a mistake which others may make at some point. It was pretty silly but here goes:

IPAddress ip;

// -- other code --

ip = "255.0.0.0";
// Above will use the IPAddress(bool isIP6) constructor
// and will make and empty IPv6 address.

// What I meant to do
ip = IPAddress(String("255.0.0.0"));

Maybe something could be done to make this mistake harder to make in future? Explicit constructor?

This also works strangely:

IPAddress ip("255.255.255.0");
// Makes empty IPv6

#2

Thanks, I’ll sort that out.


#3

@ed95
I see you have removed the constructor. This works as I can still set isIPv6 manually.

However the static IPAddress::any(bool isIP6) uses the IPV4 uint32_t constructor for me now.

IPAddress::any(true).toString()
// Returns 0.0.0.1 (i.e. a IPv4 address constructed with uint32_t(true)

#4

Ah yeah, didn’t spot that one. Thanks, there will be a fix on develop shortly.


#5

I am using the new class quite extensively (to replace our internal ipv4 only equivalent). I now have a collection of util functions, of which maybe some could go upstream. An obvious choice would be extending == to cover > and <. Using something along the lines of the below perhaps?

namespace
{
    int compare (const IPAddress& ip1, const IPAddress& ip2)
    {
        // Need to note in docs that comparing IPv4 address with IPv6 address
        // may not give altogether sensible results.
        for (int i = 0; i < (ip1.isIPv6 ? 16 : 4); i++)
        {
            if (ip1.address[i] > ip2.address[i]) return 1;
            if (ip1.address[i] < ip2.address[i]) return -1;
        }

        return 0;
    }
}

namespace juce
{
bool operator>  (const IPAddress& ip1, const IPAddress& ip2) { return compare (ip1, ip2) > 0; }
bool operator<  (const IPAddress& ip1, const IPAddress& ip2) { return compare (ip1, ip2) < 0; }
bool operator>= (const IPAddress& ip1, const IPAddress& ip2) { return compare (ip1, ip2) >= 0; }
bool operator<= (const IPAddress& ip1, const IPAddress& ip2) { return compare (ip1, ip2) <= 0; }
}

I also have stuff to cover prefixes and IPv4 masks and some other bits ‘n’ bobs.


#6

Actually thinking about it, this is weird in the current behaviour

IPAddress add1 (1, 1, 1, 1);
IPAddress add2 (257, 257, 123, 456, 789, 012, 345, 678);
expectEquals((int)(add1 == add2), (int)false); // FAILS: true
expectEquals((int)(add2 == add1), (int)false);

So maybe the (ip1.isIPv6 ? 16 : 4) should become 16? Or something like the following where IPv4 are less than IPv6s?

namespace
{
    int compare (const IPAddress& ip1, const IPAddress& ip2)
    {
        if (ip1.isIPv6 != ip2.isIPv6)
        {
            if (ip1.isIPv6) return 1;
            else return -1;
        }

        for (int i = 0; i < (ip1.isIPv6 ? 16 : 4); i++)
        {
            if (ip1.address[i] > ip2.address[i]) return 1;
            if (ip1.address[i] < ip2.address[i]) return -1;
        }

        return 0;
    }
}

namespace juce
{
bool operator>  (const IPAddress& ip1, const IPAddress& ip2) { return compare (ip1, ip2) > 0; }
bool operator<  (const IPAddress& ip1, const IPAddress& ip2) { return compare (ip1, ip2) < 0; }
bool operator>= (const IPAddress& ip1, const IPAddress& ip2) { return compare (ip1, ip2) >= 0; }
bool operator<= (const IPAddress& ip1, const IPAddress& ip2) { return compare (ip1, ip2) <= 0; }
bool operator== (const IPAddress& ip1, const IPAddress& ip2) { return compare (ip1, ip2) == 0; }
bool operator!= (const IPAddress& ip1, const IPAddress& ip2) { return compare (ip1, ip2) != 0; }
}

#7

Yeah there’s a few things that need cleaning up in that class, I’ll hopefully push something shortly…


#8

OK, commit 8e463b4 will be on develop shortly and cleans a few things up in the IPAddress class. I’ve added more comparison operators and also support for IPv4-mapped IPv6 addresses with some handy static methods for converting to and from them. This makes the comparison between IPv6 and v4 addresses a bit more sensible as it will check whether the v6 address is a v4-mapped address before comparing them. If it isn’t, then the fallback is to always assume that v6 addresses are “greater” than v4 addresses.


#9

Thanks Ed. Looks great.


#10

Hello again @ed95. There is a zeroUnusedBytes(); missing in the IPAddress string constructor I think.


#11

Yep, I’ll add that. Should be on develop in a few minutes


#12

@ed95 I have found a bug in IPv6 parsing strings. Reproduce with below:

IPAddress one("0000:0000:0000:0000:0000:0000:1234:5678");
IPAddress two(one.toString());
EXPECT_EQ(one.toString(), two.toString());

#13

OK, there should be a fix on develop shortly.


#14

Hi @ed95, will these changes be available in a release soon? Nothing on master branch as far as I can see.


#15

Without giving a specific date, we should be doing a release in the next month or so that will include these changes.