ineter icon indicating copy to clipboard operation
ineter copied to clipboard

Parsing IPv4 mapped IPv6 not supported

Open lpellegr opened this issue 3 years ago • 5 comments

It seems that IPv4 mapped IPv6 addresses such as ::FFFF:83.90.47.30 is not supported:

IPv6Address.of("::FFFF:83.90.47.30")

It throws the following error:

Illegal character: . at index 15 java.lang.IllegalArgumentException: Illegal character: . at index 15

Is there any plan to support such IP addresses and conversion to IPv4?

lpellegr avatar Jul 12 '21 15:07 lpellegr

@lpellegr I never had a need to handle such addresses, so there aren't any relevant plans. What do you expect to receive as a result of IPv6Address.of("::FFFF:83.90.47.30")? An IPv6Address instance or an IPv4Address? Both are somewhat confusing results.

maltalex avatar Jul 12 '21 20:07 maltalex

Thanks for your answer.

InetAddress.getByName("::FFFF:83.90.47.30") returns an Inet4Address.

I would say the result type has no real importance as long as we have a way to move from a representation to the other: i.e. to the IPv6 representation from the IPv4 one, and vice-versa.

To make the intent very clear, an utility class could perhaps be introduced. In that case, at least 3 public methods would be needed:

  • One that parses an IPv4 mapped IPv6 address as an IPv6Address or IPv4Address. Personally, I have a preference to return an IPv6Address.
  • A second method to get an IPv4Address from the IPv6Address by using the last 32 bits of the IPv6 address.
  • A third method to get an IPv6Address from the IPv4Address.

@maltalex What do you think?

lpellegr avatar Jul 12 '21 21:07 lpellegr

Yeah, I've encountered InetAddress.getByName returning a v4 address for mapped addresses - it's something I shot myself in the foot with :)

Returning an IPv6Address does indeed sound more reasonable to me, but I'm not sure about the utility class. We could add a method to IPv6Address to return the lower 32 bits as an IPv4Address as well as a isIPv4mappedIPv6() or something similar. Granted, it won't make much sense to most IPv6Address instances, but for those that represent a mapped IPv4 there would be a convenient solution.

maltalex avatar Jul 14 '21 14:07 maltalex

We could add a method to IPv6Address to return the lower 32 bits as an IPv4Address as well as a isIPv4mappedIPv6() or something similar

Yes, this looks great. The utility class was more an internal matter to keep features and purpose isolated. New methods in IPAddress classes could then rely on the utility class.

It would be useful to have also a method in IPv4Address to create an IPv4 mapped IPv6Address.

I am not sure whether parsing should be done in the existing method or a new one. I would say in a dedicated one as the existing method to parse IPv6 addresses looks quite optimized.

@maltalex You maybe have a preference?

lpellegr avatar Jul 14 '21 15:07 lpellegr

It would be useful to have also a method in IPv4Address to create an IPv4 mapped IPv6Address.

Nice idea.

I am not sure whether parsing should be done in the existing method or a new one. I would say in a dedicated one as the existing method to parse IPv6 addresses looks quite optimized.

Ah, yes. I remember that method :) It's honestly not as awful as it looks and there are many test cases covering it, so tweaking it shouldn't be too hard. Where else would we put the parsing logic?

maltalex avatar Jul 15 '21 12:07 maltalex