Repository: bitfield/script
Pull request #42 (2020-03-28 16:37:06)

Fix Exec to handle quotes
Issue #32 highlighted a problem with the simple approach (`strings.Fields`) to splitting a command string into a suitable slice of strings for `exec.Command`. A program like this:

```go
p := script.Exec("bash -c 'docker ps'")
```

didn't work as expected, because the actual slice of strings passed to the command was:

```go
{"bash", "-c", "'docker", "ps'"}
```

which causes the shell to choke. We need a slightly smarter approach which takes into account quoted strings, and doesn't split them:

```
{"bash", "-c", "docker ps"}
```

The [creachadair/shell](https://bitbucket.org/creachadair/shell) library provides a facility (`Split`) to do exactly this. Thanks to @shumin1027 for the original report, @posener for input, and @jrswab for an initial draft PR on this.



Patch:
diff --git a/README.md b/README.md
index 23ca9bb..f04048c 100644
--- a/README.md
+++ b/README.md
@@ -345,7 +345,7 @@ fmt.Println(output)
 `Exec()` runs a given command and creates a pipe containing its combined output (`stdout` and `stderr`). If there was an error running the command, the pipe's error status will be set.
 
 ```go
-p := script.Exec("echo hello")
+p := script.Exec("bash -c 'echo hello'")
 output, err := p.String()
 fmt.Println(output)
 // Output: hello
@@ -811,6 +811,7 @@ Since `script` is designed to help you write system administration programs, a f
 
 * [cat](examples/cat/main.go) (copies stdin to stdout)
 * [cat 2](examples/cat2/main.go) (takes a list of files on the command line and concatenates their contents to stdout)
+* [execute](examples/execute/main.go)
 * [grep](examples/grep/main.go)
 * [head](examples/head/main.go)
 * [echo](examples/echo/main.go)
diff --git a/examples/execute/main.go b/examples/execute/main.go
new file mode 100644
index 0000000..73a947a
--- /dev/null
+++ b/examples/execute/main.go
@@ -0,0 +1,9 @@
+package main
+
+import (
+	"github.com/bitfield/script"
+)
+
+func main() {
+	script.Exec("bash -c 'echo hello world'").Stdout()
+}
diff --git a/filters.go b/filters.go
index e4cfa1e..4c7b938 100644
--- a/filters.go
+++ b/filters.go
@@ -13,6 +13,8 @@ import (
 	"sort"
 	"strconv"
 	"strings"
+
+	"bitbucket.org/creachadair/shell"
 )
 
 // Basename reads a list of filepaths from the pipe, one per line, and removes
@@ -108,12 +110,15 @@ func (p *Pipe) EachLine(process func(string, *strings.Builder)) *Pipe {
 // Exec runs an external command and returns a pipe containing the output. If
 // the command had a non-zero exit status, the pipe's error status will also be
 // set to the string "exit status X", where X is the integer exit status.
-func (p *Pipe) Exec(s string) *Pipe {
+func (p *Pipe) Exec(cmdLine string) *Pipe {
 	if p == nil || p.Error() != nil {
 		return p
 	}
 	q := NewPipe()
-	args := strings.Fields(s)
+	args, ok := shell.Split(cmdLine) // strings.Fields doesn't handle quotes
+	if !ok {
+		return p.WithError(fmt.Errorf("unbalanced quotes or backslashes in [%s]", cmdLine))
+	}
 	cmd := exec.Command(args[0], args[1:]...)
 	cmd.Stdin = p.Reader
 	output, err := cmd.CombinedOutput()
diff --git a/go.mod b/go.mod
index 24ac343..faeebb7 100644
--- a/go.mod
+++ b/go.mod
@@ -2,4 +2,7 @@ module github.com/bitfield/script
 
 go 1.12
 
-require github.com/google/go-cmp v0.3.1
+require (
+	bitbucket.org/creachadair/shell v0.0.6
+	github.com/google/go-cmp v0.3.1
+)
diff --git a/go.sum b/go.sum
index a6ddb1d..49c8f07 100644
--- a/go.sum
+++ b/go.sum
@@ -1,2 +1,4 @@
+bitbucket.org/creachadair/shell v0.0.6 h1:reJflDbKqnlnqb4Oo2pQ1/BqmY/eCWcNGHrIUO8qIzc=
+bitbucket.org/creachadair/shell v0.0.6/go.mod h1:8Qqi/cYk7vPnsOePHroKXDJYmb5x7ENhtiFtfZq8K+M=
 github.com/google/go-cmp v0.3.1 h1:Xye71clBPdm5HgqGwUkwhbynsUJZhDbS20FvLhQ2izg=
 github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=


Test patch:
diff --git a/sources_test.go b/sources_test.go
index 289b503..6af612f 100644
--- a/sources_test.go
+++ b/sources_test.go
@@ -5,6 +5,7 @@ import (
 	"io/ioutil"
 	"os"
 	"os/exec"
+	"strings"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -43,38 +44,67 @@ func TestEcho(t *testing.T) {
 
 func TestExec(t *testing.T) {
 	t.Parallel()
-	want := "Usage"
-	p := Exec("go")
-	if p.Error() == nil {
-		t.Error("want error from command, but got nil")
-	}
-	if p.Error().Error() != "exit status 2" {
-		t.Errorf("want error 'exit status 2' but got %v", p.Error())
-	}
-	p.SetError(nil)
-	output, err := p.String()
-	if err != nil {
-		t.Error(err)
-	}
-	matches, err := Echo(output).Match(want).CountLines()
-	if err != nil {
-		t.Error(err)
-	}
-	if matches == 0 {
-		t.Errorf("want output of command to match %q, but no matches in %q", want, output)
-	}
-	q := Exec("doesntexist")
-	if q.Error() == nil {
-		t.Error("want error executing non-existent program, but got nil")
-	}
-	// ignoring error because we already checked it
-	output, _ = q.String()
-	if output != "" {
-		t.Errorf("want zero output from running non-existent program, but got %q", output)
-	}
-	r := Exec("go help")
-	if r.Error() != nil {
-		t.Errorf("want no error running 'go help', but got %v", r.Error())
+	tcs := []struct {
+		Command           string
+		ErrExpected       bool
+		WantErrContain    string
+		WantOutputContain string
+	}{
+		{
+			Command:           "doesntexist",
+			ErrExpected:       true,
+			WantErrContain:    "file not found",
+			WantOutputContain: "",
+		},
+		{
+			Command:           "go",
+			ErrExpected:       true,
+			WantErrContain:    "exit status 2",
+			WantOutputContain: "Usage",
+		},
+		{
+			Command:           "go help",
+			ErrExpected:       false,
+			WantErrContain:    "",
+			WantOutputContain: "Usage",
+		},
+		{
+			Command:           "sh -c 'echo hello'",
+			ErrExpected:       false,
+			WantErrContain:    "",
+			WantOutputContain: "hello\n",
+		},
+		{
+			Command:           "sh -c 'echo oh no",
+			ErrExpected:       true,
+			WantErrContain:    "",
+			WantOutputContain: "",
+		},
+		{
+			Command:           "sh -c 'sh -c \"echo inception\"'",
+			ErrExpected:       false,
+			WantErrContain:    "",
+			WantOutputContain: "inception\n",
+		},
+	}
+	for _, tc := range tcs {
+		t.Run(tc.Command, func(t *testing.T) {
+			p := Exec(tc.Command)
+			if tc.ErrExpected != (p.Error() != nil) {
+				t.Fatalf("unexpected error value: %v", p.Error())
+			}
+			if p.Error() != nil && !strings.Contains(p.Error().Error(), tc.WantErrContain) {
+				t.Fatalf("want error string %q to contain %q", p.Error().Error(), tc.WantErrContain)
+			}
+			p.SetError(nil) // else p.String() would be a no-op
+			output, err := p.String()
+			if err != nil {
+				t.Fatalf("unexpected error %v", err)
+			}
+			if !strings.Contains(output, tc.WantOutputContain) {
+				t.Fatalf("want output %q to contain %q", output, tc.WantOutputContain)
+			}
+		})
 	}
 }