Clean up a few more bugs and inconsistencies in ReadLimitLengthStream.

2 files changed, 75 insertions, 26 deletions

diff --git a/Duplicati/Library/Utility/ReadLimitLengthStream.cs b/Duplicati/Library/Utility/ReadLimitLengthStream.cs
index 3faa0fa5e..06ac06907 100644
--- a/Duplicati/Library/Utility/ReadLimitLengthStream.cs
+++ b/Duplicati/Library/Utility/ReadLimitLengthStream.cs
@@ -47,6 +47,17 @@ namespace Duplicati.Library.Utility
                     {

                         m_innerStream.Seek(m_start, SeekOrigin.Begin);

                     }

+                    else if (m_innerStream.Position < m_start)

+                    {

+                        // If the underlying stream doesn't support seeking,

+                        // this will instead simulate the seek by reading until

+                        // the underlying stream is at the start position.

+                        long bytesToRead = m_start - m_innerStream.Position;

+                        for (long i = 0; i < bytesToRead; i++)

+                        {

+                            m_innerStream.ReadByte();

+                        }

+                    }

                     else

                     {

                         throw new ArgumentException("Cannot seek stream to starting position", nameof(innerStream));

@@ -98,7 +109,7 @@ namespace Duplicati.Library.Utility
                     throw new ArgumentException("Unknown SeekOrigin", nameof(origin));

             }
 
-            return ClampInnerStreamPosition(innerPosition) - m_start;
+            return this.Position;
         }
 
         public override void SetLength(long value)
@@ -141,11 +152,6 @@ namespace Duplicati.Library.Utility
             // Since this wrapper does not own the underlying stream, we do not want it to close the underlying stream
         }
 
-        public override Task CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken)
-        {
-            return m_innerStream.CopyToAsync(destination, bufferSize, cancellationToken);
-        }
-
         public override int EndRead(IAsyncResult asyncResult)
         {
             return m_innerStream.EndRead(asyncResult);
@@ -185,15 +191,36 @@ namespace Duplicati.Library.Utility
 
         private int GetAllowedCount(int count)
         {
-            long pos = m_innerStream.Position - m_start;
-            long maxCount = m_length - pos;
-            if (pos < 0 || maxCount < 0)

+            if (m_innerStream.Position < m_start)

+            {

+                // The stream is positioned before the starting point.

+                // This state is exposed externally as having Position==0,

+                // so if possible, seek to the start and then read from there.

+                if (CanSeek)

+                {

+                    this.Position = 0;

+                }

+                else

+                {

+                    // If the underlying stream doesn't support seeking though,

+                    // this will instead simulate the seek by reading until the underlying stream is at the start position.

+                    long bytesToRead = m_start - m_innerStream.Position;

+                    for (long i = 0; i < bytesToRead; i++)

+                    {

+                        m_innerStream.ReadByte();

+                    }

+                }

+            }
+
+            long pos = this.Position;
+            if (pos >= m_length)

             {

-                // The stream is positioned before the starting point or after the end.

-                // Nothing can be read.

+                // The stream is at or past the end of the limit.

+                // Nothing should be read.

                 return 0;

             }
 
+            long maxCount = m_length - pos;
             if (count > maxCount)
             {
                 return (int)maxCount;
@@ -204,16 +231,17 @@ namespace Duplicati.Library.Utility
 
         private long ClampInnerStreamPosition(long position)

         {

-            // Note that this allows this stream to have positions in the range -1 to m_length.

-            // Reading at -1 or m_length should return nothing.

-            if (position < m_start - 1)

+            // Note that this allows this stream to have positions in the range 0 to m_length.

+            // Reading at m_length should return nothing.

+            if (position < m_start)

             {

-                return m_start - 1;

+                return m_start;

             }

 

-            if (position > m_start + m_length)

+            long maxPosition = m_start + m_length;

+            if (position > maxPosition)

             {

-                return m_start + m_length;

+                return maxPosition;

             }

 

             return position;

diff --git a/Duplicati/UnitTest/UtilityTests.cs b/Duplicati/UnitTest/UtilityTests.cs
index 11497d7b1..35a063ae7 100644
--- a/Duplicati/UnitTest/UtilityTests.cs
+++ b/Duplicati/UnitTest/UtilityTests.cs
@@ -173,7 +173,7 @@ namespace Duplicati.UnitTest
 

                     // Set the position directly and read a shorter range

                     stream.Position = 2;

-                    Assert.AreEqual(2, stream.ReadAsync(readBuffer, 0, 2, CancellationToken.None).Await(), "Read count");

+                    Assert.AreEqual(2, stream.ReadAsync(readBuffer, 0, 2).Await(), "Read count");

                     assertArray(Enumerable.Range(2, 2), readBuffer.Take(2));

                 }

 

@@ -188,8 +188,23 @@ namespace Duplicati.UnitTest
                     Assert.AreEqual(4, stream.Read(readBuffer, 0, 4), "Read count");

                     assertArray(Enumerable.Range(2, 4), readBuffer.Take(4));

 

+                    // Test CopyTo

+                    using (MemoryStream destination = new MemoryStream())

+                    {

+                        stream.Position = 0;

+                        stream.CopyTo(destination);

+                        assertArray(Enumerable.Range(2, 4), destination.ToArray());

+                    }

+

+                    // Test CopyToAsync

+                    using (MemoryStream destination = new MemoryStream())

+                    {

+                        stream.Position = 0;

+                        stream.CopyToAsync(destination).Await();

+                        assertArray(Enumerable.Range(2, 4), destination.ToArray());

+                    }

+

                     // Test seeking

-                    testSeek(stream, -1, -1);

                     testSeek(stream, 0, 2);

                     testSeek(stream, 1, 3);

                     testSeek(stream, 2, 4);

@@ -203,8 +218,8 @@ namespace Duplicati.UnitTest
 

                     // Test clamping of position

                     stream.Position = -2;

-                    Assert.AreEqual(-1, stream.Position);

-                    Assert.AreEqual(1, baseStream.Position);

+                    Assert.AreEqual(0, stream.Position);

+                    Assert.AreEqual(2, baseStream.Position);

 

                     stream.Position = 9;

                     Assert.AreEqual(4, stream.Position);

@@ -212,13 +227,19 @@ namespace Duplicati.UnitTest
 

                     // Make sure changing the base stream still shows clamped positions

                     baseStream.Position = 0;

-                    Assert.AreEqual(-1, stream.Position);

+                    Assert.AreEqual(0, stream.Position);

 

                     baseStream.Position = 4;

                     Assert.AreEqual(2, stream.Position);

 

                     baseStream.Position = 10;

                     Assert.AreEqual(4, stream.Position);

+

+                    // Reading when the baseStream is positioned before the start of the limit

+                    // should still be possible, and should seek to return the correct values.

+                    baseStream.Position = 0;

+                    Assert.AreEqual(0, stream.Position);

+                    Assert.AreEqual(2, stream.ReadByte());

                 }

 

                 // Check a stream with length 0

@@ -232,7 +253,7 @@ namespace Duplicati.UnitTest
                     Assert.AreEqual(0, stream.Read(readBuffer, 0, 4), "Read count");

 

                     // Test seeking

-                    testSeek(stream, -1, -1);

+                    testSeek(stream, 0, -1);

                     testSeek(stream, 0, -1);

 

                     // Test seeking from current

@@ -242,8 +263,8 @@ namespace Duplicati.UnitTest
 

                     // Test clamping of position

                     stream.Position = -2;

-                    Assert.AreEqual(-1, stream.Position);

-                    Assert.AreEqual(1, baseStream.Position);

+                    Assert.AreEqual(0, stream.Position);

+                    Assert.AreEqual(2, baseStream.Position);

 

                     stream.Position = 9;

                     Assert.AreEqual(0, stream.Position);

@@ -251,7 +272,7 @@ namespace Duplicati.UnitTest
 

                     // Make sure changing the base stream still shows clamped positions

                     baseStream.Position = 0;

-                    Assert.AreEqual(-1, stream.Position);

+                    Assert.AreEqual(0, stream.Position);

 

                     baseStream.Position = 4;

                     Assert.AreEqual(0, stream.Position);