Cleanup | Remove dead code paths and Regexes from SqlConnectionOptions - #4544
Open
edwardneal wants to merge 12 commits into
Open
edwardneal wants to merge 12 commits into
edwardneal wants to merge 12 commits into
Conversation
ConnectionStringRegexOdbc was rendered unused by the previous commit.
This helper uses managed code rather than the the s_connectionStringValidKeyRegex Regex, but retains a Debug-only assertion.
These are now only used in debug builds
These are only used by netfx codepaths, and do not need to be instantiated on netcore
BraceQuoteValue and BraceQuoteValueQuote were only accessible from one another, and when useOdbcRules was true.
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
This pull request has been marked as stale due to inactivity for more than 30 days. If you would like to keep this pull request open, please provide an update or respond to any comments. Otherwise, it will be closed automatically in 7 days. |
github-actions
Bot
added
the
Stale
Contributor
Author
|
This PR is not stale. |
github-actions
Bot
removed
the
Stale
Contributor
|
/azp run |
|
Azure Pipelines: Successfully started running 2 pipeline(s). 1 pipeline(s) were filtered out due to trigger conditions. |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The removed branches correspond to invariant arguments, and all affected callers remain behaviorally consistent.
Review effort: Balanced
Findings: None
What changed in this PR
Removes unreachable ODBC parser branches and avoids unnecessary regex initialization while preserving SQL connection-string behavior.
Changes:
- Simplifies parser state and method signatures.
- Restricts validation regexes to required build targets.
- Updates affected callers.
| File | Description |
|---|---|
SqlConnectionOptions.Debug.cs |
Removes unused ODBC debug parsing. |
SqlConnectionOptions.cs |
Simplifies parsing and validation paths. |
SqlConnectionInternal.cs |
Updates local-host verification call. |
DbConnectionString.netfx.cs |
Updates restriction parsing call. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
paulmedynski
requested changes
Sep 29, 2026
| private const string ConnectionStringValidKeyPattern = "^(?![;\\s])[^\\p{Cc}]+(?<!\\s)$"; // key not allowed to start with semi-colon or space or contain non-visible characters or end with space | ||
| private const string ConnectionStringValidValuePattern = "^[^\u0000]*$"; // value not allowed to contain embedded null | ||
| #if NETFRAMEWORK | ||
| private const string ConnectionStringQuoteValuePattern = "^[^\"'=;\\s\\p{Cc}]*$"; // generally do not quote the value if it matches the pattern |
Contributor
There was a problem hiding this comment.
We don't need this string constant either - it is only used in the regex constructor below.
| internal sealed partial class SqlConnectionOptions | ||
| { | ||
| #if DEBUG | ||
| private const string ConnectionStringValidKeyPattern = "^(?![;\\s])[^\\p{Cc}]+(?<!\\s)$"; // key not allowed to start with semi-colon or space or contain non-visible characters or end with space |
Contributor
There was a problem hiding this comment.
These string constants can be folded into the regex constructors as well.
Also renamed ConnectionStringRegex to fit wider naming convention
paulmedynski
approved these changes
Sep 29, 2026
Contributor
|
/azp run |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
SqlConnectionOptionshas a handful of code paths which were inherited from the need to write a generic parser for ODBC connection strings. These code paths are no longer used, so this PR just cleans them up.In the case of
GetKeyValuePair, there are a few code paths which could never be called, and codecov demonstrates this. In the other cases, they were criteria which would always be met.One interesting point emerges from this:
SqlConnectionOptionshas four statically-initialised compiled Regex instances. One of them is completely unused, one is only used by netfx code and two are only used in Debug builds. I've cleaned these up, so the static constructor for the class sheds some load.This is cleanup work which has an incidental performance benefit - it doesn't have a benchmark attached to it, and I couldn't see any results because we're dealing with static constructors.
For review, this can move commit-by-commit.
Issues
None.
Testing
All unit tests for connection string parsing continue to pass. All changes can be statically verified.