Skip to content
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

[release/9.0] Fix Encoding regression #111367

Merged
merged 3 commits into from
Jan 14, 2025

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Jan 13, 2025

Fixes #110521

Backport of #111320 to release/9.0

/cc @tarekgh

Customer Impact

  • Customer reported
  • Found internally

In .NET 9.0, users working with multi-byte encodings like GB18030 may encounter unexpected exceptions when decoding bytes into a Unicode surrogate pair. It's important to note that encoding can be used implicitly, such as when reading streams, which significantly broadens the potential impact of this issue.

Regression

  • Yes
  • No

This is a regression in .NET 9.0 from the change #97950. The change has modified the logic in EncodingCharBuffer.AddChar to handle uninitialized chars differently causing regression.

Testing

The regression tests have passed. I manually debugged the specific scenario and confirmed that the fix is effective. Additionally, I added a test case to cover the previous failure, ensuring that this regression will be detected if it occurs again in the future.

Risk

Low, we didn't change any logic more than handling the failing case when dealing with null buffers.

IMPORTANT: If this backport is for a servicing release, please verify that:

  • The PR target branch is release/X.0-staging, not release/X.0.

Package authoring no longer needed in .NET 9

IMPORTANT: Starting with .NET 9, you no longer need to edit a NuGet package's csproj to enable building and bump the version.
Keep in mind that we still need package authoring in .NET 8 and older versions.

Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-text-encoding
See info in area-owners.md if you want to be subscribed.

@tarekgh tarekgh added the Servicing-consider Issue for next servicing release review label Jan 13, 2025
@tarekgh tarekgh added this to the 9.0.x milestone Jan 13, 2025
@tarekgh tarekgh self-assigned this Jan 13, 2025
@tarekgh
Copy link
Member

tarekgh commented Jan 13, 2025

CC @ericstj @artl93

@tarekgh
Copy link
Member

tarekgh commented Jan 14, 2025

/ba-g the failure is a build timeout which is seen in other PRs too.

@tarekgh tarekgh added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Jan 14, 2025
@carlossanlop carlossanlop merged commit 55b2924 into release/9.0 Jan 14, 2025
83 of 89 checks passed
@carlossanlop carlossanlop modified the milestones: 9.0.x, 9.0.2 Jan 14, 2025
@carlossanlop carlossanlop deleted the backport/pr-111320-to-release/9.0 branch January 14, 2025 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants