From 6003413a682ff94bff3b4bc9f89aa1a5c12633a4 Mon Sep 17 00:00:00 2001 From: Darren Shepherd Date: Fri, 9 Aug 2024 19:20:09 -0700 Subject: [PATCH] Revert "bug: change quoting behavior" --- pkg/engine/cmd.go | 93 ---------------------------- pkg/engine/cmd_test.go | 135 ----------------------------------------- 2 files changed, 228 deletions(-) delete mode 100644 pkg/engine/cmd_test.go diff --git a/pkg/engine/cmd.go b/pkg/engine/cmd.go index 311c743a..57dfd477 100644 --- a/pkg/engine/cmd.go +++ b/pkg/engine/cmd.go @@ -123,9 +123,6 @@ func (e *Engine) runCommand(ctx Context, tool types.Tool, input string, toolCate } cmd, stop, err := e.newCommand(ctx.Ctx, extraEnv, tool, input) if err != nil { - if toolCategory == NoCategory { - return fmt.Sprintf("ERROR: got (%v) while parsing command", err), nil - } return "", err } defer stop() @@ -271,12 +268,6 @@ func (e *Engine) newCommand(ctx context.Context, extraEnv []string, tool types.T }) } - // After we determined the interpreter we again interpret the args by env vars - args, err = replaceVariablesForInterpreter(interpreter, envMap) - if err != nil { - return nil, nil, err - } - if runtime.GOOS == "windows" && (args[0] == "/bin/bash" || args[0] == "/bin/sh") { args[0] = path.Base(args[0]) } @@ -323,87 +314,3 @@ func (e *Engine) newCommand(ctx context.Context, extraEnv []string, tool types.T cmd.Env = compressEnv(envvars) return cmd, stop, nil } - -func replaceVariablesForInterpreter(interpreter string, envMap map[string]string) ([]string, error) { - var parts []string - for i, part := range splitByQuotes(interpreter) { - if i%2 == 0 { - part = os.Expand(part, func(s string) string { - return envMap[s] - }) - // We protect newly resolved env vars from getting replaced when we do the second Expand - // after shlex. Yeah, crazy. I'm guessing this isn't secure, but just trying to avoid a foot gun. - part = os.Expand(part, func(s string) string { - return "${__" + s + "}" - }) - } - parts = append(parts, part) - } - - parts, err := shlex.Split(strings.Join(parts, "")) - if err != nil { - return nil, err - } - - for i, part := range parts { - parts[i] = os.Expand(part, func(s string) string { - if strings.HasPrefix(s, "__") { - return "${" + s[2:] + "}" - } - return envMap[s] - }) - } - - return parts, nil -} - -// splitByQuotes will split a string by parsing matching double quotes (with \ as the escape character). -// The return value conforms to the following properties -// 1. s == strings.Join(result, "") -// 2. Even indexes are strings that were not in quotes. -// 3. Odd indexes are strings that were quoted. -// -// Example: s = `In a "quoted string" quotes can be escaped with \"` -// -// result = [`In a `, `"quoted string"`, ` quotes can be escaped with \"`] -func splitByQuotes(s string) (result []string) { - var ( - buf strings.Builder - inEscape, inQuote bool - ) - - for _, c := range s { - if inEscape { - buf.WriteRune(c) - inEscape = false - continue - } - - switch c { - case '"': - if inQuote { - buf.WriteRune(c) - } - result = append(result, buf.String()) - buf.Reset() - if !inQuote { - buf.WriteRune(c) - } - inQuote = !inQuote - case '\\': - inEscape = true - buf.WriteRune(c) - default: - buf.WriteRune(c) - } - } - - if buf.Len() > 0 { - if inQuote { - result = append(result, "") - } - result = append(result, buf.String()) - } - - return -} diff --git a/pkg/engine/cmd_test.go b/pkg/engine/cmd_test.go deleted file mode 100644 index 15f72036..00000000 --- a/pkg/engine/cmd_test.go +++ /dev/null @@ -1,135 +0,0 @@ -// File: cmd_test.go -package engine - -import "testing" - -func TestSplitByQuotes(t *testing.T) { - tests := []struct { - name string - input string - expected []string - }{ - { - name: "NoQuotes", - input: "Hello World", - expected: []string{"Hello World"}, - }, - { - name: "ValidQuote", - input: `"Hello" "World"`, - expected: []string{``, `"Hello"`, ` `, `"World"`}, - }, - { - name: "ValidQuoteWithEscape", - input: `"Hello\" World"`, - expected: []string{``, `"Hello\" World"`}, - }, - { - name: "Nothing", - input: "", - expected: []string{}, - }, - { - name: "SpaceInsideQuote", - input: `"Hello World"`, - expected: []string{``, `"Hello World"`}, - }, - { - name: "SingleChar", - input: "H", - expected: []string{"H"}, - }, - { - name: "SingleQuote", - input: `"Hello`, - expected: []string{``, ``, `"Hello`}, - }, - { - name: "ThreeQuotes", - input: `Test "Hello "World" End\"`, - expected: []string{`Test `, `"Hello "`, `World`, ``, `" End\"`}, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := splitByQuotes(tt.input) - if !equal(got, tt.expected) { - t.Errorf("splitByQuotes() = %v, want %v", got, tt.expected) - } - }) - } -} - -// Helper function to assert equality of two string slices. -func equal(a, b []string) bool { - if len(a) != len(b) { - return false - } - for i, v := range a { - if v != b[i] { - return false - } - } - return true -} - -// Testing for replaceVariablesForInterpreter -func TestReplaceVariablesForInterpreter(t *testing.T) { - tests := []struct { - name string - interpreter string - envMap map[string]string - expected []string - shouldFail bool - }{ - { - name: "No quotes", - interpreter: "/bin/bash -c ${COMMAND} tail", - envMap: map[string]string{"COMMAND": "echo Hello!"}, - expected: []string{"/bin/bash", "-c", "echo", "Hello!", "tail"}, - }, - { - name: "Quotes Variables", - interpreter: `/bin/bash -c "${COMMAND}" tail`, - envMap: map[string]string{"COMMAND": "Hello, World!"}, - expected: []string{"/bin/bash", "-c", "Hello, World!", "tail"}, - }, - { - name: "Double escape", - interpreter: `/bin/bash -c "${COMMAND}" ${TWO} tail`, - envMap: map[string]string{ - "COMMAND": "Hello, World!", - "TWO": "${COMMAND}", - }, - expected: []string{"/bin/bash", "-c", "Hello, World!", "${COMMAND}", "tail"}, - }, - { - name: "aws cli issue", - interpreter: "aws ${ARGS}", - envMap: map[string]string{ - "ARGS": `ec2 describe-instances --region us-east-1 --query 'Reservations[*].Instances[*].{Instance:InstanceId,State:State.Name}'`, - }, - expected: []string{ - `aws`, - `ec2`, - `describe-instances`, - `--region`, `us-east-1`, - `--query`, `Reservations[*].Instances[*].{Instance:InstanceId,State:State.Name}`, - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := replaceVariablesForInterpreter(tt.interpreter, tt.envMap) - if (err != nil) != tt.shouldFail { - t.Errorf("replaceVariablesForInterpreter() error = %v, want %v", err, tt.shouldFail) - return - } - if !equal(got, tt.expected) { - t.Errorf("replaceVariablesForInterpreter() = %v, want %v", got, tt.expected) - } - }) - } -}