Win32 input mode breaks WSL .exe interop

[lhecker]

Windows Terminal version

1.19.2682.0

Windows build number

No response

Steps to reproduce

• Replace system conhost.exe with a recent v1.19 OpenConsole.exe
  (I can send sfpcopy.exe (the recommended tool for this) on request if anyone wants to test this.)
• Launch WSL
• Run an .exe that reads input via ReadConsoleInputW, for instance: https://gist.github.com/lhecker/43e562d5a1370d2582bb5d6eed4e4f3e

Expected Behavior

Input is preserved as it is typed.

Actual Behavior

There are two independent issues at play here:

• ConPTY deadlocks on startup
  ConhostInternalGetSet::ReturnResponse translates a VT response into a sequence of INPUT_RECORDs and calls InputBuffer::Write. That one calls TerminalInput::HandleKey which translates KEY_EVENTs type into Win32 input sequences. In other words, VT responses are being double-encoded in conhost.
  At some point when ConPTY starts, VtIo::StartIfNeeded() is called which calls RequestCursor() under WSL (= "\x1b[6n" request) and waits for a response. ConPTY now deadlocks because it won't double-decode the response.
• Console apps receive still-encoded events as input
  Even with that issue fixed, there's still a change in behavior, as the Win32 console application still receives undecoded Win32 input sequences as a series of INPUT_RECORDs. I.e. typing "a" yields 2 dozen INPUT_RECORDs.

Note to self:

• Retest dwControlKeyState behaviour changed between Windows 10/Windows 11 #16266 after fixing this

[lhecker]

@j4james FYI this regression was caused by #15476, but I haven't really understood yet why it happens or how to fix it. I'll try to figure it out later, but if you immediately know what the issue is (and it's not a problem for you) let me know, and I'll fix it ASAP. 🙂

[zadjii-msft]

@craigloewen-msft as an FYI. We may have broken windows interop for WSL for insiders. We're looking into it.

[zadjii-msft]

See also:

• mouse events are sent incorrectly in win32-input-mode #15083
• Mouse input differences when using ENABLE_VIRTUAL_TERMINAL_INPUT #15296

[lhecker]

Probably also causes #16307

[j4james]

From what @DHowett said in #15476 (comment), my understanding was that the conpty DLL was tightly coupled to the openconsole version. So I'm wondering whether it's possible that you're getting a version mismatch by replacing the system conhost, if you aren't also using the corresponding conpty DLL?

If it's not something like that, I have no idea what the issue could be. But I haven't had a chance to look at the code in detail (and probably won't have much time again until the weekend).

[j4james]

I have an idea what the problem might be, but I don't really know how WSL works behind the scenes so my understanding may be incorrect.

1. I'm assuming when you launch a Windows executable from within WSL it acts like a third-party conpty client, and creates a conpty connection.
2. I'm assuming it would do this without the PSEUDOCONSOLE_WIN32_INPUT_MODE flag, but since that flag was removed in PR 15475, we now act as if it's always set.
3. Without that flag set, conhost would not previously have sent DECSET sequences to enable win32 input mode and focus event mode, but now it does that regardless.
4. It was assumed that change would be harmless, because a client that didn't understand those modes would ignore them, but I suspect WSL is actually passing them up to the parent terminal.
5. This would result in the parent terminal enabling win32 input mode, and from then on any keystrokes would be sent to WSL in a format that it doesn't understand (possibly also affecting VT query responses).

If the above assumptions are correct, we're probably going to need to revert most if not all of PR #15476 in order to fix this, and come up with another solution for #15461.

[lhecker]

The way I see it, the fundamental problem is that the same Mode::Win32 flag is being used for both "input" into and "output" from the InputBuffer and indiscriminately of the purpose of the read.

By default it should only take affect when INPUT_RECORDs are being read for communication between a terminal and ConPTY but since we only have a single InputBuffer::Read() and a single InputBuffer::Write() function it taints even all of the public APIs that don't get those win32-input-sequences. This is especially weird for internal APIs like the aforementioned ConhostInternalGetSet::ReturnResponse which is already is text-based and hence the double-encoding. So, if I don't misunderstand the purpose of the Win32 DECSET sequence I think it's actually fine that it's being passed up through WSL. It just shouldn't be immediately visible to console clients.

Do we support console clients requesting win32-input-sequences? If so, then that doesn't change the above IMO, because a client requesting it shouldn't change the encoding for any other attached client.

Fixing the double-encoding is trivial with just a couple lines changed, but fixing the Win32 sequences being visible to console clients is difficult since it requires us to delay encoding to the time of Read() and make it dependent on the requester. I've started writing a modernized InputBuffer but it's fairly messy given our current architecture and the vast compatibility requirements. 🤦

I suspect we'll end up having to revert most of #15476 after all... 😢

[DHowett]

Do we support console clients requesting win32-input-sequences?

I don't see why we shouldn't! It's not different from enabling mouse mode with DECSET 1003.

If so, then that doesn't change the above IMO, because a client requesting it shouldn't change the encoding for any other attached client.

As above, it's not different from enabling mouse mode with DECSET 1003. Unfortunately, we can't know who's requesting it and on whose behalf it is being requested. Even if we track the client application, how do we know that ssh.exe isn't passing input to two different processes--one of which supports win32 input, and one of which does not?

[lhecker]

Even if we track the client application, how do we know that ssh.exe isn't passing input to two different processes--one of which supports win32 input, and one of which does not?

In my vision, win32-input-sequences would be requested automatically by ConPTY at all times, just like it works right now. It's just that console clients (= in case of SSH on the server side) should not see those in form of 20 INPUT_RECORDs per actual input character if they didn't ask for it. That part is easy to do as far as I can see.

[lhecker]

See... I'm actually an idiot. The reason it ACTUALLY happens is because of this:

terminal/src/terminal/parser/stateMachine.cpp

Lines 2139 to 2142 in 12318d9

	// Fortunately, for VT input, each keystroke comes in as an individual
	// write operation. So, if at the end of processing a string for the
	// InputEngine, we find that we're not in the Ground state, that implies
	// that we've processed some input, but not dispatched it yet. This

Unfortunately, that statement is entirely wrong. The most basic example for this is that our input buffer is a basic

char buffer[256];

and if your input pipe got more characters than that, the comment's assumption will be broken, because the read will be split up into multiple chunks. Second of all, however, WSL doesn't use anonymous pipes and so they can split up inputs into arbitrary chunks down to individual bytes. That's why we see broken win32 input sequences randomly in WSL.

[DHowett]

That part is easy to do as far as I can see.

Yyeehehhhhh.. eh. Eaugh.

When an application requests ENABLE_VIRTUAL_TERMINAL_INPUT under ConPTY, we pass the input received through ConPTY directly through to the application. It allows the terminal and the client application to support input modalities that conhost doesn't know about.

[j4james]

I did a little hack experiment with fixing the ConhostInternalGetSet::ReturnResponse implementation, so it sends the response directly to InputBuffer::_HandleTerminalInputCallback (instead of creating input events, which get converted into VT sequences, which then converted back into input events).

This fixes the problem with VT query responses not working correctly in conhost when win32-input mode is set (I'm almost sure we already had an issue for that, but I couldn't find it). It also fixes the conpty deadlock on startup that you're getting when running an exe from within WSL.

I can't reproduce the second problem though. Running your ReadConsoleInputW test app, I only see two events per key press, i.e. the up and down events. The keypresses are not showing up as a bunch of undecoded win32-input sequences.

That said, you still have a problem when you exit the exe and return to WSL, because the terminal is still in win32-input mode at that point. Since your typical WSL shell is not going to be expecting that, you get a lot of garbage output at the prompt when you type.

I can work around that by adding a win32-input reset after the exe call, e.g. testinput.exe; printf "\e[?9001l", but that's obviously not a practical solution for users. Someone needs to be responsible for restoring that mode, and I'm not sure where that should happen.

[zadjii-msft]

We're pretty sure this was fixed, just not actually marked as closed.