Merge pull request #52 from rkdl/fix-breaking-args-with-whitespace

fix lets breaking strings with whitespace in append arguments

commands/command/cmd.go

@@ -6,27 +6,26 @@ import (
 	"strings"
 )
 
-func stringPartition(s, sep string) (string, string, string) {
-	sepPos := strings.Index(s, sep)
-	if sepPos == -1 { // no separator found
-		return s, "", ""
-	}
-
-	split := strings.SplitN(s, sep, 2)
-
-	return split[0], sep, split[1]
-}
+// A workaround function which helps to prevent breaking
+// strings with special symbols (' ', '*', '$', '#'...)
+// When you run a command with an argument containing one of these, you put it into quotation marks:
+// lets alembic -n dev revision --autogenerate -m "revision message"
+// which makes shell understand that "revision message" is a single argument, but not two args
+// The problem is, lets constructs a script string
+// and then passes it to an appropriate interpreter (sh -c $SCRIPT)
+// so we need to wrap args with quotation marks to prevent breaking
+// This also solves problem with json params: --key='{"value": 1}' => '--key={"value": 1}'
+func escapeArgs(args []string) []string {
+	var escapedArgs []string
 
-// e.g if value is a json --key='{"value": 1}'
-// it will wrap value in '' - --key=''{"value": 1}''
-// and when escaped it become --key='{"value": 1}'
-func escapeFlagValue(str string) string {
-	if strings.Contains(str, "=") {
-		key, sep, val := stringPartition(str, "=")
-		str = strings.Join([]string{key, fmt.Sprintf("'%s'", val)}, sep)
+	for _, arg := range args {
+		// wraps every argument with quotation marks to avoid ambiguity
+		// TODO: maybe use some kind of blacklist symbols to wrap only necessary args
+		escapedArg := fmt.Sprintf("'%s'", arg)
+		escapedArgs = append(escapedArgs, escapedArg)
 	}
 
-	return str
+	return escapedArgs
 }
 
 func parseAndValidateCmd(cmd interface{}, newCmd *Command) error {
@@ -48,19 +47,18 @@ func parseAndValidateCmd(cmd interface{}, newCmd *Command) error {
 
 			cmdList = append(cmdList, fmt.Sprintf("%s", v))
 		}
+
+		// a list of arguments to be appended to commands in lets.yaml
+		var proxyArgs []string
 		// cut binary path and command name
 		if len(os.Args) > 1 {
-			cmdList = append(cmdList, os.Args[2:]...)
+			proxyArgs = os.Args[2:]
 		} else if len(os.Args) == 1 {
-			cmdList = append(cmdList, os.Args[1:]...)
-		}
-
-		var escapedCmdList []string
-		for _, val := range cmdList {
-			escapedCmdList = append(escapedCmdList, escapeFlagValue(val))
+			proxyArgs = os.Args[1:]
 		}
 
-		newCmd.Cmd = strings.TrimSpace(strings.Join(escapedCmdList, " "))
+		fullCommandList := append(cmdList, escapeArgs(proxyArgs)...)
+		newCmd.Cmd = strings.TrimSpace(strings.Join(fullCommandList, " "))
 	default:
 		return newParseCommandError(
 			"must be either string or list of string",

commands/command/cmd_test.go

@@ -6,6 +6,23 @@ import (
 	"testing"
 )
 
+// that's how shell does it
+func simulateProcessShellArgs(inputCmdList []string) []string {
+	var cmdList []string
+
+	for _, arg := range inputCmdList {
+		isEnquoted := len(arg) >= 2 && (arg[0] == '\'' && arg[len(arg)-1] == '\'')
+		if isEnquoted {
+			quoteless := arg[1 : len(arg)-1]
+			cmdList = append(cmdList, quoteless)
+		} else {
+			cmdList = append(cmdList, arg)
+		}
+	}
+
+	return cmdList
+}
+
 func TestCommandFieldCmd(t *testing.T) {
 	t.Run("so subcommand in os.Args", func(t *testing.T) {
 		testCmd := NewCommand("test-cmd")
@@ -52,19 +69,36 @@ func TestCommandFieldCmd(t *testing.T) {
 		if err != nil {
 			t.Fatalf("unexpected error: %s", err)
 		}
-		exp := strings.Join(append([]string{"echo", "Hello"}, "one", "two", "--there", `--four=''{"age": 20}''`), " ")
+		exp := strings.Join(append([]string{"echo", "Hello"}, "'one'", "'two'", "'--there'", `'--four='{"age": 20}''`), " ")
 		if testCmd.Cmd != exp {
 			t.Errorf("wrong output. \nexpect: %s \ngot:    %s", exp, testCmd.Cmd)
 		}
 	})
 }
 
-func TestEscapeKeyValueFlagValue(t *testing.T) {
+func TestEscapeArguments(t *testing.T) {
 	t.Run("escape value if json", func(t *testing.T) {
-		escaped := escapeFlagValue(`--kwargs='{"age": 20}'`)
-		exp := `--kwargs=''{"age": 20}''`
+		jsonArg := `--kwargs={"age": 20}`
+		escaped := escapeArgs([]string{jsonArg})[0]
+		exp := `'--kwargs={"age": 20}'`
 		if escaped != exp {
 			t.Errorf("wrong output. \nexpect: %s \ngot:    %s", exp, escaped)
 		}
 	})
+
+	t.Run("escape string with whitespace", func(t *testing.T) {
+		letsCmd := "lets commitCrime"
+		appendArgs := "-m 'azaza lalka'"
+		fullCommand := strings.Join([]string{letsCmd, appendArgs}, " ")
+
+		cmdList := simulateProcessShellArgs(strings.Split(fullCommand, " "))
+
+		args := cmdList[2:]
+		escapedArgs := escapeArgs(args)
+		resultArgs := strings.Join(simulateProcessShellArgs(escapedArgs), " ")
+
+		if resultArgs != appendArgs {
+			t.Errorf("wrong output. \nexpect: %s \ngot:    %s", appendArgs, resultArgs)
+		}
+	})
 }

tests/command_options.bats

@@ -140,3 +140,32 @@ setup() {
     [[ "${lines[1]}" = "Usage: lets options-wrong-usage-xxx" ]]
 }
 
+@test "command_options: should not break json argument" {
+    run lets test-proxy-options \
+        start \
+        path.to.pythonModule.py \
+        --kwargs='{"sobaka": true}' \
+        '--json={"x": 25, "y": [1, 2, 3]}'
+    printf "%s\n" "${lines[@]}"
+
+    [[ $status = 0 ]]
+    linesLen="${#lines[@]}"
+    [[ $linesLen = 4 ]]
+    [[ "${lines[0]}" = 'start' ]]
+    [[ "${lines[1]}" = 'path.to.pythonModule.py' ]]
+    [[ "${lines[2]}" = '--kwargs={"sobaka": true}' ]]
+    [[ "${lines[3]}" = '--json={"x": 25, "y": [1, 2, 3]}' ]]
+}
+
+@test "command_options: should not break string argument with whitespace" {
+    run lets test-proxy-options generate somethingUseful -m "my message contains whitespace!!"
+    printf "%s\n" "${lines[@]}"
+
+    [[ $status = 0 ]]
+    linesLen="${#lines[@]}"
+    [[ $linesLen = 4 ]]
+    [[ "${lines[0]}" = "generate" ]]
+    [[ "${lines[1]}" = "somethingUseful" ]]
+    [[ "${lines[2]}" = "-m" ]]
+    [[ "${lines[3]}" = "my message contains whitespace!!" ]]
+}

tests/command_options/echoArgs.sh

@@ -0,0 +1,4 @@
+#!/bin/bash
+for i; do 
+    echo $i
+done

tests/command_options/lets.yaml

@@ -27,3 +27,8 @@ commands:
     options: |
       Usage: lets options-wrong-usage-xxx
     cmd: echo "Wrong"
+
+  test-proxy-options:
+    description: Test passthrough options (lets must append them to ./echoArgs.sh)
+    cmd: 
+      - ./echoArgs.sh