Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove pending spec for Path#drive with IPv6 UNC host names #13190

Merged
merged 1 commit into from
Mar 17, 2023

Conversation

HertzDevil
Copy link
Contributor

The given \\2001:4860:4860::8888\share\ is an invalid UNC string, as IPv6 addresses should be represented like \\2001-4860-4860--8888.ipv6-literal.net\share\ instead. No further action is necessary as Path doesn't have methods to convert the UNC host name into a Socket::IPAddress, even for the IPv4 case.

@HertzDevil HertzDevil added platform:windows Windows support based on the MSVC toolchain / Win32 API kind:specs topic:stdlib:text labels Mar 15, 2023
Copy link
Member

@straight-shoota straight-shoota left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could add a spec to ensure \\2001-4860-4860--8888.ipv6-literal.net\share\ works as expected, although it has nothing special about it. But maybe just to be sure?
I'm happy without this as well.

@straight-shoota straight-shoota added this to the 1.8.0 milestone Mar 15, 2023
@straight-shoota straight-shoota merged commit fc2250a into crystal-lang:master Mar 17, 2023
# The first component is either an IPv4 address or a hostname.
# IPv6 addresses are converted into hostnames by replacing all `:`s with
# `-`s, and then appending `.ipv6-literal.net`, so raw IPv6 addresses cannot
# appear here.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest adding an example to clearly illustrate the conversion.

Copy link
Member

@straight-shoota straight-shoota Mar 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a purely informational explanation why there is no need to consider IPv6 addresses specifically. For our purposes, they are just regular host names. We're not doing any such conversion ourselves and details of the external conversion are irrelevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:specs platform:windows Windows support based on the MSVC toolchain / Win32 API topic:stdlib:text
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants