Fix position handling

src/ImageSharp/IO/BufferedReadStream.cs

@@ -49,7 +49,7 @@ internal sealed class BufferedReadStream : Stream
 
         this.BaseStream = stream;
         this.Length = stream.Length;
-        this.Position = (int)stream.Position;
+        this.readerPosition = stream.Position;
         this.BufferSize = configuration.StreamProcessingBufferSize;
         this.maxBufferIndex = this.BufferSize - 1;
         this.readBuffer = ArrayPool<byte>.Shared.Rent(this.BufferSize);
@@ -96,9 +96,8 @@ internal sealed class BufferedReadStream : Stream
             else
             {
                 // Base stream seek will throw for us if invalid.
-                this.BaseStream.Seek(value, SeekOrigin.Begin);
                 this.readerPosition = value;
-                this.readBufferIndex = this.BufferSize;
+                this.FillReadBuffer();
             }
         }
     }
@@ -147,6 +146,7 @@ internal sealed class BufferedReadStream : Stream
         }
 
         this.readerPosition++;
+
         unsafe
         {
             return this.pinnedReadBuffer[this.readBufferIndex++];
@@ -202,7 +202,7 @@ internal sealed class BufferedReadStream : Stream
         if (this.readerPosition != baseStream.Position)
         {
             baseStream.Seek(this.readerPosition, SeekOrigin.Begin);
-            this.readerPosition = (int)baseStream.Position;
+            this.readerPosition = baseStream.Position;
         }
 
         // Reset to trigger full read on next attempt.

tests/ImageSharp.Tests/IO/BufferedReadStreamTests.cs

@@ -10,12 +10,10 @@ public class BufferedReadStreamTests
     private readonly Configuration configuration;
 
     public BufferedReadStreamTests()
-    {
+        => this.configuration = Configuration.CreateDefaultInstance();
-        this.configuration = Configuration.CreateDefaultInstance();
-    }
 
     public static readonly TheoryData<int> BufferSizes =
-        new TheoryData<int>()
+        new()
         {
             1, 2, 4, 8,
             16, 97, 503,
@@ -28,10 +26,9 @@ public class BufferedReadStreamTests
     public void BufferedStreamCanReadSingleByteFromOrigin(int bufferSize)
     {
         this.configuration.StreamProcessingBufferSize = bufferSize;
-        using (MemoryStream stream = this.CreateTestStream(bufferSize * 3))
+        using MemoryStream stream = CreateTestStream(bufferSize * 3);
-        {
         byte[] expected = stream.ToArray();
-            using (var reader = new BufferedReadStream(this.configuration, stream))
+        using (BufferedReadStream reader = new(this.configuration, stream))
         {
             Assert.Equal(expected[0], reader.ReadByte());
 
@@ -43,18 +40,16 @@ public class BufferedReadStreamTests
         // Position of the stream should be reset on disposal.
         Assert.Equal(1, stream.Position);
     }
-    }
 
     [Theory]
     [MemberData(nameof(BufferSizes))]
     public void BufferedStreamCanReadSingleByteFromOffset(int bufferSize)
     {
         this.configuration.StreamProcessingBufferSize = bufferSize;
-        using (MemoryStream stream = this.CreateTestStream(bufferSize * 3))
+        using MemoryStream stream = CreateTestStream(bufferSize * 3);
-        {
         byte[] expected = stream.ToArray();
         int offset = expected.Length / 2;
-            using (var reader = new BufferedReadStream(this.configuration, stream))
+        using (BufferedReadStream reader = new(this.configuration, stream))
         {
             reader.Position = offset;
 
@@ -67,18 +62,16 @@ public class BufferedReadStreamTests
 
         Assert.Equal(offset + 1, stream.Position);
     }
-    }
 
     [Theory]
     [MemberData(nameof(BufferSizes))]
     public void BufferedStreamCanReadSubsequentSingleByteCorrectly(int bufferSize)
     {
         this.configuration.StreamProcessingBufferSize = bufferSize;
-        using (MemoryStream stream = this.CreateTestStream(bufferSize * 3))
+        using MemoryStream stream = CreateTestStream(bufferSize * 3);
-        {
         byte[] expected = stream.ToArray();
         int i;
-            using (var reader = new BufferedReadStream(this.configuration, stream))
+        using (BufferedReadStream reader = new(this.configuration, stream))
         {
             for (i = 0; i < expected.Length; i++)
             {
@@ -104,19 +97,16 @@ public class BufferedReadStreamTests
 
         Assert.Equal(i, stream.Position);
     }
-    }
 
     [Theory]
     [MemberData(nameof(BufferSizes))]
     public void BufferedStreamCanReadMultipleBytesFromOrigin(int bufferSize)
     {
         this.configuration.StreamProcessingBufferSize = bufferSize;
-        using (MemoryStream stream = this.CreateTestStream(bufferSize * 3))
+        using MemoryStream stream = CreateTestStream(bufferSize * 3);
-        {
+        byte[] buffer = new byte[2];
-            var buffer = new byte[2];
         byte[] expected = stream.ToArray();
-            using (var reader = new BufferedReadStream(this.configuration, stream))
+        using BufferedReadStream reader = new(this.configuration, stream);
-            {
         Assert.Equal(2, reader.Read(buffer, 0, 2));
         Assert.Equal(expected[0], buffer[0]);
         Assert.Equal(expected[1], buffer[1]);
@@ -125,21 +115,17 @@ public class BufferedReadStreamTests
         Assert.True(stream.Position >= bufferSize);
         Assert.Equal(buffer.Length, reader.Position);
     }
-        }
-    }
 
     [Theory]
     [MemberData(nameof(BufferSizes))]
     public void BufferedStreamCanReadSubsequentMultipleByteCorrectly(int bufferSize)
     {
         this.configuration.StreamProcessingBufferSize = bufferSize;
-        using (MemoryStream stream = this.CreateTestStream(bufferSize * 3))
+        using MemoryStream stream = CreateTestStream(bufferSize * 3);
-        {
         const int increment = 2;
-            var buffer = new byte[2];
+        byte[] buffer = new byte[2];
         byte[] expected = stream.ToArray();
-            using (var reader = new BufferedReadStream(this.configuration, stream))
+        using BufferedReadStream reader = new(this.configuration, stream);
-            {
         for (int i = 0, o = 0; i < expected.Length / increment; i++, o += increment)
         {
             // Check values are correct.
@@ -176,21 +162,17 @@ public class BufferedReadStreamTests
             Assert.True(stream.Position > bufferSize * 2);
         }
     }
-        }
-    }
 
     [Theory]
     [MemberData(nameof(BufferSizes))]
     public void BufferedStreamCanReadSubsequentMultipleByteSpanCorrectly(int bufferSize)
     {
         this.configuration.StreamProcessingBufferSize = bufferSize;
-        using (MemoryStream stream = this.CreateTestStream(bufferSize * 3))
+        using MemoryStream stream = CreateTestStream(bufferSize * 3);
-        {
         const int increment = 2;
         Span<byte> buffer = new byte[2];
         byte[] expected = stream.ToArray();
-            using (var reader = new BufferedReadStream(this.configuration, stream))
+        using BufferedReadStream reader = new(this.configuration, stream);
-            {
         for (int i = 0, o = 0; i < expected.Length / increment; i++, o += increment)
         {
             // Check values are correct.
@@ -227,27 +209,22 @@ public class BufferedReadStreamTests
             Assert.True(stream.Position > bufferSize * 2);
         }
     }
-        }
-    }
 
     [Theory]
     [MemberData(nameof(BufferSizes))]
     public void BufferedStreamCanSkip(int bufferSize)
     {
         this.configuration.StreamProcessingBufferSize = bufferSize;
-        using (MemoryStream stream = this.CreateTestStream(bufferSize * 4))
+        using MemoryStream stream = CreateTestStream(bufferSize * 4);
-        {
         byte[] expected = stream.ToArray();
-            using (var reader = new BufferedReadStream(this.configuration, stream))
+        using BufferedReadStream reader = new(this.configuration, stream);
-            {
+        const int skip = 1;
-                int skip = 1;
+        const int plusOne = 1;
-                int plusOne = 1;
         int skip2 = bufferSize;
 
         // Skip
         reader.Skip(skip);
         Assert.Equal(skip, reader.Position);
-                Assert.Equal(stream.Position, reader.Position);
 
         // Read
         Assert.Equal(expected[skip], reader.ReadByte());
@@ -259,11 +236,8 @@ public class BufferedReadStreamTests
         int position = skip + plusOne + skip2;
 
         Assert.Equal(position, reader.Position);
-                Assert.Equal(stream.Position, reader.Position);
         Assert.Equal(expected[position], reader.ReadByte());
     }
-        }
-    }
 
     [Theory]
     [MemberData(nameof(BufferSizes))]
@@ -272,11 +246,10 @@ public class BufferedReadStreamTests
         this.configuration.StreamProcessingBufferSize = bufferSize;
 
         // Create a stream smaller than the default buffer length
-        using (MemoryStream stream = this.CreateTestStream(Math.Max(1, bufferSize / 4)))
+        using MemoryStream stream = CreateTestStream(Math.Max(1, bufferSize / 4));
-        {
         byte[] expected = stream.ToArray();
         int offset = expected.Length / 2;
-            using (var reader = new BufferedReadStream(this.configuration, stream))
+        using (BufferedReadStream reader = new(this.configuration, stream))
         {
             reader.Position = offset;
 
@@ -289,73 +262,87 @@ public class BufferedReadStreamTests
 
         Assert.Equal(offset + 1, stream.Position);
     }
-    }
 
     [Theory]
     [MemberData(nameof(BufferSizes))]
     public void BufferedStreamReadsCanReadAllAsSingleByteFromOrigin(int bufferSize)
     {
         this.configuration.StreamProcessingBufferSize = bufferSize;
-        using (MemoryStream stream = this.CreateTestStream(bufferSize * 3))
+        using MemoryStream stream = CreateTestStream(bufferSize * 3);
-        {
         byte[] expected = stream.ToArray();
-            using (var reader = new BufferedReadStream(this.configuration, stream))
+        using BufferedReadStream reader = new(this.configuration, stream);
-            {
         for (int i = 0; i < expected.Length; i++)
         {
             Assert.Equal(expected[i], reader.ReadByte());
         }
     }
-        }
-    }
 
     [Theory]
     [MemberData(nameof(BufferSizes))]
     public void BufferedStreamThrowsOnNegativePosition(int bufferSize)
     {
         this.configuration.StreamProcessingBufferSize = bufferSize;
-        using (MemoryStream stream = this.CreateTestStream(bufferSize))
+        using MemoryStream stream = CreateTestStream(bufferSize);
-        {
+        using BufferedReadStream reader = new(this.configuration, stream);
-            using (var reader = new BufferedReadStream(this.configuration, stream))
-            {
         Assert.Throws<ArgumentOutOfRangeException>(() => reader.Position = -stream.Length);
     }
-        }
-    }
 
     [Theory]
     [MemberData(nameof(BufferSizes))]
     public void BufferedStreamCanSetPositionToEnd(int bufferSize)
     {
         this.configuration.StreamProcessingBufferSize = bufferSize;
-        using (MemoryStream stream = this.CreateTestStream(bufferSize * 2))
+        using MemoryStream stream = CreateTestStream(bufferSize * 2);
-        {
+        using BufferedReadStream reader = new(this.configuration, stream);
-            using (var reader = new BufferedReadStream(this.configuration, stream))
-            {
         reader.Position = reader.Length;
     }
-        }
-    }
 
     [Theory]
     [MemberData(nameof(BufferSizes))]
     public void BufferedStreamCanSetPositionPastTheEnd(int bufferSize)
     {
         this.configuration.StreamProcessingBufferSize = bufferSize;
-        using (MemoryStream stream = this.CreateTestStream(bufferSize * 2))
+        using MemoryStream stream = CreateTestStream(bufferSize * 2);
-        {
+        using BufferedReadStream reader = new(this.configuration, stream);
-            using (var reader = new BufferedReadStream(this.configuration, stream))
-            {
         reader.Position = reader.Length + 1;
         Assert.Equal(stream.Length + 1, stream.Position);
     }
+
+    [Fact]
+    public void BufferedStreamCanSetPositionMultipleTimes()
+    {
+        Configuration configuration = new()
+        {
+            StreamProcessingBufferSize = 16
+        };
+
+        byte[] buffer = new byte[255];
+        for (int i = 0; i < buffer.Length; i++)
+        {
+            buffer[i] = (byte)i;
         }
+
+        BufferedReadStream bufferedStream = new(configuration, new MemoryStream(buffer));
+
+        // Read more then fits into the buffer.
+        for (int i = 0; i < 20; i++)
+        {
+            bufferedStream.ReadByte();
+        }
+
+        // Set the Position twice.
+        bufferedStream.Position = 10;
+        bufferedStream.Position = 3;
+
+        // readValue is 25, but should be 3
+        int readValue = bufferedStream.ReadByte();
+        Assert.Equal(3, readValue);
     }
 
-    private MemoryStream CreateTestStream(int length)
+    private static MemoryStream CreateTestStream(int length)
     {
-        var buffer = new byte[length];
+        byte[] buffer = new byte[length];
-        var random = new Random();
+        Random random = new();
         random.NextBytes(buffer);
 
         return new EvilStream(buffer);
@@ -371,8 +358,6 @@ public class BufferedReadStreamTests
         }
 
         public override int Read(byte[] buffer, int offset, int count)
-        {
+            => base.Read(buffer, offset, 1);
-            return base.Read(buffer, offset, 1);
-        }
     }
 }