From a1bb029ad1375537862ea9060a4844cf65ba6761 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Sat, 17 Mar 2018 20:16:47 -0700 Subject: [PATCH 1/4] forward the remaining of the stdin args to the server --- arguments.go | 66 +++++++++++++++++++++++++++++++++++++++++++++++ cli/parse_test.go | 10 +++++-- command.go | 11 ++++---- http/client.go | 10 +++++-- request.go | 30 +++++++++++++++------ 5 files changed, 109 insertions(+), 18 deletions(-) create mode 100644 arguments.go diff --git a/arguments.go b/arguments.go new file mode 100644 index 00000000..8955df82 --- /dev/null +++ b/arguments.go @@ -0,0 +1,66 @@ +package cmds + +import ( + "bufio" + "io" +) + +// StdinArguments is used to iterate through arguments piped through stdin. +type StdinArguments interface { + io.ReadCloser + // Returns the next argument passed via stdin. + // + // This method will never return an error along with a value, it will + // return one or the other. + // + // Once all arguments have been read, it will return "", io.EOF + Next() (string, error) +} + +type arguments struct { + reader *bufio.Reader + closer io.Closer +} + +func newArguments(r io.ReadCloser) *arguments { + return &arguments{ + reader: bufio.NewReader(r), + closer: r, + } +} + +// Read implements the io.Reader interface +func (a *arguments) Read(b []byte) (int, error) { + return a.reader.Read(b) +} + +// Close implements the io.Closer interface +func (a *arguments) Close() error { + return a.closer.Close() +} + +// WriteTo implements the io.WriterTo interface +func (a *arguments) WriteTo(w io.Writer) (int64, error) { + return a.reader.WriteTo(w) +} + +// Next returns the next argument +func (a *arguments) Next() (string, error) { + s, err := a.reader.ReadString('\n') + switch err { + case io.EOF: + if s == "" { + return "", io.EOF + } + // drop the error. + return s, nil + case nil: + l := len(s) + if l >= 2 && s[l-2] == '\r' { + return s[:l-2], nil + } + return s[:l-1], nil + default: + return "", err + } +} diff --git a/cli/parse_test.go b/cli/parse_test.go index d811a37b..1bf68789 100644 --- a/cli/parse_test.go +++ b/cli/parse_test.go @@ -503,8 +503,14 @@ func TestBodyArgs(t *testing.T) { } var bodyArgs words - for s.Scan() { - bodyArgs = append(bodyArgs, s.Text()) + for { + next, err := s.Next() + if err == io.EOF { + break + } else if err != nil { + t.Fatal(err) + } + bodyArgs = append(bodyArgs, next) } if !sameWords(bodyArgs, tc.varArgs) { diff --git a/command.go b/command.go index 0fd4a7dc..4be0e71f 100644 --- a/command.go +++ b/command.go @@ -9,7 +9,6 @@ output to the user, including text, JSON, and XML marshallers. package cmds import ( - "bufio" "errors" "fmt" "io" @@ -234,7 +233,7 @@ func (c *Command) CheckArguments(req *Request) error { switch err { case io.EOF: case nil: - req.bodyArgs = bufio.NewScanner(fi) + req.bodyArgs = newArguments(fi) // Can't pass files and stdin arguments. req.Files = nil default: @@ -267,13 +266,13 @@ func (c *Command) CheckArguments(req *Request) error { // Can we get it from stdin? if argDef.SupportsStdin && req.bodyArgs != nil { - if req.bodyArgs.Scan() { + next, err := req.bodyArgs.Next() + if err == nil { // Found it! - req.Arguments = append(req.Arguments, req.bodyArgs.Text()) + req.Arguments = append(req.Arguments, next) continue } - // Nope! Maybe we had a read error? - if err := req.bodyArgs.Err(); err != nil { + if err != io.EOF { return err } // No, just missing. diff --git a/http/client.go b/http/client.go index 690c1405..e8a5b7da 100644 --- a/http/client.go +++ b/http/client.go @@ -138,8 +138,14 @@ func (c *client) Send(req *cmds.Request) (cmds.Response, error) { var fileReader *files.MultiFileReader var reader io.Reader - - if req.Files != nil { + if bodyArgs := req.BodyArgs(); bodyArgs != nil { + // In the end, this wraps a file reader in a file reader. + // However, such is life. + fileReader = files.NewMultiFileReader(files.NewSliceFile("", "", []files.File{ + files.NewReaderFile("stdin", "", bodyArgs, nil), + }), true) + reader = fileReader + } else if req.Files != nil { fileReader = files.NewMultiFileReader(req.Files, true) reader = fileReader } diff --git a/request.go b/request.go index 96f1b48f..9177f501 100644 --- a/request.go +++ b/request.go @@ -1,9 +1,9 @@ package cmds import ( - "bufio" "context" "fmt" + "io" "reflect" "github.com/ipfs/go-ipfs-cmdkit" @@ -21,7 +21,7 @@ type Request struct { Files files.File - bodyArgs *bufio.Scanner + bodyArgs *arguments } // NewRequest returns a request initialized with given arguments @@ -50,8 +50,17 @@ func NewRequest(ctx context.Context, path []string, opts cmdkit.OptMap, args []s } // BodyArgs returns a scanner that returns arguments passed in the body as tokens. -func (req *Request) BodyArgs() *bufio.Scanner { - return req.bodyArgs +// +// Returns nil if there are no arguments to be consumed via stdin. +func (req *Request) BodyArgs() StdinArguments { + // dance to make sure we return an *untyped* nil. + // DO NOT just return `req.bodyArgs`. + // If you'd like to complain, go to + // https://github.com/golang/go/issues/. + if req.bodyArgs != nil { + return req.bodyArgs + } + return nil } func (req *Request) ParseBodyArgs() error { @@ -60,11 +69,16 @@ func (req *Request) ParseBodyArgs() error { return nil } - for s.Scan() { - req.Arguments = append(req.Arguments, s.Text()) + for { + next, err := s.Next() + if err != nil { + if err == io.EOF { + return nil + } + return err + } + req.Arguments = append(req.Arguments, next) } - - return s.Err() } func (req *Request) SetOption(name string, value interface{}) { From 31223407b9edce17fe2fd4e01c8db01c5fdb1c48 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Sun, 18 Mar 2018 11:08:28 -0700 Subject: [PATCH 2/4] make StdinArguments interface mimic bufio.Scanner --- arguments.go | 82 +++++++++++++++++++++++++++++++---------------- cli/parse_test.go | 13 +++----- command.go | 7 ++-- request.go | 13 ++------ 4 files changed, 65 insertions(+), 50 deletions(-) diff --git a/arguments.go b/arguments.go index 8955df82..661a2226 100644 --- a/arguments.go +++ b/arguments.go @@ -6,20 +6,28 @@ import ( ) // StdinArguments is used to iterate through arguments piped through stdin. +// +// It closely mimics the bufio.Scanner interface but also implements the +// ReadCloser interface. type StdinArguments interface { io.ReadCloser - // Returns the next argument passed via stdin. - // - // This method will never return an error along with a value, it will - // return one or the other. - // - // Once all arguments have been read, it will return "", io.EOF - Next() (string, error) + + // Scan reads in the next argument and returns true if there is an + // argument to read. + Scan() bool + + // Argument returns the next argument. + Argument() string + + // Err returns any errors encountered when reading in arguments. + Err() error } type arguments struct { - reader *bufio.Reader - closer io.Closer + argument string + err error + reader *bufio.Reader + closer io.Closer } func newArguments(r io.ReadCloser) *arguments { @@ -29,38 +37,56 @@ func newArguments(r io.ReadCloser) *arguments { } } -// Read implements the io.Reader interface +// Read implements the io.Reader interface. func (a *arguments) Read(b []byte) (int, error) { return a.reader.Read(b) } -// Close implements the io.Closer interface +// Close implements the io.Closer interface. func (a *arguments) Close() error { return a.closer.Close() } -// WriteTo implements the io.WriterTo interface +// WriteTo implements the io.WriterTo interface. func (a *arguments) WriteTo(w io.Writer) (int64, error) { return a.reader.WriteTo(w) } -// Next returns the next argument -func (a *arguments) Next() (string, error) { +// Err returns any errors encountered when reading in arguments. +func (a *arguments) Err() error { + if a.err == io.EOF { + return nil + } + return a.err +} + +// Argument returns the last argument read in. +func (a *arguments) Argument() string { + return a.argument +} + +// Scan reads in the next argument and returns true if there is an +// argument to read. +func (a *arguments) Scan() bool { + if a.err != nil { + return false + } + s, err := a.reader.ReadString('\n') - switch err { - case io.EOF: - if s == "" { - return "", io.EOF + if err != nil { + a.err = err + if err == io.EOF && len(s) > 0 { + a.argument = s + return true } - // drop the error. - return s, nil - case nil: - l := len(s) - if l >= 2 && s[l-2] == '\r' { - return s[:l-2], nil - } - return s[:l-1], nil - default: - return "", err + return false + } + + l := len(s) + if l >= 2 && s[l-2] == '\r' { + a.argument = s[:l-2] + } else { + a.argument = s[:l-1] } + return true } diff --git a/cli/parse_test.go b/cli/parse_test.go index 1bf68789..3da47985 100644 --- a/cli/parse_test.go +++ b/cli/parse_test.go @@ -503,14 +503,11 @@ func TestBodyArgs(t *testing.T) { } var bodyArgs words - for { - next, err := s.Next() - if err == io.EOF { - break - } else if err != nil { - t.Fatal(err) - } - bodyArgs = append(bodyArgs, next) + for s.Scan() { + bodyArgs = append(bodyArgs, s.Argument()) + } + if err := s.Err(); err != nil { + t.Fatal(err) } if !sameWords(bodyArgs, tc.varArgs) { diff --git a/command.go b/command.go index 4be0e71f..a2a26c5c 100644 --- a/command.go +++ b/command.go @@ -266,13 +266,12 @@ func (c *Command) CheckArguments(req *Request) error { // Can we get it from stdin? if argDef.SupportsStdin && req.bodyArgs != nil { - next, err := req.bodyArgs.Next() - if err == nil { + if req.bodyArgs.Scan() { // Found it! - req.Arguments = append(req.Arguments, next) + req.Arguments = append(req.Arguments, req.bodyArgs.Argument()) continue } - if err != io.EOF { + if err := req.bodyArgs.Err(); err != nil { return err } // No, just missing. diff --git a/request.go b/request.go index 9177f501..2784ffa0 100644 --- a/request.go +++ b/request.go @@ -3,7 +3,6 @@ package cmds import ( "context" "fmt" - "io" "reflect" "github.com/ipfs/go-ipfs-cmdkit" @@ -69,16 +68,10 @@ func (req *Request) ParseBodyArgs() error { return nil } - for { - next, err := s.Next() - if err != nil { - if err == io.EOF { - return nil - } - return err - } - req.Arguments = append(req.Arguments, next) + for s.Scan() { + req.Arguments = append(req.Arguments, s.Argument()) } + return s.Err() } func (req *Request) SetOption(name string, value interface{}) { From 49a26f3bae69a097cc2f28e2052bd5f2a5dbea91 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Sun, 18 Mar 2018 12:21:52 -0700 Subject: [PATCH 3/4] add a bunch of tests cases for stdin arguments --- arguments_test.go | 112 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 arguments_test.go diff --git a/arguments_test.go b/arguments_test.go new file mode 100644 index 00000000..e9ed47ca --- /dev/null +++ b/arguments_test.go @@ -0,0 +1,112 @@ +package cmds + +import ( + "bytes" + "io/ioutil" + "testing" +) + +func TestArguments(t *testing.T) { + var testCases = []struct { + input string + arguments []string + }{ + { + input: "", + arguments: []string{}, + }, + { + input: "\n", + arguments: []string{""}, + }, + { + input: "\r\n", + arguments: []string{""}, + }, + { + input: "\r", + arguments: []string{"\r"}, + }, + { + input: "one", + arguments: []string{"one"}, + }, + { + input: "one\n", + arguments: []string{"one"}, + }, + { + input: "one\r\n", + arguments: []string{"one"}, + }, + { + input: "one\r", + arguments: []string{"one\r"}, + }, + { + input: "one\n\ntwo", + arguments: []string{"one", "", "two"}, + }, + { + input: "first\nsecond\nthird", + arguments: []string{"first", "second", "third"}, + }, + { + input: "first\r\nsecond\nthird", + arguments: []string{"first", "second", "third"}, + }, + { + input: "first\nsecond\nthird\n", + arguments: []string{"first", "second", "third"}, + }, + { + input: "first\r\nsecond\r\nthird\r\n", + arguments: []string{"first", "second", "third"}, + }, + { + input: "first\nsecond\nthird\n\n", + arguments: []string{"first", "second", "third", ""}, + }, + { + input: "\nfirst\nsecond\nthird\n", + arguments: []string{"", "first", "second", "third"}, + }, + } + + for i, tc := range testCases { + for cut := 0; cut <= len(tc.arguments); cut++ { + args := newArguments(ioutil.NopCloser(bytes.NewBufferString(tc.input))) + for j, arg := range tc.arguments[:cut] { + if !args.Scan() { + t.Errorf("in test case %d, missing argument %d", i, j) + continue + } + got := args.Argument() + if got != arg { + t.Errorf("in test case %d, expected argument %d to be %s, got %s", i, j, arg, got) + } + if args.Err() != nil { + t.Error(args.Err()) + } + } + args = newArguments(args) + // Tests stopping in the middle. + for j, arg := range tc.arguments[cut:] { + if !args.Scan() { + t.Errorf("in test case %d, missing argument %d", i, j+cut) + continue + } + got := args.Argument() + if got != arg { + t.Errorf("in test case %d, expected argument %d to be %s, got %s", i, j+cut, arg, got) + } + if args.Err() != nil { + t.Error(args.Err()) + } + } + if args.Scan() { + t.Errorf("in test case %d, got too many arguments", i) + } + } + } +} From 9160bdcc1601eda9bb3f569d51f26751d36a7e33 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Sun, 18 Mar 2018 12:34:46 -0700 Subject: [PATCH 4/4] don't call Fatal from a goroutine Apparently, it's not thread-safe. --- command_test.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/command_test.go b/command_test.go index 98a147ab..a976a6c6 100644 --- a/command_test.go +++ b/command_test.go @@ -383,9 +383,10 @@ func TestCancel(t *testing.T) { go func() { err := re.Emit("abc") if err != context.Canceled { - t.Fatalf("re: expected context.Canceled but got %v", err) + t.Errorf("re: expected context.Canceled but got %v", err) + } else { + t.Log("re.Emit err:", err) } - t.Log("re.Emit err:", err) re.Close() close(wait) }() @@ -394,8 +395,9 @@ func TestCancel(t *testing.T) { _, err = res.Next() if err != context.Canceled { - t.Fatalf("res: expected context.Canceled but got %v", err) + t.Errorf("res: expected context.Canceled but got %v", err) + } else { + t.Log("res.Emit err:", err) } - t.Log("res.Emit err:", err) <-wait }