Fix Seek Operations in SftpFileStream (#910)

* Fix offset operations in SftpFileStream.Seek
* Fix seek exception message and add default case for invalid seek origin
* Use named params when throwing ArgumentException
* Add tests for seeking from end of file

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

@@ -0,0 +1,103 @@
+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_OriginEndAndOffsetNegative : SftpFileStreamTestBase
+    {
+        private Random _random;
+        private string _path;
+        private FileMode _fileMode;
+        private FileAccess _fileAccess;
+        private int _bufferSize;
+        private uint _readBufferSize;
+        private uint _writeBufferSize;
+        private int _length;
+        private byte[] _handle;
+        private SftpFileStream _target;
+        private int _offset;
+        private SftpFileAttributes _attributes;
+        private long _actual;
+
+        protected override void SetupData()
+        {
+            base.SetupData();
+
+            _random = new Random();
+            _path = _random.Next().ToString();
+            _fileMode = FileMode.OpenOrCreate;
+            _fileAccess = FileAccess.Write;
+            _bufferSize = _random.Next(5, 1000);
+            _readBufferSize = (uint)_random.Next(5, 1000);
+            _writeBufferSize = (uint)_random.Next(5, 1000);
+            _length = _random.Next(5, 10000);
+            _handle = GenerateRandom(_random.Next(1, 10), _random);
+            _offset = _random.Next(-_length, -1);
+            _attributes = SftpFileAttributes.Empty;
+        }
+
+        protected override void SetupMocks()
+        {
+            SftpSessionMock.InSequence(MockSequence)
+                           .Setup(session => session.RequestOpen(_path, Flags.Write | Flags.CreateNewOrOpen, false))
+                           .Returns(_handle);
+            SftpSessionMock.InSequence(MockSequence)
+                           .Setup(session => session.CalculateOptimalReadLength((uint)_bufferSize))
+                           .Returns(_readBufferSize);
+            SftpSessionMock.InSequence(MockSequence)
+                           .Setup(session => session.CalculateOptimalWriteLength((uint)_bufferSize, _handle))
+                           .Returns(_writeBufferSize);
+            SftpSessionMock.InSequence(MockSequence)
+                           .Setup(session => session.IsOpen)
+                           .Returns(true);
+            SftpSessionMock.InSequence(MockSequence)
+                           .Setup(session => session.RequestFStat(_handle, false))
+                           .Returns(_attributes);
+            SftpSessionMock.InSequence(MockSequence)
+                           .Setup(session => session.RequestFSetStat(_handle, _attributes));
+            SftpSessionMock.InSequence(MockSequence).Setup(session => session.IsOpen).Returns(true);
+            SftpSessionMock.InSequence(MockSequence)
+                           .Setup(session => session.RequestFStat(_handle, false))
+                           .Returns(_attributes);
+        }
+
+        protected override void Arrange()
+        {
+            base.Arrange();
+
+            _target = new SftpFileStream(SftpSessionMock.Object, _path, _fileMode, _fileAccess, _bufferSize);
+            _target.SetLength(_length);
+        }
+
+        protected override void Act()
+        {
+            _actual = _target.Seek(_offset, SeekOrigin.End);
+        }
+
+        [TestMethod]
+        public void SeekShouldHaveReturnedOffset()
+        {
+            Assert.AreEqual(_attributes.Size + _offset, _actual);
+        }
+
+        [TestMethod]
+        public void IsOpenOnSftpSessionShouldHaveBeenInvokedTwice()
+        {
+            SftpSessionMock.Verify(session => session.IsOpen, Times.Exactly(2));
+        }
+
+        [TestMethod]
+        public void PositionShouldReturnOffset()
+        {
+            SftpSessionMock.InSequence(MockSequence).Setup(session => session.IsOpen).Returns(true);
+
+            Assert.AreEqual(_attributes.Size + _offset, _target.Position);
+
+            SftpSessionMock.Verify(session => session.IsOpen, Times.Exactly(3));
+        }
+    }
+}

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

@@ -0,0 +1,103 @@
+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_OriginEndAndOffsetPositive : SftpFileStreamTestBase
+    {
+        private Random _random;
+        private string _path;
+        private FileMode _fileMode;
+        private FileAccess _fileAccess;
+        private int _bufferSize;
+        private uint _readBufferSize;
+        private uint _writeBufferSize;
+        private int _length;
+        private byte[] _handle;
+        private SftpFileStream _target;
+        private int _offset;
+        private SftpFileAttributes _attributes;
+        private long _actual;
+
+        protected override void SetupData()
+        {
+            base.SetupData();
+
+            _random = new Random();
+            _path = _random.Next().ToString();
+            _fileMode = FileMode.OpenOrCreate;
+            _fileAccess = FileAccess.Write;
+            _bufferSize = _random.Next(5, 1000);
+            _readBufferSize = (uint)_random.Next(5, 1000);
+            _writeBufferSize = (uint)_random.Next(5, 1000);
+            _length = _random.Next(5, 10000);
+            _handle = GenerateRandom(_random.Next(1, 10), _random);
+            _offset = _random.Next(1, int.MaxValue);
+            _attributes = SftpFileAttributes.Empty;
+        }
+
+        protected override void SetupMocks()
+        {
+            SftpSessionMock.InSequence(MockSequence)
+                           .Setup(session => session.RequestOpen(_path, Flags.Write | Flags.CreateNewOrOpen, false))
+                           .Returns(_handle);
+            SftpSessionMock.InSequence(MockSequence)
+                           .Setup(session => session.CalculateOptimalReadLength((uint)_bufferSize))
+                           .Returns(_readBufferSize);
+            SftpSessionMock.InSequence(MockSequence)
+                           .Setup(session => session.CalculateOptimalWriteLength((uint)_bufferSize, _handle))
+                           .Returns(_writeBufferSize);
+            SftpSessionMock.InSequence(MockSequence)
+                           .Setup(session => session.IsOpen)
+                           .Returns(true);
+            SftpSessionMock.InSequence(MockSequence)
+                           .Setup(session => session.RequestFStat(_handle, false))
+                           .Returns(_attributes);
+            SftpSessionMock.InSequence(MockSequence)
+                           .Setup(session => session.RequestFSetStat(_handle, _attributes));
+            SftpSessionMock.InSequence(MockSequence).Setup(session => session.IsOpen).Returns(true);
+            SftpSessionMock.InSequence(MockSequence)
+                           .Setup(session => session.RequestFStat(_handle, false))
+                           .Returns(_attributes);
+        }
+
+        protected override void Arrange()
+        {
+            base.Arrange();
+
+            _target = new SftpFileStream(SftpSessionMock.Object, _path, _fileMode, _fileAccess, _bufferSize);
+            _target.SetLength(_length);
+        }
+
+        protected override void Act()
+        {
+            _actual = _target.Seek(_offset, SeekOrigin.End);
+        }
+
+        [TestMethod]
+        public void SeekShouldHaveReturnedOffset()
+        {
+            Assert.AreEqual(_attributes.Size + _offset, _actual);
+        }
+
+        [TestMethod]
+        public void IsOpenOnSftpSessionShouldHaveBeenInvokedTwice()
+        {
+            SftpSessionMock.Verify(session => session.IsOpen, Times.Exactly(2));
+        }
+
+        [TestMethod]
+        public void PositionShouldReturnOffset()
+        {
+            SftpSessionMock.InSequence(MockSequence).Setup(session => session.IsOpen).Returns(true);
+
+            Assert.AreEqual(_attributes.Size + _offset, _target.Position);
+
+            SftpSessionMock.Verify(session => session.IsOpen, Times.Exactly(3));
+        }
+    }
+}

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

@@ -0,0 +1,103 @@
+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_OriginEndAndOffsetZero : SftpFileStreamTestBase
+    {
+        private Random _random;
+        private string _path;
+        private FileMode _fileMode;
+        private FileAccess _fileAccess;
+        private int _bufferSize;
+        private uint _readBufferSize;
+        private uint _writeBufferSize;
+        private int _length;
+        private byte[] _handle;
+        private SftpFileStream _target;
+        private int _offset;
+        private SftpFileAttributes _attributes;
+        private long _actual;
+
+        protected override void SetupData()
+        {
+            base.SetupData();
+
+            _random = new Random();
+            _path = _random.Next().ToString();
+            _fileMode = FileMode.OpenOrCreate;
+            _fileAccess = FileAccess.Write;
+            _bufferSize = _random.Next(5, 1000);
+            _readBufferSize = (uint)_random.Next(5, 1000);
+            _writeBufferSize = (uint)_random.Next(5, 1000);
+            _length = _random.Next(5, 10000);
+            _handle = GenerateRandom(_random.Next(1, 10), _random);
+            _offset = 0;
+            _attributes = SftpFileAttributes.Empty;
+        }
+
+        protected override void SetupMocks()
+        {
+            SftpSessionMock.InSequence(MockSequence)
+                           .Setup(session => session.RequestOpen(_path, Flags.Write | Flags.CreateNewOrOpen, false))
+                           .Returns(_handle);
+            SftpSessionMock.InSequence(MockSequence)
+                           .Setup(session => session.CalculateOptimalReadLength((uint)_bufferSize))
+                           .Returns(_readBufferSize);
+            SftpSessionMock.InSequence(MockSequence)
+                           .Setup(session => session.CalculateOptimalWriteLength((uint)_bufferSize, _handle))
+                           .Returns(_writeBufferSize);
+            SftpSessionMock.InSequence(MockSequence)
+                           .Setup(session => session.IsOpen)
+                           .Returns(true);
+            SftpSessionMock.InSequence(MockSequence)
+                           .Setup(session => session.RequestFStat(_handle, false))
+                           .Returns(_attributes);
+            SftpSessionMock.InSequence(MockSequence)
+                           .Setup(session => session.RequestFSetStat(_handle, _attributes));
+            SftpSessionMock.InSequence(MockSequence).Setup(session => session.IsOpen).Returns(true);
+            SftpSessionMock.InSequence(MockSequence)
+                           .Setup(session => session.RequestFStat(_handle, false))
+                           .Returns(_attributes);
+        }
+
+        protected override void Arrange()
+        {
+            base.Arrange();
+
+            _target = new SftpFileStream(SftpSessionMock.Object, _path, _fileMode, _fileAccess, _bufferSize);
+            _target.SetLength(_length);
+        }
+
+        protected override void Act()
+        {
+            _actual = _target.Seek(_offset, SeekOrigin.End);
+        }
+
+        [TestMethod]
+        public void SeekShouldHaveReturnedSize()
+        {
+            Assert.AreEqual(_attributes.Size, _actual);
+        }
+
+        [TestMethod]
+        public void IsOpenOnSftpSessionShouldHaveBeenInvokedTwice()
+        {
+            SftpSessionMock.Verify(session => session.IsOpen, Times.Exactly(2));
+        }
+
+        [TestMethod]
+        public void PositionShouldReturnSize()
+        {
+            SftpSessionMock.InSequence(MockSequence).Setup(session => session.IsOpen).Returns(true);
+
+            Assert.AreEqual(_attributes.Size, _target.Position);
+
+            SftpSessionMock.Verify(session => session.IsOpen, Times.Exactly(3));
+        }
+    }
+}

src/Renci.SshNet/Sftp/SftpFileStream.cs

@@ -788,26 +788,6 @@ namespace Renci.SshNet.Sftp
                 {
                     // Flush the write buffer and then seek.
                     FlushWriteBuffer();
-
-                    switch (origin)
-                    {
-                        case SeekOrigin.Begin:
-                            newPosn = offset;
-                            break;
-                        case SeekOrigin.Current:
-                            newPosn = _position + offset;
-                            break;
-                        case SeekOrigin.End:
-                            var attributes = _session.RequestFStat(_handle, false);
-                            newPosn = attributes.Size - offset;
-                            break;
-                    }
-
-                    if (newPosn == -1)
-                    {
-                        throw new EndOfStreamException("End of stream.");
-                    }
-                    _position = newPosn;
                 }
                 else
                 {
@@ -838,29 +818,31 @@ namespace Renci.SshNet.Sftp
                     // Abandon the read buffer.
                     _bufferPosition = 0;
                     _bufferLen = 0;
+                }
 
-                    // Seek to the new position.
-                    switch (origin)
-                    {
-                        case SeekOrigin.Begin:
-                            newPosn = offset;
-                            break;
-                        case SeekOrigin.Current:
-                            newPosn = _position + offset;
-                            break;
-                        case SeekOrigin.End:
-                            var attributes = _session.RequestFStat(_handle, false);
-                            newPosn = attributes.Size - offset;
-                            break;
-                    }
-
-                    if (newPosn < 0)
-                    {
-                        throw new EndOfStreamException();
-                    }
+                // Seek to the new position.
+                switch (origin)
+                {
+                    case SeekOrigin.Begin:
+                        newPosn = offset;
+                        break;
+                    case SeekOrigin.Current:
+                        newPosn = _position + offset;
+                        break;
+                    case SeekOrigin.End:
+                        var attributes = _session.RequestFStat(_handle, false);
+                        newPosn = attributes.Size + offset;
+                        break;
+                    default:
+                        throw new ArgumentException(message: "Invalid seek origin.", paramName: "origin");
+                }
 
-                    _position = newPosn;
+                if (newPosn < 0)
+                {
+                    throw new EndOfStreamException();
                 }
+
+                _position = newPosn;
                 return _position;
             }
         }