OK following #6494 this is the PR for the extraction of the Piwik\IP class into the Network component. I have also included IP anonymization.

Feedback is welcome!

Classes

I moved most methods into a new IP class that is not static anymore, it's a value object. Usage example:

// Factory methods:
$ip = IP::fromStringIP('127.0.0.1');
// IPv6
$ip = IP::fromStringIP('::1');
// In binary format:
$ip = IP::fromBinaryIP("\x7F\x00\x00\x01");

echo $ip->toString(); // 127.0.0.1
echo $ip->toBinary();

// IPv4 & IPv6
if ($ip instanceof IPv4) {}
if ($ip instanceof IPv6) {}

// Hostname reverse lookup
echo $ip->getHostname();

if ($ip->isInRange('192.168.1.1/32')) {}
if ($ip->isInRange('192.168.*.*')) {}

// Anonymize an IP by setting X bytes to null bytes
$ip->anonymize(2);

As you can see, there are factory methods in the base IP class. Then there are 2 implementations: IPv4 and IPv6. I know at first it sounds like unnecessary to split it in 3 classes like that, but it actually makes a lot of sense and the code is better.

Take IP anonymization for example. With a single IP class:

    public function anonymize()
    {
        if ($this->isIPv4()) {
            // do this
        } else {
            // do that
        }
    }

With IP + IPv4 and IPv6 classes:

class IP
{
    public function anonymize();
}
class IPv4
{
    public function anonymize()
    {
        // do this
    }
}
class IPv6
{
    public function anonymize()
    {
        // do that
    }
}

Polymorphism FTW!

In general, everywhere isIPv4() or isIPv6() was used, it was to implement a custom behavior. That's what OOP is for. So that's (IMO) a good replacement. And there's no impact to the user, only better code and better tests.

There were also some methods that didn't belong in an IP object, so I put them in a IPUtils class:

echo IPUtils::binaryToStringIP("\x7F\x00\x00\x01");
echo IPUtils::stringToBinaryIP('127.0.0.1');

// Sanitization methods
$sanitizedIp = IPUtils::sanitizeIp($_GET['ip']);
$sanitizedIpRange = IPUtils::sanitizeIpRange($_GET['ipRange']);

// IP range
$bounds = IPUtils::getIPRangeBounds('192.168.1.*');
echo $bounds[0]; // 192.168.1.0
echo $bounds[1]; // 192.168.1.255

Naming

As a matter of naming, I have replaced the P2N() and N2P() notation with something more explicit: binary format and string format. After looking online, "presentation format" and "network address format" where not so common terms. Hope it's OK, let me know if you have better ideas. It can be changed.

Namespace

In the new component we have now:

• Piwik\Network\IP
• Piwik\Network\IPv4
• Piwik\Network\IPv6
• Piwik\Network\IPUtils

Should it be moved to the subnamespace Piwik\Network\IP\?

I'm wondering if it's necessary if someday we add other classes to that component.

Deprecated old Piwik\IP class

I have deprecated all the methods of the old IP class that I have moved to the new component. I have also replaced almost all the usages of the old IP class by the new ones throughout the codebase. So Piwik\IP class is almost never used anymore.

However some methods where not moved:

• getIpFromHeader()
• getNonProxyIpFromHeader()
• getLastIpFromList($csv)

Those are either specific to the current request or just too specific at all.

TODO

• Update the changelog
• Publish on Packagist
• Update composer.json to use a tagged version and remove the URL to the git repository
• Search and replace usages of old Piwik\IP in public and pro plugins?
• Add tests for deprecated methods