-
Notifications
You must be signed in to change notification settings - Fork 10.3k
HTTP/3: Complete support for UseHttps #42774
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
Conversation
@@ -65,6 +79,20 @@ public QuicConnectionListener(QuicTransportOptions options, ILogger log, EndPoin | |||
EndPoint = listenEndPoint; | |||
} | |||
|
|||
private void ValidateServerAuthenticationOptions(SslServerAuthenticationOptions serverAuthenticationOptions) |
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.
Could we throw here to make this more discoverable on the server?
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.
This is in the QuicListenerOptions.ConnectionOptionsCallback
. Errors will get eaten. That's why I made these log.
src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionListener.cs
Show resolved
Hide resolved
0e66909
to
1e88bfd
Compare
src/Servers/Connections.Abstractions/src/PublicAPI.Unshipped.txt
Outdated
Show resolved
Hide resolved
eb87cdf
to
355dd72
Compare
src/Servers/Kestrel/Core/src/Internal/Infrastructure/TransportManager.cs
Outdated
Show resolved
Hide resolved
src/Servers/Kestrel/Core/src/Internal/Infrastructure/TransportManager.cs
Outdated
Show resolved
Hide resolved
d08dfce
to
0daa28e
Compare
src/Servers/Connections.Abstractions/src/Microsoft.AspNetCore.Connections.Abstractions.csproj
Outdated
Show resolved
Hide resolved
@@ -2,11 +2,17 @@ | |||
"solution": { | |||
"path": "..\\..\\..\\AspNetCore.sln", | |||
"projects": [ | |||
"src\\DataProtection\\Abstractions\\src\\Microsoft.AspNetCore.DataProtection.Abstractions.csproj", |
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.
Are these changes required/intentional?
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.
They're required to build the Kestrel solution. The recently added web transport sample project uses WebApplication and references the Microsoft.AspNetCore
project. It requires these dependencies.
@JamesNK, this change will be considered for inclusion in the blog post for the release it'll ship in. Nice work! Please ensure that the original comment in this thread contains a clear explanation of what the change does, why it's important (what problem does it solve?), and, if relevant, include things like code samples and/or performance numbers. This content may not be exactly what goes into the blog post, but it will help the team putting together the announcement. Thanks! |
@JamesNK I put the blog-candidate label on this PR, but we can probably do a section including this and the other HTTP/3 changes going into RC 1. |
Fixes #42831
Fixes #34858
Question:
SslServerAuthenticationOptions
and anIList<SslApplicationProtocol>
on a feature collection. Should a new type be added to the connection abstractions project for sharing this information? Delegates and lists in a feature collection feels a bit hacky. e.g. https://github.com/dotnet/aspnetcore/blob/06a1f45f79c300c732b0171ce6318a130d05a083/src/Servers/Kestrel/Core/src/TlsHandshakeCallbackOptions.cs(can't use these
TlsHandshakeCallbackOptions
orTlsHandshakeCallbackContext
because they're in Kestrel and Quic transport doesn't have a dependency)