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

x/crypto/ssh/terminal: GetSize returns incorrect results #27743

Closed
egazz opened this issue Sep 18, 2018 · 6 comments
Closed

x/crypto/ssh/terminal: GetSize returns incorrect results #27743

egazz opened this issue Sep 18, 2018 · 6 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows
Milestone

Comments

@egazz
Copy link

egazz commented Sep 18, 2018

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go1.11 windows/amd64

Does this issue reproduce with the latest release?

Yes (since as far as I know this is the latest release).

What operating system and processor architecture are you using (go env)?

Windows 10, amd64.

What did you do?

package main

import (
	"fmt"
	"os"
	"golang.org/x/crypto/ssh/terminal"
)

func main() {

	termID := int(os.Stdout.Fd())
	termWidth, termHeight, termSizeErr := terminal.GetSize(termID)
	
	if termSizeErr != nil {
	
		panic(termSizeErr)
		
	}
	
	fmt.Printf("Width: %v, height: %v", termWidth, termHeight)

}

What did you expect to see?

Accurate height and width of the terminal.

What did you see instead?

Height is way off. My terminal window is 30 lines tall, but this returns a height of 9001. Width was correct, however.

In addition, I noticed that this issue remains. On Windows, getting the terminal handle via os.Stdin.Fd() results in an error, while os.Stdout.Fd() results in the above. However, on Linux, using Stdin works properly.

@gopherbot gopherbot added this to the Unreleased milestone Sep 18, 2018
@dmitshur dmitshur added OS-Windows NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Sep 19, 2018
@alexbrainman
Copy link
Member

I think, it kind of works

image

There are many definitions of terminal height and width.

I looked at the code, and GetSize uses GetConsoleScreenBufferInfo Windows API. What should this package use instead? Why?

Thank you.

Alex

@egazz
Copy link
Author

egazz commented Sep 24, 2018

Hi, thanks for the reply.

I will say what I expected was the window size. I can imagine scenarios where one may need both, but for a basic console app, I think size of the window, i.e. the viewable area, is more important.

For example, if I wanted to know how much space I have for writing text, I'd need to know the dimensions (same thing for clearing the screen, unless there's a method that I haven't found yet....) The same is true for determining whether things need to be redrawn if the terminal window is resized.

I noticed this library uses syscall to reference TIOCGWINSZ, which according to this is part of the Linux kernel at least. I don't know if it'd work on Windows, though.

@alexbrainman
Copy link
Member

I don't know if it'd work on Windows, though.

That code does not run on Windows - note how first line of that source file says

// +build !windows

Alex

@egazz
Copy link
Author

egazz commented Sep 26, 2018

That was my concern. It looks, then, like the only way to get this information is sending a command such as stty via os.Stdin?

But back to my original issue, at the least it seems like the documentation should make it clearer that GetSize in fact gets the buffer size, not the window size. It's also worth noting that this behavior is different on Linux, where on at least some terminal emulators (such as urxvt) it returns the window size.

@alexbrainman
Copy link
Member

It looks, then, like the only way to get this information is sending a command such as stty via os.Stdin?

There is no stty command on Windows, if that is what you are suggesting.

Alex

@gopherbot
Copy link

Change https://golang.org/cl/163538 mentions this issue: ssh/terminal: fix GetSize on Windows

bored-engineer pushed a commit to bored-engineer/ssh that referenced this issue Oct 13, 2019
Return window size instead of buffer size.

Fixes golang/go#27743

Change-Id: Ib1cd249f5680d86d505032e51d9102c2718ddf6f
Reviewed-on: https://go-review.googlesource.com/c/163538
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
bored-engineer pushed a commit to bored-engineer/ssh that referenced this issue Oct 13, 2019
Return window size instead of buffer size.

Fixes golang/go#27743

Change-Id: Ib1cd249f5680d86d505032e51d9102c2718ddf6f
Reviewed-on: https://go-review.googlesource.com/c/163538
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
bored-engineer pushed a commit to bored-engineer/ssh that referenced this issue Oct 13, 2019
Return window size instead of buffer size.

Fixes golang/go#27743

Change-Id: Ib1cd249f5680d86d505032e51d9102c2718ddf6f
Reviewed-on: https://go-review.googlesource.com/c/163538
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators Feb 28, 2020
gopherbot pushed a commit to golang/term that referenced this issue Oct 16, 2020
Return window size instead of buffer size.

Fixes golang/go#27743

Change-Id: Ib1cd249f5680d86d505032e51d9102c2718ddf6f
Reviewed-on: https://go-review.googlesource.com/c/163538
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 28, 2022
Return window size instead of buffer size.

Fixes golang/go#27743

Change-Id: Ib1cd249f5680d86d505032e51d9102c2718ddf6f
Reviewed-on: https://go-review.googlesource.com/c/163538
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 29, 2022
Return window size instead of buffer size.

Fixes golang/go#27743

Change-Id: Ib1cd249f5680d86d505032e51d9102c2718ddf6f
Reviewed-on: https://go-review.googlesource.com/c/163538
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 29, 2022
Return window size instead of buffer size.

Fixes golang/go#27743

Change-Id: Ib1cd249f5680d86d505032e51d9102c2718ddf6f
Reviewed-on: https://go-review.googlesource.com/c/163538
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
LewiGoddard pushed a commit to LewiGoddard/crypto that referenced this issue Feb 16, 2023
Return window size instead of buffer size.

Fixes golang/go#27743

Change-Id: Ib1cd249f5680d86d505032e51d9102c2718ddf6f
Reviewed-on: https://go-review.googlesource.com/c/163538
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
BiiChris pushed a commit to BiiChris/crypto that referenced this issue Sep 15, 2023
Return window size instead of buffer size.

Fixes golang/go#27743

Change-Id: Ib1cd249f5680d86d505032e51d9102c2718ddf6f
Reviewed-on: https://go-review.googlesource.com/c/163538
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows
Projects
None yet
Development

No branches or pull requests

4 participants