Disposing TarEntry.DataStream causes TarReader.GetNextEntry to throw ObjectDisposedException

[ericstj]

Description

When disposing the stream of a single TarEntry, I cannot advance the TarReader to the next entry.

Consider the following sample:

using (var tarReader = new TarReader(unseekableStream))
{
    TarEntry? entry;
    while ((entry = tarReader.GetNextEntry()) != null)
    {
         Stream s = entry.DataStream;
         s.CopyTo(someOtherStream);
         s.Dispose();
    }
}

This will throw

System.ObjectDisposedException
  HResult=0x80131622
  Message=Cannot access a disposed object.
Object name: 'System.Formats.Tar.SubReadStream'.
  Source=System.Private.CoreLib
  StackTrace:
System.Private.CoreLib.dll!System.ThrowHelper.ThrowObjectDisposedException(object instance) Line 458
	at /_/src/libraries/System.Private.CoreLib/src/System/ThrowHelper.cs(458)
System.Private.CoreLib.dll!System.ObjectDisposedException.ThrowIf(bool condition, object instance) Line 61
	at /_/src/libraries/System.Private.CoreLib/src/System/ObjectDisposedException.cs(61)
System.Formats.Tar.dll!System.Formats.Tar.SubReadStream.Position.get() Line 52
	at /_/src/libraries/System.Formats.Tar/src/System/Formats/Tar/SubReadStream.cs(52)
System.Formats.Tar.dll!System.Formats.Tar.TarReader.AdvanceDataStreamIfNeeded() Line 228
	at /_/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarReader.cs(228)
System.Formats.Tar.dll!System.Formats.Tar.TarReader.GetNextEntry(bool copyData) Line 133
	at /_/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarReader.cs(133)

The suspect code is here:

runtime/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarReader.cs

Lines 200 to 237 in 84b3339

	internal void AdvanceDataStreamIfNeeded()
	{
	if (_previouslyReadEntry == null)
	{
	return;
	}
	if (_archiveStream.CanSeek)
	{
	Debug.Assert(_previouslyReadEntry._header._endOfHeaderAndDataAndBlockAlignment > 0);
	_archiveStream.Position = _previouslyReadEntry._header._endOfHeaderAndDataAndBlockAlignment;
	}
	else if (_previouslyReadEntry._header._size > 0)
	{
	// When working with seekable streams, every time we return an entry, we avoid advancing the pointer beyond the data section
	// This is so the user can read the data if desired. But if the data was not read by the user, we need to advance the pointer
	// here until it's located at the beginning of the next entry header.
	// This should only be done if the previous entry came from a TarReader and it still had its original SubReadStream or SeekableSubReadStream.
	if (_previouslyReadEntry._header._dataStream is not SubReadStream dataStream)
	{
	return;
	}
	if (!dataStream.HasReachedEnd)
	{
	// If the user did not advance the position, we need to make sure the position
	// pointer is located at the beginning of the next header.
	if (dataStream.Position < (_previouslyReadEntry._header._size - 1))
	{
	long bytesToSkip = _previouslyReadEntry._header._size - dataStream.Position;
	TarHelpers.AdvanceStream(_archiveStream, bytesToSkip);
	dataStream.HasReachedEnd = true; // Now the pointer is beyond the limit, so any read attempts should throw
	}
	}
	TarHelpers.SkipBlockAlignmentPadding(_archiveStream, _previouslyReadEntry._header._size);
	}
	}

Reproduction Steps

See above

Expected behavior

No exception thrown

Actual behavior

ObjectDisposedException

Regression?

No

Known Workarounds

Copy the entry to a memory stream. Copy the archive to a memory stream before opening (so it is seek-able). Etc

Configuration

No response

Other information

I noticed when debugging this that the state of the SubReadStream was the following:

Note that _positionInSuperStream is the same as _endInSuperStream. I noticed that the HasReachedEnd property is checked before accessing Position which will throw. It seems like HasReachedEnd checks both the fields mentioned, but will only treat it as true if greater, perhaps it should have been greater or equal?

[dotnet-policy-service]

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

[tmds]

This code determines how much of the superstream was read through the SubReadStream by calling the Position property. That getter throws ODE because the SubReadStream got disposed:

runtime/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarReader.cs

Lines 226 to 233 in 7dc3669

	// If the user did not advance the position, we need to make sure the position
	// pointer is located at the beginning of the next header.
	if (dataStream.Position < (_previouslyReadEntry._header._size - 1))
	{
	long bytesToSkip = _previouslyReadEntry._header._size - dataStream.Position;
	TarHelpers.AdvanceStream(_archiveStream, bytesToSkip);
	dataStream.HasReachedEnd = true; // Now the pointer is beyond the limit, so any read attempts should throw
	}