Reading input byte by byte causes lines with even numbers of characters

[paperdave]

Windows Terminal version

1.18.3181.0

Windows build number

10.0.22621.0

Other Software

This bug report is a minimized reproduction of oven-sh/bun#8504.

Steps to reproduce

Demo application "input.c"

#include <stdio.h>
#include <windows.h>

int main()
{
    HANDLE hStdin;
    DWORD len;
    char buf[1];

    hStdin = GetStdHandle(STD_INPUT_HANDLE);
    if (hStdin == INVALID_HANDLE_VALUE)
        printf("GetStdHandle failed with %lu\n", GetLastError());

    while (1)
    {
        if (!ReadFile(hStdin, buf, 1, &len, NULL)) // number of records read
            printf("ReadConsoleInput failed with %lu\n", GetLastError());

        printf("byte: %d\n", buf[0]);
    }
    return 0;
}

(This program is mimicking the behavior of the Zig standard library function std.io.Reader().readUntilDelimiterAlloc, which does a byte by byte read until it hits a specified character)

clang input.c (I used llvm 16.0.4, but I do not think this matters)

Run in Windows terminal. .\a.exe

If you type 0 or an even number of characters, the \n byte 10 is not read. only the \r byte is.

If you type an odd number of characters, everything is as normal.

Expected Behavior

I expect all lines to end with \r\n, to match the behavior of conhost.exe, ConEmu, Visual Studio Code's Integrated terminal, and also various terminals on Linux/MacOS over an ssh connection.

Actual Behavior

my inputs are: 1<return>12<return>123<return><return>, and then an S pressed by mistake when trying to take a screenshot.

[lhecker]

I've found that this PR fixed the issue: #16313
The issue was caused by this PR: #14745

It's planned to be shipped in the next service release for both 1.18 (Stable) and 1.19 (Preview). I'm sorry that it's been taking some time to ship this! But since the bug only started appearing in 1.18, I personally believe it's fine to not hotfix Zig just yet. (Although I do sort of think that using a buffered reader is a great idea, if it's an option. The ReadFile syscalls are very expensive.)

If you want to test the fix in the meantime, you can find the nightly Canary build here: https://aka.ms/terminal-canary-installer

[paperdave]

Thank you.

In Bun i have changed our code to use a buffered reader for all platforms. This makes the readByte call less expensive since it will buffer an amount already.

It probably makes sense for us to leave that commit in regardless because it improves performance slightly. These are only used for terminal input, where the ReadFile is going to always be blocked on user input (bun init and the prompt() JS api).

Maybe I'll revert my change if there are issues consuming stdin from a user program while using the prompt api, but I suspect this case is very unlikely, and by the time this becomes an issue for anyone, Windows Terminal should be propagated to everyone.