Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unknown input AND output regression between v1/v2 conhost #16410

Open
htcfreek opened this issue Dec 2, 2023 · 27 comments
Open

Unknown input AND output regression between v1/v2 conhost #16410

htcfreek opened this issue Dec 2, 2023 · 27 comments
Labels
Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Attention The core contributors need to come back around and look at this ASAP. Needs-Tag-Fix Doesn't match tag requirements Product-Conhost For issues in the Console codebase

Comments

@htcfreek
Copy link

htcfreek commented Dec 2, 2023

Description of the new feature/enhancement

Please add a button or menu entry that people can open the old CMD window settings. Sometimes there are situations where you need to change them. For example to enable legacy conhost behavior.

Proposed technical implementation details (optional)

@htcfreek htcfreek added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label Dec 2, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added Needs-Tag-Fix Doesn't match tag requirements Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Dec 2, 2023
@zadjii-msft
Copy link
Member

So, here's an interesting question - why would you like access to the v1 console host/? We've got a mind to deprecate that entirely.

I'd honestly think it weird to have a quick shortcut to the conhost propsheet inside the Terminal itself. They're two different apps, they should have separate settings IMO.

@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Dec 6, 2023
@htcfreek
Copy link
Author

htcfreek commented Dec 6, 2023

So, here's an interesting question - why would you like access to the v1 console host/? We've got a mind to deprecate that entirely.

I'd honestly think it weird to have a quick shortcut to the conhost propsheet inside the Terminal itself. They're two different apps, they should have separate settings IMO.

In case I have an old app that requires the "Use legacy console" setting to be enabled for cmd. (Or make the setting no sense when using WT?)

@microsoft-github-policy-service microsoft-github-policy-service bot added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Dec 6, 2023
@DHowett
Copy link
Member

DHowett commented Dec 6, 2023

Okay, given that we are actively working on removing the legacy console, can you please let us know what application that is and how it breaks with the built-in non-legacy Windows Console Host?

@DHowett DHowett added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Attention The core contributors need to come back around and look at this ASAP. labels Dec 6, 2023
@htcfreek
Copy link
Author

htcfreek commented Dec 6, 2023

Okay, given that we are actively working on removing the legacy console, can you please let us know what application that is and how it breaks with the built-in non-legacy Windows Console Host?

@DHowett
I will ask my colleague if we can share some information with you tomorrow.

Would it be possible to send you a PM/Mail with further information if required?

@microsoft-github-policy-service microsoft-github-policy-service bot added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Dec 6, 2023
@DHowett
Copy link
Member

DHowett commented Dec 7, 2023

@htcfreek Yes please! My e-mail is on my GitHub profile.

@DHowett DHowett added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Attention The core contributors need to come back around and look at this ASAP. labels Dec 7, 2023
@htcfreek
Copy link
Author

htcfreek commented Dec 7, 2023

Okay, given that we are actively working on removing the legacy console, can you please let us know what application that is and how it breaks with the built-in non-legacy Windows Console Host?

@DHowett
Unfortunately I don't have much information. But I will share the one I have with you. Maybe it helps. (And by the way the problematic application will replaced in summer 2024.)

What I know is that it is an old self made IBM38 application which was ported using CICS runtime to run on Windows/DOS. My colleague told me that when our users start the app without "Legacy console mode" then they can not jump using the tab key between the fields and the mouse isn't working too. (We don't have WT installed for our users. So they definitely use the conhost)

Additional test I made:
I tried to use Windows Terminal but the rendering isn't working there. Even with having Atlas Engine enabled.

In WT:
image

In Conhost:
image

@microsoft-github-policy-service microsoft-github-policy-service bot added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Dec 7, 2023
@j4james
Copy link
Collaborator

j4james commented Dec 7, 2023

This looks like the app might be using NUL characters in place of spaces. If so, that could be a case of the app needing to be run with VT mode disabled (#4954/#6973), or possibly NUL characters are being written directly to the buffer, which won't be passed on by conpty (#4363/#6265).

However, neither case explains why the legacy console option is needed. What does the app look like in Conhost without the "Use legacy console" checked. Is it the same appearance as Windows Terminal? If so, is it possible that your system has the HKEY_CURRENT_USER\Console\VirtualTerminalLevel registry entry set to 1?

@htcfreek
Copy link
Author

htcfreek commented Dec 7, 2023

@j4james
There are two different problems: The output problem in WT and the input problem in Conhost (and maybe WT).

  1. In conhost with and without "legacy console" enabled it looks correct. Only in WT the output/layout is broken.
  2. We need the "legacy console" mode only to work around the input (tab key and mouse) problems described in my last comment.
  3. I only want to tried the app in WT to check if we have the input problems there too. But I failed because of the output/layout problem.

@carlos-zamora carlos-zamora added Needs-Discussion Something that requires a team discussion before we can proceed and removed Needs-Attention The core contributors need to come back around and look at this ASAP. labels Dec 13, 2023
@carlos-zamora carlos-zamora removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Dec 13, 2023
@zadjii-msft
Copy link
Member

Is there any chance you could link us to this application to debug it/? I'd rather us figure out why this isn't working (even in conhost v2), rather than offer a bail-out to something we're actively trying to deprecate 😄

@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jan 22, 2024
@zadjii-msft zadjii-msft removed the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label Jan 22, 2024
@zadjii-msft zadjii-msft added Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Conhost For issues in the Console codebase and removed Needs-Discussion Something that requires a team discussion before we can proceed labels Jan 22, 2024
@zadjii-msft zadjii-msft changed the title Add button/menu to open old CMD settings Unknown input AND output regression between v1/v2 conhost Jan 22, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. label Jan 26, 2024
@htcfreek
Copy link
Author

Is there any chance you could link us to this application to debug it?

Unfortunately not. It is a self development of our company. And the only person who knows about the code, can make changes and can provide more details is out of office since a long time. And we can't send you a copy of the application.

@microsoft-github-policy-service microsoft-github-policy-service bot added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. labels Jan 27, 2024
@riverar
Copy link

riverar commented Feb 17, 2024

Some context: The runtime is providing IBM 3270 series terminal emulation (screen, keyboard, data channels, etc.) for Customer Information Control System (CICS) purposes. I wonder if use of console feature ENABLE_WRAP_AT_EOL_OUTPUT might be one place to start. (Forewarning, this might be a deep rabbit hole!)

Here's example CICS Runtime 6.0.27 behavior across several configurations:

Windows Console Host

windows_console_host.mp4

Legacy Windows Console Host

windows_console_host_legacy.mp4

Windows Terminal:

windows_terminal.mp4

@riverar
Copy link

riverar commented Feb 20, 2024

@zadjii-msft Is there anything else I can provide to help with your conhost deprecation efforts? (I presume you need to know which console APIs the runtime is using here, etc.)

As it currently stands, this entire class of apps will fail to function if conhost v1 goes away. There's no indication the vendor is working on an update.

@DHowett
Copy link
Member

DHowett commented Feb 20, 2024

conhost deprecation efforts

FWIW, we are not deprecating all of conhost. We're deprecating only the v1/legacy console (which was forked after Windows 8.1 and frozen in time.)

At this point, a live repro would be the best... but I understand that probably CICS Runtime requires a bunch of setup that isn't available to us. Hmm.

@j4james
Copy link
Collaborator

j4james commented Feb 20, 2024

@DHowett Didn't you have some kind of debug feature that could be used to log console API calls? That might be helpful here, because I'm curious to see what input APIs they are using that might be the cause of the tab key and mouse operations failing.

As far as the output corruption is concerned, I'm pretty certain that's just one of the NUL character issues I mentioned above. I can reproduce the exact same failed rendering of the CICS banner by writing NUL characters to most of the unused areas of the display.

@riverar
Copy link

riverar commented Feb 20, 2024

I can get that information for you outside any logging facility that may exist. Sounds like that coupled with some test cases will be needed to move forward here so I can work on that.

@o-sdn-o
Copy link

o-sdn-o commented Feb 20, 2024

vtm can log Win32 Console API calls (with Log switch enabled). Perhaps it will be useful in this case.

image

@lhecker
Copy link
Member

lhecker commented Feb 21, 2024

What we currently support are TraceLogging events of the Microsoft.Windows.Console.Host provider, aka fe1ff234-1f09-50a8-d38d-c44fab43e818. It contains an output similar to vtm:
image

We have instructions to record a trace here: https://github.com/microsoft/terminal/wiki/Troubleshooting-Tips#capturing-a-debug-etl-trace
@riverar If it's not too much to ask, would it be possible to try and record a trace with our .wprp profile? If you need any help, let me know, but given mach2 I suspect that this will be unlikely. 😅


@j4james You're thinking of https://github.com/microsoft/terminal/tree/dev/duhowett/hax/conhost_dump_replay
And yeah that would allow us to capture a complete, reproducible dump of the console calls. But it's currently not in any prebuilt binary. We could consider making one to debug this though.

@DHowett
Copy link
Member

DHowett commented Feb 21, 2024

Unfortunately, conhost_dump_replay doesn't capture generated input nor does it work for the legacy console; however, I can imagine introducing a version of it that acts as an interposer between the client and the server rather than as its own server. Hmm.

@DHowett
Copy link
Member

DHowett commented Feb 21, 2024

Oh, it might be best to use the DefTerm profile if you do go the WPR route. It's not documented on that page, but if you swap !Terminal.Verbose for !DefTerm.Verbose it will enable more tracing providers.

@j4james
Copy link
Collaborator

j4james commented Feb 21, 2024

Unfortunately, conhost_dump_replay doesn't capture generated input nor does it work for the legacy console;

I don't think that's necessarily a problem. Even running in the new conhost, we should be able to get a general idea of what the app is doing: what input modes are set; what read APIs are called. And from that we could potentially produce our own test app that does the same sort of thing, and see how it performs in v1 conhost compared to v2 conhost.

@DHowett
Copy link
Member

DHowett commented Feb 21, 2024

@j4james Oh! You're totally right!

Alright, very few folks have actually run through this scenario end-to-end, so... it might break.

OpenConsole-Dump-Replay-20240221.tar.gz

  • Run OpenConsole cmd
  • Do your repro scenario
  • exit or otherwise close the new console
  • Notice the conhost_log.bin that has been written in the CWD
  • (Optionally,) run OpenConsole conhost_log.bin to watch it play back
  • Send us conhost_log.bin in whatever way you feel safest.

@riverar
Copy link

riverar commented Feb 22, 2024

Per request:
conhost_log.zip

@riverar
Copy link

riverar commented Feb 22, 2024

And another with more complex menu navigation and even an unhandled exception (not console's fault) during a print operation.
conhost_log.zip

Note this OpenConsole doesn't reproduce the formatting issues above. Suspect that's expected and we're just using this OpenConsole build for tracing. But do correct me if I'm wrong here.

@j4james
Copy link
Collaborator

j4james commented Mar 3, 2024

@riverar Thanks for the logs. I had a chance to look at them this weekend, and I think I've figured out at least part of the what is going wrong.

  1. For the output, the app appears to be using the WriteConsoleOutputCharacter API with strings containing NUL characters, and that's the reason why the layout breaks on Windows Terminal. This is a known issue that we need to address.
  2. For the mouse input, it looks like the input mode is switching between 1E7 and 1F7, which means it's toggling the ENABLE_MOUSE_INPUT mode, but that in itself is not enough to make mouse events work - you also need to disable ENABLE_QUICK_EDIT_MODE. I'm guessing that maybe the v1 console has that disabled by default, and the v2 console has it enabled, but you should still be able to disable it manually from the console settings menu if you can't change the code (you'll need to restart the console though).
  3. For the keyboard input, it appears to be using the PeekConsoleInput and ReadConsoleInput APIs to read key events, but I can't see any reason why those would behave differently in the v2 console compared to the v1 console. I could definitely read a Tab key event easily enough in my tests, so I'm not sure what's going wrong there.

DHowett pushed a commit that referenced this issue Mar 6, 2024
When using the legacy console APIs, it's possible to write arbitrary
codepoints into the buffer. If any of those codepoints are in the C0 or
C1 range, and the buffer contents are forwarded over conpty, they can
end up mistakenly interpreted as controls by the connected terminal.

This PR fixes that issue by converting any C0 and C1 codepoints in the
buffer into printable glyphs before forwarding them over conpty. I've
used the C0 glyphs from the DOS 437 codepage and just a `?` for the C1
codepoints, since that's what you would typically have seen in the v1
console with a raster font.

Although this doesn't address the main problem in #16410, it should at
least fix the rendering issues they're seeing when running their app in
Windows Terminal.

I've confirmed that the test case in #4363 now looks the same in Windows
Terminal as it does in conhost, and I've tested the Windows version of
the terminal game [Gorched], and confirmed that it now works correctly
in Window Terminal.

[Gorched]: https://github.com/zladovan/gorched

Closes #4363
Closes #6265
DHowett pushed a commit that referenced this issue Mar 8, 2024
When using the legacy console APIs, it's possible to write arbitrary
codepoints into the buffer. If any of those codepoints are in the C0 or
C1 range, and the buffer contents are forwarded over conpty, they can
end up mistakenly interpreted as controls by the connected terminal.

This PR fixes that issue by converting any C0 and C1 codepoints in the
buffer into printable glyphs before forwarding them over conpty. I've
used the C0 glyphs from the DOS 437 codepage and just a `?` for the C1
codepoints, since that's what you would typically have seen in the v1
console with a raster font.

Although this doesn't address the main problem in #16410, it should at
least fix the rendering issues they're seeing when running their app in
Windows Terminal.

I've confirmed that the test case in #4363 now looks the same in Windows
Terminal as it does in conhost, and I've tested the Windows version of
the terminal game [Gorched], and confirmed that it now works correctly
in Window Terminal.

[Gorched]: https://github.com/zladovan/gorched

Closes #4363
Closes #6265

(cherry picked from commit 563b731)
Service-Card-Id: 92001662
Service-Version: 1.20
DHowett pushed a commit that referenced this issue Mar 8, 2024
When using the legacy console APIs, it's possible to write arbitrary
codepoints into the buffer. If any of those codepoints are in the C0 or
C1 range, and the buffer contents are forwarded over conpty, they can
end up mistakenly interpreted as controls by the connected terminal.

This PR fixes that issue by converting any C0 and C1 codepoints in the
buffer into printable glyphs before forwarding them over conpty. I've
used the C0 glyphs from the DOS 437 codepage and just a `?` for the C1
codepoints, since that's what you would typically have seen in the v1
console with a raster font.

Although this doesn't address the main problem in #16410, it should at
least fix the rendering issues they're seeing when running their app in
Windows Terminal.

I've confirmed that the test case in #4363 now looks the same in Windows
Terminal as it does in conhost, and I've tested the Windows version of
the terminal game [Gorched], and confirmed that it now works correctly
in Window Terminal.

[Gorched]: https://github.com/zladovan/gorched

Closes #4363
Closes #6265

(cherry picked from commit 563b731)
Service-Card-Id: 92001661
Service-Version: 1.19
@determin1st
Copy link

determin1st commented Apr 19, 2024

  1. For the keyboard input, it appears to be using the PeekConsoleInput and ReadConsoleInput APIs to read key events, but I can't see any reason why those would behave differently in the v2 console compared to the v1 console. I could definitely read a Tab key event easily enough in my tests, so I'm not sure what's going wrong there.

the "new" one doesnt supply virtual key code on key pressed down, only on key pressed up.

the "new" conhost im reffering has 10.0.18362.1 file version, living in my virtualbox world (never would have i rise above 7th win)

so the problem here is a simple lack of testing. testing tool that would fit is the showkey-like program that prints received input.

here, i supply some screens
bJZXA3H4_110623115910lola

{} means character only, [] has virtual key code

bJZXA3H4_110623115910lola - копия (2)

@lhecker
Copy link
Member

lhecker commented Apr 19, 2024

I've been using this to test ReadConsoleInput: https://gist.github.com/lhecker/43e562d5a1370d2582bb5d6eed4e4f3e

If I run that in a build 18362 conhost, I never get INPUT_RECORDs without virtual key code on key down. I tried it with the A API instead of W and it didn't happen either, nor when I tried the 0x1E7 mode mentioned above. I'm sure I must be doing something wrong, but it's unclear to me what that should be.

Is there any specific keys that you press that reproduce the issue? Otherwise I'd also of course be happy to test some PHP code if you have anything that would reproduce such an issue. 🙂

@determin1st
Copy link

mm, ooke, that's the ReadConsoleInputW version i use.. it's the ENABLE_VIRTUAL_TERMINAL_INPUT flag that goes to SetConsoleMode and brings that behavior, not sure why i enabled it for (probably because it says "conjunction" word in the docs). its not really php, php calls windows api through FFI extension, cant supply it not operating properly for now. so maybe not the case, as the old app wont touch it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Attention The core contributors need to come back around and look at this ASAP. Needs-Tag-Fix Doesn't match tag requirements Product-Conhost For issues in the Console codebase
Projects
None yet
Development

No branches or pull requests

9 participants