-
Notifications
You must be signed in to change notification settings - Fork 526
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
Support for changing alpn on listener callback (#2418) #2959
Support for changing alpn on listener callback (#2418) #2959
Conversation
Co-authored-by: Nick Banks <nibanks@microsoft.com>
Co-authored-by: Nick Banks <nibanks@microsoft.com>
/azp run ci |
…hub.com/liveans/msquic into liveans/support-callback-neg-alpns-2418
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run ci |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run ci |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run ci |
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks so much for doing this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't fully understand the changes here, but we don't need any API changes. What we need is to be able to narrow down ALPN list when we call ConnectionSetConfiguration
after we got NEW_CONNECTION
. Note that this needs to be asynchronous since we're invoking user provided async callback, so we cannot set it within NEW_CONNECTION
event.
The configuration already supports setting ALPN, we just need msquic to handle it for inbound connections.
@@ -1820,6 +1820,9 @@ internal unsafe partial struct _NEW_CONNECTION_e__Struct | |||
|
|||
[NativeTypeName("HQUIC")] | |||
internal QUIC_HANDLE* Connection; | |||
|
|||
[NativeTypeName("const uint8_t *")] | |||
internal byte* NewNegotiatedAlpn; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's this good for? And shouldn't it also have length specified somewhere?
I'll make a new PR for this to correct the behavior @ManickaP |
Description
Closes #2418. This pr aims to support changing alpns on ListenerCallback via the
NewNegotiatedAlpn
field.Testing
Alpn.ChangeAlpn test case already added, also extended that test case to check out of bounds and invalid address.
Documentation
Yes, example usage can be found under
QUIC_LISTENER_EVENT.md