From 8f1717a69ad2d923df1393f5a1be6328894db392 Mon Sep 17 00:00:00 2001 From: "ferenc.vizkeleti" Date: Tue, 23 Jan 2024 12:23:08 +0100 Subject: [PATCH 1/7] Making all unit tests pass locally. Excluded MD5 tests on net462 because I get System.InvalidOperationException: 'This implementation is not part of the Windows Platform FIPS validated cryptographic algorithms.' SshdConfig: do not throw for "Include", just do nothing. Modified failing dos2unix parameters in Dockerfile.TestServer. Forceing LF line ending for key files used by integration tests, otherwise using them causes error. SftpClientTest.Test_Sftp_Multiple_Async_Upload_And_Download_10Files_5MB_Each times out for maxFiles=10, decreasing to 2 to make the test pass. --- .gitattributes | 5 ++++- test/Renci.SshNet.IntegrationTests/Dockerfile.TestServer | 6 ++++-- .../OldIntegrationTests/SftpClientTest.Upload.cs | 4 +++- test/Renci.SshNet.TestTools.OpenSSH/SshdConfig.cs | 2 ++ .../Classes/Common/HostKeyEventArgsTest.cs | 6 ++++++ test/Renci.SshNet.Tests/Classes/PrivateKeyFileTest.cs | 8 +++++++- test/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj | 4 ++++ 7 files changed, 30 insertions(+), 5 deletions(-) diff --git a/.gitattributes b/.gitattributes index 83d35b020..fa94b297a 100644 --- a/.gitattributes +++ b/.gitattributes @@ -17,4 +17,7 @@ *.snk binary # Ensure key files have LF endings for easier usage with ssh-keygen -test/Data/* eol=lf \ No newline at end of file +# Also, the dockerfile used for integration tests fails if key files have cr-lf +test/Data/* eol=lf +test/Renci.SshNet.IntegrationTests/server/**/* eol=lf +test/Renci.SshNet.IntegrationTests/user/* eol=lf diff --git a/test/Renci.SshNet.IntegrationTests/Dockerfile.TestServer b/test/Renci.SshNet.IntegrationTests/Dockerfile.TestServer index 19ef6e19e..628f1a88c 100644 --- a/test/Renci.SshNet.IntegrationTests/Dockerfile.TestServer +++ b/test/Renci.SshNet.IntegrationTests/Dockerfile.TestServer @@ -10,7 +10,8 @@ RUN apk update && apk upgrade --no-cache && \ apk add --no-cache openssh && \ # install openssh-server-pam to allow for keyboard-interactive authentication apk add --no-cache openssh-server-pam && \ - dos2unix /etc/ssh/* && \ + # must not use * for dos2unix parameter otherwise it tries to process folders too and fails + dos2unix /etc/ssh/ssh*key && \ chmod 400 /etc/ssh/ssh*key && \ sed -i 's/#PasswordAuthentication yes/PasswordAuthentication yes/' /etc/ssh/sshd_config && \ sed -i 's/#LogLevel\s*INFO/LogLevel DEBUG3/' /etc/ssh/sshd_config && \ @@ -28,7 +29,8 @@ RUN apk update && apk upgrade --no-cache && \ adduser -D sshnet && \ passwd -u sshnet && \ echo 'sshnet:ssh4ever' | chpasswd && \ - dos2unix /home/sshnet/.ssh/* && \ + # must not use * for dos2unix parameter otherwise it tries to process folders too and fails + dos2unix /home/sshnet/.ssh/*_key* && \ chown -R sshnet:sshnet /home/sshnet && \ chmod -R 700 /home/sshnet/.ssh && \ chmod -R 644 /home/sshnet/.ssh/authorized_keys && \ diff --git a/test/Renci.SshNet.IntegrationTests/OldIntegrationTests/SftpClientTest.Upload.cs b/test/Renci.SshNet.IntegrationTests/OldIntegrationTests/SftpClientTest.Upload.cs index 91c248bd3..743306325 100644 --- a/test/Renci.SshNet.IntegrationTests/OldIntegrationTests/SftpClientTest.Upload.cs +++ b/test/Renci.SshNet.IntegrationTests/OldIntegrationTests/SftpClientTest.Upload.cs @@ -78,7 +78,9 @@ public void Test_Sftp_Upload_Forbidden() [TestCategory("Sftp")] public void Test_Sftp_Multiple_Async_Upload_And_Download_10Files_5MB_Each() { - var maxFiles = 10; + // Works for up to 2 files, but fails for more files (on my machine). + // I get either "Renci.SshNet.Common.SshException: Channel was closed" or timeout exception. + var maxFiles = 2; var maxSize = 5; RemoveAllFiles(); diff --git a/test/Renci.SshNet.TestTools.OpenSSH/SshdConfig.cs b/test/Renci.SshNet.TestTools.OpenSSH/SshdConfig.cs index 5e3025711..77c4e6dc1 100644 --- a/test/Renci.SshNet.TestTools.OpenSSH/SshdConfig.cs +++ b/test/Renci.SshNet.TestTools.OpenSSH/SshdConfig.cs @@ -385,6 +385,8 @@ private static void ProcessGlobalOption(SshdConfig sshdConfig, string line) case "AuthorizedKeysFile": case "PasswordAuthentication": case "GatewayPorts": + // Had to add this otherwise docker container setup in integration test fails. + case "Include": break; default: throw new NotSupportedException($"Global option '{name}' is not supported."); diff --git a/test/Renci.SshNet.Tests/Classes/Common/HostKeyEventArgsTest.cs b/test/Renci.SshNet.Tests/Classes/Common/HostKeyEventArgsTest.cs index 84d899f8c..131bda1f8 100644 --- a/test/Renci.SshNet.Tests/Classes/Common/HostKeyEventArgsTest.cs +++ b/test/Renci.SshNet.Tests/Classes/Common/HostKeyEventArgsTest.cs @@ -45,6 +45,10 @@ public void HostKeyEventArgsConstructorTest() Assert.AreEqual(2048, target.KeyLength); } +// Excluding on net462 platform, because using MD5 hash throws: System.InvalidOperationException: +// 'This implementation is not part of the Windows Platform FIPS validated cryptographic algorithms.' +#if !NET462_TARGET_FRAMEWORK + /// ///A test for MD5 calculation in HostKeyEventArgs Constructor /// @@ -59,6 +63,8 @@ public void HostKeyEventArgsConstructorTest_VerifyMD5() } +#endif + /// ///A test for SHA256 calculation in HostKeyEventArgs Constructor /// diff --git a/test/Renci.SshNet.Tests/Classes/PrivateKeyFileTest.cs b/test/Renci.SshNet.Tests/Classes/PrivateKeyFileTest.cs index dfd02044a..a237c6bad 100644 --- a/test/Renci.SshNet.Tests/Classes/PrivateKeyFileTest.cs +++ b/test/Renci.SshNet.Tests/Classes/PrivateKeyFileTest.cs @@ -1,4 +1,8 @@ -using Microsoft.VisualStudio.TestTools.UnitTesting; +// Excluding these tests on net462 platform, beacuse using MD5 hash throws: System.InvalidOperationException: +// 'This implementation is not part of the Windows Platform FIPS validated cryptographic algorithms.' +#if !NET462_TARGET_FRAMEWORK + +using Microsoft.VisualStudio.TestTools.UnitTesting; using Renci.SshNet.Common; using Renci.SshNet.Tests.Common; using System; @@ -695,3 +699,5 @@ private static void TestRsaKeyFile(PrivateKeyFile rsaPrivateKeyFile) } } } + +#endif diff --git a/test/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj b/test/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj index 5ba659daa..bc99302c9 100644 --- a/test/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj +++ b/test/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj @@ -3,6 +3,10 @@ net462;net6.0;net7.0;net8.0 + + $(DefineConstants);NET462_TARGET_FRAMEWORK + + From ed38b3112abd19e518840c61f6d66065cee6fc04 Mon Sep 17 00:00:00 2001 From: "ferenc.vizkeleti" Date: Tue, 23 Jan 2024 13:51:41 +0100 Subject: [PATCH 2/7] Added SshCommand.InputStream. --- src/Renci.SshNet/Common/ChannelInputStream.cs | 219 ++++++++++++++++++ src/Renci.SshNet/SshCommand.cs | 16 ++ 2 files changed, 235 insertions(+) create mode 100644 src/Renci.SshNet/Common/ChannelInputStream.cs diff --git a/src/Renci.SshNet/Common/ChannelInputStream.cs b/src/Renci.SshNet/Common/ChannelInputStream.cs new file mode 100644 index 000000000..b1a5452f2 --- /dev/null +++ b/src/Renci.SshNet/Common/ChannelInputStream.cs @@ -0,0 +1,219 @@ +using System; +using System.IO; + +using Renci.SshNet.Channels; + +namespace Renci.SshNet.Common +{ + /// + /// ChannelInputStream is a one direction stream intended for channel data. + /// + public class ChannelInputStream : Stream + { + /// + /// Channel to send data to. + /// + private readonly IChannelSession _channel; + + /// + /// Total bytes passed through the stream. + /// + private long _totalPosition; + + /// + /// Indicates whether the current instance was disposed. + /// + private bool _isDisposed; + + internal ChannelInputStream(IChannelSession channel) + { + _channel = channel; + } + + /// + /// When overridden in a derived class, clears all buffers for this stream and causes any buffered data to be written to the underlying device. + /// + /// An I/O error occurs. + /// Methods were called after the stream was closed. + /// + /// Once flushed, any subsequent read operations no longer block until requested bytes are available. Any write operation reactivates blocking + /// reads. + /// + public override void Flush() + { + } + + /// + /// When overridden in a derived class, sets the position within the current stream. + /// + /// + /// The new position within the current stream. + /// + /// A byte offset relative to the origin parameter. + /// A value of type indicating the reference point used to obtain the new position. + /// The stream does not support seeking, such as if the stream is constructed from a pipe or console output. + public override long Seek(long offset, SeekOrigin origin) + { + throw new NotSupportedException(); + } + + /// + /// When overridden in a derived class, sets the length of the current stream. + /// + /// The desired length of the current stream in bytes. + /// The stream does not support both writing and seeking, such as if the stream is constructed from a pipe or console output. + public override void SetLength(long value) + { + throw new NotSupportedException(); + } + + /// + /// When overridden in a derived class, reads a sequence of bytes from the current stream and advances the position within the stream by the number of bytes read. + /// + /// + /// The total number of bytes read into the buffer. This can be less than the number of bytes requested if that many bytes are not currently available, or zero if the stream is closed or end of the stream has been reached. + /// + /// An array of bytes. When this method returns, the buffer contains the specified byte array with the values between offset and (offset + count - 1) replaced by the bytes read from the current source. + /// The zero-based byte offset in buffer at which to begin storing the data read from the current stream. + /// The maximum number of bytes to be read from the current stream. + /// The sum of offset and count is larger than the buffer length. + /// Methods were called after the stream was closed. + /// The stream does not support reading. + /// is null. + /// An I/O error occurs. + /// offset or count is negative. + public override int Read(byte[] buffer, int offset, int count) + { + throw new NotSupportedException(); + } + + /// + /// When overridden in a derived class, writes a sequence of bytes to the current stream and advances the current position within this stream by the number of bytes written. + /// + /// An array of bytes. This method copies count bytes from buffer to the current stream. + /// The zero-based byte offset in buffer at which to begin copying bytes to the current stream. + /// The number of bytes to be written to the current stream. + /// An I/O error occurs. + /// The stream does not support writing. + /// Methods were called after the stream was closed. + /// is null. + /// The sum of offset and count is greater than the buffer length. + /// offset or count is negative. + public override void Write(byte[] buffer, int offset, int count) + { + if (buffer == null) + { + throw new ArgumentNullException(nameof(buffer)); + } + + if (offset + count > buffer.Length) + { + throw new ArgumentException("The sum of offset and count is greater than the buffer length."); + } + + if (offset < 0 || count < 0) + { + throw new ArgumentOutOfRangeException(nameof(offset), "offset or count is negative."); + } + + if (_isDisposed) + { + throw CreateObjectDisposedException(); + } + + if (count == 0) + { + return; + } + + _channel.SendData(buffer, offset, count); + _totalPosition += count; + _channel.SendEof(); + } + + /// + /// Releases the unmanaged resources used by the Stream and optionally releases the managed resources. + /// + /// true to release both managed and unmanaged resources; false to release only unmanaged resources. + /// + /// Disposing a will interrupt blocking read and write operations. + /// + protected override void Dispose(bool disposing) + { + base.Dispose(disposing); + + if (!_isDisposed) + { + _isDisposed = true; + if (_totalPosition > 0 && _channel.IsOpen) + { + _channel.SendEof(); + } + } + } + + /// + /// Gets a value indicating whether the current stream supports reading. + /// + /// + /// true if the stream supports reading; otherwise, false. + /// + public override bool CanRead + { + get { return false; } + } + + /// + /// Gets a value indicating whether the current stream supports seeking. + /// + /// + /// true if the stream supports seeking; otherwise, false. + /// + public override bool CanSeek + { + get { return false; } + } + + /// + /// Gets a value indicating whether the current stream supports writing. + /// + /// + /// true if the stream supports writing; otherwise, false. + /// + public override bool CanWrite + { + get { return true; } + } + + /// + /// Gets the length in bytes of the stream. + /// + /// + /// A long value representing the length of the stream in bytes. + /// + /// A class derived from Stream does not support seeking. + /// Methods were called after the stream was closed. + public override long Length + { + get { throw new NotSupportedException(); } + } + + /// + /// Gets or sets the position within the current stream. + /// + /// + /// The current position within the stream. + /// + /// The stream does not support seeking. + public override long Position + { + get { return _totalPosition; } + set { throw new NotSupportedException(); } + } + + private ObjectDisposedException CreateObjectDisposedException() + { + return new ObjectDisposedException(GetType().FullName); + } + } +} diff --git a/src/Renci.SshNet/SshCommand.cs b/src/Renci.SshNet/SshCommand.cs index a7769ab2c..1f9c7310c 100644 --- a/src/Renci.SshNet/SshCommand.cs +++ b/src/Renci.SshNet/SshCommand.cs @@ -64,6 +64,11 @@ public class SshCommand : IDisposable public Stream ExtendedOutputStream { get; private set; } #pragma warning restore CA1859 // Use concrete types when possible for improved performance + /// + /// Gets the input stream. + /// + public Stream InputStream { get; private set; } + /// /// Gets the command execution result. /// @@ -252,6 +257,10 @@ public IAsyncResult BeginExecute(AsyncCallback callback, object state) _channel = CreateChannel(); _channel.Open(); + + // Initialize the input stream + InputStream = new ChannelInputStream(_channel); + _ = _channel.SendExecRequest(CommandText); return _asyncResult; @@ -552,6 +561,13 @@ protected virtual void Dispose(bool disposing) _channel = null; } + var inputStream = InputStream; + if (inputStream != null) + { + inputStream.Dispose(); + InputStream = null; + } + var outputStream = OutputStream; if (outputStream != null) { From 1883838ad23b6c7c3c5db65bb26a33130a8c6ed6 Mon Sep 17 00:00:00 2001 From: "ferenc.vizkeleti" Date: Tue, 23 Jan 2024 17:11:31 +0100 Subject: [PATCH 3/7] Added an integration test for SshCommand.InputStream. --- src/Renci.SshNet/Common/ChannelInputStream.cs | 2 ++ .../SshClientTests.cs | 19 +++++++++++++++++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/src/Renci.SshNet/Common/ChannelInputStream.cs b/src/Renci.SshNet/Common/ChannelInputStream.cs index b1a5452f2..f6747c4d9 100644 --- a/src/Renci.SshNet/Common/ChannelInputStream.cs +++ b/src/Renci.SshNet/Common/ChannelInputStream.cs @@ -128,6 +128,8 @@ public override void Write(byte[] buffer, int offset, int count) _channel.SendData(buffer, offset, count); _totalPosition += count; + + // Must send EOF, otherwise SshCommand.EndExecute never gets called. _channel.SendEof(); } diff --git a/test/Renci.SshNet.IntegrationTests/SshClientTests.cs b/test/Renci.SshNet.IntegrationTests/SshClientTests.cs index b737b343f..01841b824 100644 --- a/test/Renci.SshNet.IntegrationTests/SshClientTests.cs +++ b/test/Renci.SshNet.IntegrationTests/SshClientTests.cs @@ -17,12 +17,27 @@ public SshClientTests() [TestMethod] public void Echo_Command_with_all_characters() { - var builder = new StringBuilder(); var response = _sshClient.RunCommand("echo $'test !@#$%^&*()_+{}:,./<>[];\\|'"); Assert.AreEqual("test !@#$%^&*()_+{}:,./<>[];\\|\n", response.Result); } - + + [TestMethod] + public void Send_InputStream_to_Command() + { + var inputByteArray = Encoding.UTF8.GetBytes("Hello world!"); + + // Make the server echo back the input file with "cat" + var command = _sshClient.CreateCommand("cat"); + + var asyncResult = command.BeginExecute(); + command.InputStream.Write(inputByteArray); + command.EndExecute(asyncResult); + + Assert.AreEqual("Hello world!", command.Result); + Assert.AreEqual(string.Empty, command.Error); + } + public void Dispose() { _sshClient.Disconnect(); From 742e4bfc50dcf0756e7d047211b980bea29bb8a1 Mon Sep 17 00:00:00 2001 From: "ferenc.vizkeleti" Date: Mon, 29 Jan 2024 09:30:33 +0100 Subject: [PATCH 4/7] Reverting changes made to unit tests unrelated to this PR. --- .../OldIntegrationTests/SftpClientTest.Upload.cs | 4 +--- test/Renci.SshNet.TestTools.OpenSSH/SshdConfig.cs | 1 - .../Classes/Common/HostKeyEventArgsTest.cs | 6 ------ test/Renci.SshNet.Tests/Classes/PrivateKeyFileTest.cs | 8 +------- test/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj | 4 ---- 5 files changed, 2 insertions(+), 21 deletions(-) diff --git a/test/Renci.SshNet.IntegrationTests/OldIntegrationTests/SftpClientTest.Upload.cs b/test/Renci.SshNet.IntegrationTests/OldIntegrationTests/SftpClientTest.Upload.cs index 743306325..91c248bd3 100644 --- a/test/Renci.SshNet.IntegrationTests/OldIntegrationTests/SftpClientTest.Upload.cs +++ b/test/Renci.SshNet.IntegrationTests/OldIntegrationTests/SftpClientTest.Upload.cs @@ -78,9 +78,7 @@ public void Test_Sftp_Upload_Forbidden() [TestCategory("Sftp")] public void Test_Sftp_Multiple_Async_Upload_And_Download_10Files_5MB_Each() { - // Works for up to 2 files, but fails for more files (on my machine). - // I get either "Renci.SshNet.Common.SshException: Channel was closed" or timeout exception. - var maxFiles = 2; + var maxFiles = 10; var maxSize = 5; RemoveAllFiles(); diff --git a/test/Renci.SshNet.TestTools.OpenSSH/SshdConfig.cs b/test/Renci.SshNet.TestTools.OpenSSH/SshdConfig.cs index 77c4e6dc1..a0129249d 100644 --- a/test/Renci.SshNet.TestTools.OpenSSH/SshdConfig.cs +++ b/test/Renci.SshNet.TestTools.OpenSSH/SshdConfig.cs @@ -385,7 +385,6 @@ private static void ProcessGlobalOption(SshdConfig sshdConfig, string line) case "AuthorizedKeysFile": case "PasswordAuthentication": case "GatewayPorts": - // Had to add this otherwise docker container setup in integration test fails. case "Include": break; default: diff --git a/test/Renci.SshNet.Tests/Classes/Common/HostKeyEventArgsTest.cs b/test/Renci.SshNet.Tests/Classes/Common/HostKeyEventArgsTest.cs index 131bda1f8..84d899f8c 100644 --- a/test/Renci.SshNet.Tests/Classes/Common/HostKeyEventArgsTest.cs +++ b/test/Renci.SshNet.Tests/Classes/Common/HostKeyEventArgsTest.cs @@ -45,10 +45,6 @@ public void HostKeyEventArgsConstructorTest() Assert.AreEqual(2048, target.KeyLength); } -// Excluding on net462 platform, because using MD5 hash throws: System.InvalidOperationException: -// 'This implementation is not part of the Windows Platform FIPS validated cryptographic algorithms.' -#if !NET462_TARGET_FRAMEWORK - /// ///A test for MD5 calculation in HostKeyEventArgs Constructor /// @@ -63,8 +59,6 @@ public void HostKeyEventArgsConstructorTest_VerifyMD5() } -#endif - /// ///A test for SHA256 calculation in HostKeyEventArgs Constructor /// diff --git a/test/Renci.SshNet.Tests/Classes/PrivateKeyFileTest.cs b/test/Renci.SshNet.Tests/Classes/PrivateKeyFileTest.cs index a237c6bad..dfd02044a 100644 --- a/test/Renci.SshNet.Tests/Classes/PrivateKeyFileTest.cs +++ b/test/Renci.SshNet.Tests/Classes/PrivateKeyFileTest.cs @@ -1,8 +1,4 @@ -// Excluding these tests on net462 platform, beacuse using MD5 hash throws: System.InvalidOperationException: -// 'This implementation is not part of the Windows Platform FIPS validated cryptographic algorithms.' -#if !NET462_TARGET_FRAMEWORK - -using Microsoft.VisualStudio.TestTools.UnitTesting; +using Microsoft.VisualStudio.TestTools.UnitTesting; using Renci.SshNet.Common; using Renci.SshNet.Tests.Common; using System; @@ -699,5 +695,3 @@ private static void TestRsaKeyFile(PrivateKeyFile rsaPrivateKeyFile) } } } - -#endif diff --git a/test/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj b/test/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj index bc99302c9..5ba659daa 100644 --- a/test/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj +++ b/test/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj @@ -3,10 +3,6 @@ net462;net6.0;net7.0;net8.0 - - $(DefineConstants);NET462_TARGET_FRAMEWORK - - From 7ac41c1b2bfe5fab2c1706cdbb270a3cdf0b0a03 Mon Sep 17 00:00:00 2001 From: "ferenc.vizkeleti" Date: Wed, 31 Jan 2024 08:57:15 +0100 Subject: [PATCH 5/7] Moved ChannelInputStream's EOF sending from Write to Dispose. Replace SshCommand.InputStream with CreateInputStream to emphasise that a (disposable) resource is created here. EndExecute also closes the _inputStream to make sure that EOF is sent (in case the user forgot to dispose the input stream). Added more unit tests: sending the input one byte at a time, not disposing the input stream, calling CreateInputStream before BeginExecute or AfterEndExecute throws exception. --- src/Renci.SshNet/Common/ChannelInputStream.cs | 11 ++- src/Renci.SshNet/SshCommand.cs | 29 +++++-- .../SshClientTests.cs | 80 ++++++++++++++++++- 3 files changed, 102 insertions(+), 18 deletions(-) diff --git a/src/Renci.SshNet/Common/ChannelInputStream.cs b/src/Renci.SshNet/Common/ChannelInputStream.cs index f6747c4d9..5396ac9f3 100644 --- a/src/Renci.SshNet/Common/ChannelInputStream.cs +++ b/src/Renci.SshNet/Common/ChannelInputStream.cs @@ -128,9 +128,6 @@ public override void Write(byte[] buffer, int offset, int count) _channel.SendData(buffer, offset, count); _totalPosition += count; - - // Must send EOF, otherwise SshCommand.EndExecute never gets called. - _channel.SendEof(); } /// @@ -142,16 +139,18 @@ public override void Write(byte[] buffer, int offset, int count) /// protected override void Dispose(bool disposing) { - base.Dispose(disposing); - if (!_isDisposed) { _isDisposed = true; - if (_totalPosition > 0 && _channel.IsOpen) + + // Closing the InputStream requires sending EOF. + if (_totalPosition > 0 && _channel?.IsOpen == true) { _channel.SendEof(); } } + + base.Dispose(disposing); } /// diff --git a/src/Renci.SshNet/SshCommand.cs b/src/Renci.SshNet/SshCommand.cs index 1f9c7310c..5b6363368 100644 --- a/src/Renci.SshNet/SshCommand.cs +++ b/src/Renci.SshNet/SshCommand.cs @@ -31,6 +31,7 @@ public class SshCommand : IDisposable private StringBuilder _error; private bool _hasError; private bool _isDisposed; + private ChannelInputStream _inputStream; /// /// Gets the command text. @@ -65,9 +66,23 @@ public class SshCommand : IDisposable #pragma warning restore CA1859 // Use concrete types when possible for improved performance /// - /// Gets the input stream. + /// Creates and returns the input stream for the command. /// - public Stream InputStream { get; private set; } + /// + /// The stream that can be used to transfer data to the command's input stream. + /// + #pragma warning disable CA1859 // Use concrete types when possible for improved performance + public Stream CreateInputStream() +#pragma warning restore CA1859 // Use concrete types when possible for improved performance + { + if (_channel == null) + { + throw new InvalidOperationException($"The input stream can be used only after calling BeginExecute and before calling EndExecute."); + } + + _inputStream ??= new ChannelInputStream(_channel); + return _inputStream; + } /// /// Gets the command execution result. @@ -222,7 +237,6 @@ public IAsyncResult BeginExecute(AsyncCallback callback, object state) AsyncState = state, }; - // When command re-executed again, create a new channel if (_channel is not null) { throw new SshException("Invalid operation."); @@ -258,9 +272,6 @@ public IAsyncResult BeginExecute(AsyncCallback callback, object state) _channel = CreateChannel(); _channel.Open(); - // Initialize the input stream - InputStream = new ChannelInputStream(_channel); - _ = _channel.SendExecRequest(CommandText); return _asyncResult; @@ -310,6 +321,8 @@ public string EndExecute(IAsyncResult asyncResult) throw new ArgumentException("EndExecute can only be called once for each asynchronous operation."); } + _inputStream?.Close(); + // wait for operation to complete (or time out) WaitOnHandle(_asyncResult.AsyncWaitHandle); @@ -561,11 +574,11 @@ protected virtual void Dispose(bool disposing) _channel = null; } - var inputStream = InputStream; + var inputStream = _inputStream; if (inputStream != null) { inputStream.Dispose(); - InputStream = null; + _inputStream = null; } var outputStream = OutputStream; diff --git a/test/Renci.SshNet.IntegrationTests/SshClientTests.cs b/test/Renci.SshNet.IntegrationTests/SshClientTests.cs index 01841b824..0476312ee 100644 --- a/test/Renci.SshNet.IntegrationTests/SshClientTests.cs +++ b/test/Renci.SshNet.IntegrationTests/SshClientTests.cs @@ -28,14 +28,86 @@ public void Send_InputStream_to_Command() var inputByteArray = Encoding.UTF8.GetBytes("Hello world!"); // Make the server echo back the input file with "cat" - var command = _sshClient.CreateCommand("cat"); + using (var command = _sshClient.CreateCommand("cat")) + { + var asyncResult = command.BeginExecute(); + using (var inputStream = command.CreateInputStream()) + { + inputStream.Write(inputByteArray); + } + + command.EndExecute(asyncResult); + + Assert.AreEqual("Hello world!", command.Result); + Assert.AreEqual(string.Empty, command.Error); + } + } + + [TestMethod] + public void Send_InputStream_to_Command_OneByteAtATime() + { + var inputByteArray = Encoding.UTF8.GetBytes("Hello world!"); + + // Make the server echo back the input file with "cat" + using (var command = _sshClient.CreateCommand("cat")) + { + var asyncResult = command.BeginExecute(); + + using (var inputStream = command.CreateInputStream()) + { + for (var i = 0; i < inputByteArray.Length; i++) + { + inputStream.WriteByte(inputByteArray[i]); + } + } + + command.EndExecute(asyncResult); + + Assert.AreEqual("Hello world!", command.Result); + Assert.AreEqual(string.Empty, command.Error); + } + } + + [TestMethod] + public void Send_InputStream_to_Command_InputStreamNotInUsingBlock_StillWorks() + { + var inputByteArray = Encoding.UTF8.GetBytes("Hello world!"); + + // Make the server echo back the input file with "cat" + using (var command = _sshClient.CreateCommand("cat")) + { + var asyncResult = command.BeginExecute(); + + var inputStream = command.CreateInputStream(); + for (var i = 0; i < inputByteArray.Length; i++) + { + inputStream.WriteByte(inputByteArray[i]); + } + + command.EndExecute(asyncResult); + + Assert.AreEqual("Hello world!", command.Result); + Assert.AreEqual(string.Empty, command.Error); + } + } + + [TestMethod] + public void CreateInputStream_BeforeBeginExecute_ThrowsInvalidOperationException() + { + var command = _sshClient.CreateCommand("ls"); + + Assert.ThrowsException(command.CreateInputStream); + } + + [TestMethod] + public void CreateInputStream_AfterEndExecute_ThrowsInvalidOperationException() + { + var command = _sshClient.CreateCommand("ls"); var asyncResult = command.BeginExecute(); - command.InputStream.Write(inputByteArray); command.EndExecute(asyncResult); - Assert.AreEqual("Hello world!", command.Result); - Assert.AreEqual(string.Empty, command.Error); + Assert.ThrowsException(command.CreateInputStream); } public void Dispose() From 03781c10ce2efdf6d0c64e1f98caf4500dcdd949 Mon Sep 17 00:00:00 2001 From: "ferenc.vizkeleti" Date: Fri, 2 Feb 2024 07:59:15 +0100 Subject: [PATCH 6/7] Fixing review comments. --- src/Renci.SshNet/Common/ChannelInputStream.cs | 7 ++----- src/Renci.SshNet/SshCommand.cs | 7 ++++++- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/Renci.SshNet/Common/ChannelInputStream.cs b/src/Renci.SshNet/Common/ChannelInputStream.cs index 5396ac9f3..f509ac38b 100644 --- a/src/Renci.SshNet/Common/ChannelInputStream.cs +++ b/src/Renci.SshNet/Common/ChannelInputStream.cs @@ -8,7 +8,7 @@ namespace Renci.SshNet.Common /// /// ChannelInputStream is a one direction stream intended for channel data. /// - public class ChannelInputStream : Stream + internal sealed class ChannelInputStream : Stream { /// /// Channel to send data to. @@ -134,9 +134,6 @@ public override void Write(byte[] buffer, int offset, int count) /// Releases the unmanaged resources used by the Stream and optionally releases the managed resources. /// /// true to release both managed and unmanaged resources; false to release only unmanaged resources. - /// - /// Disposing a will interrupt blocking read and write operations. - /// protected override void Dispose(bool disposing) { if (!_isDisposed) @@ -144,7 +141,7 @@ protected override void Dispose(bool disposing) _isDisposed = true; // Closing the InputStream requires sending EOF. - if (_totalPosition > 0 && _channel?.IsOpen == true) + if (disposing && _totalPosition > 0 && _channel?.IsOpen == true) { _channel.SendEof(); } diff --git a/src/Renci.SshNet/SshCommand.cs b/src/Renci.SshNet/SshCommand.cs index 5b6363368..7aa013ad8 100644 --- a/src/Renci.SshNet/SshCommand.cs +++ b/src/Renci.SshNet/SshCommand.cs @@ -80,7 +80,12 @@ public Stream CreateInputStream() throw new InvalidOperationException($"The input stream can be used only after calling BeginExecute and before calling EndExecute."); } - _inputStream ??= new ChannelInputStream(_channel); + if (_inputStream != null) + { + throw new InvalidOperationException($"The input stream already exists."); + } + + _inputStream = new ChannelInputStream(_channel); return _inputStream; } From f29213fcc8931eb1887a4d491288df002c16c326 Mon Sep 17 00:00:00 2001 From: Rob Hague Date: Tue, 6 Feb 2024 16:41:55 +0100 Subject: [PATCH 7/7] Fix build error after #1286 --- test/Renci.SshNet.IntegrationTests/SshClientTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Renci.SshNet.IntegrationTests/SshClientTests.cs b/test/Renci.SshNet.IntegrationTests/SshClientTests.cs index 0476312ee..2cfeb53c3 100644 --- a/test/Renci.SshNet.IntegrationTests/SshClientTests.cs +++ b/test/Renci.SshNet.IntegrationTests/SshClientTests.cs @@ -34,7 +34,7 @@ public void Send_InputStream_to_Command() using (var inputStream = command.CreateInputStream()) { - inputStream.Write(inputByteArray); + inputStream.Write(inputByteArray, 0, inputByteArray.Length); } command.EndExecute(asyncResult);