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

Use a plain char array to pass connection input #17710

Merged
merged 3 commits into from
Aug 14, 2024

Conversation

lhecker
Copy link
Member

@lhecker lhecker commented Aug 13, 2024

HSTRING does not permit strings that aren't null-terminated.
As such we'll simply use a plain char array which compiles down to
a UINT32 and wchar_t* pointer pair. Unfortunately, cppwinrt uses
char16_t in place of wchar_t, and also offers no trivial conversion
between winrt::array_view and std::wstring_view either.
As such, most of this PR is about explicit type casting.

Closes #17697

Validation Steps Performed

  • Patch the DeviceAttributes implementation in adaptDispatch.cpp
    to respond like this:
    _api.ReturnResponse({L"ABCD", 3});
  • Open a WSL shell and execute this:
    printf "\e[c"; read
  • Doesn't crash ✅

@microsoft-github-policy-service microsoft-github-policy-service bot added the Issue-Bug It either shouldn't be doing this or needs an investigation. label Aug 13, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added Area-TerminalConnection Issues pertaining to the terminal<->backend connection interface Priority-3 A description (P3) Product-Terminal The new Windows Terminal. labels Aug 13, 2024
Copy link
Member

@carlos-zamora carlos-zamora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤢, but hey, it fixed it. Thanks!

@DHowett DHowett merged commit 9c14367 into main Aug 14, 2024
20 checks passed
@DHowett DHowett deleted the dev/lhecker/17697-arrays-are-the-better-hstring branch August 14, 2024 02:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-TerminalConnection Issues pertaining to the terminal<->backend connection interface Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3) Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Potential crash when calling ITerminalConnection::WriteInput
3 participants