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

Closed
opened 2026-01-31 07:34:28 +00:00 by claunia · 2 comments
Owner

Originally created by @paperclover on GitHub (Jan 26, 2024).

Windows Terminal version

1.18.3181.0

Windows build number

10.0.22621.0

Other Software

This bug report is a minimized reproduction of https://github.com/oven-sh/bun/issues/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.

image

Originally created by @paperclover on GitHub (Jan 26, 2024). ### Windows Terminal version 1.18.3181.0 ### Windows build number 10.0.22621.0 ### Other Software This bug report is a minimized reproduction of https://github.com/oven-sh/bun/issues/8504. ### Steps to reproduce Demo application "`input.c`" ```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. ![image](https://github.com/microsoft/terminal/assets/24465214/b4c689f4-740b-43ac-b502-5d499f2eba97)
Author
Owner

@lhecker commented on GitHub (Jan 29, 2024):

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

@lhecker commented on GitHub (Jan 29, 2024): 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
Author
Owner

@paperclover commented on GitHub (Jan 29, 2024):

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.

@paperclover commented on GitHub (Jan 29, 2024): 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.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#21144