From e48b027ac3b8e19f5ce3e16783431d3b2c022df9 Mon Sep 17 00:00:00 2001 From: HuKeping Date: Sat, 7 May 2016 18:11:51 +0800 Subject: [PATCH 1/6] Introduce flag --input on notary verify If --input(-i for short) was provided, we read the from a file, instead of STDIN. Signed-off-by: Hu Keping --- cmd/notary/tuf.go | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/cmd/notary/tuf.go b/cmd/notary/tuf.go index ff1b0c6e0..cbaa96544 100644 --- a/cmd/notary/tuf.go +++ b/cmd/notary/tuf.go @@ -91,6 +91,8 @@ type tufCommander struct { roles []string sha256 string sha512 string + + input string } func (t *tufCommander) AddToCommand(cmd *cobra.Command) { @@ -98,7 +100,6 @@ func (t *tufCommander) AddToCommand(cmd *cobra.Command) { cmd.AddCommand(cmdTufStatusTemplate.ToCommand(t.tufStatus)) cmd.AddCommand(cmdTufPublishTemplate.ToCommand(t.tufPublish)) cmd.AddCommand(cmdTufLookupTemplate.ToCommand(t.tufLookup)) - cmd.AddCommand(cmdTufVerifyTemplate.ToCommand(t.tufVerify)) cmdTufList := cmdTufListTemplate.ToCommand(t.tufList) cmdTufList.Flags().StringSliceVarP( @@ -118,6 +119,10 @@ func (t *tufCommander) AddToCommand(cmd *cobra.Command) { cmdTufAddHash.Flags().StringVar(&t.sha256, notary.SHA256, "", "hex encoded sha256 of the target to add") cmdTufAddHash.Flags().StringVar(&t.sha512, notary.SHA512, "", "hex encoded sha512 of the target to add") cmd.AddCommand(cmdTufAddHash) + + cmdTufVerify := cmdTufVerifyTemplate.ToCommand(t.tufVerify) + cmdTufVerify.Flags().StringVarP(&t.input, "input", "i", "", "Read from a file, instead of STDIN") + cmd.AddCommand(cmdTufVerify) } func (t *tufCommander) tufAddByHash(cmd *cobra.Command, args []string) error { @@ -482,10 +487,21 @@ func (t *tufCommander) tufVerify(cmd *cobra.Command, args []string) error { return err } - // Reads all of the data on STDIN - payload, err := ioutil.ReadAll(os.Stdin) - if err != nil { - return fmt.Errorf("Error reading content from STDIN: %v", err) + var payload []byte + if t.input != "" { + // Reads from the given file + // + // Please be noticed that ReadFile will cut off the size if it was over 1e9. + // Thus, if the size of the file exceeds 1GB, the over part will not be + // loaded into the buffer. + if payload, err = ioutil.ReadFile(t.input); err != nil { + return err + } + } else { + // Reads all of the data on STDIN + if payload, err = ioutil.ReadAll(os.Stdin); err != nil { + return fmt.Errorf("Error reading content from STDIN: %v", err) + } } gun := args[0] From 75ac7f6f7255f484fd7992e62eddb65b01d79f6b Mon Sep 17 00:00:00 2001 From: HuKeping Date: Sat, 7 May 2016 19:26:40 +0800 Subject: [PATCH 2/6] Introduce flag --output on notary verify If --output(-o for short) was provided, we write to a file, instead of STDOUT. Signed-off-by: Hu Keping --- cmd/notary/tuf.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/cmd/notary/tuf.go b/cmd/notary/tuf.go index cbaa96544..f67c5c870 100644 --- a/cmd/notary/tuf.go +++ b/cmd/notary/tuf.go @@ -92,7 +92,8 @@ type tufCommander struct { sha256 string sha512 string - input string + input string + output string } func (t *tufCommander) AddToCommand(cmd *cobra.Command) { @@ -122,6 +123,7 @@ func (t *tufCommander) AddToCommand(cmd *cobra.Command) { cmdTufVerify := cmdTufVerifyTemplate.ToCommand(t.tufVerify) cmdTufVerify.Flags().StringVarP(&t.input, "input", "i", "", "Read from a file, instead of STDIN") + cmdTufVerify.Flags().StringVarP(&t.output, "output", "o", "", "Write to a file, instead of STDOUT") cmd.AddCommand(cmdTufVerify) } @@ -532,7 +534,14 @@ func (t *tufCommander) tufVerify(cmd *cobra.Command, args []string) error { return fmt.Errorf("data not present in the trusted collection, %v", err) } - _, _ = os.Stdout.Write(payload) + if t.output != "" { + if err := ioutil.WriteFile(t.output, payload, 0644); err != nil { + return err + } + } else { + _, _ = os.Stdout.Write(payload) + } + return nil } From 747a296e86f257b5a8a7cc6643e74cc804c9488f Mon Sep 17 00:00:00 2001 From: HuKeping Date: Sat, 7 May 2016 19:46:03 +0800 Subject: [PATCH 3/6] Introduce flag --quiet on notary verify If --quiet(-q for short) was provided, we output nothing except errors. Signed-off-by: Hu Keping --- cmd/notary/tuf.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/cmd/notary/tuf.go b/cmd/notary/tuf.go index f67c5c870..969827ad0 100644 --- a/cmd/notary/tuf.go +++ b/cmd/notary/tuf.go @@ -94,6 +94,7 @@ type tufCommander struct { input string output string + quiet bool } func (t *tufCommander) AddToCommand(cmd *cobra.Command) { @@ -124,6 +125,7 @@ func (t *tufCommander) AddToCommand(cmd *cobra.Command) { cmdTufVerify := cmdTufVerifyTemplate.ToCommand(t.tufVerify) cmdTufVerify.Flags().StringVarP(&t.input, "input", "i", "", "Read from a file, instead of STDIN") cmdTufVerify.Flags().StringVarP(&t.output, "output", "o", "", "Write to a file, instead of STDOUT") + cmdTufVerify.Flags().BoolVarP(&t.quiet, "quiet", "q", false, "No output except for errors") cmd.AddCommand(cmdTufVerify) } @@ -534,6 +536,12 @@ func (t *tufCommander) tufVerify(cmd *cobra.Command, args []string) error { return fmt.Errorf("data not present in the trusted collection, %v", err) } + // We only get here when everythings goes well, since the flag "quiet" was + // provided, we output nothing but just return. + if t.quiet { + return nil + } + if t.output != "" { if err := ioutil.WriteFile(t.output, payload, 0644); err != nil { return err From 3b2d9e969f34e32529f6fd6b5bdd8fe30b9f4ca7 Mon Sep 17 00:00:00 2001 From: HuKeping Date: Sat, 7 May 2016 20:29:59 +0800 Subject: [PATCH 4/6] Add a file to keep all the helper functions To prevent the tuf.go growing even more bigger, and refactor some code. Signed-off-by: Hu Keping --- cmd/notary/tuf.go | 35 ++++--------------------------- cmd/notary/util.go | 51 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 31 deletions(-) create mode 100644 cmd/notary/util.go diff --git a/cmd/notary/tuf.go b/cmd/notary/tuf.go index 969827ad0..f49280789 100644 --- a/cmd/notary/tuf.go +++ b/cmd/notary/tuf.go @@ -4,7 +4,6 @@ import ( "bufio" "encoding/hex" "fmt" - "io/ioutil" "net" "net/http" "net/url" @@ -491,21 +490,9 @@ func (t *tufCommander) tufVerify(cmd *cobra.Command, args []string) error { return err } - var payload []byte - if t.input != "" { - // Reads from the given file - // - // Please be noticed that ReadFile will cut off the size if it was over 1e9. - // Thus, if the size of the file exceeds 1GB, the over part will not be - // loaded into the buffer. - if payload, err = ioutil.ReadFile(t.input); err != nil { - return err - } - } else { - // Reads all of the data on STDIN - if payload, err = ioutil.ReadAll(os.Stdin); err != nil { - return fmt.Errorf("Error reading content from STDIN: %v", err) - } + payload, err := getPayload(t) + if err != nil { + return err } gun := args[0] @@ -536,21 +523,7 @@ func (t *tufCommander) tufVerify(cmd *cobra.Command, args []string) error { return fmt.Errorf("data not present in the trusted collection, %v", err) } - // We only get here when everythings goes well, since the flag "quiet" was - // provided, we output nothing but just return. - if t.quiet { - return nil - } - - if t.output != "" { - if err := ioutil.WriteFile(t.output, payload, 0644); err != nil { - return err - } - } else { - _, _ = os.Stdout.Write(payload) - } - - return nil + return feedback(t, payload) } type passwordStore struct { diff --git a/cmd/notary/util.go b/cmd/notary/util.go new file mode 100644 index 000000000..1a71f14b6 --- /dev/null +++ b/cmd/notary/util.go @@ -0,0 +1,51 @@ +package main + +import ( + "fmt" + "io/ioutil" + "os" +) + +// getPayload is a helper function to get the content used to be verified +// either from an existing file or STDIN. +func getPayload(t *tufCommander) ([]byte, error) { + + // Reads from the given file + if t.input != "" { + // Please be noticed that ReadFile will cut off the size if it was over 1e9. + // Thus, if the size of the file exceeds 1GB, the over part will not be + // loaded into the buffer. + payload, err := ioutil.ReadFile(t.input) + if err != nil { + return nil, err + } + return payload, nil + } + + // Reads all of the data on STDIN + payload, err := ioutil.ReadAll(os.Stdin) + if err != nil { + return nil, fmt.Errorf("Error reading content from STDIN: %v", err) + } + return payload, nil +} + +// feedback is a helper function to print the payload to a file or STDOUT or keep quiet +// due to the value of flag "quiet" and "output". +func feedback(t *tufCommander, payload []byte) error { + // We only get here when everythings goes well, since the flag "quiet" was + // provided, we output nothing but just return. + if t.quiet { + return nil + } + + // Flag "quiet" was not "true", that's why we get here. + if t.output != "" { + if err := ioutil.WriteFile(t.output, payload, 0644); err != nil { + return err + } + } + + os.Stdout.Write(payload) + return nil +} From 0f5d22f95f810aa3234e3e6f9b9a62a754ce628d Mon Sep 17 00:00:00 2001 From: HuKeping Date: Sat, 7 May 2016 21:25:26 +0800 Subject: [PATCH 5/6] Add test for those helper functions Signed-off-by: Hu Keping --- cmd/notary/util_test.go | 54 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 cmd/notary/util_test.go diff --git a/cmd/notary/util_test.go b/cmd/notary/util_test.go new file mode 100644 index 000000000..8949ffd4c --- /dev/null +++ b/cmd/notary/util_test.go @@ -0,0 +1,54 @@ +package main + +import ( + "fmt" + "io/ioutil" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestGetPayload(t *testing.T) { + tempDir, err := ioutil.TempDir("", "test-get-payload") + require.NoError(t, err) + defer os.RemoveAll(tempDir) + + file, err := os.Create(filepath.Join(tempDir, "content.txt")) + require.NoError(t, err) + + fmt.Fprintf(file, "Release date: June 10, 2016 - Director: Duncan Jones") + file.Close() + + commander := &tufCommander{ + input: file.Name(), + } + + payload, err := getPayload(commander) + require.NoError(t, err) + require.Equal(t, "Release date: June 10, 2016 - Director: Duncan Jones", string(payload)) +} + +func TestFeedback(t *testing.T) { + tempDir, err := ioutil.TempDir("", "test-feedback") + require.NoError(t, err) + defer os.RemoveAll(tempDir) + + file, err := os.Create(filepath.Join(tempDir, "content.txt")) + require.NoError(t, err) + + // Expect it to print nothing since "quiet" takes priority. + commander := &tufCommander{ + output: file.Name(), + quiet: true, + } + + payload := []byte("Release date: June 10, 2016 - Director: Duncan Jones") + err = feedback(commander, payload) + require.NoError(t, err) + + content, err := ioutil.ReadFile(file.Name()) + require.NoError(t, err) + require.Equal(t, "", string(content)) +} From 31a5ebcf4b4ecd02f37a0c67edae5ed04e949e44 Mon Sep 17 00:00:00 2001 From: HuKeping Date: Tue, 10 May 2016 09:47:33 +0800 Subject: [PATCH 6/6] From riyazdf's review Prior to this patch, if we provide a value to the output flag, we write both to the file as well as stdout which is not consistent with the flag description and also not the intended. Signed-off-by: Hu Keping --- cmd/notary/util.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/cmd/notary/util.go b/cmd/notary/util.go index 1a71f14b6..887204f08 100644 --- a/cmd/notary/util.go +++ b/cmd/notary/util.go @@ -12,7 +12,7 @@ func getPayload(t *tufCommander) ([]byte, error) { // Reads from the given file if t.input != "" { - // Please be noticed that ReadFile will cut off the size if it was over 1e9. + // Please note that ReadFile will cut off the size if it was over 1e9. // Thus, if the size of the file exceeds 1GB, the over part will not be // loaded into the buffer. payload, err := ioutil.ReadFile(t.input) @@ -33,7 +33,7 @@ func getPayload(t *tufCommander) ([]byte, error) { // feedback is a helper function to print the payload to a file or STDOUT or keep quiet // due to the value of flag "quiet" and "output". func feedback(t *tufCommander, payload []byte) error { - // We only get here when everythings goes well, since the flag "quiet" was + // We only get here when everything goes well, since the flag "quiet" was // provided, we output nothing but just return. if t.quiet { return nil @@ -41,9 +41,7 @@ func feedback(t *tufCommander, payload []byte) error { // Flag "quiet" was not "true", that's why we get here. if t.output != "" { - if err := ioutil.WriteFile(t.output, payload, 0644); err != nil { - return err - } + return ioutil.WriteFile(t.output, payload, 0644) } os.Stdout.Write(payload)