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

TDS8 Add Server Certificate Support #1822

Merged
merged 25 commits into from
Nov 10, 2022

Conversation

lcheunglci
Copy link
Contributor

This adds the parameter Server Certificate to the connection string. The server certificate is a local path to the SSL Certificate that has been download from the server. This certificate will be manually validated during the SSL Handshake. When HostNameInCertificate is specified with ServerCertificate, HNIC will take precedence. I added very basic functional test to check for the connection string parameter. Additional manual tests will be added in a later PR.

@codecov
Copy link

codecov bot commented Nov 1, 2022

Codecov Report

Base: 69.25% // Head: 69.22% // Decreases project coverage by -0.02% ⚠️

Coverage data is based on head (1c26fff) compared to base (2a54bc5).
Patch coverage: 54.44% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1822      +/-   ##
==========================================
- Coverage   69.25%   69.22%   -0.03%     
==========================================
  Files         292      292              
  Lines       61604    61727     +123     
==========================================
+ Hits        42662    42733      +71     
- Misses      18942    18994      +52     
Flag Coverage Δ
addons 92.38% <ø> (ø)
netcore 73.30% <55.23%> (-0.04%) ⬇️
netfx 67.62% <61.95%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...core/src/Interop/SNINativeMethodWrapper.Windows.cs 98.83% <ø> (ø)
...core/src/Microsoft/Data/SqlClient/SNI/SNICommon.cs 55.12% <0.00%> (-14.23%) ⬇️
...c/Microsoft/Data/SqlClient/TdsParserStateObject.cs 23.30% <ø> (+0.08%) ⬆️
...a.SqlClient/netcore/src/Resources/StringsHelper.cs 26.66% <ø> (ø)
...c/Microsoft/Data/Interop/SNINativeMethodWrapper.cs 57.14% <ø> (ø)
.../Microsoft/Data/Common/DbConnectionStringCommon.cs 64.05% <ø> (ø)
...oft/Data/SqlClient/TdsParserSafeHandles.Windows.cs 93.20% <ø> (ø)
...e/src/Microsoft/Data/SqlClient/SNI/SNITcpHandle.cs 62.62% <45.45%> (-0.87%) ⬇️
...nt/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs 70.84% <52.70%> (-0.21%) ⬇️
.../netcore/src/Microsoft/Data/SqlClient/TdsParser.cs 73.33% <55.35%> (-0.03%) ⬇️
... and 17 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@DavoudEshtehari DavoudEshtehari added this to the 5.1.0-preview2 milestone Nov 1, 2022
@DavoudEshtehari DavoudEshtehari added this to In Progress in SqlClient v5.1 via automation Nov 1, 2022
@cheenamalhotra cheenamalhotra added the 🆕 Public API Use this label for new API additions to the driver. label Nov 2, 2022
Co-authored-by: David Engel <dengel1012@gmail.com>
@JRahnama
Copy link
Member

JRahnama commented Nov 7, 2022

/azurepipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Co-authored-by: David Engel <dengel1012@gmail.com>
@JRahnama
Copy link
Member

the failure seems not relevant .Connection Open() without retries took longer than expected (2 seconds) on Mac machine.

@lcheunglci
Copy link
Contributor Author

The mac failures seem to intermittently occur on the pipeline in all the PRs recently, so I just have to re-run it until it succeeds.

SqlClient v5.1 automation moved this from In Progress to Review Approved Nov 10, 2022
@lcheunglci lcheunglci merged commit f0f36f2 into dotnet:main Nov 10, 2022
SqlClient v5.1 automation moved this from Review Approved to Done Nov 10, 2022
@lcheunglci lcheunglci deleted the TDS8-Add-ServerCertificate-Support branch November 10, 2022 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🆕 Public API Use this label for new API additions to the driver.
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

6 participants