-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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: refactor clientHello #34541
quic: refactor clientHello #34541
Conversation
The actions failures are new warnings (treated as errors):
|
CI is good on this! |
@addaleax, please take another look when you have the opportunity to do so. I've added a new |
b8b6ce0
to
3f902ed
Compare
2342fb7
to
b95bf9d
Compare
Refactor the `'clientHello'` event into a `clientHelloHandler` configuration option and async function. Remove the addContext API as it's not needed.
Alternative to `CallbackScope` that handles destroying the `QuicSession` in the try_catch cleanup.
3b7a7ee
to
ff8677f
Compare
Regular CI and QUIC CI are good to go on this |
Refactor the `'clientHello'` event into a `clientHelloHandler` configuration option and async function. Remove the addContext API as it's not needed. PR-URL: #34541 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Alternative to `CallbackScope` that handles destroying the `QuicSession` in the try_catch cleanup. PR-URL: #34541 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
PR-URL: #34541 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Landed in e5dacc2...6e65f26 |
First commit here is from #34533 which needs to land first.
Second commit here is the important one in this PR. This refactors the
'clientHello'
event into an async function passed as anquicsocket.listen()
option. This function is only ever called once at a very specific point in theQuicServerSession
lifecycle, using it as an event is unnecessary. The commit changes the way it works. If appropriate to do so, user code may return a newSecureContext
from the function (previously that was done in the OCSP function, which really didn't make sense).Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes