Two quotes next to each other result in one escaped quote; Add Split Example,...

Two quotes next to each other result in one escaped quote; Add Split Example, add/refactor tests for every platform.

middleware/commands.go

@@ -2,23 +2,23 @@ package middleware
 
 import (
 	"errors"
-	"fmt"
 	"runtime"
-	"strings"
 	"unicode"
 
 	"github.com/flynn/go-shlex"
 )
 
+var runtimeGoos = runtime.GOOS
+
 // SplitCommandAndArgs takes a command string and parses it
 // shell-style into the command and its separate arguments.
 func SplitCommandAndArgs(command string) (cmd string, args []string, err error) {
 	var parts []string
 
-	if runtime.GOOS == "windows" {
+	if runtimeGoos == "windows" {
 		parts = parseWindowsCommand(command) // parse it Windows-style
 	} else {
-		parts, err = shlex.Split(command) // parse it Unix-style
+		parts, err = parseUnixCommand(command) // parse it Unix-style
 		if err != nil {
 			err = errors.New("error parsing command: " + err.Error())
 			return
@@ -38,112 +38,76 @@ func SplitCommandAndArgs(command string) (cmd string, args []string, err error)
 	return
 }
 
-// parseWindowsCommand is a sad but good-enough attempt to
-// split a command into the command and its arguments like
-// the Windows command line would; only basic parsing is
-// supported. This function has to be used on Windows instead
+// parseUnixCommand parses a unix style command line and returns the
+// command and its arguments or an error
+func parseUnixCommand(cmd string) ([]string, error) {
+	return shlex.Split(cmd)
+}
+
+// parseWindowsCommand parses windows command lines and
+// returns the command and the arguments as an array. It
+// should be able to parse commonly used command lines.
+// Only basic syntax is supported:
+//  - spaces in double quotes are not token delimiters
+//  - double quotes are escaped by either backspace or another double quote
+//  - except for the above case backspaces are path separators (not special)
+//
+// Many sources point out that escaping quotes using backslash can be unsafe.
+// Use two double quotes when possible. (Source: http://stackoverflow.com/a/31413730/2616179 )
+//
+// This function has to be used on Windows instead
 // of the shlex package because this function treats backslash
 // characters properly.
-//
-// Loosely based off the rules here: http://stackoverflow.com/a/4094897/1048862
-// True parsing is much, much trickier.
-func parseWindowsCommand2(cmd string) []string {
+func parseWindowsCommand(cmd string) []string {
+	const backslash = '\\'
+	const quote = '"'
+
 	var parts []string
 	var part string
-	var quoted bool
-	var backslashes int
-
-	for _, ch := range cmd {
-		if ch == '\\' {
-			backslashes++
-			continue
-		}
-		var evenBacksl = (backslashes % 2) == 0
-		if backslashes > 0 && ch != '\\' {
-			numBacksl := (backslashes / 2) + 1
-			if ch == '"' {
-				numBacksl--
-			}
-			part += strings.Repeat(`\`, numBacksl)
-			backslashes = 0
-		}
+	var inQuotes bool
+	var lastRune rune
 
-		if quoted {
-			if ch == '"' && evenBacksl {
-				quoted = false
-				continue
-			}
-			part += string(ch)
-			continue
-		}
+	for i, ch := range cmd {
 
-		if unicode.IsSpace(ch) && len(part) > 0 {
-			parts = append(parts, part)
-			part = ""
-			continue
+		if i != 0 {
+			lastRune = rune(cmd[i-1])
 		}
 
-		if ch == '"' && evenBacksl {
-			quoted = true
+		if ch == backslash {
+			// put it in the part - for now we don't know if it's an
+			// escaping char or path separator
+			part += string(ch)
 			continue
 		}
 
+		if ch == quote {
+			if lastRune == backslash {
+				// remove the backslash from the part and add the escaped quote instead
+				part = part[:len(part)-1]
 				part += string(ch)
+				continue
 			}
 
-	if len(part) > 0 {
-		parts = append(parts, part)
-		part = ""
-	}
-
-	return parts
-}
-
-func parseWindowsCommand(cmd string) []string {
-	var parts []string
-	var part string
-	var inQuotes bool
-	var wasBackslash bool
-
-	prefix := "DEBUG:"
-
-	fmt.Println(prefix, "Parsing cmd:", cmd)
-
-	for i, ch := range cmd {
-		fmt.Println("  ", prefix, "Looking at char:", string(ch), "at index", string(i))
-
-		if ch == '\\' {
-			wasBackslash = true
-			// put it in the part - for now we don't know if it's escaping char or path separator
+			if lastRune == quote {
+				// revert the last change of the inQuotes state
+				// it was an escaping quote
+				inQuotes = !inQuotes
 				part += string(ch)
 				continue
 			}
 
-		if ch == '"' {
-			if wasBackslash {
-				// remove the backslash from the part and add the escaped quote instead
-				part = part[:len(part)-1]
-				part += string(ch)
-				wasBackslash = false
-				continue
-			} else {
 			// normal escaping quotes
-				fmt.Println("    ", prefix, "and it's a quote")
 			inQuotes = !inQuotes
 			continue
 
 		}
-		}
 
 		if unicode.IsSpace(ch) && !inQuotes && len(part) > 0 {
-			fmt.Println("    ", prefix, "and it's a space outside quotes")
 			parts = append(parts, part)
 			part = ""
-			wasBackslash = false
 			continue
 		}
 
-		wasBackslash = false
 		part += string(ch)
 	}
 
@@ -152,6 +116,5 @@ func parseWindowsCommand(cmd string) []string {
 		part = ""
 	}
 
-	fmt.Println(prefix, strings.Join(parts, ","))
 	return parts
 }

middleware/commands_test.go

@@ -2,107 +2,176 @@ package middleware
 
 import (
 	"fmt"
+	"runtime"
 	"strings"
 	"testing"
 )
 
+func TestParseUnixCommand(t *testing.T) {
+	tests := []struct {
+		input    string
+		expected []string
+	}{
+		// 0 - emtpy command
+		{
+			input:    ``,
+			expected: []string{},
+		},
+		// 1 - command without arguments
+		{
+			input:    `command`,
+			expected: []string{`command`},
+		},
+		// 2 - command with single argument
+		{
+			input:    `command arg1`,
+			expected: []string{`command`, `arg1`},
+		},
+		// 3 - command with multiple arguments
+		{
+			input:    `command arg1 arg2`,
+			expected: []string{`command`, `arg1`, `arg2`},
+		},
+		// 4 - command with single argument with space character - in quotes
+		{
+			input:    `command "arg1 arg1"`,
+			expected: []string{`command`, `arg1 arg1`},
+		},
+		// 5 - command with multiple spaces and tab character
+		{
+			input:    "command arg1    arg2\targ3",
+			expected: []string{`command`, `arg1`, `arg2`, `arg3`},
+		},
+		// 6 - command with single argument with space character - escaped with backspace
+		{
+			input:    `command arg1\ arg2`,
+			expected: []string{`command`, `arg1 arg2`},
+		},
+		// 7 - single quotes should escape special chars
+		{
+			input:    `command 'arg1\ arg2'`,
+			expected: []string{`command`, `arg1\ arg2`},
+		},
+	}
+
+	for i, test := range tests {
+		errorPrefix := fmt.Sprintf("Test [%d]: ", i)
+		errorSuffix := fmt.Sprintf(" Command to parse: [%s]", test.input)
+		actual, _ := parseUnixCommand(test.input)
+		if len(actual) != len(test.expected) {
+			t.Errorf(errorPrefix+"Expected %d parts, got %d: %#v."+errorSuffix, len(test.expected), len(actual), actual)
+			continue
+		}
+		for j := 0; j < len(actual); j++ {
+			if expectedPart, actualPart := test.expected[j], actual[j]; expectedPart != actualPart {
+				t.Errorf(errorPrefix+"Expected: %v Actual: %v (index %d)."+errorSuffix, expectedPart, actualPart, j)
+			}
+		}
+	}
+}
+
 func TestParseWindowsCommand(t *testing.T) {
 	tests := []struct {
 		input    string
 		expected []string
 	}{
-		{ // 0
+		{ // 0 - empty command - do not fail
+			input:    ``,
+			expected: []string{},
+		},
+		{ // 1 - cmd without args
 			input:    `cmd`,
 			expected: []string{`cmd`},
 		},
-		{ // 1
+		{ // 2 - multiple args
 			input:    `cmd arg1 arg2`,
 			expected: []string{`cmd`, `arg1`, `arg2`},
 		},
-		{ // 2
+		{ // 3 - multiple args with space
 			input:    `cmd "combined arg" arg2`,
 			expected: []string{`cmd`, `combined arg`, `arg2`},
 		},
-		{ // 3
+		{ // 4 - path without spaces
 			input:    `mkdir C:\Windows\foo\bar`,
 			expected: []string{`mkdir`, `C:\Windows\foo\bar`},
 		},
-		{ // 4
+		{ // 5 - command with space in quotes
 			input:    `"command here"`,
 			expected: []string{`command here`},
 		},
-		{ // 5
+		{ // 6 - argument with escaped quotes (two quotes)
+			input:    `cmd ""arg""`,
+			expected: []string{`cmd`, `"arg"`},
+		},
+		{ // 7 - argument with escaped quotes (backslash)
 			input:    `cmd \"arg\"`,
 			expected: []string{`cmd`, `"arg"`},
 		},
-		{ // 6
-			input:    `cmd "a \"quoted value\""`,
-			expected: []string{`cmd`, `a "quoted value"`},
+		{ // 8 - two quotes (escaped) inside an inQuote element
+			input:    `cmd "a ""quoted value"`,
+			expected: []string{`cmd`, `a "quoted value`},
+		},
+		// TODO - see how many quotes are dislayed if we use "", """, """""""
+		{ // 9 - two quotes outside an inQuote element
+			input:    `cmd a ""quoted value`,
+			expected: []string{`cmd`, `a`, `"quoted`, `value`},
 		},
-		{ // 7
+		{ // 10 - path with space in quotes
 			input:    `mkdir "C:\directory name\foobar"`,
 			expected: []string{`mkdir`, `C:\directory name\foobar`},
 		},
-		{ // 8
+		{ // 11 - space without quotes
 			input:    `mkdir C:\ space`,
 			expected: []string{`mkdir`, `C:\`, `space`},
 		},
-		{ // 9
+		{ // 12 - space in quotes
 			input:    `mkdir "C:\ space"`,
 			expected: []string{`mkdir`, `C:\ space`},
 		},
-		// 10
-		{
+		{ // 13 - UNC
 			input:    `mkdir \\?\C:\Users`,
 			expected: []string{`mkdir`, `\\?\C:\Users`},
 		},
-		// 11
-		{
+		{ // 14 - UNC with space
 			input:    `mkdir "\\?\C:\Program Files"`,
 			expected: []string{`mkdir`, `\\?\C:\Program Files`},
 		},
+
+		{ // 15 - unclosed quotes - treat as if the path ends with quote
+			input:    `mkdir "c:\Program files`,
+			expected: []string{`mkdir`, `c:\Program files`},
+		},
+		{ // 16 - quotes used inside the argument
+			input:    `mkdir "c:\P"rogra"m f"iles`,
+			expected: []string{`mkdir`, `c:\Program files`},
+		},
 	}
-	var nTests int
+
 	for i, test := range tests {
-		fmt.Printf("====== Test %d ======\n", i)
-		actual := parseWindowsCommand(test.input)
-		if len(actual) != len(test.expected) {
-			fmt.Printf("Test %d: Expected %d parts, got %d: %#v", i, len(test.expected), len(actual), actual)
-			fmt.Println()
-			t.Errorf("Test %d: Expected %d parts, got %d: %#v", i, len(test.expected), len(actual), actual)
-			continue
-		}
-		for j := 0; j < len(actual); j++ {
-			if expectedPart, actualPart := test.expected[j], actual[j]; expectedPart != actualPart {
-				fmt.Printf("Test %d: Expected: %v Actual: %v (index %d)", i, expectedPart, actualPart, j)
-				fmt.Println()
-				t.Errorf("Test %d: Expected: %v Actual: %v (index %d)", i, expectedPart, actualPart, j)
-			}
-		}
-		nTests += 1
-	}
+		errorPrefix := fmt.Sprintf("Test [%d]: ", i)
+		errorSuffix := fmt.Sprintf(" Command to parse: [%s]", test.input)
 
-	for _, test := range tests {
-		fmt.Printf("====== Test %d ======\n", nTests)
-		actual := parseWindowsCommand2(test.input)
+		actual := parseWindowsCommand(test.input)
 		if len(actual) != len(test.expected) {
-			fmt.Printf("Test %d: Expected %d parts, got %d: %#v", nTests, len(test.expected), len(actual), actual)
-			fmt.Println()
-			t.Errorf("Test %d: Expected %d parts, got %d: %#v", nTests, len(test.expected), len(actual), actual)
+			t.Errorf(errorPrefix+"Expected %d parts, got %d: %#v."+errorSuffix, len(test.expected), len(actual), actual)
 			continue
 		}
 		for j := 0; j < len(actual); j++ {
 			if expectedPart, actualPart := test.expected[j], actual[j]; expectedPart != actualPart {
-				fmt.Printf("Test %d: Expected: %v Actual: %v (index %d)", nTests, expectedPart, actualPart, j)
-				fmt.Println()
-				t.Errorf("Test %d: Expected: %v Actual: %v (index %d)", nTests, expectedPart, actualPart, j)
+				t.Errorf(errorPrefix+"Expected: %v Actual: %v (index %d)."+errorSuffix, expectedPart, actualPart, j)
 			}
 		}
-		nTests += 1
 	}
 }
 
 func TestSplitCommandAndArgs(t *testing.T) {
+
+	// force linux parsing. It's more robust and covers error cases
+	runtimeGoos = "linux"
+	defer func() {
+		runtimeGoos = runtime.GOOS
+	}()
+
 	var parseErrorContent = "error parsing command:"
 	var noCommandErrContent = "no command contained in"
 
@@ -112,56 +181,42 @@ func TestSplitCommandAndArgs(t *testing.T) {
 		expectedArgs       []string
 		expectedErrContent string
 	}{
-		// Test case 0 - emtpy command
+		// 0 - emtpy command
 		{
 			input:              ``,
 			expectedCommand:    ``,
 			expectedArgs:       nil,
 			expectedErrContent: noCommandErrContent,
 		},
-		// Test case 1 - command without arguments
+		// 1 - command without arguments
 		{
 			input:              `command`,
 			expectedCommand:    `command`,
 			expectedArgs:       nil,
 			expectedErrContent: ``,
 		},
-		// Test case 2 - command with single argument
+		// 2 - command with single argument
 		{
 			input:              `command arg1`,
 			expectedCommand:    `command`,
 			expectedArgs:       []string{`arg1`},
 			expectedErrContent: ``,
 		},
-		// Test case 3 - command with multiple arguments
+		// 3 - command with multiple arguments
 		{
 			input:              `command arg1 arg2`,
 			expectedCommand:    `command`,
 			expectedArgs:       []string{`arg1`, `arg2`},
 			expectedErrContent: ``,
 		},
-		// Test case 4 - command with single argument with space character - in quotes
-		{
-			input:              `command "arg1 arg1"`,
-			expectedCommand:    `command`,
-			expectedArgs:       []string{`arg1 arg1`},
-			expectedErrContent: ``,
-		},
-		// Test case 5 - command with multiple spaces and tab character
-		{
-			input:              "command arg1    arg2\targ3",
-			expectedCommand:    `command`,
-			expectedArgs:       []string{`arg1`, `arg2`, "arg3"},
-			expectedErrContent: "",
-		},
-		// Test case 6 - command with unclosed quotes
+		// 4 - command with unclosed quotes
 		{
 			input:              `command "arg1 arg2`,
 			expectedCommand:    "",
 			expectedArgs:       nil,
 			expectedErrContent: parseErrorContent,
 		},
-		// Test case 7 - command with unclosed quotes
+		// 5 - command with unclosed quotes
 		{
 			input:              `command 'arg1 arg2"`,
 			expectedCommand:    "",
@@ -188,14 +243,14 @@ func TestSplitCommandAndArgs(t *testing.T) {
 
 		// test if command matches
 		if test.expectedCommand != actualCommand {
-			t.Errorf("Expected command: [%s], actual: [%s]."+errorSuffix, test.expectedCommand, actualCommand)
+			t.Errorf(errorPrefix+"Expected command: [%s], actual: [%s]."+errorSuffix, test.expectedCommand, actualCommand)
 		}
 
 		// test if arguments match
 		if len(test.expectedArgs) != len(actualArgs) {
-			t.Errorf("Wrong number of arguments! Expected [%v], actual [%v]."+errorSuffix, test.expectedArgs, actualArgs)
-		}
-
+			t.Errorf(errorPrefix+"Wrong number of arguments! Expected [%v], actual [%v]."+errorSuffix, test.expectedArgs, actualArgs)
+		} else {
+			// test args only if the count matches.
 			for j, actualArg := range actualArgs {
 				expectedArg := test.expectedArgs[j]
 				if actualArg != expectedArg {
@@ -203,4 +258,34 @@ func TestSplitCommandAndArgs(t *testing.T) {
 				}
 			}
 		}
+	}
+}
+
+func ExampleSplitCommandAndArgs() {
+	var commandLine string
+	var command string
+	var args []string
+
+	// just for the test - change GOOS and reset it at the end of the test
+	runtimeGoos = "windows"
+	defer func() {
+		runtimeGoos = runtime.GOOS
+	}()
+
+	commandLine = `mkdir /P "C:\Program Files"`
+	command, args, _ = SplitCommandAndArgs(commandLine)
+
+	fmt.Printf("Windows: %s: %s [%s]\n", commandLine, command, strings.Join(args, ","))
+
+	// set GOOS to linux
+	runtimeGoos = "linux"
+
+	commandLine = `mkdir -p /path/with\ space`
+	command, args, _ = SplitCommandAndArgs(commandLine)
+
+	fmt.Printf("Linux: %s: %s [%s]\n", commandLine, command, strings.Join(args, ","))
+
+	// Output:
+	// Windows: mkdir /P "C:\Program Files": mkdir [/P,C:\Program Files]
+	// Linux: mkdir -p /path/with\ space: mkdir [-p,/path/with space]
 }