Bugs in `Console` members `KeyAvailable` and `ReadKey()` on Windows platform

[mawosoft]

General

The public static System.Console API mostly redirects to the platform-specific implementations in internal static System.ConsolePal. For Windows, that's in ConsolePal.Windows.cs, so I will refer to the latter directly.

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

Line 284 in 5474ab5

public static bool KeyAvailable

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

Line 323 in 5474ab5

public static ConsoleKeyInfo ReadKey(bool intercept)

Bug 1: Inconsistent logic between KeyAvailable and ReadKey()

The code peeking at the input queue in KeyAvailable uses a different logic than the code reading the input queue in ReadKey(). As a result, when invoking KeyAvailable,

• An unicode char synthesized via a Alt key-up event will be silently eaten and is no longer available to a subsequent ReadKey() call. OTOH, ReadKey() without previous KeyAvailable will read that char correctly.
  This affects input via an Alt plus numpad number sequence, pasted chars without key equivalent, and problably IME-generated chars as well.

• A keydown event in the input queue for any combination of Alt with numpad number keys, arrow keys, page up/down keys, Home/End keys, and the Insert key will return true, signaling that a subsequent call to ReadKey() should return that key and should not block.
  Unfortunately, the opposite happens, because ReadKey() silently eats these keys in anticipation of a subsequent synthesized key (which will only happen for some of these, see below). As a result, a call to ReadKey() will block and not return, despite KeyAvailable having signaled otherwise.

Bug 2: ReadKey() incorrectly skips valid keydown events

This is again related to synthesized unicode chars from an Alt plus numpad number input sequence, which are surfaced via a subsequent Alt key-up event.

The code correctly skips such keydown events for virtual numpad number keys. Each of the corresponding physical keys has a second virtual key that gets sent when NumLock is off, so ReadKey() skips them as well, because synthesizing happens regardless of the NumLock state.

However, on most keyboards, those keys exist twice - once on the numpad, and once on the arrow pad and control pad. Only the keys on the numpad will produce a synthesized unicode char, but ReadKey() lumps them together causing the keys on the control and arrow pad to be silently eaten.

Proposed fix

main...mawosoft:runtime:fix-consolepal-windows

I can submit a PR if that's desirable.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-console
See info in area-owners.md if you want to be subscribed.

[adamsitnik]

Hi @mawosoft !

Thank you for a detailed bug description and foremost for offering help.

I would be happy to review your PR, but we need to take .NET 9 release schedule under consideration.

On 22 of July we are going to perform a snap for Preview 7 and this will be the last preview for .NET 9. After that, we won't accept any risky changes until November (your proposed changes LGTM, but we don't have automated test coverage for this code and I find any non-trivial changes in that area to by risky).

So if you can send the PR this week, please do so. If not, let's do it once our main branch becomes .NET 10.

Thank you!

[adamsitnik]

fixed with #104984