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

Fix Encoding regression #111320

Merged
merged 3 commits into from
Jan 14, 2025
Merged

Fix Encoding regression #111320

merged 3 commits into from
Jan 14, 2025

Conversation

tarekgh
Copy link
Member

@tarekgh tarekgh commented Jan 12, 2025

#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
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.

@ericstj
Copy link
Member

ericstj commented Jan 13, 2025

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

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

Copy link
Contributor

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

@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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants