Console.Unix: for echoing, write back to the terminal instead of writing to Console.Out. (#94414)

* Console.Unix: for echoing, write back to the terminal instead of writing to Console.Out.

On Unix .NET implements its own echo to make Console behave similar to Windows.

The echo implementation was writing to standard output, which has the unintended effect
of writing the echo characters into a redirected standard output.

This changes to write the echo to the stdin terminal where they were read from.

Author: tmds

src/libraries/Common/src/Interop/Unix/System.Native/Interop.GetWindowWidth.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Runtime.InteropServices;
+using Microsoft.Win32.SafeHandles;
 
 internal static partial class Interop
 {
@@ -18,7 +19,7 @@ internal struct WinSize
         };
 
         [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetWindowSize", SetLastError = true)]
-        internal static partial int GetWindowSize(out WinSize winSize);
+        internal static partial int GetWindowSize(SafeFileHandle terminalHandle, out WinSize winSize);
 
         [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_SetWindowSize", SetLastError = true)]
         internal static partial int SetWindowSize(in WinSize winSize);

src/libraries/Common/src/Interop/Unix/System.Native/Interop.InitializeTerminalAndSignalHandling.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Runtime.InteropServices;
+using Microsoft.Win32.SafeHandles;
 
 internal static partial class Interop
 {
@@ -12,6 +13,6 @@ internal static partial class Sys
         internal static partial bool InitializeTerminalAndSignalHandling();
 
         [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_SetKeypadXmit", StringMarshalling = StringMarshalling.Utf8)]
-        internal static partial void SetKeypadXmit(string terminfoString);
+        internal static partial void SetKeypadXmit(SafeFileHandle terminalHandle, string terminfoString);
     }
 }

src/libraries/System.Console/src/System/ConsolePal.Unix.ConsoleStream.cs

@@ -47,7 +47,7 @@ public override int Read(Span<byte> buffer) =>
                     ConsolePal.Read(_handle, buffer);
 
             public override void Write(ReadOnlySpan<byte> buffer) =>
-                ConsolePal.Write(_handle, buffer);
+                ConsolePal.WriteFromConsoleStream(_handle, buffer);
 
             public override void Flush()
             {

src/libraries/System.Console/src/System/ConsolePal.Unix.cs

@@ -37,6 +37,7 @@ internal static partial class ConsolePal
         private static int s_windowWidth;   // Cached WindowWidth, -1 when invalid.
         private static int s_windowHeight;  // Cached WindowHeight, invalid when s_windowWidth == -1.
         private static int s_invalidateCachedSettings = 1; // Tracks whether we should invalidate the cached settings.
+        private static SafeFileHandle? s_terminalHandle; // Tracks the handle used for writing to the terminal.
 
         /// <summary>Gets the lazily-initialized terminal information for the terminal.</summary>
         public static TerminalFormatStrings TerminalFormatStringsInstance { get { return s_terminalFormatStringsInstance.Value; } }
@@ -127,13 +128,7 @@ public static ConsoleKeyInfo ReadKey(bool intercept)
                 throw new InvalidOperationException(SR.InvalidOperation_ConsoleReadKeyOnFile);
             }
 
-            bool previouslyProcessed;
-            ConsoleKeyInfo keyInfo = StdInReader.ReadKey(out previouslyProcessed);
-
-            if (!intercept && !previouslyProcessed && keyInfo.KeyChar != '\0')
-            {
-                Console.Write(keyInfo.KeyChar);
-            }
+            ConsoleKeyInfo keyInfo = StdInReader.ReadKey(intercept);
             return keyInfo;
         }
 
@@ -205,7 +200,7 @@ public static string Title
                 if (!string.IsNullOrEmpty(titleFormat))
                 {
                     string ansiStr = TermInfo.ParameterizedStrings.Evaluate(titleFormat, value);
-                    WriteStdoutAnsiString(ansiStr, mayChangeCursorPosition: false);
+                    WriteTerminalAnsiString(ansiStr, mayChangeCursorPosition: false);
                 }
             }
         }
@@ -214,15 +209,15 @@ public static void Beep()
         {
             if (!Console.IsOutputRedirected)
             {
-                WriteStdoutAnsiString(TerminalFormatStringsInstance.Bell, mayChangeCursorPosition: false);
+                WriteTerminalAnsiString(TerminalFormatStringsInstance.Bell, mayChangeCursorPosition: false);
             }
         }
 
         public static void Clear()
         {
             if (!Console.IsOutputRedirected)
             {
-                WriteStdoutAnsiString(TerminalFormatStringsInstance.Clear);
+                WriteTerminalAnsiString(TerminalFormatStringsInstance.Clear);
             }
         }
 
@@ -231,6 +226,11 @@ public static void SetCursorPosition(int left, int top)
             if (Console.IsOutputRedirected)
                 return;
 
+            SetTerminalCursorPosition(left, top);
+        }
+
+        public static void SetTerminalCursorPosition(int left, int top)
+        {
             lock (Console.Out)
             {
                 if (TryGetCachedCursorPosition(out int leftCurrent, out int topCurrent) &&
@@ -244,7 +244,7 @@ public static void SetCursorPosition(int left, int top)
                 if (!string.IsNullOrEmpty(cursorAddressFormat))
                 {
                     string ansiStr = TermInfo.ParameterizedStrings.Evaluate(cursorAddressFormat, top, left);
-                    WriteStdoutAnsiString(ansiStr);
+                    WriteTerminalAnsiString(ansiStr);
                 }
 
                 SetCachedCursorPosition(left, top);
@@ -355,19 +355,18 @@ private static void GetWindowSize(out int width, out int height)
                 // Invalidate before reading cached values.
                 CheckTerminalSettingsInvalidated();
 
-                if (s_windowWidth == -1)
+                Interop.Sys.WinSize winsize;
+                if (s_windowWidth == -1 &&
+                    s_terminalHandle != null &&
+                    Interop.Sys.GetWindowSize(s_terminalHandle, out winsize) == 0)
                 {
-                    Interop.Sys.WinSize winsize;
-                    if (Interop.Sys.GetWindowSize(out winsize) == 0)
-                    {
-                        s_windowWidth = winsize.Col;
-                        s_windowHeight = winsize.Row;
-                    }
-                    else
-                    {
-                        s_windowWidth = TerminalFormatStringsInstance.Columns;
-                        s_windowHeight = TerminalFormatStringsInstance.Lines;
-                    }
+                    s_windowWidth = winsize.Col;
+                    s_windowHeight = winsize.Row;
+                }
+                else
+                {
+                    s_windowWidth = TerminalFormatStringsInstance.Columns;
+                    s_windowHeight = TerminalFormatStringsInstance.Lines;
                 }
                 width = s_windowWidth;
                 height = s_windowHeight;
@@ -403,7 +402,7 @@ public static bool CursorVisible
             {
                 if (!Console.IsOutputRedirected)
                 {
-                    WriteStdoutAnsiString(value ?
+                    WriteTerminalAnsiString(value ?
                         TerminalFormatStringsInstance.CursorVisible :
                         TerminalFormatStringsInstance.CursorInvisible);
                 }
@@ -412,6 +411,11 @@ public static bool CursorVisible
 
         public static (int Left, int Top) GetCursorPosition()
         {
+            if (Console.IsInputRedirected || Console.IsOutputRedirected)
+            {
+                return (0, 0);
+            }
+
             TryGetCursorPosition(out int left, out int top);
             return (left, top);
         }
@@ -436,14 +440,9 @@ public static (int Left, int Top) GetCursorPosition()
         /// <param name="reinitializeForRead">Indicates whether this method is called as part of a on-going Read operation.</param>
         internal static bool TryGetCursorPosition(out int left, out int top, bool reinitializeForRead = false)
         {
-            left = top = 0;
+            Debug.Assert(!Console.IsInputRedirected);
 
-            // Getting the cursor position involves both writing out a request string and
-            // parsing a response string from the terminal.  So if anything is redirected, bail.
-            if (Console.IsInputRedirected || Console.IsOutputRedirected)
-            {
-                return false;
-            }
+            left = top = 0;
 
             int cursorVersion;
             lock (Console.Out)
@@ -485,7 +484,7 @@ internal static bool TryGetCursorPosition(out int left, out int top, bool reinit
                 {
                     // Write out the cursor position report request.
                     Debug.Assert(!string.IsNullOrEmpty(TerminalFormatStrings.CursorPositionReport));
-                    WriteStdoutAnsiString(TerminalFormatStrings.CursorPositionReport, mayChangeCursorPosition: false);
+                    WriteTerminalAnsiString(TerminalFormatStrings.CursorPositionReport, mayChangeCursorPosition: false);
 
                     // Read the cursor position report (CPR), of the form \ESC[row;colR. This is not
                     // as easy as it sounds.  Prior to the CPR having been supplied to stdin, other
@@ -802,7 +801,7 @@ private static void WriteSetColorString(bool foreground, ConsoleColor color)
             string evaluatedString = s_fgbgAndColorStrings[fgbgIndex, ccValue]; // benign race
             if (evaluatedString != null)
             {
-                WriteStdoutAnsiString(evaluatedString);
+                WriteTerminalAnsiString(evaluatedString);
                 return;
             }
 
@@ -842,7 +841,7 @@ private static void WriteSetColorString(bool foreground, ConsoleColor color)
                     int ansiCode = consoleColorToAnsiCode[ccValue] % maxColors;
                     evaluatedString = TermInfo.ParameterizedStrings.Evaluate(formatString, ansiCode);
 
-                    WriteStdoutAnsiString(evaluatedString);
+                    WriteTerminalAnsiString(evaluatedString);
 
                     s_fgbgAndColorStrings[fgbgIndex, ccValue] = evaluatedString; // benign race
                 }
@@ -854,7 +853,7 @@ private static void WriteResetColorString()
         {
             if (ConsoleUtils.EmitAnsiColorCodes)
             {
-                WriteStdoutAnsiString(TerminalFormatStringsInstance.Reset);
+                WriteTerminalAnsiString(TerminalFormatStringsInstance.Reset);
             }
         }
 
@@ -897,16 +896,17 @@ private static unsafe void EnsureInitializedCore()
                         throw new Win32Exception();
                     }
 
+                    s_terminalHandle = !Console.IsOutputRedirected ? Interop.Sys.FileDescriptors.STDOUT_FILENO :
+                                       !Console.IsInputRedirected  ? Interop.Sys.FileDescriptors.STDIN_FILENO :
+                                       null;
+
                     // Provide the native lib with the correct code from the terminfo to transition us into
                     // "application mode".  This will both transition it immediately, as well as allow
                     // the native lib later to handle signals that require re-entering the mode.
-                    if (!Console.IsOutputRedirected)
+                    if (s_terminalHandle != null &&
+                        TerminalFormatStringsInstance.KeypadXmit is string keypadXmit)
                     {
-                        string? keypadXmit = TerminalFormatStringsInstance.KeypadXmit;
-                        if (keypadXmit != null)
-                        {
-                            Interop.Sys.SetKeypadXmit(keypadXmit);
-                        }
+                        Interop.Sys.SetKeypadXmit(s_terminalHandle, keypadXmit);
                     }
 
                     if (!Console.IsInputRedirected)
@@ -952,17 +952,32 @@ private static unsafe int Read(SafeFileHandle fd, Span<byte> buffer)
             }
         }
 
+        internal static void WriteToTerminal(ReadOnlySpan<byte> buffer, bool mayChangeCursorPosition = true)
+        {
+            Debug.Assert(s_terminalHandle is not null);
+
+            lock (Console.Out) // synchronize with other writers
+            {
+                Write(s_terminalHandle, buffer, mayChangeCursorPosition);
+            }
+        }
+
+        internal static unsafe void WriteFromConsoleStream(SafeFileHandle fd, ReadOnlySpan<byte> buffer)
+        {
+            EnsureConsoleInitialized();
+
+            lock (Console.Out) // synchronize with other writers
+            {
+                Write(fd, buffer);
+            }
+        }
+
         /// <summary>Writes data from the buffer into the file descriptor.</summary>
         /// <param name="fd">The file descriptor.</param>
         /// <param name="buffer">The buffer from which to write data.</param>
         /// <param name="mayChangeCursorPosition">Writing this buffer may change the cursor position.</param>
-        internal static unsafe void Write(SafeFileHandle fd, ReadOnlySpan<byte> buffer, bool mayChangeCursorPosition = true)
+        private static unsafe void Write(SafeFileHandle fd, ReadOnlySpan<byte> buffer, bool mayChangeCursorPosition = true)
         {
-            // Console initialization might emit data to stdout.
-            // In order to avoid splitting user data we need to
-            // complete it before any writes are performed.
-            EnsureConsoleInitialized();
-
             fixed (byte* p = buffer)
             {
                 byte* bufPtr = p;
@@ -1098,7 +1113,7 @@ private static void InvalidateTerminalSettings()
         /// <summary>Writes a terminfo-based ANSI escape string to stdout.</summary>
         /// <param name="value">The string to write.</param>
         /// <param name="mayChangeCursorPosition">Writing this value may change the cursor position.</param>
-        internal static void WriteStdoutAnsiString(string? value, bool mayChangeCursorPosition = true)
+        internal static void WriteTerminalAnsiString(string? value, bool mayChangeCursorPosition = true)
         {
             if (string.IsNullOrEmpty(value))
                 return;
@@ -1115,10 +1130,8 @@ internal static void WriteStdoutAnsiString(string? value, bool mayChangeCursorPo
                 data = Encoding.UTF8.GetBytes(value);
             }
 
-            lock (Console.Out) // synchronize with other writers
-            {
-                Write(Interop.Sys.FileDescriptors.STDOUT_FILENO, data, mayChangeCursorPosition);
-            }
+            EnsureConsoleInitialized();
+            WriteToTerminal(data, mayChangeCursorPosition);
         }
     }
 }

src/libraries/System.Console/src/System/ConsolePal.Wasi.cs

@@ -18,24 +18,20 @@ namespace System
     //       to also change the test class.
     internal static partial class ConsolePal
     {
-        // StdInReader is only used when input isn't redirected and we're working
-        // with an interactive terminal.  In that case, performance isn't critical
-        // and we can use a smaller buffer to minimize working set.
-        // there is no dup on WASI
         public static Stream OpenStandardInput()
         {
-            return new UnixConsoleStream(Interop.CheckIo(Interop.Sys.FileDescriptors.STDIN_FILENO), FileAccess.Read,
+            return new UnixConsoleStream(Interop.Sys.FileDescriptors.STDIN_FILENO, FileAccess.Read,
                                          useReadLine: !Console.IsInputRedirected);
         }
 
         public static Stream OpenStandardOutput()
         {
-            return new UnixConsoleStream(Interop.CheckIo(Interop.Sys.FileDescriptors.STDOUT_FILENO), FileAccess.Write);
+            return new UnixConsoleStream(Interop.Sys.FileDescriptors.STDOUT_FILENO, FileAccess.Write);
         }
 
         public static Stream OpenStandardError()
         {
-            return new UnixConsoleStream(Interop.CheckIo(Interop.Sys.FileDescriptors.STDERR_FILENO), FileAccess.Write);
+            return new UnixConsoleStream(Interop.Sys.FileDescriptors.STDERR_FILENO, FileAccess.Write);
         }
 
         public static Encoding InputEncoding
@@ -254,17 +250,21 @@ private static unsafe int Read(SafeFileHandle fd, Span<byte> buffer)
             }
         }
 
+        internal static unsafe void WriteFromConsoleStream(SafeFileHandle fd, ReadOnlySpan<byte> buffer)
+        {
+            EnsureConsoleInitialized();
+
+            lock (Console.Out) // synchronize with other writers
+            {
+                Write(fd, buffer);
+            }
+        }
+
         /// <summary>Writes data from the buffer into the file descriptor.</summary>
         /// <param name="fd">The file descriptor.</param>
         /// <param name="buffer">The buffer from which to write data.</param>
-        /// <param name="mayChangeCursorPosition">Writing this buffer may change the cursor position.</param>
-        internal static unsafe void Write(SafeFileHandle fd, ReadOnlySpan<byte> buffer, bool mayChangeCursorPosition = true)
+        private static unsafe void Write(SafeFileHandle fd, ReadOnlySpan<byte> buffer)
         {
-            // Console initialization might emit data to stdout.
-            // In order to avoid splitting user data we need to
-            // complete it before any writes are performed.
-            EnsureConsoleInitialized();
-
             fixed (byte* p = buffer)
             {
                 byte* bufPtr = p;
@@ -298,36 +298,11 @@ internal static unsafe void Write(SafeFileHandle fd, ReadOnlySpan<byte> buffer,
                             throw Interop.GetExceptionForIoErrno(errorInfo);
                         }
                     }
+
                     count -= bytesWritten;
                     bufPtr += bytesWritten;
                 }
             }
         }
-
-        /// <summary>Writes a terminfo-based ANSI escape string to stdout.</summary>
-        /// <param name="value">The string to write.</param>
-        /// <param name="mayChangeCursorPosition">Writing this value may change the cursor position.</param>
-        internal static void WriteStdoutAnsiString(string? value, bool mayChangeCursorPosition = true)
-        {
-            if (string.IsNullOrEmpty(value))
-                return;
-
-            scoped Span<byte> data;
-            if (value.Length <= 256) // except for extremely rare cases, ANSI escape strings are very short
-            {
-                data = stackalloc byte[Encoding.UTF8.GetMaxByteCount(value.Length)];
-                int bytesToWrite = Encoding.UTF8.GetBytes(value, data);
-                data = data.Slice(0, bytesToWrite);
-            }
-            else
-            {
-                data = Encoding.UTF8.GetBytes(value);
-            }
-
-            lock (Console.Out) // synchronize with other writers
-            {
-                Write(Interop.Sys.FileDescriptors.STDOUT_FILENO, data, mayChangeCursorPosition);
-            }
-        }
     }
 }