Skip to content

Fix Encoding regression#111320

Merged
tarekgh merged 3 commits intodotnet:mainfrom
tarekgh:FixEncodingRegression
Jan 14, 2025
Merged

Fix Encoding regression#111320
tarekgh merged 3 commits intodotnet:mainfrom
tarekgh:FixEncodingRegression

Conversation

@tarekgh
Copy link
Member

@tarekgh tarekgh commented Jan 12, 2025 •
edited
Loading

#110521

The change here is fixing a regression from the change #97950.
The regression occurs in the multi-byte encoding when handling a surrogate pair. It incorrectly throws an exception when the character buffer is null, which is not the expected behavior. The code should be resilient to a null buffer, avoiding exceptions and instead returning only the count of the decoded characters.

The change is not reverting #97950 as the rest of the changes are beneficial.

@tarekgh tarekgh added this to the 10.0.0 milestone Jan 12, 2025
@tarekgh tarekgh requested a review from jkotas January 12, 2025 19:49
@tarekgh
Copy link
Member Author

tarekgh commented Jan 12, 2025

CC @manandre @ericstj

@tarekgh
Copy link
Member Author

tarekgh commented Jan 12, 2025

The fix of this issue needs to be backported to the 9.0 servicing release.

This was referenced Jan 13, 2025
Open
Open
Closed
@ericstj
Copy link
Member

ericstj commented Jan 13, 2025 •
edited
Loading

Seems like System.Text.Tests.EncodingTest.TestDefaultEncodings is failing on many jobs. Could be related.

Error message
Assert.Throws() Failure: No exception was thrown
Expected: typeof(System.NotSupportedException)

Stack trace
   at System.Text.Tests.EncodingTest.TestDefaultEncodings() in /_/src/libraries/System.Text.Encoding.CodePages/tests/EncodingCodePages.cs:line 481
   at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args) in /_/src/mono/System.Private.CoreLib/src/System/Reflection/MethodBaseInvoker.Mono.cs:line 22
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(O

runtime/src/libraries/System.Text.Encoding.CodePages/tests/EncodingCodePages.cs

Line 481 in 812d504

Assert.Throws<NotSupportedException>(() => Encoding.GetEncoding((int)mapping[0]));

@tarekgh
Copy link
Member Author

tarekgh commented Jan 13, 2025

Seems like System.Text.Tests.EncodingTest.TestDefaultEncodings is failing on many jobs. Could be related.

I'll take a look

@tarekgh
Copy link
Member Author

tarekgh commented Jan 13, 2025

/backport to release/9.0

@github-actions
Copy link
Contributor

github-actions bot commented Jan 13, 2025

Started backporting to release/9.0: https://github.com/dotnet/runtime/actions/runs/12754927051

@github-actions github-actions bot mentioned this pull request Jan 13, 2025
Merged
4 tasks
@tarekgh tarekgh mentioned this pull request Jan 13, 2025
Closed
This was referenced Jan 14, 2025
Open
Open
@tarekgh
Copy link
Member Author

tarekgh commented Jan 14, 2025

/ba-g the failing build is timeout which happened in other PRs.

@tarekgh tarekgh merged commit 30834e4 into dotnet:main Jan 14, 2025
80 of 82 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Feb 14, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jkotas jkotas jkotas approved these changes

@ericstj ericstj ericstj approved these changes

Assignees

@tarekgh tarekgh

Labels

area-System.Text.Encoding

Projects

None yet

Milestone

10.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

@tarekgh @ericstj @jkotas