ReadConsoleW not handling tab completion correctly #1409

Closed
opened 2026-01-30 22:25:22 +00:00 by claunia · 6 comments
Owner

Originally created by @RadAd on GitHub (May 30, 2019).

Environment

Microsoft Windows [Version 10.0.17763.503]

Steps to reproduce

#include <Windows.H>

int main()
{
    HANDLE hStdInput = GetStdHandle(STD_INPUT_HANDLE);

    WCHAR buf[1024];
    DWORD read = 0;
    CONSOLE_READCONSOLE_CONTROL crc = {};
    crc.nLength = sizeof(crc);
    WCHAR c = L'\t';
    crc.dwCtrlWakeupMask = 1 << c;
    ReadConsoleW(hStdInput, buf, ARRAYSIZE(buf), &read, &crc);

    HANDLE hStdOutput = GetStdHandle(STD_OUTPUT_HANDLE);

    DWORD written = 0;
    WriteConsoleW(hStdOutput, L"\n", 1, &written, NULL);
    WriteConsoleW(hStdOutput, buf, read, &written, NULL);

    return 0;
}

Run the above program and type in (no return): abcdef
Move the cursor back 3 places.
Press the TAB key

Expected behavior

I expected buf to contain abc\tdef

Actual behavior

Actually buf to contain abc\tef

Notice that the tab character is in the correct place but it seems to have overwritten the character rather than inserted it. The length, however, has been increased by one but the last character is a space which wasn't typed in. This makes it impossible to correctly handling tab completion.

Originally created by @RadAd on GitHub (May 30, 2019). # Environment Microsoft Windows [Version 10.0.17763.503] # Steps to reproduce ``` #include <Windows.H> int main() { HANDLE hStdInput = GetStdHandle(STD_INPUT_HANDLE); WCHAR buf[1024]; DWORD read = 0; CONSOLE_READCONSOLE_CONTROL crc = {}; crc.nLength = sizeof(crc); WCHAR c = L'\t'; crc.dwCtrlWakeupMask = 1 << c; ReadConsoleW(hStdInput, buf, ARRAYSIZE(buf), &read, &crc); HANDLE hStdOutput = GetStdHandle(STD_OUTPUT_HANDLE); DWORD written = 0; WriteConsoleW(hStdOutput, L"\n", 1, &written, NULL); WriteConsoleW(hStdOutput, buf, read, &written, NULL); return 0; } ``` Run the above program and type in (no return): ```abcdef``` Move the cursor back 3 places. Press the ```TAB``` key # Expected behavior I expected ```buf``` to contain ```abc\tdef``` # Actual behavior Actually ```buf``` to contain ```abc\tef ``` Notice that the tab character is in the correct place but it seems to have overwritten the character rather than inserted it. The length, however, has been increased by one but the last character is a space which wasn't typed in. This makes it impossible to correctly handling tab completion.
claunia added the Product-ConhostResolution-By-DesignIssue-BugArea-Server labels 2026-01-30 22:25:22 +00:00
Author
Owner

@zadjii-msft commented on GitHub (May 30, 2019):

Is this a regression from a previous version of Windows?

I'm pretty sure this is by design with the Console API. Sounds to me like you want to set ENABLE_INSERT_MODE with SetConsoleMode.

@zadjii-msft commented on GitHub (May 30, 2019): Is this a regression from a previous version of Windows? I'm pretty sure this is by design with the Console API. Sounds to me like you want to set `ENABLE_INSERT_MODE` with [SetConsoleMode](https://docs.microsoft.com/en-us/windows/console/setconsolemode).
Author
Owner

@DHowett-MSFT commented on GitHub (May 30, 2019):

Would you mind testing this after enabling [ ] Use legacy console?

@DHowett-MSFT commented on GitHub (May 30, 2019): Would you mind testing this after enabling `[ ] Use legacy console`?
Author
Owner

@RadAd commented on GitHub (May 31, 2019):

I not sure if it is a regression, just something I have noticed now. I think the CONSOLE_READCONSOLE_CONTROL was undocumented for a long time.

ENABLE_INSERT_MODE on or off doesn't change the behaviour.

Use legacy console has no effect either.

@RadAd commented on GitHub (May 31, 2019): I not sure if it is a regression, just something I have noticed now. I think the ```CONSOLE_READCONSOLE_CONTROL``` was undocumented for a long time. ```ENABLE_INSERT_MODE``` on or off doesn't change the behaviour. ```Use legacy console``` has no effect either.
Author
Owner

@zadjii-msft commented on GitHub (Jun 6, 2019):

We're making this a bug to make sure that it hasn't regressed since previous versions.

I believe @dhowett-msft menitoned something about a rouge space character at the end of the buffer in this scenario - we should make sure it hasn't changed since previous versions

@zadjii-msft commented on GitHub (Jun 6, 2019): We're making this a bug to make sure that it hasn't regressed since previous versions. I believe @dhowett-msft menitoned something about a rouge space character at the end of the buffer in this scenario - we should make sure it hasn't changed since previous versions
Author
Owner

@DHowett-MSFT commented on GitHub (Aug 28, 2019):

Alright, I've gone and tested a wide gamut of console hosts.

version tab overwrites extra space at end of buffer
19h1 ✔️ ✔️
rs5 ✔️ ✔️
rs4 ✔️ ✔️
legacy v1 ✔️ ✔️

As unfortunate as this might be, it looks like it's not a regression. Unfortunately, with the Win32 Console APIs we don't really have the flexibility to change the little details like this. There's a very long tail of applications that could be depending on the buffer size manipulations we do here.

This code dates back as far as I can find: Windows 2000. It doesn't exist in NT 3.51 because CtrlWakeupMask doesn't exist in NT 3.51.

    if (CookedReadData->CtrlWakeupMask != 0 &&
        Char < L' ' && (CookedReadData->CtrlWakeupMask & (1 << Char))) {
        *CookedReadData->BufPtr = Char;
        CookedReadData->BytesRead += sizeof(WCHAR);
        CookedReadData->BufPtr += 1;
        CookedReadData->CurrentPosition += 1;
        CookedReadData->ControlKeyState = KeyState;
        return TRUE;
    }

Today, it looks like this.

974e95ebf7/src/host/readDataCooked.cpp (L495-L503)

When we're processing a cooked read, and the incoming character happens to be in the CTRL wakeup mask, we'll always place it at the current position in the incoming buffer, claim we read an additional character, and advance our position.

I hate to close bad things as "By Design," but I think this one has too much stuff piled on top of it for us to ever change it now.

@DHowett-MSFT commented on GitHub (Aug 28, 2019): Alright, I've gone and tested a wide gamut of console hosts. |version|tab overwrites|extra space at end of buffer| |-|-|-| |19h1|✔️|✔️| |rs5|✔️|✔️| |rs4|✔️|✔️| |legacy v1|✔️|✔️| As unfortunate as this might be, it looks like it's _not a regression_. Unfortunately, with the Win32 Console APIs we don't really have the flexibility to change the little details like this. There's a _very_ long tail of applications that could be depending on the buffer size manipulations we do here. This code dates back as far as I can find: Windows 2000. It doesn't exist in NT 3.51 because CtrlWakeupMask doesn't exist in NT 3.51. ``` if (CookedReadData->CtrlWakeupMask != 0 && Char < L' ' && (CookedReadData->CtrlWakeupMask & (1 << Char))) { *CookedReadData->BufPtr = Char; CookedReadData->BytesRead += sizeof(WCHAR); CookedReadData->BufPtr += 1; CookedReadData->CurrentPosition += 1; CookedReadData->ControlKeyState = KeyState; return TRUE; } ``` Today, it looks like this. https://github.com/microsoft/terminal/blob/974e95ebf7704b08da51c1f728ab2bc06108881f/src/host/readDataCooked.cpp#L495-L503 When we're processing a cooked read, and the incoming character happens to be in the CTRL wakeup mask, we'll _always_ place it at the current position in the incoming buffer, claim we read an additional character, and advance our position. I hate to close bad things as "By Design," but I think this one has too much stuff piled on top of it for us to ever change it now.
Author
Owner

@RadAd commented on GitHub (Aug 29, 2019):

Im not surprised its not a regression.

My current workaround is to read the lost character from the console.

I assume this was never picked up because cmd.exe throws away everything after the tab anyway.

@RadAd commented on GitHub (Aug 29, 2019): Im not surprised its not a regression. My current workaround is to read the lost character from the console. I assume this was never picked up because cmd.exe throws away everything after the tab anyway.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#1409