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

C0 characters in screen buffer are incorrectly interpreted as controls by conpty #4363

Closed
j4james opened this issue Jan 26, 2020 · 4 comments · Fixed by #16825
Closed

C0 characters in screen buffer are incorrectly interpreted as controls by conpty #4363

j4james opened this issue Jan 26, 2020 · 4 comments · Fixed by #16825
Labels
Area-Output Related to output processing (inserting text into buffer, retrieving buffer text, etc.) Area-VT Virtual Terminal sequence support In-PR This issue has a related PR Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Product-Conpty For console issues specifically related to conpty
Milestone

Comments

@j4james
Copy link
Collaborator

j4james commented Jan 26, 2020

Environment

Windows build number: Version 10.0.18362.535
Windows Terminal version (if applicable): 0.8.10091.0

Steps to reproduce

Compile the follow C++ program, and then run the resulting executable from a cmd shell in the Windows Terminal.

#include <windows.h>
#include <string>

void main() {
    HANDLE handle = GetStdHandle(STD_OUTPUT_HANDLE);

    // Enable VT mode.
    DWORD mode;
    GetConsoleMode(handle, &mode);
    mode |= ENABLE_VIRTUAL_TERMINAL_PROCESSING;
    SetConsoleMode(handle, mode);

    // Write directly to the screen buffer with control chars.
    DWORD written = 0;
    const std::wstring text = L"\x1B[31mIs this text red?\nIs this on a new line?\n";
    WriteConsoleOutputCharacterW(handle, text.c_str(), text.length(), COORD{ 0, 0 }, &written);
}

Expected behavior

The WriteConsoleOutputCharacterW api writes directly to the screen buffer, without attempting to process any control characters. I would thus expect the text message to be output entirely on the first line of the screen buffer, with the ESC and LF control characters output as glyphs, and not interpreted. This is what it looks like in a conhost cmd shell:

image

Note that the ENABLE_VIRTUAL_TERMINAL_PROCESSING mode should have no effect on this API. I've only turned it on to make sure the code is running under the same conditions in conhost as it is in WT (which enables VT mode by default).

Actual behavior

In the Windows Terminal, the ESC and LF characters are interpreted as control characters instead of being output as glyphs. As a result, you get an escape sequence changing the text color to red, and the text will be output over multiple lines.

Depending on timing, you can also end up with other parts of the screen being redrawn incorrectly, as the terminal cursor position is now out of sync with where the conhost thinks it is (notice part of the copyright message being redrawn two lines down in red).

image

@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Jan 26, 2020
@DHowett-MSFT
Copy link
Contributor

Yep, absolutely. This is pretty bad, but it's something we've kicked down the road a bit.

I was secretly hoping we could add 0x2400 to each one when we render it and get the Control Picture...

I had written "this is the actual root cause of #1965", but then I read the history and I think we just never filed the followup bug for this, and to boot I realized that you had filed it. 😁

@DHowett-MSFT DHowett-MSFT added Area-Output Related to output processing (inserting text into buffer, retrieving buffer text, etc.) Area-VT Virtual Terminal sequence support Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Conpty For console issues specifically related to conpty labels Jan 31, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Jan 31, 2020
@DHowett-MSFT DHowett-MSFT added this to the 21H1 milestone Jan 31, 2020
@DHowett-MSFT
Copy link
Contributor

#1965 (comment) and #1965 (comment) for context

@DHowett-MSFT DHowett-MSFT removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Jan 31, 2020
@j4james
Copy link
Collaborator Author

j4james commented Jan 31, 2020

Ah, that's the issue I was looking for. I knew we'd discussed this somewhere before, but all I could find was PR #1964 (which I thought at the time would have solved the problem, but clearly not in all situations).

As for how we fix it, if we want WT to be "compatible" with the legacy console, then I would have thought conpty could simply translate these characters to the Unicode replacement character (U+FFFD)? If that's the glyph they produce in conhost, then surely that's what you'd want to see in WT too.

@adipose
Copy link

adipose commented Mar 11, 2020

Piling on. I have used WriteConsoleOutputCharacter in the past to bypass printing the control characters, but now they are broken (note that in latest non-insider windows builds, the glyphs themselves are also broken on cmd.exe, but at least the formatting is incorrect).

image

vs.

image

@zadjii-msft zadjii-msft modified the milestones: Windows vNext, 22H2 Jan 4, 2022
@zadjii-msft zadjii-msft modified the milestones: 22H2, Backlog Jul 5, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added the In-PR This issue has a related PR label Mar 5, 2024
@DHowett DHowett closed this as completed in 563b731 Mar 6, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label Mar 6, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label Mar 6, 2024
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Output Related to output processing (inserting text into buffer, retrieving buffer text, etc.) Area-VT Virtual Terminal sequence support In-PR This issue has a related PR Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Product-Conpty For console issues specifically related to conpty
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants