Skip to content

Commit

Permalink
Fix Seek Operations in SftpFileStream (#910)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@lemonyte @drieseng
lemonyte authored and drieseng committed May 24, 2023
1 parent b31e857 commit 8b97a67
Show file tree
Hide file tree
Showing 4 changed files with 331 additions and 40 deletions.
103 changes: 103 additions & 0 deletions .../Sftp/SftpFileStreamTest_Seek_PositionedAtBeginningOfStream_OriginEndAndOffsetNegative.cs
View file
Original file line number Diff line number Diff line change
@@ -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));
}
}
}
103 changes: 103 additions & 0 deletions .../Sftp/SftpFileStreamTest_Seek_PositionedAtBeginningOfStream_OriginEndAndOffsetPositive.cs
View file
Original file line number Diff line number Diff line change
@@ -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));
}
}
}
103 changes: 103 additions & 0 deletions ...sses/Sftp/SftpFileStreamTest_Seek_PositionedAtBeginningOfStream_OriginEndAndOffsetZero.cs
View file
Original file line number Diff line number Diff line change
@@ -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));
}
}
}
62 changes: 22 additions & 40 deletions src/Renci.SshNet/Sftp/SftpFileStream.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -788,26 +788,6 @@ public override long Seek(long offset, SeekOrigin origin)
{
// 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
{
Expand Down Expand Up @@ -838,29 +818,31 @@ public override long Seek(long offset, SeekOrigin origin)
// 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;
}
}
Expand Down

0 comments on commit 8b97a67

Please sign in to comment.