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.

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

add test for SslClientAuthenticationOptions.ShallowClone #88557

Merged
merged 4 commits into from
Aug 29, 2023

Conversation

wfurt
Copy link
Member

@wfurt wfurt commented Jul 9, 2023

follow-up on #88155 and #86047 to address #88155 (comment)

@wfurt wfurt added area-System.Net.Security test-enhancement Improvements of test source code labels Jul 9, 2023
@wfurt wfurt added this to the 8.0.0 milestone Jul 9, 2023
@wfurt wfurt requested review from stephentoub and a team July 9, 2023 22:23
@wfurt wfurt self-assigned this Jul 9, 2023
@ghost
Copy link

ghost commented Jul 9, 2023

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

Issue Details

follow-up on #88155 and #86047 to address #88155 (comment)

Author: wfurt
Assignees: wfurt
Labels:

area-System.Net.Security, test-enhancement

Milestone: 8.0.0

Comment on lines +155 to +166
AllowRenegotiation = false,
AllowTlsResume = false,
ApplicationProtocols = new List<SslApplicationProtocol> { SslApplicationProtocol.Http11, SslApplicationProtocol.Http2 },
CertificateRevocationCheckMode = X509RevocationMode.Online,
ClientCertificates = new X509CertificateCollection() { clientCert },
EnabledSslProtocols = SslProtocols.Tls12,
EncryptionPolicy = EncryptionPolicy.RequireEncryption,
TargetHost = "foo",
CertificateChainPolicy = new X509ChainPolicy(),
RemoteCertificateValidationCallback = new RemoteCertificateValidationCallback(delegate { return true; }),
LocalCertificateSelectionCallback = new LocalCertificateSelectionCallback(delegate { return null; }),
ClientCertificateContext = SslStreamCertificateContext.Create(clientCert, null, false),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume all of these are non-default values? Might be worth a comment mentioning that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. With default value they work out of the box e.g. the test would not see if we fail to copy some value.

@wfurt wfurt merged commit 87f3817 into dotnet:main Aug 29, 2023
101 of 103 checks passed
@wfurt wfurt deleted the sslOptions branch August 29, 2023 16:22
@karelz karelz modified the milestones: 8.0.0, 9.0.0 Sep 14, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Oct 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Net.Security test-enhancement Improvements of test source code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants