Update Read(byte[] buffer, int offset, int count) to not write bytes to read buffer when the read buffer is empty and count is greater than the number of bytes read from the server.

Lazily initialize read and write buffer.

Author: drieseng

src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_OutsideOfReadBuffer.cs renamed to src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_NoBuffering.cs

@@ -8,7 +8,7 @@ namespace Renci.SshNet.Tests.Classes.Sftp
 {
     [TestClass]
     //[Ignore]
-    public class SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_OutsideOfReadBuffer : SftpFileStreamTestBase
+    public class SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_NoBuffering : SftpFileStreamTestBase
     {
         private Random _random;
         private string _path;
@@ -21,8 +21,7 @@ public class SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOf
         private SftpFileStream _target;
         private long _actual;
         private byte[] _buffer;
-        private byte[] _serverData1;
-        private byte[] _serverData2;
+        private byte[] _serverData;
 
         protected override void SetupData()
         {
@@ -36,9 +35,8 @@ protected override void SetupData()
             _readBufferSize = 20;
             _writeBufferSize = (uint) _random.Next(5, 1000);
             _handle = GenerateRandom(_random.Next(1, 10), _random);
-            _buffer = new byte[_readBufferSize + 1];
-            _serverData1 = GenerateRandom((int)  _readBufferSize, _random);
-            _serverData2 = GenerateRandom(10, _random);
+            _buffer = new byte[_readBufferSize];
+            _serverData = GenerateRandom(_buffer.Length, _random);
         }
 
         protected override void SetupMocks()
@@ -47,20 +45,17 @@ protected override void SetupMocks()
                            .Setup(p => p.RequestOpen(_path, Flags.Read | Flags.CreateNewOrOpen, false))
                            .Returns(_handle);
             SftpSessionMock.InSequence(MockSequence)
-                           .Setup(p => p.CalculateOptimalReadLength((uint) _bufferSize))
+                           .Setup(p => p.CalculateOptimalReadLength((uint)_bufferSize))
                            .Returns(_readBufferSize);
             SftpSessionMock.InSequence(MockSequence)
-                           .Setup(p => p.CalculateOptimalWriteLength((uint) _bufferSize, _handle))
+                           .Setup(p => p.CalculateOptimalWriteLength((uint)_bufferSize, _handle))
                            .Returns(_writeBufferSize);
             SftpSessionMock.InSequence(MockSequence)
                            .Setup(p => p.IsOpen)
                            .Returns(true);
             SftpSessionMock.InSequence(MockSequence)
                            .Setup(p => p.RequestRead(_handle, 0UL, _readBufferSize))
-                           .Returns(_serverData1);
-            SftpSessionMock.InSequence(MockSequence)
-                           .Setup(p => p.RequestRead(_handle, _readBufferSize, _readBufferSize))
-                           .Returns(_serverData2);
+                           .Returns(_serverData);
             SftpSessionMock.InSequence(MockSequence)
                            .Setup(p => p.IsOpen)
                            .Returns(true);
@@ -104,21 +99,24 @@ public void IsOpenOnSftpSessionShouldHaveBeenInvokedTwice()
         }
 
         [TestMethod]
-        public void ReadShouldReadFromServer()
+        public void ReadShouldReturnReadBytesFromServer()
         {
             SftpSessionMock.InSequence(MockSequence)
                            .Setup(p => p.IsOpen)
                            .Returns(true);
             SftpSessionMock.InSequence(MockSequence)
-                           .Setup(p => p.RequestRead(_handle, 0, _readBufferSize))
-                           .Returns(new byte[] {0x04});
+                           .Setup(p => p.RequestRead(_handle, 0UL, _readBufferSize))
+                           .Returns(new byte[] { 0x05, 0x04 });
 
             var buffer = new byte[1];
 
             var bytesRead = _target.Read(buffer, 0, buffer.Length);
 
             Assert.AreEqual(buffer.Length, bytesRead);
-            Assert.AreEqual(0x04, buffer[0]);
+            Assert.AreEqual(0x05, buffer[0]);
+
+            SftpSessionMock.Verify(p => p.IsOpen, Times.Exactly(3));
+            SftpSessionMock.Verify(p => p.RequestRead(_handle, 0UL, _readBufferSize), Times.Exactly(2));
         }
     }
 }

src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_WithinReadBuffer.cs renamed to src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_ReadBuffer.cs

@@ -8,7 +8,7 @@ namespace Renci.SshNet.Tests.Classes.Sftp
 {
     [TestClass]
     //[Ignore]
-    public class SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_WithinReadBuffer : SftpFileStreamTestBase
+    public class SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_ReadBuffer : SftpFileStreamTestBase
     {
         private Random _random;
         private string _path;
@@ -21,7 +21,8 @@ public class SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOf
         private SftpFileStream _target;
         private long _actual;
         private byte[] _buffer;
-        private byte[] _serverData;
+        private byte[] _serverData1;
+        private byte[] _serverData2;
 
         protected override void SetupData()
         {
@@ -35,8 +36,9 @@ protected override void SetupData()
             _readBufferSize = 20;
             _writeBufferSize = (uint) _random.Next(5, 1000);
             _handle = GenerateRandom(_random.Next(1, 10), _random);
-            _buffer = new byte[_readBufferSize - 5];
-            _serverData = GenerateRandom((int) _readBufferSize, _random);
+            _buffer = new byte[2]; // should be less than size of read buffer
+            _serverData1 = GenerateRandom((int) _readBufferSize, _random);
+            _serverData2 = GenerateRandom((int) _readBufferSize, _random);
         }
 
         protected override void SetupMocks()
@@ -45,17 +47,17 @@ protected override void SetupMocks()
                            .Setup(p => p.RequestOpen(_path, Flags.Read | Flags.CreateNewOrOpen, false))
                            .Returns(_handle);
             SftpSessionMock.InSequence(MockSequence)
-                           .Setup(p => p.CalculateOptimalReadLength((uint)_bufferSize))
+                           .Setup(p => p.CalculateOptimalReadLength((uint) _bufferSize))
                            .Returns(_readBufferSize);
             SftpSessionMock.InSequence(MockSequence)
-                           .Setup(p => p.CalculateOptimalWriteLength((uint)_bufferSize, _handle))
+                           .Setup(p => p.CalculateOptimalWriteLength((uint) _bufferSize, _handle))
                            .Returns(_writeBufferSize);
             SftpSessionMock.InSequence(MockSequence)
                            .Setup(p => p.IsOpen)
                            .Returns(true);
             SftpSessionMock.InSequence(MockSequence)
                            .Setup(p => p.RequestRead(_handle, 0UL, _readBufferSize))
-                           .Returns(_serverData);
+                           .Returns(_serverData1);
             SftpSessionMock.InSequence(MockSequence)
                            .Setup(p => p.IsOpen)
                            .Returns(true);
@@ -99,54 +101,39 @@ public void IsOpenOnSftpSessionShouldHaveBeenInvokedTwice()
         }
 
         [TestMethod]
-        public void ReadLessThanReadBufferSizeShouldReturnBytesFromReadBuffer()
+        public void ReadBytesThatWereNotBufferedBeforeSeekShouldReadBytesFromServer()
         {
             SftpSessionMock.InSequence(MockSequence)
                            .Setup(p => p.IsOpen)
                            .Returns(true);
-
-            var buffer = new byte[_readBufferSize - 3];
-
-            var bytesRead = _target.Read(buffer, 0, buffer.Length);
-
-            Assert.AreEqual(buffer.Length, bytesRead);
-            Assert.IsTrue(_serverData.Take(buffer.Length).IsEqualTo(buffer));
-        }
-
-        [TestMethod]
-        public void ReadReadBufferSizeShouldReturnBytesFromReadBuffer()
-        {
             SftpSessionMock.InSequence(MockSequence)
-                           .Setup(p => p.IsOpen)
-                           .Returns(true);
+                           .Setup(p => p.RequestRead(_handle, 0UL, _readBufferSize))
+                           .Returns(_serverData2);
 
-            var buffer = new byte[_readBufferSize];
+            var bytesRead = _target.Read(_buffer, 0, _buffer.Length);
 
-            var bytesRead = _target.Read(buffer, 0, buffer.Length);
+            Assert.AreEqual(_buffer.Length, bytesRead);
+            Assert.IsTrue(_serverData2.Take(_buffer.Length).IsEqualTo(_buffer));
 
-            Assert.AreEqual(buffer.Length, bytesRead);
-            Assert.IsTrue(_serverData.IsEqualTo(buffer));
+            SftpSessionMock.Verify(p => p.IsOpen, Times.Exactly(3));
+            SftpSessionMock.Verify(p => p.RequestRead(_handle, 0UL, _readBufferSize), Times.Exactly(2));
         }
 
         [TestMethod]
-        public void ReadMoreThanReadBufferSizeShouldReturnBytesFromReadBufferAndReadRemaningBytesFromServer()
+        public void ReadBytesThatWereBufferedBeforeSeekShouldReadBytesFromServer()
         {
-            var serverData2 = GenerateRandom(6, _random);
-
             SftpSessionMock.InSequence(MockSequence)
                            .Setup(p => p.IsOpen)
                            .Returns(true);
             SftpSessionMock.InSequence(MockSequence)
-                           .Setup(p => p.RequestRead(_handle, _readBufferSize, _readBufferSize))
-                           .Returns(serverData2);
-
-            var buffer = new byte[_readBufferSize + 4];
+                           .Setup(p => p.RequestRead(_handle, 0UL, _readBufferSize))
+                           .Returns(_serverData2);
 
+            var buffer = new byte[_buffer.Length + 1]; // we read one byte that was previously buffered
             var bytesRead = _target.Read(buffer, 0, buffer.Length);
 
             Assert.AreEqual(buffer.Length, bytesRead);
-            Assert.IsTrue(_serverData.Take((int) _readBufferSize).IsEqualTo(buffer.Take((int) _readBufferSize)));
-            Assert.IsTrue(serverData2.Take(4).IsEqualTo(buffer.Take((int) _readBufferSize, 4)));
+            Assert.IsTrue(_serverData2.Take(buffer.Length).IsEqualTo(buffer));
         }
     }
 }

src/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj

@@ -439,8 +439,8 @@
     <Compile Include="Classes\Sftp\SftpFileStreamTest_Seek_PositionedAtBeginningOfStream_OriginBeginAndOffsetNegative.cs" />
     <Compile Include="Classes\Sftp\SftpFileStreamTest_Seek_PositionedAtBeginningOfStream_OriginBeginAndOffsetPositive.cs" />
     <Compile Include="Classes\Sftp\SftpFileStreamTest_Seek_PositionedAtBeginningOfStream_OriginBeginAndOffsetZero.cs" />
-    <Compile Include="Classes\Sftp\SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_OutsideOfReadBuffer.cs" />
-    <Compile Include="Classes\Sftp\SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_WithinReadBuffer.cs" />
+    <Compile Include="Classes\Sftp\SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_NoBuffering.cs" />
+    <Compile Include="Classes\Sftp\SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_ReadBuffer.cs" />
     <Compile Include="Classes\Sftp\SftpFileStreamTest_SetLength_Closed.cs" />
     <Compile Include="Classes\Sftp\SftpFileStreamTest_SetLength_Disposed.cs" />
     <Compile Include="Classes\Sftp\SftpFileStreamTest_SetLength_SessionNotOpen.cs" />