-
Notifications
You must be signed in to change notification settings - Fork 311
Fix crash when Data Source starts with a comma #3250
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
Fix crash when Data Source starts with a comma #3250
Conversation
@dotnet-policy-service agree |
src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlConnectionTest.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.
Great fisrt open-source contribution! Little bugs like these can be very frustrating for users, so we appreciate you taking the time to fix one :)
src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlConnectionTest.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlConnectionTest.cs
Outdated
Show resolved
Hide resolved
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
@paulmedynski Thank you for the feedback, it helps a lot 😄 I've updated the test name and replaced Record.Exception with Assert.Throws() as suggested. Let me know if you'd like anything else adjusted — happy to improve further! |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
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.
Changes look great - thanks!
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3250 +/- ##
==========================================
- Coverage 72.69% 0 -72.70%
==========================================
Files 303 0 -303
Lines 59718 0 -59718
==========================================
- Hits 43414 0 -43414
+ Misses 16304 0 -16304
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:
|
Fixes #3110
this PR fixes a crash in
ADP.IsEndpoint(...)
caused by an unsafe call toLastIndexOf('\\', length - 1, length - 1)
when the Data Source in the connection string starts with a comma (e.g."Data Source=,"
).When
length == 0
, the method would previously throw anArgumentOutOfRangeException
.---Fix
Added a defensive check to ensure
length > 0
before callingLastIndexOf
, which prevents invalid index access.--- Tests
ConnectionString_WithOnlyComma_ShouldNotThrow
This is my first open-source contribution — excited to be part of the project! Let me know if you'd like any changes.