From db768bd215f2bad110bbd5180ecaaae29da62c30 Mon Sep 17 00:00:00 2001 From: Igor Halfeld Date: Thu, 9 Jul 2020 19:21:47 -0300 Subject: [PATCH 1/2] fix buff size --- cmd/context.go | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/cmd/context.go b/cmd/context.go index 12a95c205..2ce36e5f9 100644 --- a/cmd/context.go +++ b/cmd/context.go @@ -31,8 +31,8 @@ func newContextCommand(config *settings.Config) *cobra.Command { } listCommand := &cobra.Command{ - Short: "List all contexts", - Use: "list ", + Short: "List all contexts", + Use: "list ", PreRunE: initClient, RunE: func(cmd *cobra.Command, args []string) error { return listContexts(cl, args[0], args[1]) @@ -41,8 +41,8 @@ func newContextCommand(config *settings.Config) *cobra.Command { } showContextCommand := &cobra.Command{ - Short: "Show a context", - Use: "show ", + Short: "Show a context", + Use: "show ", PreRunE: initClient, RunE: func(cmd *cobra.Command, args []string) error { return showContext(cl, args[0], args[1], args[2]) @@ -51,8 +51,8 @@ func newContextCommand(config *settings.Config) *cobra.Command { } storeCommand := &cobra.Command{ - Short: "Store a new environment variable in the named context. The value is read from stdin.", - Use: "store-secret ", + Short: "Store a new environment variable in the named context. The value is read from stdin.", + Use: "store-secret ", PreRunE: initClient, RunE: func(cmd *cobra.Command, args []string) error { return storeEnvVar(cl, args[0], args[1], args[2], args[3]) @@ -61,8 +61,8 @@ func newContextCommand(config *settings.Config) *cobra.Command { } removeCommand := &cobra.Command{ - Short: "Remove an environment variable from the named context", - Use: "remove-secret ", + Short: "Remove an environment variable from the named context", + Use: "remove-secret ", PreRunE: initClient, RunE: func(cmd *cobra.Command, args []string) error { return removeEnvVar(cl, args[0], args[1], args[2], args[3]) @@ -71,8 +71,8 @@ func newContextCommand(config *settings.Config) *cobra.Command { } createContextCommand := &cobra.Command{ - Short: "Create a new context", - Use: "create ", + Short: "Create a new context", + Use: "create ", PreRunE: initClient, RunE: func(cmd *cobra.Command, args []string) error { return createContext(cl, args[0], args[1], args[2]) @@ -82,8 +82,8 @@ func newContextCommand(config *settings.Config) *cobra.Command { force := false deleteContextCommand := &cobra.Command{ - Short: "Delete the named context", - Use: "delete ", + Short: "Delete the named context", + Use: "delete ", PreRunE: initClient, RunE: func(cmd *cobra.Command, args []string) error { return deleteContext(cl, force, args[0], args[1], args[2]) @@ -176,7 +176,13 @@ func readSecretValue() (string, error) { return string(bytes), err } else { fmt.Print("Enter secret value and press enter: ") - reader := bufio.NewReader(os.Stdin) + + buffSize, err := os.Stdin.Stat() + if err != nil { + return "", err + } + + reader := bufio.NewReaderSize(os.Stdin, int(buffSize.Size())) str, err := reader.ReadString('\n') return strings.TrimRight(str, "\n"), err } From de18f39fac1ad529e35da2a5613d1189ffcd1031 Mon Sep 17 00:00:00 2001 From: Igor Halfeld Date: Fri, 10 Jul 2020 16:34:41 -0300 Subject: [PATCH 2/2] better if/else block and fix code coverage --- cmd/context.go | 32 ++++++++++++++++++-------------- cmd/context_test.go | 29 +++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 14 deletions(-) diff --git a/cmd/context.go b/cmd/context.go index 2ce36e5f9..b5d514ccd 100644 --- a/cmd/context.go +++ b/cmd/context.go @@ -3,7 +3,7 @@ package cmd import ( "bufio" "fmt" - "io/ioutil" + "io" "os" "strings" @@ -169,23 +169,27 @@ func showContext(client *client.Client, vcsType, orgName, contextName string) er return nil } -func readSecretValue() (string, error) { +// ReadSecretValue reads a secret from a buffer +func ReadSecretValue() (string, error) { stat, _ := os.Stdin.Stat() + + buffSize, err := os.Stdin.Stat() + if err != nil { + return "", err + } + + reader := bufio.NewReaderSize(os.Stdin, int(buffSize.Size())) + if (stat.Mode() & os.ModeCharDevice) == 0 { - bytes, err := ioutil.ReadAll(os.Stdin) + bytes := make([]byte, buffSize.Size()) + _, err := io.ReadFull(reader, bytes) return string(bytes), err - } else { - fmt.Print("Enter secret value and press enter: ") + } - buffSize, err := os.Stdin.Stat() - if err != nil { - return "", err - } + fmt.Print("Enter secret value and press enter: ") - reader := bufio.NewReaderSize(os.Stdin, int(buffSize.Size())) - str, err := reader.ReadString('\n') - return strings.TrimRight(str, "\n"), err - } + str, err := reader.ReadString('\n') + return strings.TrimRight(str, "\n"), err } func createContext(client *client.Client, vcsType, orgName, contextName string) error { @@ -207,7 +211,7 @@ func storeEnvVar(client *client.Client, vcsType, orgName, contextName, varName s if err != nil { return err } - secretValue, err := readSecretValue() + secretValue, err := ReadSecretValue() if err != nil { return errors.Wrap(err, "Failed to read secret value from stdin") diff --git a/cmd/context_test.go b/cmd/context_test.go index 934b8b42c..9c0053031 100644 --- a/cmd/context_test.go +++ b/cmd/context_test.go @@ -1,9 +1,13 @@ package cmd_test import ( + "io/ioutil" + "log" + "os" "os/exec" "github.com/CircleCI-Public/circleci-cli/clitest" + "github.com/CircleCI-Public/circleci-cli/cmd" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "github.com/onsi/gomega/gbytes" @@ -36,6 +40,31 @@ You can create a new personal API token here: https://circleci.com/account/api`)) Eventually(session).Should(clitest.ShouldFail()) }) + + It("ReadSecretValue should read correctly os.Stdin wihout given a error", func() { + By("running the command") + content := []byte("stdin mock!") + tempFile, err := ioutil.TempFile("", "stdin-mock") + if err != nil { + log.Fatalln(err) + } + + if _, err := tempFile.Write(content); err != nil { + log.Fatal(err) + } + + if _, err := tempFile.Seek(0, 0); err != nil { + log.Fatal(err) + } + + defer os.Remove(tempFile.Name()) + + os.Stdin = tempFile + result, err := cmd.ReadSecretValue() + + Expect(err).ShouldNot(HaveOccurred()) + Expect(result).Should(Equal(string(content))) + }) }) // TODO: add integration tests for happy path cases