From 66d685830373c716fdf740c3373706056b93ad65 Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Thu, 2 Mar 2023 20:00:11 +0000 Subject: use common shell quoting utility * only suited for printable ascii * always single-quotes if quoting needed * may quote more than strictly necessary --- client/go/internal/cli/cmd/document_test.go | 2 +- client/go/internal/cli/cmd/query_test.go | 2 +- client/go/internal/curl/curl.go | 3 +- client/go/internal/curl/curl_test.go | 4 +- client/go/internal/util/shell_quote.go | 68 +++++++++++++++++++++++++++++ client/go/internal/util/shell_quote_test.go | 27 ++++++++++++ client/go/internal/vespa/load_env.go | 56 +----------------------- 7 files changed, 101 insertions(+), 61 deletions(-) create mode 100644 client/go/internal/util/shell_quote.go create mode 100644 client/go/internal/util/shell_quote_test.go 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) -- cgit v1.2.3