-
Notifications
You must be signed in to change notification settings - Fork 317
[Port main->6.0] Enhance TDS Login ClientInterfaceName field with client info #3200
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
- Moved logic to build client interface name into its own ClientInterface class. - Added tests for ClientInterface. - Reverted changes to Application Name. - Removed unnecessary SQP_PROVIDER_NAME constant.
- Fixed vulnerable transitive dependencies.
- Removed duplicate System.Text.Json version.
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.
PR Overview
This PR ports changes from main to release/6.0 to enhance the TDS Login ClientInterfaceName field behavior, update related tests, and remove obsolete constants. Key changes include:
- Introducing the ClientInterface class and a new test (ConnectionOpen_ClientInterfaceName) to validate its behavior.
- Replacing TdsEnums.SQL_PROVIDER_NAME with Common.DbConnectionStringDefaults.ApplicationName in error reporting and source properties.
- Removing the SQL_PROVIDER_NAME constant from TdsEnums and updating TdsParser to use ClientInterface.Name.
Reviewed Changes
| File | Description |
|---|---|
| src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlConnectionBasicTests.cs | Added test to verify the ClientInterfaceName in the LOGIN7 packet |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlException.cs | Updated the Source property to use the new application name |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlError.cs | Updated error source from SQL_PROVIDER_NAME to ApplicationName |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsEnums.cs | Removed the obsolete SQL_PROVIDER_NAME constant |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParser.cs | Updated clientInterfaceName assignment to use ClientInterface.Name |
| src/Microsoft.Data.SqlClient/tests/tools/TDS/TDS.Servers/GenericTDSServer.cs | Added a delegate for processing and capturing validated LOGIN7 requests |
Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParser.cs:125
- Consider adding a validation to ensure that the length of ClientInterface.Name does not exceed TdsEnums.MAXLEN_CLIENTINTERFACE, similar to the previous debug assert.
string clientInterfaceName = ClientInterface.Name;
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release/6.0 #3200 +/- ##
===============================================
- Coverage 72.78% 72.64% -0.15%
===============================================
Files 285 286 +1
Lines 59162 59235 +73
===============================================
- Hits 43064 43034 -30
- Misses 16098 16201 +103
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Port of PR #3149 from main to release/6.0:
Spec is here: https://microsoft.sharepoint-df.com/:w:/t/sqldevx/EZYUHqdnoXhKoQv0kxcM1QUB2V_uTCWN9GHfrDQY2tojmA?e=tf4xco
Additional changes: