use common shell quoting utilityarnej/use-our-shell-quote

* only suited for printable ascii
* always single-quotes if quoting needed
* may quote more than strictly necessary

7 files changed, 101 insertions, 61 deletions

diff --git a/client/go/internal/cli/cmd/document_test.go b/client/go/internal/cli/cmd/document_test.go
index bf9cc0404dc..214ab60c76f 100644
--- a/client/go/internal/cli/cmd/document_test.go
+++ b/client/go/internal/cli/cmd/document_test.go
@@ -113,7 +113,7 @@ func assertDocumentSend(arguments []string, expectedOperation string, expectedMe
 		}
 	}
 	if verbose {
-		expectedCurl := "curl -X " + expectedMethod + " -H 'Content-Type: application/json' --data-binary @" + expectedPayloadFile + " " + expectedURL + "\n"
+		expectedCurl := "curl -X " + expectedMethod + " -H 'Content-Type: application/json' --data-binary '@" + expectedPayloadFile + "' " + expectedURL + "\n"
 		assert.Equal(t, expectedCurl, stderr.String())
 	}
 	assert.Equal(t, "Success: "+expectedOperation+" "+expectedDocumentId+"\n", stdout.String())
diff --git a/client/go/internal/cli/cmd/query_test.go b/client/go/internal/cli/cmd/query_test.go
index 94b5a485b9d..1caf6d33e70 100644
--- a/client/go/internal/cli/cmd/query_test.go
+++ b/client/go/internal/cli/cmd/query_test.go
@@ -27,7 +27,7 @@ func TestQueryVerbose(t *testing.T) {
 	cli.httpClient = client
 
 	assert.Nil(t, cli.Run("query", "-v", "select from sources * where title contains 'foo'"))
-	assert.Equal(t, "curl http://127.0.0.1:8080/search/\\?timeout=10s\\&yql=select+from+sources+%2A+where+title+contains+%27foo%27\n", stderr.String())
+	assert.Equal(t, "curl 'http://127.0.0.1:8080/search/?timeout=10s&yql=select+from+sources+%2A+where+title+contains+%27foo%27'\n", stderr.String())
 	assert.Equal(t, "{\n    \"query\": \"result\"\n}\n", stdout.String())
 }
 
diff --git a/client/go/internal/curl/curl.go b/client/go/internal/curl/curl.go
index b70e0f824a3..0ee21a7f4e1 100644
--- a/client/go/internal/curl/curl.go
+++ b/client/go/internal/curl/curl.go
@@ -7,7 +7,6 @@ import (
 	"os/exec"
 	"runtime"
 
-	"github.com/kballard/go-shellquote"
 	"github.com/vespa-engine/vespa/client/go/internal/util"
 )
 
@@ -77,7 +76,7 @@ func (c *Command) Args() []string {
 func (c *Command) String() string {
 	args := []string{c.Path}
 	args = append(args, c.Args()...)
-	return shellquote.Join(args...)
+	return util.ShellQuoteArgs(args...)
 }
 
 func (c *Command) Header(key, value string) {
diff --git a/client/go/internal/curl/curl_test.go b/client/go/internal/curl/curl_test.go
index 448e1e5199f..a8ccade3866 100644
--- a/client/go/internal/curl/curl_test.go
+++ b/client/go/internal/curl/curl_test.go
@@ -17,7 +17,7 @@ func TestPost(t *testing.T) {
 	c.WithBodyFile("file.json")
 	c.Header("Content-Type", "application/json")
 
-	assert.Equal(t, "curl --key key.pem --cert cert.pem -X POST -H 'Content-Type: application/json' --data-binary @file.json https://example.com", c.String())
+	assert.Equal(t, "curl --key key.pem --cert cert.pem -X POST -H 'Content-Type: application/json' --data-binary '@file.json' https://example.com", c.String())
 }
 
 func TestGet(t *testing.T) {
@@ -30,7 +30,7 @@ func TestGet(t *testing.T) {
 	c.Param("yql", "select * from sources * where title contains 'foo';")
 	c.Param("hits", "5")
 
-	assert.Equal(t, `curl --key key.pem --cert cert.pem https://example.com\?hits=5\&yql=select+%2A+from+sources+%2A+where+title+contains+%27foo%27%3B`, c.String())
+	assert.Equal(t, `curl --key key.pem --cert cert.pem 'https://example.com?hits=5&yql=select+%2A+from+sources+%2A+where+title+contains+%27foo%27%3B'`, c.String())
 }
 
 func TestRawArgs(t *testing.T) {
diff --git a/client/go/internal/util/shell_quote.go b/client/go/internal/util/shell_quote.go
new file mode 100644
index 00000000000..f4ee01ea6b8
--- /dev/null
+++ b/client/go/internal/util/shell_quote.go
@@ -0,0 +1,68 @@
+// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+// load default environment variables (from $VESPA_HOME/conf/vespa/default-env.txt)
+// Author: arnej
+
+package util
+
+import (
+	"bytes"
+)
+
+func needQuoting(s string) bool {
+	for _, ch := range s {
+		switch {
+		case (ch >= 'A' && ch <= 'Z'):
+		case (ch >= 'a' && ch <= 'z'):
+		case (ch >= '0' && ch <= '9'):
+		case ch == '-':
+		case ch == '_':
+		case ch == '.':
+		case ch == ':':
+		case ch == '/':
+			// all above: nop
+		default:
+			return true
+		}
+	}
+	return false
+}
+
+func quote(s string, t *bytes.Buffer) {
+	const singleQuote = '\''
+	if !needQuoting(s) {
+		t.WriteString(s)
+		return
+	}
+	t.WriteByte(singleQuote)
+	for _, ch := range s {
+		switch {
+		case ch == '\'' || ch == '\\':
+			t.WriteByte(singleQuote)
+			t.WriteByte('\\')
+			t.WriteByte(byte(ch))
+			t.WriteByte(singleQuote)
+		case ch < 32 || ch > 127:
+			t.WriteByte('?')
+		default:
+			t.WriteByte(byte(ch))
+		}
+	}
+	t.WriteByte(singleQuote)
+}
+
+func ShellQuoteArgs(args ...string) string {
+	var buf bytes.Buffer
+	for idx, s := range args {
+		if idx > 0 {
+			buf.WriteByte(' ')
+		}
+		quote(s, &buf)
+	}
+	return buf.String()
+}
+
+func ShellQuote(s string) string {
+	var buf bytes.Buffer
+	quote(s, &buf)
+	return buf.String()
+}
diff --git a/client/go/internal/util/shell_quote_test.go b/client/go/internal/util/shell_quote_test.go
new file mode 100644
index 00000000000..7a90a0c0d3b
--- /dev/null
+++ b/client/go/internal/util/shell_quote_test.go
@@ -0,0 +1,27 @@
+// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package util
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestShellQuote(t *testing.T) {
+	assert.Equal(t, "foo", ShellQuote("foo"))
+	assert.Equal(t, "'foo bar'", ShellQuote("foo bar"))
+	assert.Equal(t, "foo-bar", ShellQuote("foo-bar"))
+	assert.Equal(t, "foo_bar", ShellQuote("foo_bar"))
+	assert.Equal(t, "' foo'", ShellQuote(" foo"))
+	assert.Equal(t, "'foo '", ShellQuote("foo "))
+	assert.Equal(t, "' foo '", ShellQuote(" foo "))
+	assert.Equal(t, `'a'\''b'`, ShellQuote("a'b"))
+	assert.Equal(t, `''\'''`, ShellQuote("'"))
+	assert.Equal(t, `'"'`, ShellQuote(`"`))
+	assert.Equal(t, `'z?z?z?z'`, ShellQuote("z\u2318z\tz\rz"))
+}
+
+func TestShellQuoteArgs(t *testing.T) {
+	assert.Equal(t, "foo 123 bar", ShellQuoteArgs("foo", "123", "bar"))
+	assert.Equal(t, "'fo oo' '12 3' 'b ar'", ShellQuoteArgs("fo oo", "12 3", "b ar"))
+}
diff --git a/client/go/internal/vespa/load_env.go b/client/go/internal/vespa/load_env.go
index 6c41c1fece5..7fffb2494fe 100644
--- a/client/go/internal/vespa/load_env.go
+++ b/client/go/internal/vespa/load_env.go
@@ -180,7 +180,7 @@ func (p *shellEnvExporter) fallbackVar(varName, varVal string) {
 }
 func (p *shellEnvExporter) overrideVar(varName, varVal string) {
 	delete(p.unsetVars, varName)
-	p.exportVars[varName] = shellQuote(varVal)
+	p.exportVars[varName] = util.ShellQuote(varVal)
 }
 func (p *shellEnvExporter) unsetVar(varName string) {
 	delete(p.exportVars, varName)
@@ -196,60 +196,6 @@ func (p *shellEnvExporter) currentValue(varName string) string {
 	return os.Getenv(varName)
 }
 
-func shellQuote(s string) string {
-	l := 0
-	nq := false
-	for _, ch := range s {
-		switch {
-		case (ch >= 'A' && ch <= 'Z') ||
-			(ch >= 'a' && ch <= 'z') ||
-			(ch >= '0' && ch <= '9'):
-			l++
-		case ch == '_' || ch == ' ':
-			l++
-			nq = true
-		case ch == '\'' || ch == '\\':
-			l = l + 4
-			nq = true
-		default:
-			l++
-			nq = true
-		}
-	}
-	if nq {
-		l = l + 2
-	}
-	res := make([]rune, l)
-	i := 0
-	if nq {
-		res[i] = '\''
-		i++
-	}
-	for _, ch := range s {
-		if ch == '\'' || ch == '\\' {
-			res[i] = '\''
-			i++
-			res[i] = '\\'
-			i++
-			res[i] = ch
-			i++
-			res[i] = '\''
-		} else {
-			res[i] = ch
-		}
-		i++
-	}
-	if nq {
-		res[i] = '\''
-		i++
-	}
-	if i != l {
-		err := fmt.Errorf("expected length %d but was %d", l, i)
-		util.JustExitWith(err)
-	}
-	return string(res)
-}
-
 func (p *shellEnvExporter) dump() {
 	for vn, vv := range p.exportVars {
 		fmt.Printf("%s=%s\n", vn, vv)