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

Sending negative values to ResizePseudoConsole returns S_OK #3447

Closed
ZoeyR opened this issue Nov 5, 2019 · 2 comments · Fixed by #3936
Closed

Sending negative values to ResizePseudoConsole returns S_OK #3447

ZoeyR opened this issue Nov 5, 2019 · 2 comments · Fixed by #3936
Labels
Area-Server Down in the muck of API call servicing, interprocess communication, eventing, etc. Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Conpty For console issues specifically related to conpty Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.

Comments

@ZoeyR
Copy link
Contributor

ZoeyR commented Nov 5, 2019

Environment

Windows build number: 18363,418
Windows Terminal version (if applicable):

Any other software?

Steps to reproduce

Call ResizePseudoConsole with negative values stored in the passed COORD structure.

Expected behavior

returned HRESULT should indicate failure since a pty cannot have a negative size

Actual behavior

returned HRESULT indicates success.

@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 Nov 5, 2019
@miniksa
Copy link
Member

miniksa commented Nov 5, 2019

HRESULT _ResizePseudoConsole(_In_ const PseudoConsole* const pPty, _In_ const COORD size)
{
if (pPty == NULL)
{
return E_INVALIDARG;
}
unsigned short signalPacket[3];
signalPacket[0] = PTY_SIGNAL_RESIZE_WINDOW;
signalPacket[1] = size.X;
signalPacket[2] = size.Y;
BOOL fSuccess = WriteFile(pPty->hSignal, signalPacket, sizeof(signalPacket), NULL, NULL);
return fSuccess ? S_OK : HRESULT_FROM_WIN32(GetLastError());
}

The COORD which has two signed SHORTs is being pushed into a structure of unsigned short and apparently nothing is complaining or validating that.

@miniksa miniksa added Area-Server Down in the muck of API call servicing, interprocess communication, eventing, etc. Help Wanted We encourage anyone to jump in on these. Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Conpty For console issues specifically related to conpty and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Nov 5, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Nov 5, 2019
@ghost ghost added In-PR This issue has a related PR and removed In-PR This issue has a related PR labels Dec 12, 2019
@ghost ghost added the In-PR This issue has a related PR label Dec 12, 2019
@ghost ghost added Needs-Tag-Fix Doesn't match tag requirements and removed Help Wanted We encourage anyone to jump in on these. labels Dec 13, 2019
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Dec 13, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Dec 13, 2019
@ghost
Copy link

ghost commented Jan 14, 2020

🎉This issue was addressed in #3936, which has now been successfully released as Windows Terminal Preview v0.8.10091.0.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Server Down in the muck of API call servicing, interprocess communication, eventing, etc. Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Conpty For console issues specifically related to conpty Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
3 participants