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.

Sign up for GitHub

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

Jump to bottom

[QUIC] QuicConnection API follow up #72027

Closed
2 tasks
ManickaP opened this issue Jul 12, 2022 · 3 comments · Fixed by #72366
Closed
2 tasks

[QUIC] QuicConnection API follow up #72027

ManickaP opened this issue Jul 12, 2022 · 3 comments · Fixed by #72366
Assignees
@ManickaP
Labels
area-System.Net.Quic
Milestone
7.0.0

Comments

@ManickaP
Copy link
Member

  • it should AFAIK. It would be nice to have some assert about it. It differs among platforms but MsQUIC now uses native layout e.g. same as SocketAddress.

Originally posted by @wfurt in #71783 (comment)

  • It would be nice to throw or at least log. I know this is what we we in SslStream for historical reasons but the failures when we silently ignore given certificate are annoying IMHO.
    (this could be separate work)

Originally posted by @wfurt in #71783 (comment)
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jul 12, 2022
@ghost
Copy link

ghost commented Jul 12, 2022

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

Issue Details
  • it should AFAIK. It would be nice to have some assert about it. It differs among platforms but MsQUIC now uses native layout e.g. same as SocketAddress.

Originally posted by @wfurt in #71783 (comment)

  • It would be nice to throw or at least log. I know this is what we we in SslStream for historical reasons but the failures when we silently ignore given certificate are annoying IMHO.
    (this could be separate work)

Originally posted by @wfurt in #71783 (comment)

Author: ManickaP
Assignees: -
Labels:

area-System.Net.Quic
Milestone: -

@ManickaP ManickaP mentioned this issue Jul 12, 2022
Merged
@ManickaP ManickaP removed the untriaged New issue has not been triaged by the area owner label Jul 12, 2022
@ManickaP ManickaP added this to the 7.0.0 milestone Jul 12, 2022
@ManickaP ManickaP self-assigned this Jul 14, 2022
@ManickaP
Copy link
Member Author

It would be nice to have some assert about it.

@wfurt how would we assert that the layout is the same? We have an assert about size, but I'm not sure what else we could do, any ideas?

@wfurt
Copy link
Member

wfurt commented Jul 14, 2022

perhaps separate test for the helper? Serialize given buffer as SocketAddress and compare equality?
AFAIK the only one special case was macOS were the layout was different. Conceptually, test would probably fail on platforms where we do it wrong. Perhaps just remove the comment with the doubts....?

@ManickaP ManickaP mentioned this issue Jul 18, 2022
Merged
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jul 18, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jul 18, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Aug 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@ManickaP ManickaP

Labels
area-System.Net.Quic
Projects
None yet
Milestone
7.0.0
Development

Successfully merging a pull request may close this issue.

[QUIC] API changes follow up ManickaP/runtime
2 participants
@ManickaP @wfurt