From bcc8def223653d6c8718c180a43b79bd75eed7a6 Mon Sep 17 00:00:00 2001 From: Gert Driesen Date: Sun, 19 Mar 2017 20:37:14 +0100 Subject: [PATCH 01/10] No longer track write position separately. Reading from the SftpFileStream will not also affect the position at which a subsequent write will be performed. The position at which a write is performed in the server file is now always the current position minus the position in the buffer. --- src/Renci.SshNet/Sftp/SftpFileStream.cs | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/src/Renci.SshNet/Sftp/SftpFileStream.cs b/src/Renci.SshNet/Sftp/SftpFileStream.cs index fbc4d4674..13ba18cb0 100644 --- a/src/Renci.SshNet/Sftp/SftpFileStream.cs +++ b/src/Renci.SshNet/Sftp/SftpFileStream.cs @@ -28,7 +28,6 @@ public class SftpFileStream : Stream private bool _canRead; private bool _canSeek; private bool _canWrite; - private ulong _serverFilePosition; private readonly object _lock = new object(); @@ -262,7 +261,6 @@ internal SftpFileStream(ISftpSession session, string path, FileMode mode, FileAc { var attributes = _session.RequestFStat(_handle, false); _position = attributes.Size; - _serverFilePosition = (ulong) attributes.Size; } } @@ -347,9 +345,6 @@ public override int Read(byte[] buffer, int offset, int count) var data = _session.RequestRead(_handle, (ulong) _position, (uint) _readBufferSize); - // TODO: don't we need to take into account the number of bytes read (data.Length) ? - _serverFilePosition = (ulong) _position; - if (data.Length == 0) { break; @@ -422,11 +417,11 @@ public override int ReadByte() if (_bufferPosition >= _bufferLen) { _bufferPosition = 0; + _bufferLen = 0; var data = _session.RequestRead(_handle, (ulong) _position, (uint) _readBufferSize); _bufferLen = data.Length; - _serverFilePosition = (ulong) _position; if (_bufferLen == 0) { @@ -501,7 +496,6 @@ public override long Seek(long offset, SeekOrigin origin) throw new EndOfStreamException("End of stream."); } _position = newPosn; - _serverFilePosition = (ulong)newPosn; } else { @@ -645,8 +639,7 @@ public override void Write(byte[] buffer, int offset, int count) { using (var wait = new AutoResetEvent(false)) { - _session.RequestWrite(_handle, _serverFilePosition, buffer, offset, tempLen, wait); - _serverFilePosition += (ulong) tempLen; + _session.RequestWrite(_handle, (ulong) _position, buffer, offset, tempLen, wait); } } else @@ -668,8 +661,7 @@ public override void Write(byte[] buffer, int offset, int count) { using (var wait = new AutoResetEvent(false)) { - _session.RequestWrite(_handle, _serverFilePosition, _writeBuffer, 0, _bufferPosition, wait); - _serverFilePosition += (ulong) _bufferPosition; + _session.RequestWrite(_handle, (ulong) (_position - _bufferPosition), _writeBuffer, 0, _bufferPosition, wait); } _bufferPosition = 0; @@ -699,8 +691,7 @@ public override void WriteByte(byte value) { using (var wait = new AutoResetEvent(false)) { - _session.RequestWrite(_handle, _serverFilePosition, _writeBuffer, 0, _bufferPosition, wait); - _serverFilePosition += (ulong) _bufferPosition; + _session.RequestWrite(_handle, (ulong) (_position - _bufferPosition), _writeBuffer, 0, _bufferPosition, wait); } _bufferPosition = 0; @@ -779,8 +770,7 @@ private void FlushWriteBuffer() { using (var wait = new AutoResetEvent(false)) { - _session.RequestWrite(_handle, _serverFilePosition, _writeBuffer, 0, _bufferPosition, wait); - _serverFilePosition += (ulong) _bufferPosition; + _session.RequestWrite(_handle, (ulong) (_position - _bufferPosition), _writeBuffer, 0, _bufferPosition, wait); } _bufferPosition = 0; From 40efaa6e5a4c4712968b1360b7a0dba8d84ad6a7 Mon Sep 17 00:00:00 2001 From: Gert Driesen Date: Mon, 5 Jun 2017 19:10:49 +0200 Subject: [PATCH 02/10] Move back to writing all read bytes to read buffer, not only those that exceed the user supplied buffer. --- ...fferAndReadLessBytesFromServerThanCount.cs | 153 ++++++++++++++++++ ...ferAndReadMoreBytesFromServerThanCount.cs} | 20 +-- ...ngOfStream_OriginBeginAndOffsetNegative.cs | 97 +++++++++++ ...ngOfStream_OriginBeginAndOffsetPositive.cs | 88 ++++++++++ ...inningOfStream_OriginBeginAndOffsetZero.cs | 16 ++ ...nBeginAndOffsetZero_OutsideOfReadBuffer.cs | 124 ++++++++++++++ ...iginBeginAndOffsetZero_WithinReadBuffer.cs | 46 +++++- .../Renci.SshNet.Tests.csproj | 6 +- src/Renci.SshNet/Sftp/SftpFileStream.cs | 23 ++- 9 files changed, 547 insertions(+), 26 deletions(-) create mode 100644 src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Read_ReadMode_NoDataInReaderBufferAndReadLessBytesFromServerThanCount.cs rename src/Renci.SshNet.Tests/Classes/Sftp/{SftpFileStreamTest_Read_ReadMode_NoDataInReaderBufferAndReadMoreBytesThanCount.cs => SftpFileStreamTest_Read_ReadMode_NoDataInReaderBufferAndReadMoreBytesFromServerThanCount.cs} (84%) create mode 100644 src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Seek_PositionedAtBeginningOfStream_OriginBeginAndOffsetNegative.cs create mode 100644 src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Seek_PositionedAtBeginningOfStream_OriginBeginAndOffsetPositive.cs create mode 100644 src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_OutsideOfReadBuffer.cs diff --git a/src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Read_ReadMode_NoDataInReaderBufferAndReadLessBytesFromServerThanCount.cs b/src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Read_ReadMode_NoDataInReaderBufferAndReadLessBytesFromServerThanCount.cs new file mode 100644 index 000000000..d519ab4e7 --- /dev/null +++ b/src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Read_ReadMode_NoDataInReaderBufferAndReadLessBytesFromServerThanCount.cs @@ -0,0 +1,153 @@ +using System; +using System.IO; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; +using Renci.SshNet.Common; +using Renci.SshNet.Sftp; + +namespace Renci.SshNet.Tests.Classes.Sftp +{ + [TestClass] + public class SftpFileStreamTest_Read_ReadMode_NoDataInReaderBufferAndReadLessBytesFromServerThanCount : SftpFileStreamTestBase + { + private string _path; + private SftpFileStream _target; + private byte[] _handle; + private uint _bufferSize; + private uint _readBufferSize; + private uint _writeBufferSize; + private int _actual; + private byte[] _buffer; + private byte[] _serverData1; + private byte[] _serverData2; + private int _serverData1Length; + private int _serverData2Length; + private int _numberOfBytesToRead; + + protected override void SetupData() + { + base.SetupData(); + + var random = new Random(); + _path = random.Next().ToString(); + _handle = GenerateRandom(5, random); + _bufferSize = (uint)random.Next(1, 1000); + _readBufferSize = 20; + _writeBufferSize = 500; + + _numberOfBytesToRead = 20; + _buffer = new byte[_numberOfBytesToRead]; + _serverData1Length = _numberOfBytesToRead - 5; + _serverData1 = GenerateRandom(_serverData1Length, random); + _serverData2Length = _numberOfBytesToRead - _serverData1Length + 3; + _serverData2 = GenerateRandom(_serverData2Length, random); + } + + protected override void SetupMocks() + { + SftpSessionMock.InSequence(MockSequence) + .Setup(p => p.RequestOpen(_path, Flags.Read, false)) + .Returns(_handle); + SftpSessionMock.InSequence(MockSequence) + .Setup(p => p.CalculateOptimalReadLength(_bufferSize)) + .Returns(_readBufferSize); + SftpSessionMock.InSequence(MockSequence) + .Setup(p => p.CalculateOptimalWriteLength(_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, (ulong) _serverData1.Length, _readBufferSize)) + .Returns(_serverData2); + } + + [TestCleanup] + public void TearDown() + { + SftpSessionMock.InSequence(MockSequence) + .Setup(p => p.RequestClose(_handle)); + } + + protected override void Arrange() + { + base.Arrange(); + + _target = new SftpFileStream(SftpSessionMock.Object, + _path, + FileMode.Open, + FileAccess.Read, + (int)_bufferSize); + } + + protected override void Act() + { + _actual = _target.Read(_buffer, 0, _numberOfBytesToRead); + } + + [TestMethod] + public void ReadShouldHaveReturnedTheNumberOfBytesRequested() + { + Assert.AreEqual(_buffer.Length, _actual); + } + + [TestMethod] + public void ReadShouldHaveWrittenBytesToTheCallerSuppliedBuffer() + { + Assert.IsTrue(_serverData1.IsEqualTo(_buffer.Take(_serverData1Length))); + + var bytesWrittenFromSecondRead = _numberOfBytesToRead - _serverData1Length; + Assert.IsTrue(_serverData2.Take(bytesWrittenFromSecondRead).IsEqualTo(_buffer.Take(_serverData1Length, bytesWrittenFromSecondRead))); + } + + [TestMethod] + public void PositionShouldReturnNumberOfBytesWrittenToBuffer() + { + SftpSessionMock.InSequence(MockSequence).Setup(p => p.IsOpen).Returns(true); + + Assert.AreEqual(_buffer.Length, _target.Position); + + SftpSessionMock.Verify(p => p.IsOpen, Times.Exactly(2)); + } + + [TestMethod] + public void ReadShouldReturnAllRemaningBytesFromReadBufferWhenCountIsEqualToNumberOfRemainingBytes() + { + SftpSessionMock.InSequence(MockSequence).Setup(p => p.IsOpen).Returns(true); + + var numberOfBytesRemainingInReadBuffer = _serverData1Length + _serverData2Length - _numberOfBytesToRead; + + _buffer = new byte[numberOfBytesRemainingInReadBuffer]; + + var actual = _target.Read(_buffer, 0, _buffer.Length); + + Assert.AreEqual(_buffer.Length, actual); + Assert.IsTrue(_serverData2.Take(_numberOfBytesToRead - _serverData1Length, _buffer.Length).IsEqualTo(_buffer)); + + SftpSessionMock.Verify(p => p.IsOpen, Times.Exactly(2)); + } + + [TestMethod] + public void ReadShouldReturnAllRemaningBytesFromReadBufferAndReadAgainWhenCountIsGreaterThanNumberOfRemainingBytesAndNewReadReturnsZeroBytes() + { + SftpSessionMock.InSequence(MockSequence).Setup(p => p.IsOpen).Returns(true); + SftpSessionMock.InSequence(MockSequence).Setup(p => p.RequestRead(_handle, (ulong) (_serverData1Length + _serverData2Length), _readBufferSize)).Returns(Array.Empty); + + var numberOfBytesRemainingInReadBuffer = _serverData1Length + _serverData2Length - _numberOfBytesToRead; + + _buffer = new byte[numberOfBytesRemainingInReadBuffer + 1]; + + var actual = _target.Read(_buffer, 0, _buffer.Length); + + Assert.AreEqual(numberOfBytesRemainingInReadBuffer, actual); + Assert.IsTrue(_serverData2.Take(_numberOfBytesToRead - _serverData1Length, numberOfBytesRemainingInReadBuffer).IsEqualTo(_buffer.Take(numberOfBytesRemainingInReadBuffer))); + Assert.AreEqual(0, _buffer[numberOfBytesRemainingInReadBuffer]); + + SftpSessionMock.Verify(p => p.IsOpen, Times.Exactly(2)); + SftpSessionMock.Verify(p => p.RequestRead(_handle, (ulong) (_serverData1Length + _serverData2Length), _readBufferSize)); + } + } +} diff --git a/src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Read_ReadMode_NoDataInReaderBufferAndReadMoreBytesThanCount.cs b/src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Read_ReadMode_NoDataInReaderBufferAndReadMoreBytesFromServerThanCount.cs similarity index 84% rename from src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Read_ReadMode_NoDataInReaderBufferAndReadMoreBytesThanCount.cs rename to src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Read_ReadMode_NoDataInReaderBufferAndReadMoreBytesFromServerThanCount.cs index d1956e654..aaa31dbb8 100644 --- a/src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Read_ReadMode_NoDataInReaderBufferAndReadMoreBytesThanCount.cs +++ b/src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Read_ReadMode_NoDataInReaderBufferAndReadMoreBytesFromServerThanCount.cs @@ -7,7 +7,7 @@ namespace Renci.SshNet.Tests.Classes.Sftp { [TestClass] - public class SftpFileStreamTest_Read_ReadMode_NoDataInReaderBufferAndReadMoreBytesThanCount : SftpFileStreamTestBase + public class SftpFileStreamTest_Read_ReadMode_NoDataInReaderBufferAndReadMoreBytesFromServerThanCount : SftpFileStreamTestBase { private string _path; private SftpFileStream _target; @@ -18,7 +18,7 @@ public class SftpFileStreamTest_Read_ReadMode_NoDataInReaderBufferAndReadMoreByt private int _actual; private byte[] _buffer; private byte[] _serverData; - private int _numberOfBytesInReadBuffer; + private int _numberOfBytesToWriteToReadBuffer; private int _numberOfBytesToRead; protected override void SetupData() @@ -34,8 +34,8 @@ protected override void SetupData() _numberOfBytesToRead = 20; _buffer = new byte[_numberOfBytesToRead]; - _numberOfBytesInReadBuffer = 10; - _serverData = GenerateRandom(_buffer.Length + _numberOfBytesInReadBuffer, random); + _numberOfBytesToWriteToReadBuffer = 10; // should be less than _readBufferSize + _serverData = GenerateRandom(_numberOfBytesToRead + _numberOfBytesToWriteToReadBuffer, random); } protected override void SetupMocks() @@ -81,9 +81,9 @@ protected override void Act() } [TestMethod] - public void ReadShouldHaveReturnedTheNumberOfBytesWrittenToBuffer() + public void ReadShouldHaveReturnedTheNumberOfBytesRequested() { - Assert.AreEqual(_buffer.Length, _actual); + Assert.AreEqual(_numberOfBytesToRead, _actual); } [TestMethod] @@ -107,12 +107,12 @@ public void ReadShouldReturnAllRemaningBytesFromReadBufferWhenCountIsEqualToNumb { SftpSessionMock.InSequence(MockSequence).Setup(p => p.IsOpen).Returns(true); - _buffer = new byte[_numberOfBytesInReadBuffer]; + _buffer = new byte[_numberOfBytesToWriteToReadBuffer]; - var actual = _target.Read(_buffer, 0, _numberOfBytesInReadBuffer); + var actual = _target.Read(_buffer, 0, _numberOfBytesToWriteToReadBuffer); - Assert.AreEqual(_numberOfBytesInReadBuffer, actual); - Assert.IsTrue(_serverData.Take(_numberOfBytesToRead, _numberOfBytesInReadBuffer).IsEqualTo(_buffer)); + Assert.AreEqual(_numberOfBytesToWriteToReadBuffer, actual); + Assert.IsTrue(_serverData.Take(_numberOfBytesToRead, _numberOfBytesToWriteToReadBuffer).IsEqualTo(_buffer)); SftpSessionMock.Verify(p => p.IsOpen, Times.Exactly(2)); } diff --git a/src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Seek_PositionedAtBeginningOfStream_OriginBeginAndOffsetNegative.cs b/src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Seek_PositionedAtBeginningOfStream_OriginBeginAndOffsetNegative.cs new file mode 100644 index 000000000..712880d85 --- /dev/null +++ b/src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Seek_PositionedAtBeginningOfStream_OriginBeginAndOffsetNegative.cs @@ -0,0 +1,97 @@ +using System; +using System.IO; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; +using Renci.SshNet.Sftp; + +namespace Renci.SshNet.Tests.Classes.Sftp +{ + [TestClass] + public class SftpFileStreamTest_Seek_PositionedAtBeginningOfStream_OriginBeginAndOffsetNegative : SftpFileStreamTestBase + { + private Random _random; + private string _path; + private FileMode _fileMode; + private FileAccess _fileAccess; + private int _bufferSize; + private uint _readBufferSize; + private uint _writeBufferSize; + private byte[] _handle; + private SftpFileStream _target; + private int _offset; + private EndOfStreamException _actualException; + + protected override void SetupData() + { + base.SetupData(); + + _random = new Random(); + _path = _random.Next().ToString(); + _fileMode = FileMode.OpenOrCreate; + _fileAccess = FileAccess.Read; + _bufferSize = _random.Next(5, 1000); + _readBufferSize = (uint)_random.Next(5, 1000); + _writeBufferSize = (uint)_random.Next(5, 1000); + _handle = GenerateRandom(_random.Next(1, 10), _random); + _offset = _random.Next(int.MinValue, -1); + } + + protected override void SetupMocks() + { + SftpSessionMock.InSequence(MockSequence) + .Setup(p => p.RequestOpen(_path, Flags.Read | Flags.CreateNewOrOpen, false)) + .Returns(_handle); + SftpSessionMock.InSequence(MockSequence) + .Setup(p => p.CalculateOptimalReadLength((uint)_bufferSize)) + .Returns(_readBufferSize); + SftpSessionMock.InSequence(MockSequence) + .Setup(p => p.CalculateOptimalWriteLength((uint)_bufferSize, _handle)) + .Returns(_writeBufferSize); + SftpSessionMock.InSequence(MockSequence).Setup(p => p.IsOpen).Returns(true); + } + + protected override void Arrange() + { + base.Arrange(); + + _target = new SftpFileStream(SftpSessionMock.Object, _path, _fileMode, _fileAccess, _bufferSize); + } + + protected override void Act() + { + try + { + _target.Seek(_offset, SeekOrigin.Begin); + Assert.Fail(); + } + catch (EndOfStreamException ex) + { + _actualException = ex; + } + } + + [TestMethod] + public void SeekShouldHaveThrownEndOfStreamException() + { + Assert.IsNotNull(_actualException); + Assert.IsNull(_actualException.InnerException); + Assert.AreEqual("Attempted to read past the end of the stream.", _actualException.Message); + } + + [TestMethod] + public void IsOpenOnSftpSessionShouldHaveBeenInvokedOnce() + { + SftpSessionMock.Verify(p => p.IsOpen, Times.Once); + } + + [TestMethod] + public void PositionShouldReturnZero() + { + SftpSessionMock.InSequence(MockSequence).Setup(p => p.IsOpen).Returns(true); + + Assert.AreEqual(0L, _target.Position); + + SftpSessionMock.Verify(p => p.IsOpen, Times.Exactly(2)); + } + } +} diff --git a/src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Seek_PositionedAtBeginningOfStream_OriginBeginAndOffsetPositive.cs b/src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Seek_PositionedAtBeginningOfStream_OriginBeginAndOffsetPositive.cs new file mode 100644 index 000000000..c0f368b8c --- /dev/null +++ b/src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Seek_PositionedAtBeginningOfStream_OriginBeginAndOffsetPositive.cs @@ -0,0 +1,88 @@ +using System; +using System.IO; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; +using Renci.SshNet.Sftp; + +namespace Renci.SshNet.Tests.Classes.Sftp +{ + [TestClass] + public class SftpFileStreamTest_Seek_PositionedAtBeginningOfStream_OriginBeginAndOffsetPositive : SftpFileStreamTestBase + { + private Random _random; + private string _path; + private FileMode _fileMode; + private FileAccess _fileAccess; + private int _bufferSize; + private uint _readBufferSize; + private uint _writeBufferSize; + private byte[] _handle; + private SftpFileStream _target; + private int _offset; + private EndOfStreamException _actualException; + private long _actual; + + protected override void SetupData() + { + base.SetupData(); + + _random = new Random(); + _path = _random.Next().ToString(); + _fileMode = FileMode.OpenOrCreate; + _fileAccess = FileAccess.Read; + _bufferSize = _random.Next(5, 1000); + _readBufferSize = (uint)_random.Next(5, 1000); + _writeBufferSize = (uint)_random.Next(5, 1000); + _handle = GenerateRandom(_random.Next(1, 10), _random); + _offset = _random.Next(1, int.MaxValue); + } + + protected override void SetupMocks() + { + SftpSessionMock.InSequence(MockSequence) + .Setup(p => p.RequestOpen(_path, Flags.Read | Flags.CreateNewOrOpen, false)) + .Returns(_handle); + SftpSessionMock.InSequence(MockSequence) + .Setup(p => p.CalculateOptimalReadLength((uint)_bufferSize)) + .Returns(_readBufferSize); + SftpSessionMock.InSequence(MockSequence) + .Setup(p => p.CalculateOptimalWriteLength((uint)_bufferSize, _handle)) + .Returns(_writeBufferSize); + SftpSessionMock.InSequence(MockSequence).Setup(p => p.IsOpen).Returns(true); + } + + protected override void Arrange() + { + base.Arrange(); + + _target = new SftpFileStream(SftpSessionMock.Object, _path, _fileMode, _fileAccess, _bufferSize); + } + + protected override void Act() + { + _actual = _target.Seek(_offset, SeekOrigin.Begin); + } + + [TestMethod] + public void SeekShouldHaveReturnedOffset() + { + Assert.AreEqual(_offset, _actual); + } + + [TestMethod] + public void IsOpenOnSftpSessionShouldHaveBeenInvokedOnce() + { + SftpSessionMock.Verify(p => p.IsOpen, Times.Once); + } + + [TestMethod] + public void PositionShouldReturnOffset() + { + SftpSessionMock.InSequence(MockSequence).Setup(p => p.IsOpen).Returns(true); + + Assert.AreEqual(_offset, _target.Position); + + SftpSessionMock.Verify(p => p.IsOpen, Times.Exactly(2)); + } + } +} diff --git a/src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Seek_PositionedAtBeginningOfStream_OriginBeginAndOffsetZero.cs b/src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Seek_PositionedAtBeginningOfStream_OriginBeginAndOffsetZero.cs index 98ae83bf8..27e8d96fc 100644 --- a/src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Seek_PositionedAtBeginningOfStream_OriginBeginAndOffsetZero.cs +++ b/src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Seek_PositionedAtBeginningOfStream_OriginBeginAndOffsetZero.cs @@ -65,5 +65,21 @@ public void SeekShouldHaveReturnedZero() { Assert.AreEqual(0L, _actual); } + + [TestMethod] + public void IsOpenOnSftpSessionShouldHaveBeenInvokedOnce() + { + SftpSessionMock.Verify(p => p.IsOpen, Times.Once); + } + + [TestMethod] + public void PositionShouldReturnZero() + { + SftpSessionMock.InSequence(MockSequence).Setup(p => p.IsOpen).Returns(true); + + Assert.AreEqual(0L, _target.Position); + + SftpSessionMock.Verify(p => p.IsOpen, Times.Exactly(2)); + } } } diff --git a/src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_OutsideOfReadBuffer.cs b/src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_OutsideOfReadBuffer.cs new file mode 100644 index 000000000..f30a64109 --- /dev/null +++ b/src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_OutsideOfReadBuffer.cs @@ -0,0 +1,124 @@ +using System; +using System.IO; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; +using Renci.SshNet.Sftp; + +namespace Renci.SshNet.Tests.Classes.Sftp +{ + [TestClass] + //[Ignore] + public class SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_OutsideOfReadBuffer : SftpFileStreamTestBase + { + private Random _random; + private string _path; + private FileMode _fileMode; + private FileAccess _fileAccess; + private int _bufferSize; + private uint _readBufferSize; + private uint _writeBufferSize; + private byte[] _handle; + private SftpFileStream _target; + private long _actual; + private byte[] _buffer; + private byte[] _serverData1; + private byte[] _serverData2; + + protected override void SetupData() + { + base.SetupData(); + + _random = new Random(); + _path = _random.Next().ToString(); + _fileMode = FileMode.OpenOrCreate; + _fileAccess = FileAccess.Read; + _bufferSize = _random.Next(5, 1000); + _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); + } + + protected override void SetupMocks() + { + SftpSessionMock.InSequence(MockSequence) + .Setup(p => p.RequestOpen(_path, Flags.Read | Flags.CreateNewOrOpen, false)) + .Returns(_handle); + SftpSessionMock.InSequence(MockSequence) + .Setup(p => p.CalculateOptimalReadLength((uint) _bufferSize)) + .Returns(_readBufferSize); + SftpSessionMock.InSequence(MockSequence) + .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); + SftpSessionMock.InSequence(MockSequence) + .Setup(p => p.IsOpen) + .Returns(true); + } + + protected override void Arrange() + { + base.Arrange(); + + _target = new SftpFileStream(SftpSessionMock.Object, _path, _fileMode, _fileAccess, _bufferSize); + _target.Read(_buffer, 0, _buffer.Length); + } + + protected override void Act() + { + _actual = _target.Seek(0L, SeekOrigin.Begin); + } + + [TestMethod] + public void SeekShouldHaveReturnedZero() + { + Assert.AreEqual(0L, _actual); + } + + [TestMethod] + public void PositionShouldReturnZero() + { + SftpSessionMock.InSequence(MockSequence) + .Setup(p => p.IsOpen) + .Returns(true); + + Assert.AreEqual(0L, _target.Position); + + SftpSessionMock.Verify(p => p.IsOpen, Times.Exactly(3)); + } + + [TestMethod] + public void IsOpenOnSftpSessionShouldHaveBeenInvokedTwice() + { + SftpSessionMock.Verify(p => p.IsOpen, Times.Exactly(2)); + } + + [TestMethod] + public void ReadShouldReadFromServer() + { + SftpSessionMock.InSequence(MockSequence) + .Setup(p => p.IsOpen) + .Returns(true); + SftpSessionMock.InSequence(MockSequence) + .Setup(p => p.RequestRead(_handle, 0, _readBufferSize)) + .Returns(new byte[] {0x04}); + + var buffer = new byte[1]; + + var bytesRead = _target.Read(buffer, 0, buffer.Length); + + Assert.AreEqual(buffer.Length, bytesRead); + Assert.AreEqual(0x04, buffer[0]); + } + } +} diff --git a/src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_WithinReadBuffer.cs b/src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_WithinReadBuffer.cs index 50b3172af..0a5afba4e 100644 --- a/src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_WithinReadBuffer.cs +++ b/src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_WithinReadBuffer.cs @@ -7,7 +7,7 @@ namespace Renci.SshNet.Tests.Classes.Sftp { [TestClass] - [Ignore] + //[Ignore] public class SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_WithinReadBuffer : SftpFileStreamTestBase { private Random _random; @@ -93,7 +93,28 @@ public void PositionShouldReturnZero() } [TestMethod] - public void ReadUpToReadBufferSizeShouldReturnBytesFromReadBuffer() + public void IsOpenOnSftpSessionShouldHaveBeenInvokedTwice() + { + SftpSessionMock.Verify(p => p.IsOpen, Times.Exactly(2)); + } + + [TestMethod] + public void ReadLessThanReadBufferSizeShouldReturnBytesFromReadBuffer() + { + 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) @@ -106,5 +127,26 @@ public void ReadUpToReadBufferSizeShouldReturnBytesFromReadBuffer() Assert.AreEqual(buffer.Length, bytesRead); Assert.IsTrue(_serverData.IsEqualTo(buffer)); } + + [TestMethod] + public void ReadMoreThanReadBufferSizeShouldReturnBytesFromReadBufferAndReadRemaningBytesFromServer() + { + 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]; + + 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))); + } } } diff --git a/src/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj b/src/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj index fa976e89c..439f8d9e2 100644 --- a/src/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj +++ b/src/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj @@ -434,8 +434,12 @@ - + + + + + diff --git a/src/Renci.SshNet/Sftp/SftpFileStream.cs b/src/Renci.SshNet/Sftp/SftpFileStream.cs index 13ba18cb0..db7e2b9e6 100644 --- a/src/Renci.SshNet/Sftp/SftpFileStream.cs +++ b/src/Renci.SshNet/Sftp/SftpFileStream.cs @@ -340,18 +340,18 @@ public override int Read(byte[] buffer, int offset, int count) var bytesAvailableInBuffer = _bufferLen - _bufferPosition; if (bytesAvailableInBuffer <= 0) { - _bufferPosition = 0; - _bufferLen = 0; - var data = _session.RequestRead(_handle, (ulong) _position, (uint) _readBufferSize); - if (data.Length == 0) + _bufferPosition = 0; + _bufferLen = data.Length; + + if (_bufferLen == 0) { break; } - // determine number of bytes that we can read into caller-provided buffer - var bytesToWriteToCallerBuffer = Math.Min(data.Length, count); + // determine number of bytes that we can write into caller-provided buffer + var bytesToWriteToCallerBuffer = Math.Min(_bufferLen, count); // write bytes to caller-provided buffer Buffer.BlockCopy(data, 0, buffer, offset, bytesToWriteToCallerBuffer); // advance offset to start writing bytes into caller-provided buffer @@ -362,13 +362,10 @@ public override int Read(byte[] buffer, int offset, int count) readLen += bytesToWriteToCallerBuffer; // update stream position _position += bytesToWriteToCallerBuffer; - - if (data.Length > bytesToWriteToCallerBuffer) - { - // copy remaining bytes to read buffer - _bufferLen = data.Length - bytesToWriteToCallerBuffer; - Buffer.BlockCopy(data, bytesToWriteToCallerBuffer, _readBuffer, 0, _bufferLen); - } + // update position in read buffer + _bufferPosition = bytesToWriteToCallerBuffer; + // write read bytes to read buffer + Buffer.BlockCopy(data, 0, _readBuffer, 0, _bufferLen); } else { From 6fd1cc78c390dce8d502477f1825bf9dc19dafd4 Mon Sep 17 00:00:00 2001 From: Gert Driesen Date: Mon, 12 Jun 2017 21:15:23 +0200 Subject: [PATCH 03/10] 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. --- ...m_OriginBeginAndOffsetZero_NoBuffering.cs} | 30 +++-- ...am_OriginBeginAndOffsetZero_ReadBuffer.cs} | 57 ++++----- .../Renci.SshNet.Tests.csproj | 4 +- src/Renci.SshNet/Sftp/SftpFileStream.cs | 111 +++++++++++------- 4 files changed, 109 insertions(+), 93 deletions(-) rename src/Renci.SshNet.Tests/Classes/Sftp/{SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_OutsideOfReadBuffer.cs => SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_NoBuffering.cs} (80%) rename src/Renci.SshNet.Tests/Classes/Sftp/{SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_WithinReadBuffer.cs => SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_ReadBuffer.cs} (69%) diff --git a/src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_OutsideOfReadBuffer.cs b/src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_NoBuffering.cs similarity index 80% rename from src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_OutsideOfReadBuffer.cs rename to src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_NoBuffering.cs index f30a64109..29b683375 100644 --- a/src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_OutsideOfReadBuffer.cs +++ b/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)); } } } diff --git a/src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_WithinReadBuffer.cs b/src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_ReadBuffer.cs similarity index 69% rename from src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_WithinReadBuffer.cs rename to src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_ReadBuffer.cs index 0a5afba4e..624a9759f 100644 --- a/src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_WithinReadBuffer.cs +++ b/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)); } } } diff --git a/src/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj b/src/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj index 439f8d9e2..9944f5c97 100644 --- a/src/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj +++ b/src/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj @@ -439,8 +439,8 @@ - - + + diff --git a/src/Renci.SshNet/Sftp/SftpFileStream.cs b/src/Renci.SshNet/Sftp/SftpFileStream.cs index db7e2b9e6..fbb718ee3 100644 --- a/src/Renci.SshNet/Sftp/SftpFileStream.cs +++ b/src/Renci.SshNet/Sftp/SftpFileStream.cs @@ -18,9 +18,9 @@ public class SftpFileStream : Stream // Buffer information. private readonly int _readBufferSize; - private readonly byte[] _readBuffer; + private byte[] _readBuffer; private readonly int _writeBufferSize; - private readonly byte[] _writeBuffer; + private byte[] _writeBuffer; private int _bufferPosition; private int _bufferLen; private long _position; @@ -253,9 +253,7 @@ internal SftpFileStream(ISftpSession session, string path, FileMode mode, FileAc // or SSH_FXP_WRITE message _readBufferSize = (int) session.CalculateOptimalReadLength((uint) bufferSize); - _readBuffer = new byte[_readBufferSize]; _writeBufferSize = (int) session.CalculateOptimalWriteLength((uint) bufferSize, _handle); - _writeBuffer = new byte[_writeBufferSize]; if (mode == FileMode.Append) { @@ -342,52 +340,69 @@ public override int Read(byte[] buffer, int offset, int count) { var data = _session.RequestRead(_handle, (ulong) _position, (uint) _readBufferSize); - _bufferPosition = 0; - _bufferLen = data.Length; - - if (_bufferLen == 0) + if (data.Length == 0) { + _bufferPosition = 0; + _bufferLen = 0; + break; } - // determine number of bytes that we can write into caller-provided buffer - var bytesToWriteToCallerBuffer = Math.Min(_bufferLen, count); + var bytesToWriteToCallerBuffer = count; + if (bytesToWriteToCallerBuffer >= data.Length) + { + // write all data read to caller-provided buffer + bytesToWriteToCallerBuffer = data.Length; + // reset buffer since we will skip buffering + _bufferPosition = 0; + _bufferLen = 0; + } + else + { + // determine number of bytes that we should write into read buffer + var bytesToWriteToReadBuffer = data.Length - bytesToWriteToCallerBuffer; + // write remaining bytes to read buffer + Buffer.BlockCopy(data, count, GetOrCreateReadBuffer(), 0, bytesToWriteToReadBuffer); + // update position in read buffer + _bufferPosition = 0; + // update number of bytes in read buffer + _bufferLen = bytesToWriteToReadBuffer; + } + // write bytes to caller-provided buffer Buffer.BlockCopy(data, 0, buffer, offset, bytesToWriteToCallerBuffer); // advance offset to start writing bytes into caller-provided buffer offset += bytesToWriteToCallerBuffer; - // update number of bytes left to read - count -= bytesToWriteToCallerBuffer; // record total number of bytes read into caller-provided buffer readLen += bytesToWriteToCallerBuffer; + // signal that all caller-requested bytes are read + count -= bytesToWriteToCallerBuffer; // update stream position _position += bytesToWriteToCallerBuffer; - // update position in read buffer - _bufferPosition = bytesToWriteToCallerBuffer; - // write read bytes to read buffer - Buffer.BlockCopy(data, 0, _readBuffer, 0, _bufferLen); } else { - // determine number of bytes that we can write from read buffer to caller-provided buffer - var bytesToWriteToCallerBuffer = Math.Min(bytesAvailableInBuffer, count); + // limit the number of bytes to use from read buffer to the caller-request number of bytes + if (bytesAvailableInBuffer > count) + bytesAvailableInBuffer = count; + // copy data from read buffer to the caller-provided buffer - Buffer.BlockCopy(_readBuffer, _bufferPosition, buffer, offset, bytesToWriteToCallerBuffer); + Buffer.BlockCopy(GetOrCreateReadBuffer(), _bufferPosition, buffer, offset, bytesAvailableInBuffer); // update position in read buffer - _bufferPosition += bytesToWriteToCallerBuffer; + _bufferPosition += bytesAvailableInBuffer; // advance offset to start writing bytes into caller-provided buffer offset += bytesAvailableInBuffer; - // update number of bytes left to read - count -= bytesToWriteToCallerBuffer; // record total number of bytes read into caller-provided buffer - readLen += bytesToWriteToCallerBuffer; + readLen += bytesAvailableInBuffer; + // update number of bytes left to read + count -= bytesAvailableInBuffer; // update stream position - _position += bytesToWriteToCallerBuffer; + _position += bytesAvailableInBuffer; } } } - // Return the number of bytes that were read to the caller. + // return the number of bytes that were read to the caller. return readLen; } @@ -410,28 +425,32 @@ public override int ReadByte() // Setup the object for reading. SetupRead(); + byte[] readBuffer; + // Read more data into the internal buffer if necessary. if (_bufferPosition >= _bufferLen) { - _bufferPosition = 0; - _bufferLen = 0; - var data = _session.RequestRead(_handle, (ulong) _position, (uint) _readBufferSize); - - _bufferLen = data.Length; - - if (_bufferLen == 0) + if (data.Length == 0) { // We've reached EOF. return -1; } - Buffer.BlockCopy(data, 0, _readBuffer, 0, _bufferLen); + readBuffer = GetOrCreateReadBuffer(); + Buffer.BlockCopy(data, 0, readBuffer, 0, data.Length); + + _bufferPosition = 0; + _bufferLen = data.Length; + } + else + { + readBuffer = GetOrCreateReadBuffer(); } // Extract the next byte from the buffer. ++_position; - return _readBuffer[_bufferPosition++]; + return readBuffer[_bufferPosition++]; } } @@ -501,8 +520,7 @@ public override long Seek(long offset, SeekOrigin origin) if (origin == SeekOrigin.Begin) { newPosn = _position - _bufferPosition; - if (offset >= newPosn && offset < - (newPosn + _bufferLen)) + if (offset >= newPosn && offset < (newPosn + _bufferLen)) { _bufferPosition = (int)(offset - newPosn); _position = offset; @@ -515,8 +533,7 @@ public override long Seek(long offset, SeekOrigin origin) if (newPosn >= (_position - _bufferPosition) && newPosn < (_position - _bufferPosition + _bufferLen)) { - _bufferPosition = - (int)(newPosn - (_position - _bufferPosition)); + _bufferPosition = (int) (newPosn - (_position - _bufferPosition)); _position = newPosn; return _position; } @@ -642,7 +659,7 @@ public override void Write(byte[] buffer, int offset, int count) else { // No: copy the data to the write buffer first. - Buffer.BlockCopy(buffer, offset, _writeBuffer, _bufferPosition, tempLen); + Buffer.BlockCopy(buffer, offset, GetOrCreateWriteBuffer(), _bufferPosition, tempLen); _bufferPosition += tempLen; } @@ -658,7 +675,7 @@ public override void Write(byte[] buffer, int offset, int count) { using (var wait = new AutoResetEvent(false)) { - _session.RequestWrite(_handle, (ulong) (_position - _bufferPosition), _writeBuffer, 0, _bufferPosition, wait); + _session.RequestWrite(_handle, (ulong) (_position - _bufferPosition), GetOrCreateWriteBuffer(), 0, _bufferPosition, wait); } _bufferPosition = 0; @@ -742,6 +759,20 @@ protected override void Dispose(bool disposing) } } + private byte[] GetOrCreateReadBuffer() + { + if (_readBuffer == null) + _readBuffer = new byte[_readBufferSize]; + return _readBuffer; + } + + private byte[] GetOrCreateWriteBuffer() + { + if (_writeBuffer == null) + _writeBuffer = new byte[_writeBufferSize]; + return _writeBuffer; + } + /// /// Flushes the read data from the buffer. /// From 50ea405d5a65985ed4909b551852516f7c2f601c Mon Sep 17 00:00:00 2001 From: Gert Driesen Date: Mon, 12 Jun 2017 21:35:13 +0200 Subject: [PATCH 04/10] Sync project file. --- .../Renci.SshNet.Tests.NET35.csproj | 27 ++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/src/Renci.SshNet.Tests.NET35/Renci.SshNet.Tests.NET35.csproj b/src/Renci.SshNet.Tests.NET35/Renci.SshNet.Tests.NET35.csproj index 2c350023e..5e4891fa8 100644 --- a/src/Renci.SshNet.Tests.NET35/Renci.SshNet.Tests.NET35.csproj +++ b/src/Renci.SshNet.Tests.NET35/Renci.SshNet.Tests.NET35.csproj @@ -1202,8 +1202,26 @@ Classes\Sftp\SftpFileStreamTest_ReadByte_ReadMode_NoDataInWriteBufferAndNoDataInReadBuffer_LessDataThanReadBufferSizeAvailable.cs - - Classes\Sftp\SftpFileStreamTest_Read_ReadMode_NoDataInReaderBufferAndReadMoreBytesThanCount.cs + + Classes\Sftp\SftpFileStreamTest_Read_ReadMode_NoDataInReaderBufferAndReadLessBytesFromServerThanCount.cs + + + Classes\Sftp\SftpFileStreamTest_Read_ReadMode_NoDataInReaderBufferAndReadMoreBytesFromServerThanCount.cs + + + Classes\Sftp\SftpFileStreamTest_Seek_PositionedAtBeginningOfStream_OriginBeginAndOffsetNegative.cs + + + Classes\Sftp\SftpFileStreamTest_Seek_PositionedAtBeginningOfStream_OriginBeginAndOffsetPositive.cs + + + Classes\Sftp\SftpFileStreamTest_Seek_PositionedAtBeginningOfStream_OriginBeginAndOffsetZero.cs + + + Classes\Sftp\SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_NoBuffering.cs + + + Classes\Sftp\SftpFileStreamTest_Seek_PositionedAtMiddleOfStream_OriginBeginAndOffsetZero_ReadBuffer.cs Classes\Sftp\SftpFileStreamTest_SetLength_Closed.cs @@ -1223,6 +1241,9 @@ Classes\Sftp\SftpFileStreamTest_SetLength_SessionOpen_FIleAccessWrite.cs + + Classes\Sftp\SftpFileStreamTest_Write_SessionOpen_CountGreatherThanTwoTimesTheWriteBufferSize.cs + Classes\Sftp\SftpFileSystemInformationTest.cs @@ -1451,7 +1472,7 @@ - +