Skip to content

Conversation

apoorvdeshmukh
Copy link
Contributor

Description

Ports #3399 to release/5.1

Issues

#3397

Testing

Port contains the testcase to validate the fix

@Copilot Copilot AI review requested due to automatic review settings September 13, 2025 11:28
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR ports a UTF-8 encoding fix from the main branch to release/5.1. The fix addresses an issue where UTF-8 encoding operations were including a Byte Order Mark (BOM) when they shouldn't, which could cause data corruption or incorrect encoding behavior in SQL Server bulk copy operations.

Key changes:

  • Replaces direct use of Encoding.UTF8 with a custom UTF-8 encoder that doesn't emit BOM
  • Adds comprehensive test coverage for UTF-8 bulk copy scenarios
  • Updates both .NET Framework and .NET Core implementations consistently

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
TestBulkCopyWithUTF8.cs New test file that validates UTF-8 bulk copy operations preserve byte sequences correctly
Microsoft.Data.SqlClient.ManualTesting.Tests.csproj Adds the new test files to the project compilation
DataTestUtility.cs Adds helper method for creating test tables with specific column definitions
netfx/src/Microsoft/Data/SqlClient/TdsParser.cs Replaces UTF-8 encoding instances with BOM-less version in .NET Framework implementation
netcore/src/Microsoft/Data/SqlClient/TdsParser.cs Replaces UTF-8 encoding instances with BOM-less version in .NET Core implementation

@apoorvdeshmukh apoorvdeshmukh requested a review from a team September 13, 2025 11:31
Copy link

codecov bot commented Sep 13, 2025

Codecov Report

❌ Patch coverage is 80.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.97%. Comparing base (ee2d196) to head (5fc499d).

Files with missing lines Patch % Lines
.../netcore/src/Microsoft/Data/SqlClient/TdsParser.cs 80.00% 1 Missing ⚠️
...nt/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs 80.00% 1 Missing ⚠️
Additional details and impacted files
@@               Coverage Diff               @@
##           release/5.1    #3617      +/-   ##
===============================================
+ Coverage        65.77%   65.97%   +0.19%     
===============================================
  Files              293      293              
  Lines            61647    61649       +2     
===============================================
+ Hits             40547    40671     +124     
+ Misses           21100    20978     -122     
Flag Coverage Δ
addons 0.00% <ø> (ø)
netcore 70.26% <80.00%> (+0.15%) ⬆️
netfx 64.09% <80.00%> (+0.18%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@paulmedynski paulmedynski self-assigned this Sep 16, 2025
@benrr101
Copy link
Contributor

Afaik, this will need to have the branch merged into it to pick up CI changes once they go through.

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.

3 participants