BinaryData.Empty.ToString throws ArgumentNullException

[amis92]

Description

Using package System.Memory.Data v6+, System.BinaryData.ToString() called on an instance with zero-length data (BinaryData.FromStream(Stream.Null), BinaryData.FromString(string.Empty), BinaryData.FromBytes(Array.Empty<byte>()) etc.) throws System.ArgumentNullException.

Reproduction Steps

BinaryData.Empty.ToString()

Expected behavior

Method should return empty string, as it did/does in package v1 (1.0.0, 1.0.1, 1.0.2).
The expression BinaryData.FromString(data).ToString() == data where data is any valid non-null string instance should be true.

Actual behavior

ArgumentNullException is thrown:

System.ArgumentNullException: System.ArgumentNullException: Array cannot be null. (Parameter 'bytes')
  + System.Text.Encoding.GetString(System.Byte*, int)
  + System.BinaryData.ToString()

Regression?

The method does not throw and returns an empty string using System.Memory.Data v1.

Known Workarounds

No response

Configuration

No response

Other information

The original implementation was:

        public override string ToString()
        {
            if (MemoryMarshal.TryGetArray(_bytes, out ArraySegment<byte> segment))
                return Encoding.UTF8.GetString(segment.Array, segment.Offset, segment.Count);
            return Encoding.UTF8.GetString(_bytes.ToArray());
        }

Now it uses byte* which seems to be the problem (empty Span casts as a null ref):

runtime/src/libraries/System.Memory.Data/src/System/BinaryData.cs

Lines 190 to 197 in 7ab6c18

	public override unsafe string ToString()
	{
	ReadOnlySpan<byte> span = _bytes.Span;
	fixed (byte* ptr = span)
	{
	return Encoding.UTF8.GetString(ptr, span.Length);
	}
	}

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

Issue Details

---

Description

Using package System.Memory.Data v6+, System.BinaryData.ToString() called on an instance with zero-length data (BinaryData.FromStream(Stream.Null), BinaryData.FromString(string.Empty), BinaryData.FromBytes(Array.Empty<byte>()) etc.) throws System.ArgumentNullException.

Reproduction Steps

BinaryData.Empty.ToString()

Expected behavior

Method should return empty string, as it did/does in package v1 (1.0.0, 1.0.1, 1.0.2).
The expression BinaryData.FromString(data).ToString() == data where data is any valid non-null string instance should be true.

Actual behavior

ArgumentNullException is thrown:

System.ArgumentNullException: System.ArgumentNullException: Array cannot be null. (Parameter 'bytes')
  + System.Text.Encoding.GetString(System.Byte*, int)
  + System.BinaryData.ToString()

Regression?

The method does not throw and returns an empty string using System.Memory.Data v1.

Known Workarounds

No response

Configuration

No response

Other information

The original implementation was:

public override string ToString()
        {
            if ([MemoryMarshal](https://docs.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.memorymarshal).TryGetArray(_bytes, out [ArraySegment](https://docs.microsoft.com/en-us/dotnet/api/system.arraysegment-1)<byte> segment))
                return [Encoding](https://docs.microsoft.com/en-us/dotnet/api/system.text.encoding).UTF8.GetString(segment.Array, segment.Offset, segment.Count);
            return [Encoding](https://docs.microsoft.com/en-us/dotnet/api/system.text.encoding).UTF8.GetString(_bytes.ToArray());
        }

Now it uses byte* which seems to be the problem (empty Span casts as a null ref):

        public override unsafe string ToString()
        {
            ReadOnlySpan<byte> span = _bytes.Span;
            fixed (byte* ptr = span)
            {
                return Encoding.UTF8.GetString(ptr, span.Length);
            }
        }

Author:	amis92
Assignees:	-
Labels:	area-System.Memory, untriaged
Milestone:	-

[kasperk81]

missing if (_bytes.Length == 0) return string.Empty;

[adamsitnik]

@amis92 @kasperk81 would any of you be interested in sending a PR with a fix?

[amis92]

Any chance for this to be backported as a bugfix into 6.x packages, or is it too much of a breaking-unbreaking thing? I've found out because one of my dependencies pulled in higher version and so I couldn't really downgrade and had to make a workaround, and with 6.0 being LTS, this bug will keep being there in many places.