[jeromedalbert]

Fix for #537.

Solution

• Add a new IPv6HostParser. This parser has a high priority in the parser list so IPv6 addresses are not incorrectly picked up and parsed by HostWithPortParser's simple regex.
• Because this parser has a high priority, we need its regex to be as strict and narrow as possible to prevent it from picking up and incorrectly parsing non-IPv6 addresses. That's why it uses Resolv::IPv6::Regex from the stdlib.

Notes

My previous solution tweaked the regex of the existing IPv6HostWithPortParser, but this resulted in a regression for local hostnames, so I am using Resolv::IPv6::Regex instead, which should prove more stable. I added an extra unit test to validate the local hostname scenario.

[mattbrictson]

Thanks for the PR! Sorry for the slow response.

I think the ambiguity in the IPv6 regex means that moving it up in the parser priority will introduce problems. For example, there a regression when the hostname is "db" and a port is specified:

# Expected behavior
Host.new("db:22")
# => hostname="db", port=22

# Regression
Host.new("db:22")
# => hostname="db:22", port=nil

Is there a way to make the regex more strict to solve this? I am not very familiar with IPv6 addresses, so I'm not sure I can offer a solution, other than to stick with the status-quo [] requirement.

Maybe using Resolv::IPv6::Regex (as you originally implemented) is the way to go?

[jeromedalbert]

@mattbrictson Good point. Because the IPv6 parser still needs a high priority, we need a strict IPv6 regex. Resolv::IPv6::Regex seems to be the way to go, it is pretty involved in order to catch all the different IPv6 formats, so that should be plenty strict for our needs.

I have updated the PR and description accordingly, and added an extra unit test for the db:22 scenario you mentioned.

[mattbrictson]

Looks great!

[mattbrictson]

✨ Thanks for the additional test!