Skip to content

Commit

Permalink
Merge pull request #1561 from loadimpact/fix/1559-mintty-panic
Browse files Browse the repository at this point in the history
Fix UI panic and login errors on mintty/Windows
  • Loading branch information
Ivan Mirić authored Jul 17, 2020
2 parents 62e1b53 + 7aa5269 commit 9c8de85
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 6 deletions.
8 changes: 8 additions & 0 deletions cmd/login_cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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)
Expand Down Expand Up @@ -89,6 +94,9 @@ This will set the default token used when just "k6 run -o cloud" is passed.`,
},
},
}
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)
if err != nil {
return err
Expand Down
8 changes: 8 additions & 0 deletions cmd/login_influxdb.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 {
Expand Down Expand Up @@ -80,6 +85,9 @@ This will set the default server used when just "-o influxdb" is passed.`,
},
},
}
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)
if err != nil {
return err
Expand Down
23 changes: 17 additions & 6 deletions cmd/ui.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -252,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 = 80 // TODO: something safer, return error?
termWidth = defaultTermWidth
errTermGetSize = true
}

// Get the longest left side string length, to align progress bars
Expand Down Expand Up @@ -330,12 +333,20 @@ func showProgress(
outMutex.Unlock()
return
case <-winch:
// More responsive progress bar resizing on platforms with SIGWINCH (*nix)
termWidth, _, _ = terminal.GetSize(fd)
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 {
termWidth, _, _ = terminal.GetSize(fd)
if !errTermGetSize && winch == nil {
termWidth, _, err = terminal.GetSize(fd)
if err != nil {
termWidth = defaultTermWidth
}
}
}
renderProgressBars(true)
Expand Down
11 changes: 11 additions & 0 deletions ui/form_fields.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
package ui

import (
"bufio"
"io"
"os"
"strings"
Expand Down Expand Up @@ -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
}

Expand Down

0 comments on commit 9c8de85

Please sign in to comment.