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

QUIC: IPv6 constant looks suspicious in native helper #53495

Closed
wfurt opened this issue May 31, 2021 · 3 comments · Fixed by #53673
Closed

QUIC: IPv6 constant looks suspicious in native helper #53495

wfurt opened this issue May 31, 2021 · 3 comments · Fixed by #53673
Labels
area-System.Net.Quic enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@wfurt
Copy link
Member

wfurt commented May 31, 2021

I noticed this while reviewing #53461.

namespace System.Net.Quic.Implementations.MsQuic.Internal
{
    internal static class MsQuicAddressHelpers
    {
        internal const ushort IPv4 = 2;
        internal const ushort IPv6 = 23;

..
       internal static SOCKADDR_INET IPEndPointToINet(IPEndPoint endpoint)
       {
                       socketAddress.Ipv6._family = IPv6;

AF_INET6 is 10 on Linux and 30 on macOS. It feels like this should come from PAL.
It also seems like our tests use CreateQuicListener() that use IPEndPoint(IPAddress.Loopback, 0) so we may have test gaps for IPv6.

@ghost
Copy link

ghost commented May 31, 2021

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

I noticed this while reviewing #53461.

namespace System.Net.Quic.Implementations.MsQuic.Internal
{
    internal static class MsQuicAddressHelpers
    {
        internal const ushort IPv4 = 2;
        internal const ushort IPv6 = 23;

..
       internal static SOCKADDR_INET IPEndPointToINet(IPEndPoint endpoint)
       {
                       socketAddress.Ipv6._family = IPv6;

AF_INET6 is 10 on Linux and 30 on macOS. It feels like this should come from PAL.
It also seems like our tests use CreateQuicListener() that use IPEndPoint(IPAddress.Loopback, 0) so we may have test gaps for IPv6.

Author: wfurt
Assignees: -
Labels:

area-System.Net.Quic

Milestone: -

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label May 31, 2021
@treysis
Copy link

treysis commented May 31, 2021

Loopback/localhost shouldn't be used. It is very unclear what the system will resolve it to. Either you specify the address family, or you use literal addresses for localhost.

@ThadHouse
Copy link
Contributor

ThadHouse commented Jun 1, 2021

In MsQuic, we ensure all IPv6 family values seen through the API are 23, to match windows, and our does have users use QUIC_ADDRESS_FAMILY_INET6 which is always that value even on posix platforms.

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jun 3, 2021
@karelz karelz removed the untriaged New issue has not been triaged by the area owner label Jun 3, 2021
@karelz karelz added this to the 6.0.0 milestone Jun 3, 2021
@karelz karelz added the enhancement Product code improvement that does NOT require public API changes/additions label Jun 3, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jun 3, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jul 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Net.Quic enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants