Moving chunked output encoding to winscp.com, keeping the console API interface pure binary

(cherry picked from commit 469758e2ba06bbd2e73a0e04eb2cd9e8e38420c5)

Source commit: 6909a100a75c21ed95cfbd2d736634b49a309368

dotnet/Session.cs

@@ -1013,6 +1013,7 @@ namespace WinSCP
                         }
                         finally
                         {
+                            _process.StdOut = null;
                             // Only after disposing the group reader, so when called from onGetEndWithExit, the Check() has all failures.
                             operationResultGuard.Dispose();
                         }
@@ -1032,6 +1033,15 @@ namespace WinSCP
                     }
                 }
 
+                // should never happen
+                if (_process.StdOut != null)
+                {
+                    throw Logger.WriteException(new InvalidOperationException("Data stream already exist"));
+                }
+
+                PipeStream stream = new PipeStream(onGetEndWithExit);
+                _process.StdOut = stream;
+
                 try
                 {
                     bool downloadFound;
@@ -1047,7 +1057,6 @@ namespace WinSCP
                     }
                     if (downloadFound)
                     {
-                        ChunkedReadStream stream = new ChunkedReadStream(_process.StdOut, onGetEndWithExit);
                         callstackAndLock.DisarmLock();
                         return stream;
                     }
@@ -2219,7 +2228,7 @@ namespace WinSCP
                 throw Logger.WriteException(new SessionLocalException(this, "Aborted."));
             }
 
-            if (_process.StdOut.ReadAvailable(1))
+            if ((_process.StdOut != null) && _process.StdOut.ReadAvailable(1))
             {
                 throw new StdOutException();
             }

dotnet/internal/ChunkedReadStream.cs

@@ -1,147 +0,0 @@
-using System;
-using System.Globalization;
-using System.IO;
-
-namespace WinSCP
-{
-    internal class ChunkedReadStream : Stream
-    {
-        public ChunkedReadStream(Stream baseStream, Action onDispose)
-        {
-            _baseStream = baseStream;
-            _onDispose = onDispose;
-            _remaining = 0;
-            _eof = false;
-        }
-
-        public override bool CanRead => !_eof;
-
-        public override bool CanSeek => false;
-
-        public override bool CanWrite => false;
-
-        public override long Length => throw new NotImplementedException();
-
-        public override long Position { get => throw new NotImplementedException(); set => throw new NotImplementedException(); }
-
-        public override void Flush()
-        {
-            throw new NotImplementedException();
-        }
-
-        public override int Read(byte[] buffer, int offset, int count)
-        {
-            int result;
-            if (_eof)
-            {
-                result = 0;
-            }
-            else
-            {
-                if (_remaining == 0)
-                {
-                    string lenStr = string.Empty;
-                    while (!lenStr.EndsWith("\r\n"))
-                    {
-                        if (lenStr.Length > 64)
-                        {
-                            throw new Exception("Too long chunk length line");
-                        }
-                        int b = _baseStream.ReadByte();
-                        if (b < 0)
-                        {
-                            throw new Exception("End of stream reached while reading chunk length line");
-                        }
-                        lenStr += (char)b;
-                    }
-                    lenStr = lenStr.Trim();
-                    _remaining = int.Parse(lenStr, NumberStyles.HexNumber);
-                    if (_remaining == 0)
-                    {
-                        _eof = true;
-                    }
-                }
-
-                // Not sure if it is ok to call Read with 0
-                if (_remaining > 0)
-                {
-                    int read = Math.Min(count, _remaining);
-                    result = _baseStream.Read(buffer, offset, read);
-                    _remaining -= result;
-                }
-                else
-                {
-                    result = 0;
-                }
-
-                if (_remaining == 0)
-                {
-                    int cr = _baseStream.ReadByte();
-                    if (cr != '\r')
-                    {
-                        throw new Exception("Expected CR");
-                    }
-                    int lf = _baseStream.ReadByte();
-                    if (lf != '\n')
-                    {
-                        throw new Exception("Expected LF");
-                    }
-                }
-
-                if (_eof)
-                {
-                    // Throw any pending exception asap, not only once the stream is closed.
-                    // Also releases the lock.
-                    Closed();
-                }
-            }
-
-            return result;
-        }
-
-        public override long Seek(long offset, SeekOrigin origin)
-        {
-            throw new NotImplementedException();
-        }
-
-        public override void SetLength(long value)
-        {
-            throw new NotImplementedException();
-        }
-
-        public override void Write(byte[] buffer, int offset, int count)
-        {
-            throw new NotImplementedException();
-        }
-
-        protected override void Dispose(bool disposing)
-        {
-            try
-            {
-                // Have to consume the rest of the buffered download data, otherwise we could not continue with other downloads
-                while (!_eof)
-                {
-                    byte[] buf = new byte[10240];
-                    Read(buf, 0, buf.Length);
-                }
-                base.Dispose(disposing);
-            }
-            finally
-            {
-                Closed();
-            }
-        }
-
-        private void Closed()
-        {
-            Action onDispose = _onDispose;
-            _onDispose = null;
-            onDispose?.Invoke();
-        }
-
-        private Stream _baseStream;
-        private Action _onDispose;
-        private int _remaining;
-        private bool _eof;
-    }
-}

dotnet/internal/ConsoleCommStruct.cs

@@ -13,8 +13,9 @@ namespace WinSCP
         public uint OutputType;
         public bool WantsProgress; // since version 6
         public bool UseStdErr; // since version 10
-        public bool BinaryOutput; // since version 10
-        public bool BinaryInput; // since version 10
+        public enum StdInOut { Off, Binary, Chunked }
+        public StdInOut BinaryOutput; // since version 10
+        public StdInOut BinaryInput; // since version 10
     }
 
     [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)]

dotnet/internal/ExeSessionProcess.cs

@@ -19,7 +19,7 @@ namespace WinSCP
 
         public bool HasExited { get { return _process.HasExited; } }
         public int ExitCode { get { return _process.ExitCode; } }
-        public PipeStream StdOut { get; private set; }
+        public PipeStream StdOut { get; set; }
 
         public static ExeSessionProcess CreateForSession(Session session)
         {
@@ -99,7 +99,7 @@ namespace WinSCP
                     string.Format(CultureInfo.InvariantCulture, "/dotnet={0} ", assemblyVersionStr);
 
                 string arguments =
-                    xmlLogSwitch + "/nointeractiveinput /stdout=chunked " + assemblyVersionSwitch +
+                    xmlLogSwitch + "/nointeractiveinput /stdout " + assemblyVersionSwitch +
                     configSwitch + logSwitch + logLevelSwitch + _session.AdditionalExecutableArguments;
 
                 Tools.AddRawParameters(ref arguments, _session.RawConfiguration, "/rawconfig", false);
@@ -141,7 +141,6 @@ namespace WinSCP
         {
             using (_logger.CreateCallstack())
             {
-                StdOut = new PipeStream();
                 InitializeConsole();
                 InitializeChild();
             }
@@ -468,6 +467,12 @@ namespace WinSCP
         {
             using (_logger.CreateCallstack())
             {
+                if (!e.UseStdErr ||
+                    (e.BinaryOutput != ConsoleInitEventStruct.StdInOut.Binary))
+                {
+                    _logger.WriteException(new InvalidOperationException("Unexpected console interface options"));
+                }
+
                 e.InputType = 3; // pipe
                 e.OutputType = 3; // pipe
                 e.WantsProgress = _session.WantsProgress;
@@ -532,9 +537,21 @@ namespace WinSCP
             {
                 _logger.WriteLine("Len [{0}]", e.Len);
 
-                StdOut.Write(e.Data, 0, (int)e.Len);
-
-                _logger.WriteLine("Data written to the buffer");
+                if (StdOut == null)
+                {
+                    throw _logger.WriteException(new InvalidOperationException("Unexpected data"));
+                }
+                int len = (int)e.Len;
+                if (len > 0)
+                {
+                    StdOut.Write(e.Data, 0, len);
+                    _logger.WriteLine("Data written to the buffer");
+                }
+                else
+                {
+                    StdOut.CloseWrite();
+                    _logger.WriteLine("Data buffer closed");
+                }
             }
         }
 

dotnet/internal/PipeStream.cs

@@ -53,6 +53,14 @@ namespace WinSCP
         /// </summary>
         private bool _isDisposed;
 
+        private bool _closedWrite;
+        private Action _onDispose;
+
+        public PipeStream(Action onDispose)
+        {
+            _onDispose = onDispose;
+        }
+
         #endregion
 
         #region Public properties
@@ -145,7 +153,7 @@ namespace WinSCP
 
             lock (_buffer)
             {
-                while (!_isDisposed && !ReadAvailable(1))
+                while (!_isDisposed && !_closedWrite && !ReadAvailable(1))
                 {
                     Monitor.Wait(_buffer);
                 }
@@ -162,6 +170,13 @@ namespace WinSCP
                     buffer[readLength] = _buffer.Dequeue();
                 }
 
+                if (_closedWrite)
+                {
+                    // Throw any pending exception asap, not only once the stream is closed.
+                    // Also releases the lock.
+                    Closed();
+                }
+
                 Monitor.Pulse(_buffer);
             }
 
@@ -202,6 +217,8 @@ namespace WinSCP
             CheckDisposed();
             if (count == 0)
                 return;
+            if (_closedWrite)
+                throw new InvalidOperationException("Stream closed for writes");
 
             lock (_buffer)
             {
@@ -239,6 +256,7 @@ namespace WinSCP
                     _isDisposed = true;
                     Monitor.Pulse(_buffer);
                 }
+                Closed();
             }
         }
 
@@ -308,6 +326,19 @@ namespace WinSCP
 
         #endregion
 
+        public void CloseWrite()
+        {
+            lock (_buffer)
+            {
+                CheckDisposed();
+                if (!_closedWrite)
+                {
+                    _closedWrite = true;
+                    Monitor.Pulse(_buffer);
+                }
+            }
+        }
+
         private void CheckDisposed()
         {
             if (_isDisposed)
@@ -315,5 +346,12 @@ namespace WinSCP
                 throw new ObjectDisposedException(GetType().FullName);
             }
         }
+
+        private void Closed()
+        {
+            Action onDispose = _onDispose;
+            _onDispose = null;
+            onDispose?.Invoke();
+        }
     }
 }

source/console/Console.h

@@ -22,8 +22,9 @@ struct TConsoleCommStruct
     unsigned int OutputType;
     bool WantsProgress; // since version 6
     bool UseStdErr; // since version 10
-    bool BinaryOutput; // since version 10
-    bool BinaryInput; // since version 10
+    enum STDINOUT { OFF, BINARY, CHUNKED };
+    STDINOUT OutputFormat; // since version 10
+    STDINOUT InputFormat; // since version 10
   };
 
   struct TPrintEvent

source/console/Main.cpp

@@ -16,6 +16,8 @@ HANDLE ConsoleInput = NULL;
 HANDLE ConsoleOutput = NULL;
 HANDLE ConsoleStandardOutput = NULL;
 HANDLE ConsoleErrorOutput = NULL;
+TConsoleCommStruct::TInitEvent::STDINOUT OutputFormat;
+TConsoleCommStruct::TInitEvent::STDINOUT InputFormat;
 HANDLE Child = NULL;
 HANDLE CancelEvent = NULL;
 HANDLE InputTimerEvent = NULL;
@@ -790,12 +792,14 @@ inline void ProcessInitEvent(TConsoleCommStruct::TInitEvent& Event)
     ConsoleOutput = ConsoleErrorOutput;
   }
 
-  if (Event.BinaryOutput)
+  OutputFormat = Event.OutputFormat;
+  if (OutputFormat != TConsoleCommStruct::TInitEvent::OFF)
   {
     setmode(fileno(stdout), O_BINARY);
   }
 
-  if (Event.BinaryInput)
+  InputFormat = Event.InputFormat;
+  if (InputFormat != TConsoleCommStruct::TInitEvent::OFF)
   {
     setmode(fileno(stdin), O_BINARY);
   }
@@ -821,7 +825,16 @@ inline void ProcessInitEvent(TConsoleCommStruct::TInitEvent& Event)
 //---------------------------------------------------------------------------
 inline void ProcessTransferOutEvent(TConsoleCommStruct::TTransferEvent& Event)
 {
-  fwrite(Event.Data, 1, Event.Len, stdout);
+  if (OutputFormat == TConsoleCommStruct::TInitEvent::BINARY)
+  {
+    fwrite(Event.Data, 1, Event.Len, stdout);
+  }
+  else if (OutputFormat == TConsoleCommStruct::TInitEvent::CHUNKED)
+  {
+    fprintf(stdout, "%x\r\n", static_cast<int>(Event.Len));
+    fwrite(Event.Data, 1, Event.Len, stdout);
+    fputs("\r\n", stdout);
+  }
 }
 //---------------------------------------------------------------------------
 inline void ProcessTransferInEvent(TConsoleCommStruct::TTransferEvent& Event)

source/core/Terminal.cpp

@@ -7445,7 +7445,7 @@ void __fastcall TTerminal::SinkRobust(
   }
   __finally
   {
-    // Once we issue <download> we must terminate the chunked stream
+    // Once we issue <download> we must terminate the data stream
     if (Action.IsValid() && (CopyParam->OnTransferOut != NULL) && DebugAlwaysTrue(IsCapable[fcTransferOut]))
     {
       CopyParam->OnTransferOut(this, NULL, 0);

source/windows/ConsoleRunner.cpp

@@ -523,7 +523,7 @@ UnicodeString __fastcall TOwnConsole::FinalLogMessage()
   return UnicodeString();
 }
 //---------------------------------------------------------------------------
-enum TStdInOutMode { siomOff, siomBinary, siomChunked };
+typedef TConsoleCommStruct::TInitEvent::STDINOUT TStdInOutMode;
 //---------------------------------------------------------------------------
 class TExternalConsole : public TConsole
 {
@@ -566,7 +566,6 @@ private:
   inline void __fastcall SendEvent(int Timeout);
   void __fastcall Init();
   void __fastcall CheckHandle(HANDLE Handle, const UnicodeString & Desc);
-  void __fastcall DoTransferOut(const unsigned char * Data, size_t Len);
 };
 //---------------------------------------------------------------------------
 __fastcall TExternalConsole::TExternalConsole(
@@ -828,9 +827,9 @@ void __fastcall TExternalConsole::Init()
   {
     CommStruct->Event = TConsoleCommStruct::INIT;
     CommStruct->InitEvent.WantsProgress = false;
-    CommStruct->InitEvent.UseStdErr = (FStdOut != siomOff);
-    CommStruct->InitEvent.BinaryOutput = (FStdOut != siomOff);
-    CommStruct->InitEvent.BinaryInput = (FStdIn != siomOff);
+    CommStruct->InitEvent.UseStdErr = (FStdOut != TConsoleCommStruct::TInitEvent::OFF);
+    CommStruct->InitEvent.OutputFormat = FStdOut;
+    CommStruct->InitEvent.InputFormat = FStdIn;
   }
   __finally
   {
@@ -880,10 +879,10 @@ bool __fastcall TExternalConsole::HasFlag(TConsoleFlag Flag) const
       return FWantsProgress;
 
     case cfStdOut:
-      return (FStdOut != siomOff);
+      return (FStdOut != TConsoleCommStruct::TInitEvent::OFF);
 
     case cfStdIn:
-      return (FStdIn != siomOff);
+      return (FStdIn != TConsoleCommStruct::TInitEvent::OFF);
 
     default:
       DebugFail();
@@ -962,10 +961,11 @@ void __fastcall TExternalConsole::Progress(TScriptProgress & Progress)
   }
 }
 //---------------------------------------------------------------------------
-void __fastcall TExternalConsole::DoTransferOut(const unsigned char * Data, size_t Len)
+void __fastcall TExternalConsole::TransferOut(const unsigned char * Data, size_t Len)
 {
+  DebugAssert((Data == NULL) == (Len == 0));
   size_t Offset = 0;
-  while (Offset < Len)
+  do
   {
     TConsoleCommStruct * CommStruct = GetCommStruct();
     try
@@ -982,38 +982,7 @@ void __fastcall TExternalConsole::DoTransferOut(const unsigned char * Data, size
     }
     SendEvent(INFINITE);
   }
-}
-//---------------------------------------------------------------------------
-void __fastcall TExternalConsole::TransferOut(const unsigned char * Data, size_t Len)
-{
-  DebugAssert((Data == NULL) == (Len == 0));
-  if (FStdOut == siomBinary)
-  {
-    if (Data != NULL)
-    {
-      DoTransferOut(Data, Len);
-    }
-  }
-  else if (FStdOut == siomChunked)
-  {
-    std::vector<unsigned char> Buf;
-    // 32 is more than enough for Len in hex + twice CRLF
-    Buf.reserve(Len + 32);
-    // static_cast should not lose digits with IntToHex
-    AnsiString S = AnsiString(IntToHex(static_cast<int>(Len), 0) + L"\r\n");
-    Buf.insert(Buf.end(), S.c_str(), S.c_str() + S.Length());
-    if (Data != NULL) // not really needed
-    {
-      Buf.insert(Buf.end(), Data, Data + Len);
-    }
-    S = "\r\n";
-    Buf.insert(Buf.end(), S.c_str(), S.c_str() + S.Length());
-    DoTransferOut(&Buf.front(), Buf.size());
-  }
-  else
-  {
-    DebugFail();
-  }
+  while (Offset < Len);
 }
 //---------------------------------------------------------------------------
 size_t __fastcall TExternalConsole::TransferIn(unsigned char * Data, size_t Len)
@@ -2852,17 +2821,17 @@ TStdInOutMode ParseStdInOutMode(TProgramParams * Params, const UnicodeString & S
   UnicodeString Value;
   if (!Params->FindSwitch(Switch, Value))
   {
-    Result = siomOff;
+    Result = TConsoleCommStruct::TInitEvent::OFF;
   }
   else
   {
     if (Value.IsEmpty() || SameText(Value, STDINOUT_BINARY_VALUE))
     {
-      Result = siomBinary;
+      Result = TConsoleCommStruct::TInitEvent::BINARY;
     }
     else if (SameText(Value, STDINOUT_CHUNKED_VALUE))
     {
-      Result = siomChunked;
+      Result = TConsoleCommStruct::TInitEvent::CHUNKED;
     }
     else
     {
@@ -2890,7 +2859,7 @@ int __fastcall Console(TConsoleMode Mode)
       Configuration->Usage->Inc(L"ConsoleExternal");
       TStdInOutMode StdOut = ParseStdInOutMode(Params, STDOUT_SWITCH);
       TStdInOutMode StdIn = ParseStdInOutMode(Params, STDIN_SWITCH);
-      bool NoInteractiveInput = Params->FindSwitch(NOINTERACTIVEINPUT_SWITCH) || (StdIn != siomOff);
+      bool NoInteractiveInput = Params->FindSwitch(NOINTERACTIVEINPUT_SWITCH) || (StdIn != TConsoleCommStruct::TInitEvent::OFF);
       Console = new TExternalConsole(ConsoleInstance, NoInteractiveInput, StdOut, StdIn);
     }
     else if (Params->FindSwitch(L"Console") || (Mode != cmScripting))