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

Bug: When headless, GetLargestConsoleWindowSize reports a MUCH too small size #2712

Open
jazzdelightsme opened this issue Sep 10, 2019 · 4 comments
Labels
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. Priority-2 A description (P2) Product-Conpty For console issues specifically related to conpty
Milestone

Comments

@jazzdelightsme
Copy link
Member

Environment

ssh client: recent internal build of Windows
ssh server: recent internal build of Windows (WCOS)
vim: custom build that can load on the server

In a headless pseudoconsole session (such as over ssh), GetLargestConsoleWindowSize is reporting a maximum possible window size of 640 columns and 18 rows (on my particular SKU of Windows, which might differ from others). 640 columns isn't bad, I suppose, but 18 rows is downright tiny. I debugged a bit and found that internally (in conhost) this is due to a "screen" size in pixels of 640x300, and a font size of {1, 16}. (Of course there is not really a screen or pixels in a headless scenario, and that font size seems strange.)

Over ssh, I believe the console does not have any way to determine the ssh client's maximum possible window size (it doesn't know the size of the monitor the client is using, or the font being used, or even if it's a windows machine). It can be notified of window size changes on the client side (section 6.7: "Window Dimension Change Message" of RFC 4254), but not what the max possible size is. (Although I know very little about ssh; it's possible there's some extension or convention or something.)

So I propose that since there is no monitor hooked up to a headless session, GetLargestConsoleWindowSize should just report { SHRT_MAX, SHRT_MAX } (and let the client deal with whatever happens if the client sets the console's size too big).

(Without this, vim running in an ssh session is stuck at 18 lines high and cannot be made any bigger.)

@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 Sep 10, 2019
@jazzdelightsme jazzdelightsme changed the title Bug: When headless, GetLargestConsoleWindowSize reports a MUCH too-small size Bug: When headless, GetLargestConsoleWindowSize reports a MUCH too small size Sep 10, 2019
@jazzdelightsme
Copy link
Member Author

An alternate idea (floated by DHowett on an email) would be to limit GetLargestConsoleWindowSize to reporting just the values the pseudoconsole caller provides in Resize or on initialization so the connected client application never tries to break out.

The problem with that, as noted by DHowett, is "VT applications are allowed to resize their terminals..."

What that would mean for vim over sssh, for example, is that if you wanted to grow it (":set lines="), you'd have to exit vim, resize your console, then re-launch vim. But at least that would work.

@DHowett-MSFT DHowett-MSFT added 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 labels Sep 14, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Sep 14, 2019
@DHowett-MSFT
Copy link
Contributor

I'm tagging this one up, but leaving Triage on so we discuss it on Monday. 😄

@DHowett-MSFT DHowett-MSFT added this to the 21H1 milestone Sep 14, 2019
@DHowett-MSFT DHowett-MSFT removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Sep 16, 2019
@DHowett-MSFT
Copy link
Contributor

Current thoughts: for pseudoconsole hosts only, we can return {SHRT_MAX, SHRT_MAX} through the service locator. If not the service locator, all the API methods in "getset". We can't necessarily use the pty-requested size as the "maximum" because a few other console APIs that resize the buffer use ...Largest... as a bounds check.

@zadjii-msft
Copy link
Member

Functions in discussion:

ApiRoutines::GetLargestConsoleWindowSizeImpl
SCREEN_INFORMATION::GetLargestWindowSizeInCharacters
IWindowMetrics::GetMaxClientRectInPixels

@zadjii-msft zadjii-msft modified the milestones: Windows vNext, 22H1 Jan 4, 2022
@zadjii-msft zadjii-msft added the Priority-2 A description (P2) label Jan 4, 2022
@zadjii-msft zadjii-msft modified the milestones: 22H1, Terminal v1.14 Feb 2, 2022
@zadjii-msft zadjii-msft modified the milestones: Terminal v1.14, Backlog Mar 10, 2022
@zadjii-msft zadjii-msft added the Help Wanted We encourage anyone to jump in on these. label Mar 10, 2022
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. Help Wanted We encourage anyone to jump in on these. Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Conpty For console issues specifically related to conpty
Projects
None yet
Development

No branches or pull requests

3 participants