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

[pull] main from microsoft:main #223

Merged
merged 2 commits into from
Oct 10, 2024
Merged

[pull] main from microsoft:main #223

merged 2 commits into from
Oct 10, 2024

Conversation

pull[bot]
Copy link

@pull pull bot commented Oct 8, 2024

See Commits and Changes for more details.


Created by pull[bot]

Can you help keep this open source service alive? 💖 Please sponsor : )

j4james and others added 2 commits October 7, 2024 13:11
This PR adds support for the `S8C1T` and `S7C1T` commands, which enable
an application to choose whether the terminal should use C1 controls
when sending key sequences and query responses.

This also updates the `DOCS` command to set both the input and output
code pages. So when switched to ISO2022 mode, the C1 controls will be
transmitted as 8-bit, which is what legacy systems would be expecting.

## Detailed Description of the Pull Request / Additional comments

While adding the input code page support, I also reworked the way we
handle the code page reset in `RIS`. In the original implementation we
saved the active code page when the `DOCS` sequence was first used, and
that would become the default value for a reset.

With this PR I'm now saving the code pages whenever `SetConsoleCP` or
`SetConsoleOutputCP` is called, so those APIs now control what the
default values will be. This feels more consistent than the previous
approach. And this is how WSL sets its initial code page to UTF-8.

## Validation Steps Performed

I've added a couple of unit tests that check one of each applicable C1
control in the key sequences and query reports.

I also built myself a code page aware telnet client so I could log into
WSL in 8-bit mode, and confirmed that the C1 transmissions are working
as expected in vttest.

Closes #17931
Tests added/passed
This fixes a lot of subtle issues:
* Avoid emitting another de-/iconify VT sequence when
  we encounter a (de)iconify VT sequence during parsing.
* Avoid emitting a de-/iconify VT sequence when
  a focus event is received on the signal pipe.
* Avoid emitting such sequences on startup.
* Avoid emitting multiple such sequences
  when rapidly un-/focusing the window.

It's also a minor cleanup, because the `GA_ROOTOWNER` is not security
relevant. It was added because there was concern that someone can just
spawn a ConPTY session, tell it that it's focused, and spawn a child
which is now focused. But why would someone do that, when the console
IOCTLs to do so are not just publicly available but also documented?

I also disabled the IME window.

## Validation Steps Performed
* First:
  ```cpp
  int main() {
      for (bool show = false;; show = !show) {
          printf(show ? "Show in 3s...\n" : "Hide in 3s...\n");
          Sleep(3000);
          ShowWindow(GetConsoleWindow(), show ? SW_SHOW : SW_HIDE);
      }
  }
  ```
* PowerShell 5's `Get-Credential` gains focus ✅
* `sleep 5; Get-Credential` and focus another app. WT should start
  blinking in the taskbar. Restore it. The popup has focus ✅
* Run `:hardcopy` in vim: Window is shown centered at (0,0) ✖️
  But that's okay because it does that already anyway ✅
* `Connect-AzAccount` doesn't crash PowerShell ✅
@pull pull bot merged commit 4386bf0 into l3dlp-sandbox:main Oct 10, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants