sharpcompress/docs/DataDescriptorStream-RewindableStream-Fix.md

# DataDescriptorStream and RewindableStream Fix

## Summary

Fixed the `Zip_Uncompressed_Read_All` test failure caused by incompatibility between `DataDescriptorStream` seeking requirements and the new `RewindableStream` wrapper used in `StreamingZipHeaderFactory`.

## Problem Description

### Symptom
The test `Zip_Uncompressed_Read_All` was failing with:
```
System.NotSupportedException : Cannot seek outside buffered region.
```

### Root Cause

The issue had two related aspects:

#### 1. Double-Wrapping of RewindableStream

`StreamingZipHeaderFactory.ReadStreamHeader()` was creating a new `RewindableStream` wrapper:
```csharp
var rewindableStream = new RewindableStream(stream);
```

When `ReaderFactory.OpenReader()` already wraps the input stream with `SeekableRewindableStream` (for seekable streams), this resulted in double-wrapping:
```
DataDescriptorStream
  -> NonDisposingStream
    -> RewindableStream (new, plain)  <-- created by ReadStreamHeader
      -> SeekableRewindableStream     <-- created by ReaderFactory
        -> FileStream
```

The inner plain `RewindableStream` lost the seeking capability of `SeekableRewindableStream`.

#### 2. Recording State Interference

Even after fixing the double-wrapping using `RewindableStream.EnsureSeekable()`, there was another issue:

`StreamingZipHeaderFactory.ReadStreamHeader()` contains code to peek ahead when checking for zero-length files with `UsePostDataDescriptor`:

```csharp
rewindableStream.StartRecording();
var nextHeaderBytes = reader.ReadUInt32();
rewindableStream.Rewind(true);
```

This code was interfering with the recording state that `ReaderFactory.OpenReader()` had set up:

1. `ReaderFactory.OpenReader()` calls `bStream.StartRecording()` at position 0
2. Factory detection calls `StreamingZipHeaderFactory.ReadStreamHeader()` via `IsZipFile()`
3. Inside `ReadStreamHeader`, the above code overwrites the recorded position
4. `Rewind(true)` stops recording and seeks to the wrong position
5. When control returns to `Factory.TryOpenReader()`, it calls `stream.Rewind(true)`, but recording is already stopped, so nothing happens
6. The stream position is not at the beginning, causing subsequent reads to fail

## Solution

### Fix 1: Use EnsureSeekable instead of new RewindableStream

Changed `StreamingZipHeaderFactory.ReadStreamHeader()` to use:
```csharp
var rewindableStream = RewindableStream.EnsureSeekable(stream);
```

This method:
- Returns the existing `RewindableStream` if the stream is already one (avoids double-wrapping)
- Creates a `SeekableRewindableStream` if the underlying stream is seekable
- Creates a plain `RewindableStream` only for non-seekable streams

### Fix 2: Use direct position save/restore for SeekableRewindableStream

For the peek-ahead logic, changed the code to check for `SeekableRewindableStream` specifically and use direct position manipulation:

```csharp
if (rewindableStream is SeekableRewindableStream)
{
    // Direct position save/restore avoids interfering with caller's recording state
    var savedPosition = rewindableStream.Position;
    var nextHeaderBytes = reader.ReadUInt32();
    rewindableStream.Position = savedPosition;
    header.HasData = !IsHeader(nextHeaderBytes);
}
else
{
    // Plain RewindableStream was created fresh by EnsureSeekable, safe to use recording
    rewindableStream.StartRecording();
    var nextHeaderBytes = reader.ReadUInt32();
    rewindableStream.Rewind(true);
    header.HasData = !IsHeader(nextHeaderBytes);
}
```

This approach:
- For `SeekableRewindableStream` (reused from caller): Uses direct position save/restore to avoid clobbering the caller's recording state
- For plain `RewindableStream` (freshly created): Uses the recording mechanism which is safe since the stream isn't shared

## Files Changed

- `src/SharpCompress/Common/Zip/StreamingZipHeaderFactory.cs`
- `src/SharpCompress/Common/Zip/StreamingZipHeaderFactory.Async.cs`

## Design Notes

### Why not fix RewindableStream.CanSeek?

`RewindableStream.CanSeek` returns `true` even though it can only seek within its buffered region. We considered changing this to `false`, but:
1. It would be a breaking change for existing code that relies on `CanSeek`
2. The `RewindableStream` does provide limited seeking capability (within buffer)
3. Checking for `SeekableRewindableStream` specifically is more precise

### Stream Wrapper Hierarchy

Understanding the stream wrapper hierarchy is crucial:

**For seekable source streams (e.g., FileStream):**
```
SeekableRewindableStream (full seeking via underlying stream)
  -> FileStream
```

**For non-seekable source streams (e.g., decompression streams):**
```
RewindableStream (limited seeking via buffer)
  -> DecompressionStream
```

`DataDescriptorStream` needs backward seeking to position the stream correctly after finding the data descriptor marker. This is why proper stream wrapper selection matters.