[release/10.0] Fix handling of non-ASCII data in CBOR indefinite strings#131278
[release/10.0] Fix handling of non-ASCII data in CBOR indefinite strings#131278bartonjs wants to merge 1 commit into
Conversation
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @dotnet/area-system-formats-cbor, @bartonjs, @vcsjones |
There was a problem hiding this comment.
Pull request overview
Fixes CborReader.TryReadTextString handling for indefinite-length text strings containing non-ASCII UTF-8, ensuring destination slicing advances by the number of chars written (not the UTF-8 byte length). This aligns TryRead... behavior with the existing ReadTextString() concatenation logic and improves correctness for multi-byte UTF-8 chunks.
Changes:
- Fix
TryReadIndefiniteLengthTextStringConcatenatedto slice the destination span bychunkCharsWrittenrather than the chunk’s byte length. - Add/extend unit test coverage for non-ASCII text (e.g., “São Paulo”, “München”, Cyrillic) across
ReadTextStringandTryReadTextString, including additional conformance/error-path coverage.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/libraries/System.Formats.Cbor/tests/Reader/CborReaderTests.TextString.cs | Adds regression coverage for non-ASCII text and expands TryReadTextString coverage across success/failure paths. |
| src/libraries/System.Formats.Cbor/src/System/Formats/Cbor/Reader/CborReader.String.cs | Fixes the span-advance logic when concatenating UTF-8 chunks for indefinite-length text strings in TryRead.... |
|
/backport to release/9.0-staging |
|
Started backporting to |
|
@bartonjs backporting to |
Backport of #131235 to release/10.0
Customer Impact
When reading an indefinite-length encoded text string, TryReadTextString moves the write position forward the number of bytes read, not the number of chars written, so non-ASCII data gets exceptions that a precisely-sized buffer is too small (despite having had the size pre-validated).
Regression
It seems to have had this behavior since the method was introduced, in .NET 5.
Testing
This change includes tests that exercise the pattern better, including having a non-ASCII character in something other than the final position in a string.
Risk
Low. Existing test coverage shows no regressions for pure-ASCII and Unicode-final-only, new test coverage shows Unicode-before-final works correctly.
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.