Fixed convertIpStringToBinary fails in some scenarios

[PaulB]

Affected version

2.2.6 Patch 2

convertIpStringToBinary fails in some scenarios because it tries to handle a situation in which the input has already been converted. It would probably be better if it were just simple wrappers around inet_pton().

In particular, a valid IPv6 address in its string representation can be a valid IPv4 address in its binary representation. Similarly, a valid IPv6 address in its binary representation can be a different IPv6's string representation.

For example:

• Ip::convertIpStringToBinary("::ff") returns "::ff"
• Ip::convertIpBinaryToString(Ip::convertIpStringToBinary("::ff")) returns "58.58.102.102"
• Ip::convertIpStringToBinary(Ip::convertIpStringToBinary("3a3a:3a3a:3a3a:3a3a:3a3a:3a3a:3a3a:3a3a")) returns false, but Ip::convertIpStringToBinary(Ip::convertIpStringToBinary("3b3b:3b3b:3b3b:3b3b:3b3b:3b3b:3b3b:3b3b")) returns ";;;;;;;;;;;;;;;;"; why bother providing the illusion that it's safe to run convertIpStringToBinary only already-converted values when that clearly isn't the case?
• Ip::convertIpBinaryToString(Ip::convertIpStringtoBinary(Ip::convertIpStringToBinary("3a31:3030:2e31:3030:2e31:3030:2e31:3030"))) returns "100.100.100.100", but Ip::convertIpBinaryToString(Ip::convertIpStringtoBinary(Ip::convertIpStringToBinary("3b31:3030:2e31:3030:2e31:3030:2e31:3030"))) returns "3b31:3030:2e31:3030:2e31:3030:2e31:3030"

convertIpStringToBinary tries to ensure that it's safe to run on a binary representation, but that's impossible to do while supporting both IPv4 and IPv6: too many values become ambiguous. This is inevitably going to result in subtle bugs, especially in third-party add-ons. It would be far better to simply throw an exception if the input is not something that would be expected by inet_pton().

 

[Chris D]

Going to call this fixed but. alas, not until XF 2.3.

XF 2.3 deprecates the convertIpStringToBinary and conertIpBinaryToString functions in favour of new functions stringToBinary and binaryToString.

These essentially are wrappers around inet_pton and inet_ntop respectively.