darsyn / ip

Immutable value object for IPv4 and IPv6 addresses, including helper methods and Doctrine support.
http://darsyn.github.io/ip/
MIT License
245 stars 21 forks source link

Behaviour of Type Methods in Multi #82

Closed zanbaldwin closed 2 years ago

zanbaldwin commented 2 years ago

If Multi is holding an IPv4-embedded address, should that IP be treated purely as IPv4 or should it treated as potentially both IPv4 and IPv6?

Example

The address 0.0.0.1 (when using the compatible embedding strategy) is a loopback address if viewing as IPv6 (::1), but also not a loopback address (127.x.x.x) if viewing as an IPv4-embedded address.

Implementation

The type methods can be implemented as either of the following two examples, depending on what is decided:

// Checks if it a loopback address according to IPv6, if not try rechecking
// as IPv4 (if it's embedded). This is the current (4.0.1) behaviour.
return parent::isLoopback()
    || $this->isEmbedded()
    && (new IPv4($this->getShortBinary()))->isLoopback();

// Try as both IPv4->isLoopback() and IPv6->isLoopback().
Multi::factory('0.0.0.1', new Strategy\Compatible)->isLoopback(); // bool(true)
// If it's an IPv4-embedded address, check as IPv4 only.
// If it's not, check as IPv6.
return $this->isEmbedded()
    ? (new IPv4($this->getShortBinary()))->isLoopback()
    : parent::isLoopback();

// Try only as both IPv4->isLoopback() because it's an IPv4-embedded address.
Multi::factory('0.0.0.1', new Strategy\Compatible)->isLoopback(); // bool(false)
zanbaldwin commented 2 years ago

Solution: use second implementation (IPv4-embedded is treated as IPv4 only) to make behaviour consistent across helper and type methods, and document change.