Take into account the offset in SftpFileStream.Write(byte[] buffer, int offset, int count) when not writing to the buffer. Fixes issue #70.

Author: drieseng

src/Renci.SshNet.Tests/Classes/Sftp/Requests/SftpWriteRequestTest.cs

@@ -15,8 +15,9 @@ public class SftpWriteRequestTest
         private uint _protocolVersion;
         private uint _requestId;
         private byte[] _handle;
-        private ulong _offset;
+        private ulong _serverFileOffset;
         private byte[] _data;
+        private int _offset;
         private int _length;
 
         [TestInitialize]
@@ -28,22 +29,25 @@ public void Init()
             _requestId = (uint)random.Next(0, int.MaxValue);
             _handle = new byte[random.Next(1, 10)];
             random.NextBytes(_handle);
-            _offset = (ulong) random.Next(0, int.MaxValue);
-            _data = new byte[random.Next(5, 10)];
+            _serverFileOffset = (ulong) random.Next(0, int.MaxValue);
+            _data = new byte[random.Next(10, 15)];
             random.NextBytes(_data);
-            _length = random.Next(1, 4);
+            _offset = random.Next(0, 4);
+            _length = random.Next(5, 10);
         }
 
         [TestMethod]
         public void Constructor()
         {
-            var request = new SftpWriteRequest(_protocolVersion, _requestId, _handle, _offset, _data, _length, null);
+            var request = new SftpWriteRequest(_protocolVersion, _requestId, _handle, _serverFileOffset, _data, _offset, _length, null);
 
             Assert.AreSame(_data, request.Data);
             Assert.AreSame(_handle, request.Handle);
             Assert.AreEqual(_length, request.Length);
+            Assert.AreEqual(_offset, request.Offset);
             Assert.AreEqual(_protocolVersion, request.ProtocolVersion);
             Assert.AreEqual(_requestId, request.RequestId);
+            Assert.AreEqual(_serverFileOffset, request.ServerFileOffset);
             Assert.AreEqual(SftpMessageTypes.Write, request.SftpMessageType);
         }
 
@@ -58,8 +62,9 @@ public void Complete_SftpStatusResponse()
                 _protocolVersion,
                 _requestId,
                 _handle,
-                _offset,
+                _serverFileOffset,
                 _data,
+                _offset,
                 _length,
                 statusAction);
 
@@ -72,7 +77,7 @@ public void Complete_SftpStatusResponse()
         [TestMethod]
         public void GetBytes()
         {
-            var request = new SftpWriteRequest(_protocolVersion, _requestId, _handle, _offset, _data, _length, null);
+            var request = new SftpWriteRequest(_protocolVersion, _requestId, _handle, _serverFileOffset, _data, _offset, _length, null);
 
             var bytes = request.GetBytes();
 
@@ -82,7 +87,7 @@ public void GetBytes()
             expectedBytesLength += 4; // RequestId
             expectedBytesLength += 4; // Handle length
             expectedBytesLength += _handle.Length; // Handle
-            expectedBytesLength += 8; // Offset
+            expectedBytesLength += 8; // ServerFileOffset
             expectedBytesLength += 4; // Data length
             expectedBytesLength += _length; // Data
 
@@ -99,12 +104,12 @@ public void GetBytes()
             sshDataStream.Read(actualHandle, 0, actualHandle.Length);
             Assert.IsTrue(_handle.SequenceEqual(actualHandle));
 
-            Assert.AreEqual(_offset, sshDataStream.ReadUInt64());
+            Assert.AreEqual(_serverFileOffset, sshDataStream.ReadUInt64());
 
             Assert.AreEqual((uint) _length, sshDataStream.ReadUInt32());
             var actualData = new byte[_length];
             sshDataStream.Read(actualData, 0, actualData.Length);
-            Assert.IsTrue(_data.Take(_length).SequenceEqual(actualData));
+            Assert.IsTrue(_data.Take(_offset, _length).SequenceEqual(actualData));
 
             Assert.IsTrue(sshDataStream.IsEndOfData);
         }

src/Renci.SshNet.Tests/Classes/Sftp/SftpFileStreamTest_Write_SessionOpen_CountGreatherThanTwoTimesTheWriteBufferSize.cs

@@ -0,0 +1,187 @@
+using System;
+using System.Globalization;
+using System.IO;
+using System.Threading;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using Moq;
+using Renci.SshNet.Sftp;
+using Renci.SshNet.Sftp.Responses;
+
+namespace Renci.SshNet.Tests.Classes.Sftp
+{
+    [TestClass]
+    public class SftpFileStreamTest_Write_SessionOpen_CountGreatherThanTwoTimesTheWriteBufferSize
+    {
+        private Mock<ISftpSession> _sftpSessionMock;
+        private string _path;
+        private SftpFileStream _sftpFileStream;
+        private byte[] _handle;
+        private SftpFileAttributes _fileAttributes;
+        private uint _bufferSize;
+        private uint _readBufferSize;
+        private uint _writeBufferSize;
+        private byte[] _data;
+        private int _count;
+        private int _offset;
+        private MockSequence _sequence;
+        private Random _random;
+        private uint _expectedWrittenByteCount;
+        private int _expectedBufferedByteCount;
+        private byte[] _expectedBufferedBytes;
+
+        [TestInitialize]
+        public void Setup()
+        {
+            Arrange();
+            Act();
+        }
+
+        [TestCleanup]
+        public void TearDown()
+        {
+            if (_sftpSessionMock != null)
+            {
+                // allow Dispose to complete successfully
+                _sftpSessionMock.InSequence(_sequence)
+                    .Setup(p => p.IsOpen)
+                    .Returns(true);
+                _sftpSessionMock.InSequence(_sequence)
+                    .Setup(p => p.RequestWrite(_handle, _expectedWrittenByteCount, It.IsAny<byte[]>(), 0, _expectedBufferedByteCount, It.IsAny<AutoResetEvent>(), null));
+                _sftpSessionMock.InSequence(_sequence)
+                    .Setup(p => p.RequestClose(_handle));
+            }
+        }
+
+        protected void Arrange()
+        {
+            _random = new Random();
+            _path = _random.Next().ToString(CultureInfo.InvariantCulture);
+            _handle = new[] {(byte) _random.Next(byte.MinValue, byte.MaxValue)};
+            _fileAttributes = SftpFileAttributes.Empty;
+            _bufferSize = (uint) _random.Next(1, 1000);
+            _readBufferSize = (uint) _random.Next(0, 1000);
+            _writeBufferSize = (uint) _random.Next(500, 1000);
+            _data = new byte[(_writeBufferSize  * 2) + 15];
+            _random.NextBytes(_data);
+            _offset = _random.Next(1, 5);
+            // to get multiple SSH_FXP_WRITE messages (and verify the offset is updated correctly), we make sure
+            // the number of bytes to write is at least two times the write buffer size; we write a few extra bytes to
+            // ensure the buffer is not empty after the writes so we can verify whether Length, Dispose and Flush
+            // flush the buffer
+            _count = ((int) _writeBufferSize*2) + _random.Next(1, 5);
+
+            _expectedWrittenByteCount = (2 * _writeBufferSize);
+            _expectedBufferedByteCount = (int)(_count - _expectedWrittenByteCount);
+            _expectedBufferedBytes = _data.Take(_offset + (int)_expectedWrittenByteCount, _expectedBufferedByteCount);
+
+            _sftpSessionMock = new Mock<ISftpSession>(MockBehavior.Strict);
+
+            _sequence = new MockSequence();
+            _sftpSessionMock.InSequence(_sequence)
+                .Setup(p => p.RequestOpen(_path, Flags.Write | Flags.Truncate, true))
+                .Returns(_handle);
+            _sftpSessionMock.InSequence(_sequence).Setup(p => p.RequestFStat(_handle)).Returns(_fileAttributes);
+            _sftpSessionMock.InSequence(_sequence)
+                .Setup(p => p.CalculateOptimalReadLength(_bufferSize))
+                .Returns(_readBufferSize);
+            _sftpSessionMock.InSequence(_sequence)
+                .Setup(p => p.CalculateOptimalWriteLength(_bufferSize, _handle))
+                .Returns(_writeBufferSize);
+            _sftpSessionMock.InSequence(_sequence)
+                .Setup(p => p.IsOpen)
+                .Returns(true);
+            _sftpSessionMock.InSequence(_sequence)
+                .Setup(p => p.RequestWrite(_handle, 0, _data, _offset, (int) _writeBufferSize, It.IsAny<AutoResetEvent>(), null));
+            _sftpSessionMock.InSequence(_sequence)
+                .Setup(p => p.RequestWrite(_handle, _writeBufferSize, _data, _offset + (int) _writeBufferSize, (int)_writeBufferSize, It.IsAny<AutoResetEvent>(), null));
+
+            _sftpFileStream = new SftpFileStream(_sftpSessionMock.Object, _path, FileMode.Create, FileAccess.Write, (int) _bufferSize);
+        }
+
+        protected void Act()
+        {
+            _sftpFileStream.Write(_data, _offset, _count);
+        }
+
+        [TestMethod]
+        public void RequestWriteOnSftpSessionShouldBeInvokedTwice()
+        {
+            _sftpSessionMock.Verify(p => p.RequestWrite(_handle, 0, _data, _offset, (int)_writeBufferSize, It.IsAny<AutoResetEvent>(), null), Times.Once);
+            _sftpSessionMock.Verify(p => p.RequestWrite(_handle, _writeBufferSize, _data, _offset + (int)_writeBufferSize, (int)_writeBufferSize, It.IsAny<AutoResetEvent>(), null), Times.Once);
+        }
+
+        [TestMethod]
+        public void PositionShouldBeNumberOfBytesWrittenToFileAndNUmberOfBytesInBuffer()
+        {
+            _sftpSessionMock.InSequence(_sequence)
+                            .Setup(p => p.IsOpen)
+                            .Returns(true);
+
+            Assert.AreEqual(_count, _sftpFileStream.Position);
+        }
+
+        [TestMethod]
+        public void LengthShouldFlushBufferAndReturnSizeOfFile()
+        {
+            var lengthFileAttributes = new SftpFileAttributes(DateTime.Now, DateTime.Now, _random.Next(), _random.Next(),
+                                                        _random.Next(), (uint) _random.Next(0, int.MaxValue), null);
+            byte[] actualFlushedData = null;
+
+            _sftpSessionMock.InSequence(_sequence)
+                            .Setup(p => p.IsOpen)
+                            .Returns(true);
+            _sftpSessionMock.InSequence(_sequence)
+                            .Setup(p => p.RequestWrite(_handle, _expectedWrittenByteCount, It.IsAny<byte[]>(), 0, _expectedBufferedByteCount, It.IsAny<AutoResetEvent>(), null))
+                            .Callback<byte[], ulong, byte[], int, int, AutoResetEvent, Action<SftpStatusResponse>>((handle, serverFileOffset, data, offset, length, wait, writeCompleted) => actualFlushedData = data.Take(offset, length));
+            _sftpSessionMock.InSequence(_sequence)
+                            .Setup(p => p.RequestFStat(_handle))
+                            .Returns(lengthFileAttributes);
+
+            Assert.AreEqual(lengthFileAttributes.Size, _sftpFileStream.Length);
+            Assert.IsTrue(actualFlushedData.IsEqualTo(_expectedBufferedBytes));
+
+            _sftpSessionMock.Verify(p => p.RequestWrite(_handle, _expectedWrittenByteCount, It.IsAny<byte[]>(), 0, _expectedBufferedByteCount, It.IsAny<AutoResetEvent>(), null), Times.Once);
+        }
+
+        [TestMethod]
+        public void DisposeShouldFlushBufferAndCloseRequest()
+        {
+            byte[] actualFlushedData = null;
+
+            _sftpSessionMock.InSequence(_sequence)
+                            .Setup(p => p.IsOpen)
+                            .Returns(true);
+            _sftpSessionMock.InSequence(_sequence)
+                            .Setup(p => p.RequestWrite(_handle, _expectedWrittenByteCount, It.IsAny<byte[]>(), 0, _expectedBufferedByteCount, It.IsAny<AutoResetEvent>(), null))
+                            .Callback<byte[], ulong, byte[], int, int, AutoResetEvent, Action<SftpStatusResponse>>((handle, serverFileOffset, data, offset, length, wait, writeCompleted) => actualFlushedData = data.Take(offset, length));
+            _sftpSessionMock.InSequence(_sequence)
+                            .Setup(p => p.RequestClose(_handle));
+
+            _sftpFileStream.Dispose();
+
+            Assert.IsTrue(actualFlushedData.IsEqualTo(_expectedBufferedBytes));
+
+            _sftpSessionMock.Verify(p => p.RequestWrite(_handle, _expectedWrittenByteCount, It.IsAny<byte[]>(), 0, _expectedBufferedByteCount, It.IsAny<AutoResetEvent>(), null), Times.Once);
+            _sftpSessionMock.Verify(p => p.RequestClose(_handle), Times.Once);
+        }
+
+        [TestMethod]
+        public void FlushShouldFlushBuffer()
+        {
+            byte[] actualFlushedData = null;
+
+            _sftpSessionMock.InSequence(_sequence)
+                            .Setup(p => p.IsOpen)
+                            .Returns(true);
+            _sftpSessionMock.InSequence(_sequence)
+                            .Setup(p => p.RequestWrite(_handle, _expectedWrittenByteCount, It.IsAny<byte[]>(), 0, _expectedBufferedByteCount, It.IsAny<AutoResetEvent>(), null))
+                            .Callback<byte[], ulong, byte[], int, int, AutoResetEvent, Action<SftpStatusResponse>>((handle, serverFileOffset, data, offset, length, wait, writeCompleted) => actualFlushedData = data.Take(offset, length));
+
+            _sftpFileStream.Flush();
+
+            Assert.IsTrue(actualFlushedData.IsEqualTo(_expectedBufferedBytes));
+
+            _sftpSessionMock.Verify(p => p.RequestWrite(_handle, _expectedWrittenByteCount, It.IsAny<byte[]>(), 0, _expectedBufferedByteCount, It.IsAny<AutoResetEvent>(), null), Times.Once);
+        }
+    }
+}

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

@@ -406,6 +406,7 @@
     <Compile Include="Classes\Sftp\SftpFileStreamTest_SetLength_SessionOpen_FIleAccessRead.cs" />
     <Compile Include="Classes\Sftp\SftpFileStreamTest_SetLength_SessionOpen_FIleAccessReadWrite.cs" />
     <Compile Include="Classes\Sftp\SftpFileStreamTest_SetLength_SessionOpen_FIleAccessWrite.cs" />
+    <Compile Include="Classes\Sftp\SftpFileStreamTest_Write_SessionOpen_CountGreatherThanTwoTimesTheWriteBufferSize.cs" />
     <Compile Include="Classes\Sftp\SftpFileTest.cs" />
     <Compile Include="Classes\Sftp\SftpSessionTest_Connected_RequestRead.cs" />
     <Compile Include="Classes\Sftp\SftpSessionTest_Connected_RequestStatVfs.cs" />

src/Renci.SshNet/Sftp/ISftpSession.cs

@@ -154,14 +154,16 @@ internal interface ISftpSession : ISubsystemSession
         /// Performs SSH_FXP_WRITE request.
         /// </summary>
         /// <param name="handle">The handle.</param>
-        /// <param name="offset">The offset.</param>
-        /// <param name="data">The data to send.</param>
-        /// <param name="length">The number of bytes of <paramref name="data"/> to send.</param>
+        /// <param name="serverOffset">The the zero-based offset (in bytes) relative to the beginning of the file that the write must start at.</param>
+        /// <param name="data">The buffer holding the data to write.</param>
+        /// <param name="offset">the zero-based offset in <paramref name="data" /> at which to begin taking bytes to write.</param>
+        /// <param name="length">The length (in bytes) of the data to write.</param>
         /// <param name="wait">The wait event handle if needed.</param>
         /// <param name="writeCompleted">The callback to invoke when the write has completed.</param>
         void RequestWrite(byte[] handle,
-                          ulong offset,
+                          ulong serverOffset,
                           byte[] data,
+                          int offset,
                           int length,
                           AutoResetEvent wait,
                           Action<SftpStatusResponse> writeCompleted = null);

src/Renci.SshNet/Sftp/Requests/SftpWriteRequest.cs

@@ -12,10 +12,39 @@ public override SftpMessageTypes SftpMessageType
 
         public byte[] Handle { get; private set; }
 
-        public ulong Offset { get; private set; }
+        /// <summary>
+        /// Gets the zero-based offset (in bytes) relative to the beginning of the file that the write
+        /// must start at.
+        /// </summary>
+        /// <value>
+        /// The zero-based offset (in bytes) relative to the beginning of the file that the write must
+        /// start at.
+        /// </value>
+        public ulong ServerFileOffset { get; private set; }
 
+        /// <summary>
+        /// Gets the buffer holding the data to write.
+        /// </summary>
+        /// <value>
+        /// The buffer holding the data to write.
+        /// </value>
         public byte[] Data { get; private set; }
 
+        /// <summary>
+        /// Gets the zero-based offset in <see cref="Data" /> at which to begin taking bytes to
+        /// write.
+        /// </summary>
+        /// <value>
+        /// The zero-based offset in <see cref="Data" /> at which to begin taking bytes to write.
+        /// </value>
+        public int Offset { get; private set; }
+
+        /// <summary>
+        /// Gets the length (in bytes) of the data to write.
+        /// </summary>
+        /// <value>
+        /// The length (in bytes) of the data to write.
+        /// </value>
         public int Length { get; private set; }
 
         protected override int BufferCapacity
@@ -25,7 +54,7 @@ protected override int BufferCapacity
                 var capacity = base.BufferCapacity;
                 capacity += 4; // Handle length
                 capacity += Handle.Length; // Handle
-                capacity += 8; // Offset length
+                capacity += 8; // ServerFileOffset length
                 capacity += 4; // Data length
                 capacity += Length; // Data
                 return capacity;
@@ -35,33 +64,36 @@ protected override int BufferCapacity
         public SftpWriteRequest(uint protocolVersion,
                                 uint requestId,
                                 byte[] handle,
-                                ulong offset,
+                                ulong serverFileOffset,
                                 byte[] data,
+                                int offset,
                                 int length,
                                 Action<SftpStatusResponse> statusAction)
             : base(protocolVersion, requestId, statusAction)
         {
             Handle = handle;
-            Offset = offset;
+            ServerFileOffset = serverFileOffset;
             Data = data;
+            Offset = offset;
             Length = length;
         }
 
         protected override void LoadData()
         {
             base.LoadData();
             Handle = ReadBinary();
-            Offset = ReadUInt64();
+            ServerFileOffset = ReadUInt64();
             Data = ReadBinary();
+            Offset = 0;
             Length = Data.Length;
         }
 
         protected override void SaveData()
         {
             base.SaveData();
             WriteBinaryString(Handle);
-            Write(Offset);
-            WriteBinary(Data, 0, Length);
+            Write(ServerFileOffset);
+            WriteBinary(Data, Offset, Length);
         }
     }
 }