[API Proposal]: Convert.TryFromHexString

[Socolin]

Background and motivation

Convert.FromHexString("not-hex"); is throwing a System.FormatException when input string is not a valid hex.

Like Convert.TryFromBase64Chars() Convert.TryFromBase64String() it would be nice to have Convert.TryFromHexString(); when validating user input it's not great to be force to put a whole try/catch block arround for readability

byte[] myBytes;
try
{
	myBytes = Convert.FromHexString(inputString);
}
catch (FormatException ex)
{
	throw new InvalidXxx(ex);
}

I think It would be much nicer to do

if (!Convert.TryFromHexString(inputString, out var myBytes))
	throw new InvalidXxx(ex);

API Proposal

namespace System;

public class Convert
{
    public static bool TryFromHexString(string s, Span<byte> bytes, out int bytesWritten)
}

API Usage

if (!Convert.TryFromHexString(inputString, out var myBytes))
	throw new InvalidXxx(ex);

Alternative Designs

TryFromBase64String returns a Span bytes instead with the number bytesWritten, so maybe it should be the same, but since the number of bytes are easy to guest with hex string (just divide string length by 2) I'm not sure it's useful

public static bool TryFromHexString(string s, Span<byte> bytes, out int bytesWritten)

Risks

I don't see any

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[ghost]

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

Issue Details

---

Background and motivation

Convert.FromHexString("not-hex"); is throwing a System.FormatException when input string is not a valid hex.

Like Convert.TryFromBase64Chars() Convert.TryFromBase64String() it would be nice to have Convert.TryFromHexString(); when validating user input it's not great to be force to put a whole try/catch block arround for readability

byte[] myBytes;
try
{
	myBytes = Convert.FromHexString(inputString);
}
catch (FormatException ex)
{
	throw new InvalidXxx(ex);
}

I think It would be much nicer to do

if (!Convert.TryFromHexString(inputString, out var myBytes))
	throw new InvalidXxx(ex);

API Proposal

namespace System;

public class Convert
{
    public static bool TryFromHexString(string s, out byte[] bytes);
}

API Usage

if (!Convert.TryFromHexString(inputString, out var myBytes))
	throw new InvalidXxx(ex);

Alternative Designs

TryFromBase64String returns a Span bytes instead with the number bytesWritten, so maybe it should be the same, but since the number of bytes are easy to guest with hex string (just divide string length by 2) I'm not sure it's useful

public static bool TryFromHexString(string s, Span<byte> bytes, out int bytesWritten)

Risks

I don't see any

Author:	Socolin
Assignees:	-
Labels:	api-suggestion, area-System.Runtime, untriaged
Milestone:	-

[dakersnar]

This is a reasonable proposal. We would want to match existing patterns (the version called out in your Alternative Designs section), mostly because Span<Byte> doesn't need to be allocated. Can you update your proposal?

[ghost]

This issue has been marked needs-author-action and may be missing some important information.

[ghost]

This issue has been automatically marked no-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will remove no-recent-activity.

[Socolin]

This is a reasonable proposal. We would want to match existing patterns (the version called out in your Alternative Designs section), mostly because Span<Byte> doesn't need to be allocated. Can you update your proposal?

I've updated my proposal

[terrajobst]

Video

• We should align the shape with the existing Try(From/To)Base64 API
  • If we accept a span to write to, we should accept a span as the source
  • For string, it makes sense to have a null-rejecting overload
• TryFromHexString should throw if destination is too small because Try-pattern are supposed to have a single failure mode
  • However, that's not what the existing TryFromBase64 method does
  • Convenience wise, we believe it's worth to return false for both cases here

namespace System;

public partial class Convert
{
    public static bool TryFromHexString(string source, Span<byte> destination, out int bytesWritten);
    public static bool TryFromHexString(ReadOnlySpan<char> source, Span<byte> destination, out int bytesWritten);
    public static bool TryToHexString(ReadOnlySpan<byte> source, Span<char> destination, out int charsWritten);
}

[stephentoub]

TryFromHexString should throw if destination is too small because Try-pattern are supposed to have a single failure mode However, that's not what the existing TryFromBase64 method does Convenience wise, we believe it's worth to return false for both cases here

Having a return value of false mean something additional to "the destination buffer is too small" is a problem. It means an algorithm like:

while (true)
{
    if (TryFromHexString(..., destination, ...) break;
    destination = new byte[destination.Length * 2];
}

will OOM if given invalid input. Is the intent you're that this is never supposed to be used that way and a dev is supposed to precompute the required size?

[hrrrrustic]

What's the final decision here? Is one more API review meeting required?
I created a PR with no-throwing implementation, but changing it to throwing is easy

[kiminuo]

Is it a good time to fix #60393 as well?

edit: Ah, I missed that this issue is only about FROM HEX.
edit: edit: Ah, but #86556 adds TryToHexString as well.

[terrajobst]

Video

• FromHexString
  • We concluded we don't want the boolean because it either throws in unexpected ways or it collapses multiples errors which makes it non-trivial to recover.
  • We should use OperationStatus as the return value and drop the Try prefix
  • We should support incremental use by adding a consumed out parameter
• ToHexString
  • All bytes are valid, only error is destination is too small. Try-pattern is fine.

namespace System;

public partial class Convert
{
    public static OperationStatus FromHexString(string source, Span<byte> destination, out int charsConsumed, out int bytesWritten);
    public static OperationStatus FromHexString(ReadOnlySpan<char> source, Span<byte> destination, out int charsConsumed, out int bytesWritten);

    public static bool TryToHexString(ReadOnlySpan<byte> source, Span<char> destination, out int charsWritten);
}

[tannergooding]

This isn't going to make .NET 8. There is a pending PR, but its still in draft mode.

We'll be able to land it sometime after the repo opens for .NET 9 changes and the PR is no longer marked draft

[markusschaber]

Is there anything we can do support you to get the PR past "draft" status?

[tannergooding]

We're still locking down on .NET 8 cleanup and other issues.

The main branch is open for .NET 9 changes now and we should be getting to it "soon".

[markusschaber]

Thanks. I want it because of the possibility to provide a span as destination, this saves me a temporary array allocation and copying, at the non-cost of shorter code. :-)