Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Feb 9, 2026

Description

Backport of #3929 to release/6.1. Fixes a connection performance regression where SPN generation was triggered for non-integrated auth (e.g., SQL auth) on the native SNI path, causing unnecessary DNS lookups and ~5s connection delays.

On release/6.1, the netcore TdsParserStateObjectNative.cs had the same vulnerable pattern as main pre-fix: empty serverSPN passed through to native SNI without normalization when isIntegratedSecurity=false.

Changes to netcore/src/.../TdsParserStateObjectNative.cs:

  • Extract NormalizeServerSpn(string serverSPN, bool isIntegratedSecurity) — identical to the main implementation
  • Integrated security: returns provided SPN or string.Empty (triggers SPN generation)
  • SQL auth: returns null for empty SPN (suppresses generation), preserves explicit SPN
  • Guard resolvedSpn assignment behind !string.IsNullOrWhiteSpace check

The netfx path was not affected — release/6.1 netfx handles SPN normalization in TdsParser.cs where non-integrated auth already sets SPN to null.

Issues

Backport of #3929 (commit 104a4017).

Testing

Unit test TdsParserStateObjectNativeTests.NormalizeServerSpn_ReturnsExpectedValue already exists on the branch (ported from main with #3929). Covers all combinations of null/empty/whitespace/provided SPN × integrated/SQL auth.

No environment-dependent E2E validation performed — the regression requires a specific network topology (client → router → Wi-Fi extender → SQL Server) that is out of scope per the backport instructions.

Guidelines

Please review the contribution guidelines before submitting a pull request:

Original prompt

This section details on the original issue you should resolve

<issue_title>Port PR #3929 to release/6.1</issue_title>
<issue_description>Ask:
Please port the changes from PR #3929 that are already merged into main to the release/6.1 branch.

Source PR: #3929
Merged commit (main): 104a401
Target branch: release/6.1
Scope: Port the file diffs from the above commit exactly, with only minimal, mechanical conflict resolution if needed.

Important: Do not attempt to validate or re-run the environment-specific end-to-end tests referenced in PR #3929’s description. Those require a very specific test environment that is out of scope for this port task. Treat this as a clean backport/cherry-pick.

Why:

Keep release/6.1 aligned with the fix/changes already approved and merged into main.
Consumers targeting 6.1 need these changes without waiting for the next major line.

Porting Instructions (explicit for Copilot)

Create the backport branch from release/6.1:

Branch name suggestion: backport/pr-3929-to-release/6.1

Cherry-pick the exact merge commit (or the effective commit(s)) from main:

Commit: 104a401
If the merge commit does not cherry-pick cleanly, cherry-pick the individual change commits from PR #3929.

Resolve conflicts minimally, keeping the logic identical to main while adapting only for differences that exist in release/6.1 (e.g., namespace/file layout/versioning deltas).
Do not refactor or reformat beyond what is necessary to compile in release/6.1.
Preserve attribution:

Keep original commit messages and any Co-authored-by: trailers where possible.

Build & quick verification:

Ensure the solution builds on release/6.1.

Open a PR targeting release/6.1 with the title:

[6.1] Port PR #3929 to release/6.1

Link this issue and the original PR in the new backport PR description.

Acceptance Criteria

A PR targeting release/6.1 is opened that ports the changes from PR #3929 (commit 104a401…e66a94).
The code compiles in release/6.1 with no functional changes beyond what’s necessary for the branch.
CI for release/6.1 passes (excluding the specific environment-dependent E2E tests).
Original PR and this issue are referenced in the backport PR.
Reviewer confirms parity with main for the ported files/logic.

Constraints / Non‑Goals
No environment setup. Do not create or validate the bespoke E2E environment referenced by PR #3929.
No scope creep. This is a strict backport—avoid opportunistic changes, refactors, or style-only edits.
No API surface changes beyond what is already in the main commit being ported.

Risk & Mitigation
Risk: Minor merge conflicts due to drift in release/6.1.
Mitigation: Resolve with minimal diffs; if a construct doesn’t exist in release/6.1, adapt conservatively and note the adaptation in the PR.

References

Original PR: #3929
Merged commit on main: 104a401
Target branch: release/6.1

PR Body Template (for the backport PR)

Title: [6.1] Port PR #3929 to release/6.1
Summary: This PR backports the changes from #3929 (commit 104a401…e66a94) into release/6.1.
Notes:

No environment-dependent E2E validation performed (by design).
Only minimal, mechanical conflict resolutions applied. No refactors/formatting beyond necessity.

Links:

Original PR: #3929
Issue tracking this backport: </issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

…h on native SNI path

Apply the NormalizeServerSpn fix to netcore TdsParserStateObjectNative.cs
for the release/6.1 branch file structure.

Original PR: #3929
Co-authored-by: apoorvdeshmukh <apdeshmukh@microsoft.com>

Co-authored-by: apoorvdeshmukh <5858671+apoorvdeshmukh@users.noreply.github.com>
Copilot AI changed the title [WIP] Port changes from PR dotnet/SqlClient#3929 to release/6.1 [6.1] Port PR #3929: Avoid unintended SPN generation for non-integrated auth on native SNI path Feb 9, 2026
Copilot AI requested a review from apoorvdeshmukh February 9, 2026 06:17
@apoorvdeshmukh apoorvdeshmukh removed their assignment Feb 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Port PR #3929 to release/6.1

2 participants