From 7e1586f16bd2cffe8d4eab2aa2406c071cb283ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20Miri=C4=87?= Date: Thu, 16 Jul 2020 09:35:30 +0200 Subject: [PATCH 1/4] Fix panic on mintty/Windows Closes #1559 --- cmd/ui.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/cmd/ui.go b/cmd/ui.go index 70b272a16a4..5c6ed871ea9 100644 --- a/cmd/ui.go +++ b/cmd/ui.go @@ -46,7 +46,8 @@ const ( maxLeftLength = 30 // Amount of padding in chars between rendered progress // bar text and right-side terminal window edge. - termPadding = 1 + termPadding = 1 + defaultTermWidth = 80 ) // A writer that syncs writes with a mutex and, if the output is a TTY, clears before newlines. @@ -255,7 +256,7 @@ func showProgress( termWidth, _, err := terminal.GetSize(int(os.Stdout.Fd())) if err != nil && stdoutTTY { logger.WithError(err).Warn("error getting terminal size") - termWidth = 80 // TODO: something safer, return error? + termWidth = defaultTermWidth } // Get the longest left side string length, to align progress bars @@ -331,11 +332,17 @@ func showProgress( return case <-winch: // More responsive progress bar resizing on platforms with SIGWINCH (*nix) - termWidth, _, _ = terminal.GetSize(fd) + termWidth, _, err = terminal.GetSize(fd) + if err != nil { + termWidth = defaultTermWidth + } case <-ticker.C: // Default ticker-based progress bar resizing if winch == nil { - termWidth, _, _ = terminal.GetSize(fd) + termWidth, _, err = terminal.GetSize(fd) + if err != nil { + termWidth = defaultTermWidth + } } } renderProgressBars(true) From d17100b0e7b02c796d97c9385d52acb365fa4fb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20Miri=C4=87?= Date: Thu, 16 Jul 2020 10:10:17 +0200 Subject: [PATCH 2/4] Don't get terminal size if it fails initially Resolves https://github.com/loadimpact/k6/pull/1561/files#r455583966 --- cmd/ui.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/cmd/ui.go b/cmd/ui.go index 5c6ed871ea9..93a59a71913 100644 --- a/cmd/ui.go +++ b/cmd/ui.go @@ -253,10 +253,12 @@ func showProgress( pbs = append(pbs, s.GetProgress()) } + var errTermGetSize bool termWidth, _, err := terminal.GetSize(int(os.Stdout.Fd())) if err != nil && stdoutTTY { logger.WithError(err).Warn("error getting terminal size") termWidth = defaultTermWidth + errTermGetSize = true } // Get the longest left side string length, to align progress bars @@ -331,14 +333,16 @@ func showProgress( outMutex.Unlock() return case <-winch: - // More responsive progress bar resizing on platforms with SIGWINCH (*nix) - termWidth, _, err = terminal.GetSize(fd) - if err != nil { - termWidth = defaultTermWidth + if !errTermGetSize { + // More responsive progress bar resizing on platforms with SIGWINCH (*nix) + termWidth, _, err = terminal.GetSize(fd) + if err != nil { + termWidth = defaultTermWidth + } } case <-ticker.C: // Default ticker-based progress bar resizing - if winch == nil { + if !errTermGetSize && winch == nil { termWidth, _, err = terminal.GetSize(fd) if err != nil { termWidth = defaultTermWidth From 1e4b9585dfa6b5e154090bb2af6a6e2b2a43c3f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20Miri=C4=87?= Date: Thu, 16 Jul 2020 13:37:41 +0200 Subject: [PATCH 3/4] Fix terminal pty issues on Windows/mintty in cloud/influxdb login This falls back to plain text input which will echo the password back, but there doesn't seem to be a way around that. :-/ --- cmd/login_cloud.go | 8 ++++++++ cmd/login_influxdb.go | 8 ++++++++ ui/form_fields.go | 11 +++++++++++ 3 files changed, 27 insertions(+) diff --git a/cmd/login_cloud.go b/cmd/login_cloud.go index 453743ad5a1..dd6cae8869a 100644 --- a/cmd/login_cloud.go +++ b/cmd/login_cloud.go @@ -22,10 +22,13 @@ package cmd import ( "os" + "syscall" "github.com/pkg/errors" + "github.com/sirupsen/logrus" "github.com/spf13/afero" "github.com/spf13/cobra" + "golang.org/x/crypto/ssh/terminal" "gopkg.in/guregu/null.v3" "github.com/loadimpact/k6/lib/consts" @@ -51,6 +54,8 @@ This will set the default token used when just "k6 run -o cloud" is passed.`, k6 login cloud`[1:], Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { + // TODO: don't use a global... or maybe change the logger? + logger := logrus.StandardLogger() fs := afero.NewOsFs() k6Conf, err := getConsolidatedConfig(fs, Config{}, nil) @@ -89,6 +94,9 @@ This will set the default token used when just "k6 run -o cloud" is passed.`, }, }, } + if !terminal.IsTerminal(syscall.Stdin) { + logger.Warn("Stdin is not a terminal, falling back to plain text input") + } vals, err := form.Run(os.Stdin, stdout) if err != nil { return err diff --git a/cmd/login_influxdb.go b/cmd/login_influxdb.go index 44de72c141c..d425c1041b5 100644 --- a/cmd/login_influxdb.go +++ b/cmd/login_influxdb.go @@ -22,11 +22,14 @@ package cmd import ( "os" + "syscall" "time" "github.com/mitchellh/mapstructure" + "github.com/sirupsen/logrus" "github.com/spf13/afero" "github.com/spf13/cobra" + "golang.org/x/crypto/ssh/terminal" "github.com/loadimpact/k6/lib/types" "github.com/loadimpact/k6/stats/influxdb" @@ -42,6 +45,8 @@ var loginInfluxDBCommand = &cobra.Command{ This will set the default server used when just "-o influxdb" is passed.`, Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { + // TODO: don't use a global... or maybe change the logger? + logger := logrus.StandardLogger() fs := afero.NewOsFs() config, configPath, err := readDiskConfig(fs) if err != nil { @@ -80,6 +85,9 @@ This will set the default server used when just "-o influxdb" is passed.`, }, }, } + if !terminal.IsTerminal(syscall.Stdin) { + logger.Warn("Stdin is not a terminal, falling back to plain text input") + } vals, err := form.Run(os.Stdin, stdout) if err != nil { return err diff --git a/ui/form_fields.go b/ui/form_fields.go index 52735142255..e8c04ca7d74 100644 --- a/ui/form_fields.go +++ b/ui/form_fields.go @@ -21,6 +21,7 @@ package ui import ( + "bufio" "io" "os" "strings" @@ -122,6 +123,16 @@ func (f PasswordField) GetContents(r io.Reader) (string, error) { return "", errors.New("Cannot read password from the supplied terminal") } password, err := terminal.ReadPassword(int(stdin.Fd())) + if err != nil { + // Possibly running on Cygwin/mintty which doesn't emulate + // pseudo terminals properly, so fallback to plain text input. + // Note that passwords will be echoed if this is the case. + // See https://github.com/mintty/mintty/issues/56 + // A workaround is to use winpty or mintty compiled with + // Cygwin >=3.1.0 which supports the new ConPTY Windows API. + bufR := bufio.NewReader(r) + password, err = bufR.ReadBytes('\n') + } return string(password), err } From 7aa526921c2f00ba34abc23b7856148a54f050c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20Miri=C4=87?= Date: Thu, 16 Jul 2020 14:00:06 +0200 Subject: [PATCH 4/4] Silence linter, the conversion is required on Windows --- cmd/login_cloud.go | 2 +- cmd/login_influxdb.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/login_cloud.go b/cmd/login_cloud.go index dd6cae8869a..f440a81cd56 100644 --- a/cmd/login_cloud.go +++ b/cmd/login_cloud.go @@ -94,7 +94,7 @@ This will set the default token used when just "k6 run -o cloud" is passed.`, }, }, } - if !terminal.IsTerminal(syscall.Stdin) { + if !terminal.IsTerminal(int(syscall.Stdin)) { // nolint: unconvert logger.Warn("Stdin is not a terminal, falling back to plain text input") } vals, err := form.Run(os.Stdin, stdout) diff --git a/cmd/login_influxdb.go b/cmd/login_influxdb.go index d425c1041b5..602ccb41ee6 100644 --- a/cmd/login_influxdb.go +++ b/cmd/login_influxdb.go @@ -85,7 +85,7 @@ This will set the default server used when just "-o influxdb" is passed.`, }, }, } - if !terminal.IsTerminal(syscall.Stdin) { + if !terminal.IsTerminal(int(syscall.Stdin)) { // nolint: unconvert logger.Warn("Stdin is not a terminal, falling back to plain text input") } vals, err := form.Run(os.Stdin, stdout)