Skip to content

Commit f3ebc29

Browse files
lemonytelemonyte
authored
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
1 parent b9bc475 commit f3ebc29

4 files changed

+331
-40
lines changed

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

+103
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
using System;
2+
using System.IO;
3+
using Microsoft.VisualStudio.TestTools.UnitTesting;
4+
using Moq;
5+
using Renci.SshNet.Sftp;
6+
7+
namespace Renci.SshNet.Tests.Classes.Sftp
8+
{
9+
[TestClass]
10+
public class SftpFileStreamTest_Seek_PositionedAtBeginningOfStream_OriginEndAndOffsetNegative : SftpFileStreamTestBase
11+
{
12+
private Random _random;
13+
private string _path;
14+
private FileMode _fileMode;
15+
private FileAccess _fileAccess;
16+
private int _bufferSize;
17+
private uint _readBufferSize;
18+
private uint _writeBufferSize;
19+
private int _length;
20+
private byte[] _handle;
21+
private SftpFileStream _target;
22+
private int _offset;
23+
private SftpFileAttributes _attributes;
24+
private long _actual;
25+
26+
protected override void SetupData()
27+
{
28+
base.SetupData();
29+
30+
_random = new Random();
31+
_path = _random.Next().ToString();
32+
_fileMode = FileMode.OpenOrCreate;
33+
_fileAccess = FileAccess.Write;
34+
_bufferSize = _random.Next(5, 1000);
35+
_readBufferSize = (uint)_random.Next(5, 1000);
36+
_writeBufferSize = (uint)_random.Next(5, 1000);
37+
_length = _random.Next(5, 10000);
38+
_handle = GenerateRandom(_random.Next(1, 10), _random);
39+
_offset = _random.Next(-_length, -1);
40+
_attributes = SftpFileAttributes.Empty;
41+
}
42+
43+
protected override void SetupMocks()
44+
{
45+
SftpSessionMock.InSequence(MockSequence)
46+
.Setup(session => session.RequestOpen(_path, Flags.Write | Flags.CreateNewOrOpen, false))
47+
.Returns(_handle);
48+
SftpSessionMock.InSequence(MockSequence)
49+
.Setup(session => session.CalculateOptimalReadLength((uint)_bufferSize))
50+
.Returns(_readBufferSize);
51+
SftpSessionMock.InSequence(MockSequence)
52+
.Setup(session => session.CalculateOptimalWriteLength((uint)_bufferSize, _handle))
53+
.Returns(_writeBufferSize);
54+
SftpSessionMock.InSequence(MockSequence)
55+
.Setup(session => session.IsOpen)
56+
.Returns(true);
57+
SftpSessionMock.InSequence(MockSequence)
58+
.Setup(session => session.RequestFStat(_handle, false))
59+
.Returns(_attributes);
60+
SftpSessionMock.InSequence(MockSequence)
61+
.Setup(session => session.RequestFSetStat(_handle, _attributes));
62+
SftpSessionMock.InSequence(MockSequence).Setup(session => session.IsOpen).Returns(true);
63+
SftpSessionMock.InSequence(MockSequence)
64+
.Setup(session => session.RequestFStat(_handle, false))
65+
.Returns(_attributes);
66+
}
67+
68+
protected override void Arrange()
69+
{
70+
base.Arrange();
71+
72+
_target = new SftpFileStream(SftpSessionMock.Object, _path, _fileMode, _fileAccess, _bufferSize);
73+
_target.SetLength(_length);
74+
}
75+
76+
protected override void Act()
77+
{
78+
_actual = _target.Seek(_offset, SeekOrigin.End);
79+
}
80+
81+
[TestMethod]
82+
public void SeekShouldHaveReturnedOffset()
83+
{
84+
Assert.AreEqual(_attributes.Size + _offset, _actual);
85+
}
86+
87+
[TestMethod]
88+
public void IsOpenOnSftpSessionShouldHaveBeenInvokedTwice()
89+
{
90+
SftpSessionMock.Verify(session => session.IsOpen, Times.Exactly(2));
91+
}
92+
93+
[TestMethod]
94+
public void PositionShouldReturnOffset()
95+
{
96+
SftpSessionMock.InSequence(MockSequence).Setup(session => session.IsOpen).Returns(true);
97+
98+
Assert.AreEqual(_attributes.Size + _offset, _target.Position);
99+
100+
SftpSessionMock.Verify(session => session.IsOpen, Times.Exactly(3));
101+
}
102+
}
103+
}

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

+103
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
using System;
2+
using System.IO;
3+
using Microsoft.VisualStudio.TestTools.UnitTesting;
4+
using Moq;
5+
using Renci.SshNet.Sftp;
6+
7+
namespace Renci.SshNet.Tests.Classes.Sftp
8+
{
9+
[TestClass]
10+
public class SftpFileStreamTest_Seek_PositionedAtBeginningOfStream_OriginEndAndOffsetPositive : SftpFileStreamTestBase
11+
{
12+
private Random _random;
13+
private string _path;
14+
private FileMode _fileMode;
15+
private FileAccess _fileAccess;
16+
private int _bufferSize;
17+
private uint _readBufferSize;
18+
private uint _writeBufferSize;
19+
private int _length;
20+
private byte[] _handle;
21+
private SftpFileStream _target;
22+
private int _offset;
23+
private SftpFileAttributes _attributes;
24+
private long _actual;
25+
26+
protected override void SetupData()
27+
{
28+
base.SetupData();
29+
30+
_random = new Random();
31+
_path = _random.Next().ToString();
32+
_fileMode = FileMode.OpenOrCreate;
33+
_fileAccess = FileAccess.Write;
34+
_bufferSize = _random.Next(5, 1000);
35+
_readBufferSize = (uint)_random.Next(5, 1000);
36+
_writeBufferSize = (uint)_random.Next(5, 1000);
37+
_length = _random.Next(5, 10000);
38+
_handle = GenerateRandom(_random.Next(1, 10), _random);
39+
_offset = _random.Next(1, int.MaxValue);
40+
_attributes = SftpFileAttributes.Empty;
41+
}
42+
43+
protected override void SetupMocks()
44+
{
45+
SftpSessionMock.InSequence(MockSequence)
46+
.Setup(session => session.RequestOpen(_path, Flags.Write | Flags.CreateNewOrOpen, false))
47+
.Returns(_handle);
48+
SftpSessionMock.InSequence(MockSequence)
49+
.Setup(session => session.CalculateOptimalReadLength((uint)_bufferSize))
50+
.Returns(_readBufferSize);
51+
SftpSessionMock.InSequence(MockSequence)
52+
.Setup(session => session.CalculateOptimalWriteLength((uint)_bufferSize, _handle))
53+
.Returns(_writeBufferSize);
54+
SftpSessionMock.InSequence(MockSequence)
55+
.Setup(session => session.IsOpen)
56+
.Returns(true);
57+
SftpSessionMock.InSequence(MockSequence)
58+
.Setup(session => session.RequestFStat(_handle, false))
59+
.Returns(_attributes);
60+
SftpSessionMock.InSequence(MockSequence)
61+
.Setup(session => session.RequestFSetStat(_handle, _attributes));
62+
SftpSessionMock.InSequence(MockSequence).Setup(session => session.IsOpen).Returns(true);
63+
SftpSessionMock.InSequence(MockSequence)
64+
.Setup(session => session.RequestFStat(_handle, false))
65+
.Returns(_attributes);
66+
}
67+
68+
protected override void Arrange()
69+
{
70+
base.Arrange();
71+
72+
_target = new SftpFileStream(SftpSessionMock.Object, _path, _fileMode, _fileAccess, _bufferSize);
73+
_target.SetLength(_length);
74+
}
75+
76+
protected override void Act()
77+
{
78+
_actual = _target.Seek(_offset, SeekOrigin.End);
79+
}
80+
81+
[TestMethod]
82+
public void SeekShouldHaveReturnedOffset()
83+
{
84+
Assert.AreEqual(_attributes.Size + _offset, _actual);
85+
}
86+
87+
[TestMethod]
88+
public void IsOpenOnSftpSessionShouldHaveBeenInvokedTwice()
89+
{
90+
SftpSessionMock.Verify(session => session.IsOpen, Times.Exactly(2));
91+
}
92+
93+
[TestMethod]
94+
public void PositionShouldReturnOffset()
95+
{
96+
SftpSessionMock.InSequence(MockSequence).Setup(session => session.IsOpen).Returns(true);
97+
98+
Assert.AreEqual(_attributes.Size + _offset, _target.Position);
99+
100+
SftpSessionMock.Verify(session => session.IsOpen, Times.Exactly(3));
101+
}
102+
}
103+
}

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

+103
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
using System;
2+
using System.IO;
3+
using Microsoft.VisualStudio.TestTools.UnitTesting;
4+
using Moq;
5+
using Renci.SshNet.Sftp;
6+
7+
namespace Renci.SshNet.Tests.Classes.Sftp
8+
{
9+
[TestClass]
10+
public class SftpFileStreamTest_Seek_PositionedAtBeginningOfStream_OriginEndAndOffsetZero : SftpFileStreamTestBase
11+
{
12+
private Random _random;
13+
private string _path;
14+
private FileMode _fileMode;
15+
private FileAccess _fileAccess;
16+
private int _bufferSize;
17+
private uint _readBufferSize;
18+
private uint _writeBufferSize;
19+
private int _length;
20+
private byte[] _handle;
21+
private SftpFileStream _target;
22+
private int _offset;
23+
private SftpFileAttributes _attributes;
24+
private long _actual;
25+
26+
protected override void SetupData()
27+
{
28+
base.SetupData();
29+
30+
_random = new Random();
31+
_path = _random.Next().ToString();
32+
_fileMode = FileMode.OpenOrCreate;
33+
_fileAccess = FileAccess.Write;
34+
_bufferSize = _random.Next(5, 1000);
35+
_readBufferSize = (uint)_random.Next(5, 1000);
36+
_writeBufferSize = (uint)_random.Next(5, 1000);
37+
_length = _random.Next(5, 10000);
38+
_handle = GenerateRandom(_random.Next(1, 10), _random);
39+
_offset = 0;
40+
_attributes = SftpFileAttributes.Empty;
41+
}
42+
43+
protected override void SetupMocks()
44+
{
45+
SftpSessionMock.InSequence(MockSequence)
46+
.Setup(session => session.RequestOpen(_path, Flags.Write | Flags.CreateNewOrOpen, false))
47+
.Returns(_handle);
48+
SftpSessionMock.InSequence(MockSequence)
49+
.Setup(session => session.CalculateOptimalReadLength((uint)_bufferSize))
50+
.Returns(_readBufferSize);
51+
SftpSessionMock.InSequence(MockSequence)
52+
.Setup(session => session.CalculateOptimalWriteLength((uint)_bufferSize, _handle))
53+
.Returns(_writeBufferSize);
54+
SftpSessionMock.InSequence(MockSequence)
55+
.Setup(session => session.IsOpen)
56+
.Returns(true);
57+
SftpSessionMock.InSequence(MockSequence)
58+
.Setup(session => session.RequestFStat(_handle, false))
59+
.Returns(_attributes);
60+
SftpSessionMock.InSequence(MockSequence)
61+
.Setup(session => session.RequestFSetStat(_handle, _attributes));
62+
SftpSessionMock.InSequence(MockSequence).Setup(session => session.IsOpen).Returns(true);
63+
SftpSessionMock.InSequence(MockSequence)
64+
.Setup(session => session.RequestFStat(_handle, false))
65+
.Returns(_attributes);
66+
}
67+
68+
protected override void Arrange()
69+
{
70+
base.Arrange();
71+
72+
_target = new SftpFileStream(SftpSessionMock.Object, _path, _fileMode, _fileAccess, _bufferSize);
73+
_target.SetLength(_length);
74+
}
75+
76+
protected override void Act()
77+
{
78+
_actual = _target.Seek(_offset, SeekOrigin.End);
79+
}
80+
81+
[TestMethod]
82+
public void SeekShouldHaveReturnedSize()
83+
{
84+
Assert.AreEqual(_attributes.Size, _actual);
85+
}
86+
87+
[TestMethod]
88+
public void IsOpenOnSftpSessionShouldHaveBeenInvokedTwice()
89+
{
90+
SftpSessionMock.Verify(session => session.IsOpen, Times.Exactly(2));
91+
}
92+
93+
[TestMethod]
94+
public void PositionShouldReturnSize()
95+
{
96+
SftpSessionMock.InSequence(MockSequence).Setup(session => session.IsOpen).Returns(true);
97+
98+
Assert.AreEqual(_attributes.Size, _target.Position);
99+
100+
SftpSessionMock.Verify(session => session.IsOpen, Times.Exactly(3));
101+
}
102+
}
103+
}

‎src/Renci.SshNet/Sftp/SftpFileStream.cs

+22-40
Original file line numberDiff line numberDiff line change
@@ -788,26 +788,6 @@ public override long Seek(long offset, SeekOrigin origin)
788788
{
789789
// Flush the write buffer and then seek.
790790
FlushWriteBuffer();
791-
792-
switch (origin)
793-
{
794-
case SeekOrigin.Begin:
795-
newPosn = offset;
796-
break;
797-
case SeekOrigin.Current:
798-
newPosn = _position + offset;
799-
break;
800-
case SeekOrigin.End:
801-
var attributes = _session.RequestFStat(_handle, false);
802-
newPosn = attributes.Size - offset;
803-
break;
804-
}
805-
806-
if (newPosn == -1)
807-
{
808-
throw new EndOfStreamException("End of stream.");
809-
}
810-
_position = newPosn;
811791
}
812792
else
813793
{
@@ -838,29 +818,31 @@ public override long Seek(long offset, SeekOrigin origin)
838818
// Abandon the read buffer.
839819
_bufferPosition = 0;
840820
_bufferLen = 0;
821+
}
841822

842-
// Seek to the new position.
843-
switch (origin)
844-
{
845-
case SeekOrigin.Begin:
846-
newPosn = offset;
847-
break;
848-
case SeekOrigin.Current:
849-
newPosn = _position + offset;
850-
break;
851-
case SeekOrigin.End:
852-
var attributes = _session.RequestFStat(_handle, false);
853-
newPosn = attributes.Size - offset;
854-
break;
855-
}
856-
857-
if (newPosn < 0)
858-
{
859-
throw new EndOfStreamException();
860-
}
823+
// Seek to the new position.
824+
switch (origin)
825+
{
826+
case SeekOrigin.Begin:
827+
newPosn = offset;
828+
break;
829+
case SeekOrigin.Current:
830+
newPosn = _position + offset;
831+
break;
832+
case SeekOrigin.End:
833+
var attributes = _session.RequestFStat(_handle, false);
834+
newPosn = attributes.Size + offset;
835+
break;
836+
default:
837+
throw new ArgumentException(message: "Invalid seek origin.", paramName: "origin");
838+
}
861839

862-
_position = newPosn;
840+
if (newPosn < 0)
841+
{
842+
throw new EndOfStreamException();
863843
}
844+
845+
_position = newPosn;
864846
return _position;
865847
}
866848
}

0 commit comments

Comments
 (0)