Convert.FromHexString exception with too large buffer #105426
Conversation
dotnet-issue-labeler
bot
added
the
needs-area-label
dotnet-policy-service
bot
added
the
community-contribution
vcsjones
added
area-System.Runtime
and removed
needs-area-label
| @@ -3007,11 +3007,15 @@ public static OperationStatus FromHexString(ReadOnlySpan<char> source, Span<byte | |||
| quotient = destination.Length; | |||
| result = OperationStatus.DestinationTooSmall; | |||
| } | |||
| else if (remainder == 1) | |||
| else if (destination.Length > quotient) | |||
There was a problem hiding this comment.
Shouldn't we also be handling remainders in cases where the destination buffer precisely matches the quotient?
| else if (destination.Length > quotient) | |
| else |
Are we missing a unit test for that case?
| { | ||
| source = source.Slice(0, source.Length - 1); | ||
| } | ||
|
|
||
| result = OperationStatus.NeedMoreData; |
There was a problem hiding this comment.
If the destination buffer can fit the result and the remainder is 0, why should the status be NeedMoreData? Do we have unit test for that case?
| byte[] buffer = new byte[100]; | ||
| var status = Convert.FromHexString(hex, buffer, out int charsConsumed, out int bytesWritten); | ||
|
|
||
| Assert.Equal(OperationStatus.NeedMoreData, status); |
There was a problem hiding this comment.
I believe that this assertion is incorrect. NeedMoreData should only be returned if the parser handled partial data (i.e. there's an odd number of hex chars). If the buffer fits the result it should always return Done.
There was a problem hiding this comment.
I've pushed changes that address an additional corner case and update the behaviour such that OperationStatus.Done is being returned in cases where the destination buffer is longer than the output.
cc @adamsitnik
Should close #105405
Draft for now to see CI results