Use ArraySegment for channel data (#1650)

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

Author: Rob-Hague

src/Renci.SshNet/Channels/Channel.cs

@@ -378,9 +378,9 @@ protected virtual void OnWindowAdjust(uint bytesToAdd)
         /// Called when channel data is received.
         /// </summary>
         /// <param name="data">The data.</param>
-        protected virtual void OnData(byte[] data)
+        protected virtual void OnData(ArraySegment<byte> data)
         {
-            AdjustDataWindow(data);
+            AdjustDataWindow(data.Count);
 
             DataReceived?.Invoke(this, new ChannelDataEventArgs(LocalChannelNumber, data));
         }
@@ -392,7 +392,7 @@ protected virtual void OnData(byte[] data)
         /// <param name="dataTypeCode">The data type code.</param>
         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<ChannelDataMessage> e
             {
                 try
                 {
-                    OnData(e.Message.Data);
+                    OnData(new ArraySegment<byte>(e.Message.Data, e.Message.Offset, e.Message.Size));
                 }
                 catch (Exception ex)
                 {
@@ -768,9 +768,9 @@ private void OnChannelFailure(object sender, MessageEventArgs<ChannelFailureMess
             }
         }
 
-        private void AdjustDataWindow(byte[] messageData)
+        private void AdjustDataWindow(int count)
         {
-            LocalWindowSize -= (uint)messageData.Length;
+            LocalWindowSize -= (uint)count;
 
             // Adjust window if window size is too low
             if (LocalWindowSize < LocalPacketSize)

src/Renci.SshNet/Channels/ChannelDirectTcpip.cs

@@ -194,7 +194,7 @@ protected override void Close()
         /// Called when channel data is received.
         /// </summary>
         /// <param name="data">The data.</param>
-        protected override void OnData(byte[] data)
+        protected override void OnData(ArraySegment<byte> 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);
                     }
                 }
             }

src/Renci.SshNet/Channels/ChannelForwardedTcpip.cs

@@ -201,14 +201,14 @@ protected override void Close()
         /// Called when channel data is received.
         /// </summary>
         /// <param name="data">The data.</param>
-        protected override void OnData(byte[] data)
+        protected override void OnData(ArraySegment<byte> 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);
             }
         }
     }

src/Renci.SshNet/Common/ChannelDataEventArgs.cs

@@ -13,17 +13,22 @@ internal class ChannelDataEventArgs : ChannelEventArgs
         /// <param name="channelNumber">Channel number.</param>
         /// <param name="data">Channel data.</param>
         /// <exception cref="ArgumentNullException"><paramref name="data"/> is <see langword="null"/>.</exception>
-        public ChannelDataEventArgs(uint channelNumber, byte[] data)
+        public ChannelDataEventArgs(uint channelNumber, ArraySegment<byte> data)
             : base(channelNumber)
         {
-            ThrowHelper.ThrowIfNull(data);
+            ThrowHelper.ThrowIfNull(data.Array);
 
             Data = data;
         }
 
+        internal ChannelDataEventArgs(uint channelNumber, byte[] data)
+            : this(channelNumber, new ArraySegment<byte>(data))
+        {
+        }
+
         /// <summary>
         /// Gets channel data.
         /// </summary>
-        public byte[] Data { get; }
+        public ArraySegment<byte> Data { get; }
     }
 }

src/Renci.SshNet/Common/ChannelExtendedDataEventArgs.cs

@@ -1,4 +1,6 @@
-namespace Renci.SshNet.Common
+using System;
+
+namespace Renci.SshNet.Common
 {
     /// <summary>
     /// Provides data for <see cref="Channels.Channel.ExtendedDataReceived"/> events.
@@ -12,7 +14,7 @@ internal sealed class ChannelExtendedDataEventArgs : ChannelDataEventArgs
         /// <param name="data">Channel data.</param>
         /// <param name="dataTypeCode">Channel data type code.</param>
         public ChannelExtendedDataEventArgs(uint channelNumber, byte[] data, uint dataTypeCode)
-            : base(channelNumber, data)
+            : base(channelNumber, new ArraySegment<byte>(data))
         {
             DataTypeCode = dataTypeCode;
         }

src/Renci.SshNet/Common/SshData.cs

@@ -232,7 +232,7 @@ protected string ReadString(Encoding encoding = null)
         }
 
         /// <summary>
-        /// Reads next data type as byte array from internal buffer.
+        /// Reads a length-prefixed byte array from the internal buffer.
         /// </summary>
         /// <returns>
         /// The bytes read.
@@ -242,6 +242,20 @@ protected byte[] ReadBinary()
             return _stream.ReadBinary();
         }
 
+        /// <summary>
+        /// Reads a length-prefixed byte array from the internal buffer,
+        /// returned as a view over the buffer.
+        /// </summary>
+        /// <remarks>
+        /// When using this method, consider whether the underlying buffer is shared
+        /// or reused, and whether the returned <see cref="ArraySegment{T}"/> may
+        /// exist beyond the lifetime for which it is valid to be used.
+        /// </remarks>
+        private protected ArraySegment<byte> ReadBinarySegment()
+        {
+            return _stream.ReadBinarySegment();
+        }
+
         /// <summary>
         /// Reads next name-list data type from internal buffer.
         /// </summary>

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;
         }
 
         /// <summary>

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<byte> data)
         {
-            var chunk = Encoding.UTF8.GetString(data);
+            var chunk = Encoding.UTF8.GetString(data.Array, data.Offset, data.Count);
 
             if (ServerCapabilities is null)
             {

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();
 

src/Renci.SshNet/Sftp/SftpSession.cs

@@ -302,67 +302,65 @@ protected override void OnChannelOpen()
             WorkingDirectory = RequestRealPath(".")[0].Key;
         }
 
-        protected override void OnDataReceived(byte[] data)
+        protected override void OnDataReceived(ArraySegment<byte> data)
         {
-            ArraySegment<byte> 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;
             }
 
             // 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<byte>(
+                data = new ArraySegment<byte>(
                     _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;
                 }
 
                 // 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);
             }