-
Notifications
You must be signed in to change notification settings - Fork 5.2k
[WinHTTP] Certificate caching on WinHttpHandler to eliminate extra call to Custom Certificate Validation #111791
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
[WinHTTP] Certificate caching on WinHttpHandler to eliminate extra call to Custom Certificate Validation #111791
Conversation
…om Certificate Validation
Tagging subscribers to this area: @dotnet/ncl |
/azp run runtime-libraries-coreclr outerloop-windows |
Azure Pipelines successfully started running 1 pipeline(s). |
src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpRequestCallback.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpRequestCallback.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpRequestCallback.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpRequestCallback.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpRequestCallback.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpRequestCallback.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpRequestCallback.cs
Show resolved
Hide resolved
src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpRequestCallback.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpRequestCallback.cs
Outdated
Show resolved
Hide resolved
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 very nice for Prove of concept, but there are few things which needs to be considered before we decide on feasibility of this approach:
- decide if unbounded cache can be an memory leak issue (I have hard time to imagine use where it would)
- consider different cache key to handle virtual servers infra
src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpRequestCallback.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpRequestCallback.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpRequestCallback.cs
Outdated
Show resolved
Hide resolved
/azp run runtime-libraries-coreclr outerloop-windows |
Azure Pipelines successfully started running 1 pipeline(s). |
src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpRequestCallback.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/WinHttpHandlerTest.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs
Show resolved
Hide resolved
src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/WinHttpHandlerTest.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/WinHttpHandlerTest.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpRequestCallback.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpRequestCallback.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/WinHttpHandlerTest.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/WinHttpHandlerTest.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs
Outdated
Show resolved
Hide resolved
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.
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/WinHttpHandlerTest.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpRequestCallback.cs
Show resolved
Hide resolved
…ficate_experiment
…riment' of github.com:liveans/dotnet-runtime into winhttp_servervalidationcallback_cache_certificate_experiment
/azp run runtime-libraries-coreclr outerloop-windows |
Azure Pipelines successfully started running 1 pipeline(s). |
I'm going to merge this after CI run and I'll track this in CI for a while on main, will start backporting process afterwards. If you have further concerns with PR, ptal @antonfirsov @wfurt @MihaZupan |
…ficate_experiment
…riment' of github.com:liveans/dotnet-runtime into winhttp_servervalidationcallback_cache_certificate_experiment
/azp run runtime-libraries-coreclr outerloop-windows |
Azure Pipelines successfully started running 1 pipeline(s). |
/backport to release/9.0-staging |
Started backporting to release/9.0-staging: https://github.com/dotnet/runtime/actions/runs/14334626247 |
@liveans backporting to "release/9.0-staging" failed, the patch most likely resulted in conflicts: $ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch
Applying: Certificate caching on WinHttpHandler to eliminate extra call to Custom Certificate Validation
Applying: Review feedback
Applying: Review feedback
Applying: Framework compat + Review Feedback
Applying: Implement Timer to clear cache
error: sha1 information is lacking or useless (src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0005 Implement Timer to clear cache
Error: The process '/usr/bin/git' failed with exit code 128 Please backport manually! |
This PR goal is improving performance around CustomCertificateValidationCallback.