Partial fix IPv4-mapped IPv6 addresses: Improve formatting and simplify code

[Steffen]

Affected version

2.0.0 Beta 7

Take the IPv4 address 192.168.123.123. XenForo internally stores it as an IPv4-mapped IPv6 address which totally makes sense. But when displaying this IP address, XenForo formats it as "::ffff:c0a8:7b7b" which is technically correct but I dont' think it's very user-friendly. In fact, XenForo is the only software I know that does not simply format such addresses as "::ffff:192.168.123.123".

Wikipedia agrees: "These addresses are typically written with a 96-bit prefix in the standard IPv6 format, and the remaining 32 bits written in the customary dot-decimal notation of IPv4." – https://en.wikipedia.org/wiki/IPv6#IPv6_readiness

In fact, the built-in PHP function inet_ntop uses this user-friendly formatting automatically. I'm wondering why "XF\Util\Ip" implements a custom "convertIpBinaryToString" method instead of using the native PHP function? Is it because some arcane PHP installations might not have IPv6 support?

(The dual method "convertIpStringToBinary" could be replaced by inet_pton.)

 

[Mike]

It may be arcane, but it is possible for PHP to theoretically not actually have inet_pton/inet_ntop or, if they exist, for them not to necessarily support IPv6. You can see some of the ifdef's here regarding that: https://github.com/php/php-src/blob...668e29ba/ext/standard/basic_functions.c#L3931

For a simpler example, PHP has an IPv6 test that shows the conditions it runs on: https://github.com/php/php-src/blob...bce/ext/standard/tests/network/inet_ipv6.phpt Interestingly, there's an implication that MacOS has broken inet_* functions, which is something I saw mentions of elsewhere.

So I'm undecided what changes to make here. I have actually made changes to use this, though it's not actually quite as simple as you imply, as our functions actually handle failure differently (and handle some different forms), so it doesn't save a ton of code. It does format IPv4-mapped addresses a bit better, though that's not the most difficult thing to change.

 

[Mike]

At this point, I'm just changing the code to support displaying IPv4-mapped IPv6 addresses in a more expected way (when outputting a shortened IPv6 address). Given that there may be some situations where unexpected behavior happens with the inet_* functions, it's likely we'd have to keep manual implementation in place anyway, so in this case, flipping to inet_* would likely just add some debugging difficulty.

 

[Steffen]

Thank you for the changes to make IPv4-mapped IPv6 addresses more readable!

I did not know that the native PHP functions might not exist (could be detected), might not support IPv6 (could probably be detected as shown in the linked PHP test-case) or might just be broken on macOS (could be detected using PHP_OS).

But still, as you said, there is little value in using the native functions if the manual implementation is required for fallback purposes anyway and if the performance impact is low.

One could argue that working inet_pton / inet_ntop functions should be an installation requirement (such that a manual implementation would not be needed) but I'm fine with keeping things the way they are (with the fixed usability issue ).