Console.ReadKey should print a char, not an integer (dotnet#66609)

Co-authored-by: Jan Kotas <[email protected]>

src/libraries/Common/src/Interop/Windows/Kernel32/Interop.PeekConsoleInput.cs

@@ -10,6 +10,6 @@ internal static partial class Kernel32
     {
         [LibraryImport(Libraries.Kernel32, EntryPoint = "PeekConsoleInputW",  SetLastError = true, StringMarshalling = StringMarshalling.Utf16)]
         [return: MarshalAs(UnmanagedType.Bool)]
-        internal static partial bool PeekConsoleInput(IntPtr hConsoleInput, out InputRecord buffer, int numInputRecords_UseOne, out int numEventsRead);
+        internal static partial bool PeekConsoleInput(IntPtr hConsoleInput, out INPUT_RECORD buffer, int numInputRecords_UseOne, out int numEventsRead);
     }
 }

src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ReadConsoleInput.cs

@@ -8,24 +8,26 @@ internal static partial class Interop
 {
     internal const short KEY_EVENT = 1;
 
-    // Windows's KEY_EVENT_RECORD
     [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)]
-    internal struct KeyEventRecord
+    internal struct KEY_EVENT_RECORD
     {
-        internal BOOL keyDown;
-        internal short repeatCount;
-        internal short virtualKeyCode;
-        internal short virtualScanCode;
-        internal ushort uChar; // Union between WCHAR and ASCII char
-        internal int controlKeyState;
+        internal BOOL bKeyDown;
+        internal ushort wRepeatCount;
+        internal ushort wVirtualKeyCode;
+        internal ushort wVirtualScanCode;
+        private ushort _uChar; // Union between WCHAR and ASCII char
+        internal uint dwControlKeyState;
+
+        // _uChar is stored as short to avoid any ambiguity for interop marshaling
+        internal char uChar => (char)_uChar;
     }
 
     // Really, this is a union of KeyEventRecords and other types.
     [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)]
-    internal struct InputRecord
+    internal struct INPUT_RECORD
     {
-        internal short eventType;
-        internal KeyEventRecord keyEvent;
+        internal ushort EventType;
+        internal KEY_EVENT_RECORD keyEvent;
         // This struct is a union!  Word alignment should take care of padding!
     }
 
@@ -34,6 +36,6 @@ internal static partial class Kernel32
     {
         [LibraryImport(Libraries.Kernel32, EntryPoint = "ReadConsoleInputW",  SetLastError = true, StringMarshalling = StringMarshalling.Utf16)]
         [return: MarshalAs(UnmanagedType.Bool)]
-        internal static partial bool ReadConsoleInput(IntPtr hConsoleInput, out InputRecord buffer, int numInputRecords_UseOne, out int numEventsRead);
+        internal static partial bool ReadConsoleInput(IntPtr hConsoleInput, out INPUT_RECORD buffer, int numInputRecords_UseOne, out int numEventsRead);
     }
 }

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

@@ -177,24 +177,24 @@ internal static TextReader GetOrCreateReader()
         // we will lose repeated keystrokes when someone switches from
         // calling ReadKey to calling Read or ReadLine.  Those methods should
         // ideally flush this cache as well.
-        private static Interop.InputRecord _cachedInputRecord;
+        private static Interop.INPUT_RECORD _cachedInputRecord;
 
         // Skip non key events. Generally we want to surface only KeyDown event
         // and suppress KeyUp event from the same Key press but there are cases
         // where the assumption of KeyDown-KeyUp pairing for a given key press
         // is invalid. For example in IME Unicode keyboard input, we often see
         // only KeyUp until the key is released.
-        private static bool IsKeyDownEvent(Interop.InputRecord ir)
+        private static bool IsKeyDownEvent(Interop.INPUT_RECORD ir)
         {
-            return (ir.eventType == Interop.KEY_EVENT && ir.keyEvent.keyDown != Interop.BOOL.FALSE);
+            return (ir.EventType == Interop.KEY_EVENT && ir.keyEvent.bKeyDown != Interop.BOOL.FALSE);
         }
 
-        private static bool IsModKey(Interop.InputRecord ir)
+        private static bool IsModKey(Interop.INPUT_RECORD ir)
         {
             // We should also skip over Shift, Control, and Alt, as well as caps lock.
             // Apparently we don't need to check for 0xA0 through 0xA5, which are keys like
             // Left Control & Right Control. See the ConsoleKey enum for these values.
-            short keyCode = ir.keyEvent.virtualKeyCode;
+            ushort keyCode = ir.keyEvent.wVirtualKeyCode;
             return ((keyCode >= 0x10 && keyCode <= 0x12)
                     || keyCode == 0x14 || keyCode == 0x90 || keyCode == 0x91);
         }
@@ -218,9 +218,9 @@ internal enum ControlKeyState
         // desired effect is to translate the sequence into one Unicode KeyPress.
         // We need to keep track of the Alt+NumPad sequence and surface the final
         // unicode char alone when the Alt key is released.
-        private static bool IsAltKeyDown(Interop.InputRecord ir)
+        private static bool IsAltKeyDown(Interop.INPUT_RECORD ir)
         {
-            return (((ControlKeyState)ir.keyEvent.controlKeyState)
+            return (((ControlKeyState)ir.keyEvent.dwControlKeyState)
                               & (ControlKeyState.LeftAltPressed | ControlKeyState.RightAltPressed)) != 0;
         }
 
@@ -269,12 +269,12 @@ public static bool KeyAvailable
         {
             get
             {
-                if (_cachedInputRecord.eventType == Interop.KEY_EVENT)
+                if (_cachedInputRecord.EventType == Interop.KEY_EVENT)
                     return true;
 
                 while (true)
                 {
-                    bool r = Interop.Kernel32.PeekConsoleInput(InputHandle, out Interop.InputRecord ir, 1, out int numEventsRead);
+                    bool r = Interop.Kernel32.PeekConsoleInput(InputHandle, out Interop.INPUT_RECORD ir, 1, out int numEventsRead);
                     if (!r)
                     {
                         int errorCode = Marshal.GetLastPInvokeError();
@@ -306,20 +306,20 @@ public static bool KeyAvailable
 
         public static ConsoleKeyInfo ReadKey(bool intercept)
         {
-            Interop.InputRecord ir;
+            Interop.INPUT_RECORD ir;
             bool r;
 
             lock (s_readKeySyncObject)
             {
-                if (_cachedInputRecord.eventType == Interop.KEY_EVENT)
+                if (_cachedInputRecord.EventType == Interop.KEY_EVENT)
                 {
                     // We had a previous keystroke with repeated characters.
                     ir = _cachedInputRecord;
-                    if (_cachedInputRecord.keyEvent.repeatCount == 0)
-                        _cachedInputRecord.eventType = -1;
+                    if (_cachedInputRecord.keyEvent.wRepeatCount == 0)
+                        _cachedInputRecord.EventType = 0;
                     else
                     {
-                        _cachedInputRecord.keyEvent.repeatCount--;
+                        _cachedInputRecord.keyEvent.wRepeatCount--;
                     }
                     // We will return one key from this method, so we decrement the
                     // repeatCount here, leaving the cachedInputRecord in the "queue".
@@ -339,7 +339,7 @@ public static ConsoleKeyInfo ReadKey(bool intercept)
                             throw new InvalidOperationException(SR.InvalidOperation_ConsoleReadKeyOnFile);
                         }
 
-                        short keyCode = ir.keyEvent.virtualKeyCode;
+                        ushort keyCode = ir.keyEvent.wVirtualKeyCode;
 
                         // First check for non-keyboard events & discard them. Generally we tap into only KeyDown events and ignore the KeyUp events
                         // but it is possible that we are dealing with a Alt+NumPad unicode key sequence, the final unicode char is revealed only when
@@ -353,7 +353,7 @@ public static ConsoleKeyInfo ReadKey(bool intercept)
                                 continue;
                         }
 
-                        char ch = (char)ir.keyEvent.uChar;
+                        char ch = ir.keyEvent.uChar;
 
                         // In a Alt+NumPad unicode sequence, when the alt key is released uChar will represent the final unicode character, we need to
                         // surface this. VirtualKeyCode for this event will be Alt from the Alt-Up key event. This is probably not the right code,
@@ -377,22 +377,22 @@ public static ConsoleKeyInfo ReadKey(bool intercept)
                             continue;
                         }
 
-                        if (ir.keyEvent.repeatCount > 1)
+                        if (ir.keyEvent.wRepeatCount > 1)
                         {
-                            ir.keyEvent.repeatCount--;
+                            ir.keyEvent.wRepeatCount--;
                             _cachedInputRecord = ir;
                         }
                         break;
                     }
                 }  // we did NOT have a previous keystroke with repeated characters.
             }
 
-            ControlKeyState state = (ControlKeyState)ir.keyEvent.controlKeyState;
+            ControlKeyState state = (ControlKeyState)ir.keyEvent.dwControlKeyState;
             bool shift = (state & ControlKeyState.ShiftPressed) != 0;
             bool alt = (state & (ControlKeyState.LeftAltPressed | ControlKeyState.RightAltPressed)) != 0;
             bool control = (state & (ControlKeyState.LeftCtrlPressed | ControlKeyState.RightCtrlPressed)) != 0;
 
-            ConsoleKeyInfo info = new ConsoleKeyInfo((char)ir.keyEvent.uChar, (ConsoleKey)ir.keyEvent.virtualKeyCode, shift, alt, control);
+            ConsoleKeyInfo info = new ConsoleKeyInfo(ir.keyEvent.uChar, (ConsoleKey)ir.keyEvent.wVirtualKeyCode, shift, alt, control);
 
             if (!intercept)
                 Console.Write(ir.keyEvent.uChar);

src/libraries/System.Console/tests/ManualTests/ManualTests.cs

@@ -121,6 +121,17 @@ public static void ReadKey()
             AssertUserExpectedResults("\"console\" correctly not echoed as you typed it");
         }
 
+        [ConditionalFact(nameof(ManualTestsEnabled))]
+        public static void ReadKeyNoIntercept()
+        {
+            Console.WriteLine("Please type \"console\" (without the quotes). You should see it as you type:");
+            foreach (ConsoleKey k in new[] { ConsoleKey.C, ConsoleKey.O, ConsoleKey.N, ConsoleKey.S, ConsoleKey.O, ConsoleKey.L, ConsoleKey.E })
+            {
+                Assert.Equal(k, Console.ReadKey(intercept: false).Key);
+            }
+            AssertUserExpectedResults("\"console\" correctly echoed as you typed it");
+        }
+
         [ConditionalFact(nameof(ManualTestsEnabled))]
         public static void EnterKeyIsEnterAfterKeyAvailableCheck()
         {