From 4f83427525ebff8f14fa1d8dddb19e6aeb4d6b51 Mon Sep 17 00:00:00 2001 From: Rob Hague Date: Sat, 19 Apr 2025 10:39:15 +0100 Subject: [PATCH] Use ArraySegment for channel data The library currently allocates 4 bytes (and some) for every 1 byte of file downloaded(*). It could be 0. This takes it to 3. (*) 1. Array allocated for read of encrypted packet from socket 2. Array for decrypted packet 3. Array for channel data (removed in this change) 4. Array for sftp data packet --- src/Renci.SshNet/Channels/Channel.cs | 12 +++---- .../Channels/ChannelDirectTcpip.cs | 4 +-- .../Channels/ChannelForwardedTcpip.cs | 4 +-- .../Common/ChannelDataEventArgs.cs | 11 ++++-- .../Common/ChannelExtendedDataEventArgs.cs | 6 ++-- src/Renci.SshNet/Common/SshData.cs | 16 ++++++++- .../Messages/Connection/ChannelDataMessage.cs | 8 +++-- src/Renci.SshNet/Netconf/NetConfSession.cs | 4 +-- src/Renci.SshNet/ScpClient.cs | 12 +++---- src/Renci.SshNet/Sftp/SftpSession.cs | 36 +++++++++---------- src/Renci.SshNet/Shell.cs | 4 +-- src/Renci.SshNet/ShellStream.cs | 6 ++-- src/Renci.SshNet/SshCommand.cs | 4 +-- src/Renci.SshNet/SubsystemSession.cs | 2 +- .../Classes/Channels/ChannelStub.cs | 2 +- .../Classes/Channels/ClientChannelStub.cs | 2 +- .../Connection/ChannelDataMessageTest.cs | 4 +-- .../Classes/SubsystemSessionStub.cs | 2 +- 18 files changed, 79 insertions(+), 60 deletions(-) diff --git a/src/Renci.SshNet/Channels/Channel.cs b/src/Renci.SshNet/Channels/Channel.cs index d009c3370..bbcd1ffbd 100644 --- a/src/Renci.SshNet/Channels/Channel.cs +++ b/src/Renci.SshNet/Channels/Channel.cs @@ -378,9 +378,9 @@ protected virtual void OnWindowAdjust(uint bytesToAdd) /// Called when channel data is received. /// /// The data. - protected virtual void OnData(byte[] data) + protected virtual void OnData(ArraySegment data) { - AdjustDataWindow(data); + AdjustDataWindow(data.Count); DataReceived?.Invoke(this, new ChannelDataEventArgs(LocalChannelNumber, data)); } @@ -392,7 +392,7 @@ protected virtual void OnData(byte[] data) /// The data type code. protected virtual void OnExtendedData(byte[] data, uint dataTypeCode) { - AdjustDataWindow(data); + AdjustDataWindow(data.Length); ExtendedDataReceived?.Invoke(this, new ChannelExtendedDataEventArgs(LocalChannelNumber, data, dataTypeCode)); } @@ -651,7 +651,7 @@ private void OnChannelData(object sender, MessageEventArgs e { try { - OnData(e.Message.Data); + OnData(new ArraySegment(e.Message.Data, e.Message.Offset, e.Message.Size)); } catch (Exception ex) { @@ -768,9 +768,9 @@ private void OnChannelFailure(object sender, MessageEventArgs /// The data. - protected override void OnData(byte[] data) + protected override void OnData(ArraySegment data) { base.OnData(data); @@ -204,7 +204,7 @@ protected override void OnData(byte[] data) { if (_socket.IsConnected()) { - SocketAbstraction.Send(_socket, data, 0, data.Length); + SocketAbstraction.Send(_socket, data.Array, data.Offset, data.Count); } } } diff --git a/src/Renci.SshNet/Channels/ChannelForwardedTcpip.cs b/src/Renci.SshNet/Channels/ChannelForwardedTcpip.cs index 29808e2b4..10d8a161c 100644 --- a/src/Renci.SshNet/Channels/ChannelForwardedTcpip.cs +++ b/src/Renci.SshNet/Channels/ChannelForwardedTcpip.cs @@ -201,14 +201,14 @@ protected override void Close() /// Called when channel data is received. /// /// The data. - protected override void OnData(byte[] data) + protected override void OnData(ArraySegment data) { base.OnData(data); var socket = _socket; if (socket.IsConnected()) { - SocketAbstraction.Send(socket, data, 0, data.Length); + SocketAbstraction.Send(socket, data.Array, data.Offset, data.Count); } } } diff --git a/src/Renci.SshNet/Common/ChannelDataEventArgs.cs b/src/Renci.SshNet/Common/ChannelDataEventArgs.cs index e42b140d9..5ded6fe0b 100644 --- a/src/Renci.SshNet/Common/ChannelDataEventArgs.cs +++ b/src/Renci.SshNet/Common/ChannelDataEventArgs.cs @@ -13,17 +13,22 @@ internal class ChannelDataEventArgs : ChannelEventArgs /// Channel number. /// Channel data. /// is . - public ChannelDataEventArgs(uint channelNumber, byte[] data) + public ChannelDataEventArgs(uint channelNumber, ArraySegment data) : base(channelNumber) { - ThrowHelper.ThrowIfNull(data); + ThrowHelper.ThrowIfNull(data.Array); Data = data; } + internal ChannelDataEventArgs(uint channelNumber, byte[] data) + : this(channelNumber, new ArraySegment(data)) + { + } + /// /// Gets channel data. /// - public byte[] Data { get; } + public ArraySegment Data { get; } } } diff --git a/src/Renci.SshNet/Common/ChannelExtendedDataEventArgs.cs b/src/Renci.SshNet/Common/ChannelExtendedDataEventArgs.cs index e67ccb901..ae7f4c31d 100644 --- a/src/Renci.SshNet/Common/ChannelExtendedDataEventArgs.cs +++ b/src/Renci.SshNet/Common/ChannelExtendedDataEventArgs.cs @@ -1,4 +1,6 @@ -namespace Renci.SshNet.Common +using System; + +namespace Renci.SshNet.Common { /// /// Provides data for events. @@ -12,7 +14,7 @@ internal sealed class ChannelExtendedDataEventArgs : ChannelDataEventArgs /// Channel data. /// Channel data type code. public ChannelExtendedDataEventArgs(uint channelNumber, byte[] data, uint dataTypeCode) - : base(channelNumber, data) + : base(channelNumber, new ArraySegment(data)) { DataTypeCode = dataTypeCode; } diff --git a/src/Renci.SshNet/Common/SshData.cs b/src/Renci.SshNet/Common/SshData.cs index 22fc86b54..098408731 100644 --- a/src/Renci.SshNet/Common/SshData.cs +++ b/src/Renci.SshNet/Common/SshData.cs @@ -232,7 +232,7 @@ protected string ReadString(Encoding encoding = null) } /// - /// Reads next data type as byte array from internal buffer. + /// Reads a length-prefixed byte array from the internal buffer. /// /// /// The bytes read. @@ -242,6 +242,20 @@ protected byte[] ReadBinary() return _stream.ReadBinary(); } + /// + /// Reads a length-prefixed byte array from the internal buffer, + /// returned as a view over the buffer. + /// + /// + /// When using this method, consider whether the underlying buffer is shared + /// or reused, and whether the returned may + /// exist beyond the lifetime for which it is valid to be used. + /// + private protected ArraySegment ReadBinarySegment() + { + return _stream.ReadBinarySegment(); + } + /// /// Reads next name-list data type from internal buffer. /// diff --git a/src/Renci.SshNet/Messages/Connection/ChannelDataMessage.cs b/src/Renci.SshNet/Messages/Connection/ChannelDataMessage.cs index 5331a4ca4..8416eb687 100644 --- a/src/Renci.SshNet/Messages/Connection/ChannelDataMessage.cs +++ b/src/Renci.SshNet/Messages/Connection/ChannelDataMessage.cs @@ -120,9 +120,11 @@ protected override void LoadData() { base.LoadData(); - Data = ReadBinary(); - Offset = 0; - Size = Data.Length; + var data = ReadBinarySegment(); + + Data = data.Array; + Offset = data.Offset; + Size = data.Count; } /// diff --git a/src/Renci.SshNet/Netconf/NetConfSession.cs b/src/Renci.SshNet/Netconf/NetConfSession.cs index acc99974b..2044a7852 100644 --- a/src/Renci.SshNet/Netconf/NetConfSession.cs +++ b/src/Renci.SshNet/Netconf/NetConfSession.cs @@ -121,9 +121,9 @@ protected override void OnChannelOpen() WaitOnHandle(_serverCapabilitiesConfirmed, OperationTimeout); } - protected override void OnDataReceived(byte[] data) + protected override void OnDataReceived(ArraySegment data) { - var chunk = Encoding.UTF8.GetString(data); + var chunk = Encoding.UTF8.GetString(data.Array, data.Offset, data.Count); if (ServerCapabilities is null) { diff --git a/src/Renci.SshNet/ScpClient.cs b/src/Renci.SshNet/ScpClient.cs index d128d3cc9..56330f2ef 100644 --- a/src/Renci.SshNet/ScpClient.cs +++ b/src/Renci.SshNet/ScpClient.cs @@ -257,7 +257,7 @@ public void Upload(Stream source, string path) using (var input = ServiceFactory.CreatePipeStream()) using (var channel = Session.CreateChannelSession()) { - channel.DataReceived += (sender, e) => input.Write(e.Data, 0, e.Data.Length); + channel.DataReceived += (sender, e) => input.Write(e.Data.Array!, e.Data.Offset, e.Data.Count); channel.Closed += (sender, e) => input.Dispose(); channel.Open(); @@ -300,7 +300,7 @@ public void Upload(FileInfo fileInfo, string path) using (var input = ServiceFactory.CreatePipeStream()) using (var channel = Session.CreateChannelSession()) { - channel.DataReceived += (sender, e) => input.Write(e.Data, 0, e.Data.Length); + channel.DataReceived += (sender, e) => input.Write(e.Data.Array!, e.Data.Offset, e.Data.Count); channel.Closed += (sender, e) => input.Dispose(); channel.Open(); @@ -346,7 +346,7 @@ public void Upload(DirectoryInfo directoryInfo, string path) using (var input = ServiceFactory.CreatePipeStream()) using (var channel = Session.CreateChannelSession()) { - channel.DataReceived += (sender, e) => input.Write(e.Data, 0, e.Data.Length); + channel.DataReceived += (sender, e) => input.Write(e.Data.Array!, e.Data.Offset, e.Data.Count); channel.Closed += (sender, e) => input.Dispose(); channel.Open(); @@ -389,7 +389,7 @@ public void Download(string filename, FileInfo fileInfo) using (var input = ServiceFactory.CreatePipeStream()) using (var channel = Session.CreateChannelSession()) { - channel.DataReceived += (sender, e) => input.Write(e.Data, 0, e.Data.Length); + channel.DataReceived += (sender, e) => input.Write(e.Data.Array!, e.Data.Offset, e.Data.Count); channel.Closed += (sender, e) => input.Dispose(); channel.Open(); @@ -429,7 +429,7 @@ public void Download(string directoryName, DirectoryInfo directoryInfo) using (var input = ServiceFactory.CreatePipeStream()) using (var channel = Session.CreateChannelSession()) { - channel.DataReceived += (sender, e) => input.Write(e.Data, 0, e.Data.Length); + channel.DataReceived += (sender, e) => input.Write(e.Data.Array!, e.Data.Offset, e.Data.Count); channel.Closed += (sender, e) => input.Dispose(); channel.Open(); @@ -469,7 +469,7 @@ public void Download(string filename, Stream destination) using (var input = ServiceFactory.CreatePipeStream()) using (var channel = Session.CreateChannelSession()) { - channel.DataReceived += (sender, e) => input.Write(e.Data, 0, e.Data.Length); + channel.DataReceived += (sender, e) => input.Write(e.Data.Array!, e.Data.Offset, e.Data.Count); channel.Closed += (sender, e) => input.Dispose(); channel.Open(); diff --git a/src/Renci.SshNet/Sftp/SftpSession.cs b/src/Renci.SshNet/Sftp/SftpSession.cs index 2d651c318..b63ed01dd 100644 --- a/src/Renci.SshNet/Sftp/SftpSession.cs +++ b/src/Renci.SshNet/Sftp/SftpSession.cs @@ -302,38 +302,36 @@ protected override void OnChannelOpen() WorkingDirectory = RequestRealPath(".")[0].Key; } - protected override void OnDataReceived(byte[] data) + protected override void OnDataReceived(ArraySegment data) { - ArraySegment d = new(data); - // If the buffer is empty then skip a copy and read packets // directly out of the given data. if (_buffer.ActiveLength == 0) { - while (d.Count >= 4) + while (data.Count >= 4) { - var packetLength = BinaryPrimitives.ReadInt32BigEndian(d); + var packetLength = BinaryPrimitives.ReadInt32BigEndian(data); - if (d.Count - 4 < packetLength) + if (data.Count - 4 < packetLength) { break; } - if (!TryLoadSftpMessage(d.Slice(4, packetLength))) + if (!TryLoadSftpMessage(data.Slice(4, packetLength))) { // An error occured. return; } - d = d.Slice(4 + packetLength); + data = data.Slice(4 + packetLength); } - if (d.Count > 0) + if (data.Count > 0) { // Now buffer the remainder. - _buffer.EnsureAvailableSpace(d.Count); - d.AsSpan().CopyTo(_buffer.AvailableSpan); - _buffer.Commit(d.Count); + _buffer.EnsureAvailableSpace(data.Count); + data.AsSpan().CopyTo(_buffer.AvailableSpan); + _buffer.Commit(data.Count); } return; @@ -341,20 +339,20 @@ protected override void OnDataReceived(byte[] data) // The buffer already had some data. Append the new data and // proceed with reading out packets. - _buffer.EnsureAvailableSpace(d.Count); - d.AsSpan().CopyTo(_buffer.AvailableSpan); - _buffer.Commit(d.Count); + _buffer.EnsureAvailableSpace(data.Count); + data.AsSpan().CopyTo(_buffer.AvailableSpan); + _buffer.Commit(data.Count); while (_buffer.ActiveLength >= 4) { - d = new ArraySegment( + data = new ArraySegment( _buffer.DangerousGetUnderlyingBuffer(), _buffer.ActiveStartOffset, _buffer.ActiveLength); - var packetLength = BinaryPrimitives.ReadInt32BigEndian(d); + var packetLength = BinaryPrimitives.ReadInt32BigEndian(data); - if (d.Count - 4 < packetLength) + if (data.Count - 4 < packetLength) { break; } @@ -362,7 +360,7 @@ protected override void OnDataReceived(byte[] data) // Note: the packet data in the buffer is safe to read from // only for the duration of this load. If it needs to be stored, // callees should make their own copy. - _ = TryLoadSftpMessage(d.Slice(4, packetLength)); + _ = TryLoadSftpMessage(data.Slice(4, packetLength)); _buffer.Discard(4 + packetLength); } diff --git a/src/Renci.SshNet/Shell.cs b/src/Renci.SshNet/Shell.cs index fe43d491b..909d8f507 100644 --- a/src/Renci.SshNet/Shell.cs +++ b/src/Renci.SshNet/Shell.cs @@ -241,12 +241,12 @@ private void Session_Disconnected(object sender, EventArgs e) private void Channel_ExtendedDataReceived(object sender, ChannelExtendedDataEventArgs e) { - _extendedOutputStream?.Write(e.Data, 0, e.Data.Length); + _extendedOutputStream?.Write(e.Data.Array, e.Data.Offset, e.Data.Count); } private void Channel_DataReceived(object sender, ChannelDataEventArgs e) { - _outputStream?.Write(e.Data, 0, e.Data.Length); + _outputStream?.Write(e.Data.Array, e.Data.Offset, e.Data.Count); } private void Channel_Closed(object sender, ChannelEventArgs e) diff --git a/src/Renci.SshNet/ShellStream.cs b/src/Renci.SshNet/ShellStream.cs index d56487417..f7dc862ac 100644 --- a/src/Renci.SshNet/ShellStream.cs +++ b/src/Renci.SshNet/ShellStream.cs @@ -965,16 +965,16 @@ private void Channel_DataReceived(object? sender, ChannelDataEventArgs e) { lock (_sync) { - _readBuffer.EnsureAvailableSpace(e.Data.Length); + _readBuffer.EnsureAvailableSpace(e.Data.Count); e.Data.AsSpan().CopyTo(_readBuffer.AvailableSpan); - _readBuffer.Commit(e.Data.Length); + _readBuffer.Commit(e.Data.Count); Monitor.PulseAll(_sync); } - DataReceived?.Invoke(this, new ShellDataEventArgs(e.Data)); + DataReceived?.Invoke(this, new ShellDataEventArgs(e.Data.ToArray())); } } } diff --git a/src/Renci.SshNet/SshCommand.cs b/src/Renci.SshNet/SshCommand.cs index c3d66e07c..1a3c6945d 100644 --- a/src/Renci.SshNet/SshCommand.cs +++ b/src/Renci.SshNet/SshCommand.cs @@ -583,7 +583,7 @@ private void Channel_RequestReceived(object? sender, ChannelRequestEventArgs e) private void Channel_ExtendedDataReceived(object? sender, ChannelExtendedDataEventArgs e) { - ExtendedOutputStream.Write(e.Data, 0, e.Data.Length); + ExtendedOutputStream.Write(e.Data.Array!, e.Data.Offset, e.Data.Count); if (e.DataTypeCode == 1) { @@ -593,7 +593,7 @@ private void Channel_ExtendedDataReceived(object? sender, ChannelExtendedDataEve private void Channel_DataReceived(object? sender, ChannelDataEventArgs e) { - OutputStream.Write(e.Data, 0, e.Data.Length); + OutputStream.Write(e.Data.Array!, e.Data.Offset, e.Data.Count); } /// diff --git a/src/Renci.SshNet/SubsystemSession.cs b/src/Renci.SshNet/SubsystemSession.cs index bb3ddfc9b..6c6200f4b 100644 --- a/src/Renci.SshNet/SubsystemSession.cs +++ b/src/Renci.SshNet/SubsystemSession.cs @@ -173,7 +173,7 @@ public void SendData(byte[] data) /// Called when data is received. /// /// The data. - protected abstract void OnDataReceived(byte[] data); + protected abstract void OnDataReceived(ArraySegment data); /// /// Raises the error. diff --git a/test/Renci.SshNet.Tests/Classes/Channels/ChannelStub.cs b/test/Renci.SshNet.Tests/Classes/Channels/ChannelStub.cs index d89ec98c0..119c3d58e 100644 --- a/test/Renci.SshNet.Tests/Classes/Channels/ChannelStub.cs +++ b/test/Renci.SshNet.Tests/Classes/Channels/ChannelStub.cs @@ -68,7 +68,7 @@ protected override void OnClose() } } - protected override void OnData(byte[] data) + protected override void OnData(ArraySegment data) { base.OnData(data); diff --git a/test/Renci.SshNet.Tests/Classes/Channels/ClientChannelStub.cs b/test/Renci.SshNet.Tests/Classes/Channels/ClientChannelStub.cs index 66113f60b..07ac5dad8 100644 --- a/test/Renci.SshNet.Tests/Classes/Channels/ClientChannelStub.cs +++ b/test/Renci.SshNet.Tests/Classes/Channels/ClientChannelStub.cs @@ -72,7 +72,7 @@ protected override void OnClose() } } - protected override void OnData(byte[] data) + protected override void OnData(ArraySegment data) { base.OnData(data); diff --git a/test/Renci.SshNet.Tests/Classes/Messages/Connection/ChannelDataMessageTest.cs b/test/Renci.SshNet.Tests/Classes/Messages/Connection/ChannelDataMessageTest.cs index a82b2b418..2f96b11d8 100644 --- a/test/Renci.SshNet.Tests/Classes/Messages/Connection/ChannelDataMessageTest.cs +++ b/test/Renci.SshNet.Tests/Classes/Messages/Connection/ChannelDataMessageTest.cs @@ -145,9 +145,7 @@ public void Load() target.Load(bytes, 1, bytes.Length - 1); // skip message type - Assert.IsTrue(target.Data.SequenceEqual(data.Take(offset, size))); - Assert.AreEqual(0, target.Offset); - Assert.AreEqual(size, target.Size); + CollectionAssert.AreEqual(data.Take(offset, size), target.Data.Take(target.Offset, target.Size)); } } } diff --git a/test/Renci.SshNet.Tests/Classes/SubsystemSessionStub.cs b/test/Renci.SshNet.Tests/Classes/SubsystemSessionStub.cs index e7fab595a..1af653f1a 100644 --- a/test/Renci.SshNet.Tests/Classes/SubsystemSessionStub.cs +++ b/test/Renci.SshNet.Tests/Classes/SubsystemSessionStub.cs @@ -37,7 +37,7 @@ protected override void OnChannelOpen() } } - protected override void OnDataReceived(byte[] data) + protected override void OnDataReceived(ArraySegment data) { OnDataReceivedInvocations.Add(new ChannelDataEventArgs(0, data));