fix: parsing when there are nested quotes

remote/remote.go

@@ -9,7 +9,6 @@ import (
 	"bytes"
 	"context"
 	"encoding/binary"
-	"encoding/json"
 	"errors"
 	"fmt"
 	"io"
@@ -365,46 +364,34 @@ func validateCommand(calledCmd string) (string, error) {
 	return strings.Join(cmdParts, " "), nil
 }
 
-func getCleanWpCliArgumentArray(wpCliCmdString string) ([]string, error) {
+func getCleanWpCliArgumentArray(wpCliCmdString string) []string {
 	rawArgs := tokenizeString(wpCliCmdString)
 	cleanArgs := make([]string, 0)
-	openQuote := false
-	arg := ""
 
+	quotedNamedParamRegex := regexp.MustCompile(`^(--[a-zA-Z0-9_-]+=)"(.*)"$`)
+
+	// If the item starts and ends with a single quote, remove the quotes. For compatibility with old VIP CLI.
+	// If the item starts and ends with a double quote, remove the quotes. Additionally, replace any escaped double quotes with a single double quote
+	// Otherwise:
+	// - if the item is a named parameter with a value that is quoted, remove the quotes around the value.
+	// - if not, use the item as is.
 	for _, rawArg := range rawArgs {
-		if idx := strings.Index(rawArg, "\""); -1 != idx {
-			if idx != strings.LastIndexAny(rawArg, "\"") {
-				cleanArgs = append(cleanArgs, rawArg)
-			} else if openQuote {
-				arg = fmt.Sprintf("%s %s", arg, rawArg)
-				cleanArgs = append(cleanArgs, arg)
-				arg = ""
-				openQuote = false
-			} else {
-				arg = rawArg
-				openQuote = true
-			}
+		if strings.HasPrefix(rawArg, "'") && strings.HasSuffix(rawArg, "'") {
+			cleanArgs = append(cleanArgs, rawArg[1:len(rawArg)-1])
+		} else if strings.HasPrefix(rawArg, "\"") && strings.HasSuffix(rawArg, "\"") {
+			trimmed := rawArg[1 : len(rawArg)-1]
+			trimmed = strings.ReplaceAll(trimmed, "\\\"", "\"")
+			cleanArgs = append(cleanArgs, trimmed)
 		} else {
-			if openQuote {
-				arg = fmt.Sprintf("%s %s", arg, rawArg)
+			if matches := quotedNamedParamRegex.FindStringSubmatch(rawArg); matches != nil {
+				cleanArgs = append(cleanArgs, matches[1]+matches[2])
 			} else {
 				cleanArgs = append(cleanArgs, rawArg)
 			}
 		}
 	}
 
-	if openQuote {
-		return make([]string, 0), errors.New(fmt.Sprintf("WP CLI command is invalid: %s\n", wpCliCmdString))
-	}
-
-	// Remove quotes from the args
-	for i := range cleanArgs {
-		if !isJSONObject(cleanArgs[i]) { //don't alter JSON arguments
-			cleanArgs[i] = strings.ReplaceAll(cleanArgs[i], "\"", "")
-		}
-	}
-
-	return cleanArgs, nil
+	return cleanArgs
 }
 
 func connWriteUTF8(conn net.Conn, data []byte) (int, int, error) {
@@ -714,12 +701,7 @@ func runWpCliCmdRemote(conn net.Conn, GUID string, rows uint16, cols uint16, wpC
 	cmdArgs := make([]string, 0)
 	cmdArgs = append(cmdArgs, strings.Fields("--path="+remoteConfig.wpPath)...)
 
-	cleanArgs, err := getCleanWpCliArgumentArray(wpCliCmdString)
-	if nil != err {
-		conn.Write([]byte("WP CLI command is invalid"))
-		conn.Close()
-		return errors.New(err.Error())
-	}
+	cleanArgs := getCleanWpCliArgumentArray(wpCliCmdString)
 	log.Printf("LOG CLI Arguments (%d elements): %s", len(cleanArgs), strings.Join(cleanArgs, ", "))
 
 	cmdArgs = append(cmdArgs, cleanArgs...)
@@ -1076,28 +1058,22 @@ Splits a string into an array based on whitespace except when that whitepace is
 */
 func tokenizeString(rawString string) []string {
 	quoted := false
+	var quoteChar rune
 	var prevRune rune
 	tokenized := strings.FieldsFunc(rawString, func(r rune) bool {
-		//Tokenizing on double quotes EXCEPT when preceded by the escape char
-		if r == '"' && prevRune != '\\' {
-			quoted = !quoted
+		// Tokenizing on double or single quotes EXCEPT when preceded by the escape char
+		if (r == '"' || r == '\'') && prevRune != '\\' {
+			if !quoted {
+				quoted = true
+				quoteChar = r
+			} else if quoteChar == r {
+				quoted = false
+			}
 		}
 		prevRune = r
-		return !quoted && r == ' '
+		return !quoted && unicode.IsSpace(r)
 	})
 	out := strings.Join(tokenized, ", ")
 	log.Printf("LOG: %s", out)
 	return tokenized
 }
-
-func isJSON(str string) bool {
-	return json.Valid([]byte(str))
-}
-
-func isJSONObject(str string) bool {
-	trimmedStr := strings.TrimSpace(str)
-	if !strings.HasPrefix(trimmedStr, "{") || !strings.HasSuffix(trimmedStr, "}") {
-		return false
-	}
-	return isJSON(str)
-}

remote/remote_test.go

@@ -5,40 +5,14 @@ import (
 	"testing"
 )
 
-func TestCheckIsJSONObject(t *testing.T) {
-	tests := map[string]struct {
-		input string
-		want  bool
-	}{
-		"normal text with no quotes":                {want: false, input: "normal text"},
-		"array object":                              {want: false, input: "[1,2,3]"},
-		"valid json string that should be excluded": {want: false, input: `"normal text inside quotes"`},
-		"missing quotes json":                       {want: false, input: `{"broken":json"}`},
-		"json inside extra quotes":                  {want: false, input: `"{"broken":"json"}"`},
-		"missing closing parenthesis":               {want: false, input: `{"broken":"json object"`},
-		"wrong numerical key json":                  {want: false, input: ` { 1 : " wrong" } `},
-		"standard json":                             {want: true, input: `{"object":"json"}`},
-		"json with extra spacing":                   {want: true, input: `  { "object space" : "with spacing" } `},
-	}
-
-	for name, tc := range tests {
-		t.Run(name, func(t *testing.T) {
-			got := isJSONObject(tc.input)
-			if !reflect.DeepEqual(tc.want, got) {
-				t.Fatalf("testing '%v' isJSONObject(\"%v\") expected: %v, got: %v", name, tc.input, tc.want, got)
-			}
-		})
-	}
-}
-
 func TestValidateCommand(t *testing.T) {
 	tests := map[string]struct {
 		errString string
 		input     string
 		want      string
 	}{
-		"media import file should pass":              {errString: "", want: "media import https://example.com/cutekitties.png", input: "media import https://example.com/cutekitties.png"},
-		"vip whatever should pass":                   {errString: "", want: "vip whatever", input: "vip whatever"},
+		"media import file should pass": {errString: "", want: "media import https://example.com/cutekitties.png", input: "media import https://example.com/cutekitties.png"},
+		"vip whatever should pass":      {errString: "", want: "vip whatever", input: "vip whatever"},
 	}
 
 	for name, tc := range tests {
@@ -59,3 +33,65 @@ func TestValidateCommand(t *testing.T) {
 		})
 	}
 }
+
+func TestTokenizeString(t *testing.T) {
+	tests := map[string]struct {
+		input string
+		want  []string
+	}{
+		"no quotes":            {want: []string{"option", "update", "cow", "a"}, input: "option update cow a"},
+		"single quotes":        {want: []string{"option", "update", "cow", `'a b'`}, input: "option update cow 'a b'"},
+		"double quotes":        {want: []string{"option", "update", "cow", `"a b"`}, input: `option update cow "a b"`},
+		"nested double quotes": {want: []string{"option", "update", "cow", `"a \"b\""`}, input: `option update cow "a \"b\""`},
+		"one nested quote":     {want: []string{`"a\"b"`}, input: `"a\"b"`},
+
+		// Whitespaces
+		"newline in arguments":  {want: []string{"option", "update", "cow", "\"a\nb\""}, input: "option update cow \"a\nb\""},
+		"newline between args":  {want: []string{"option", "update", "cow", "a"}, input: "option \n update cow a"},
+		"different whitespaces": {want: []string{"option", "update", "cow", "a"}, input: "option \n \t update\v\fcow\ra"},
+
+		// Compatibility with VIP CLI bugs
+		"named parameters": {want: []string{"option", "list", `--search="a b"`}, input: `option list --search="a b"`},
+
+		// These sequences should not occur; if they do, someone is trying to break the system
+		"embedded quotes":       {want: []string{`opt""i''on`}, input: `opt""i''on`},
+		"unbalanced quotes (1)": {want: []string{`"a 'b`}, input: `"a 'b`},
+		"unbalanced quotes (2)": {want: []string{`a" 'b`}, input: `a" 'b`},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			got := tokenizeString(tc.input)
+
+			if !reflect.DeepEqual(tc.want, got) {
+				t.Fatalf("testing '%v' tokenizeString(\"%v\") expected: %v, got: %v", name, tc.input, tc.want, got)
+			}
+		})
+	}
+}
+
+func TestGetCleanWpCliArgumentArray(t *testing.T) {
+	tests := map[string]struct {
+		input string
+		want  []string
+	}{
+		"no quotes":                {want: []string{"option", "update", "cow", "a"}, input: "option update cow a"},
+		"single quotes":            {want: []string{"option", "update", "cow", "a b"}, input: "option update cow 'a b'"},
+		"double quotes":            {want: []string{"option", "update", "cow", "a b"}, input: `option update cow "a b"`},
+		"nested double quotes":     {want: []string{"option", "update", "cow", `a "b"`}, input: `option update cow "a \"b\""`},
+		"json":                     {want: []string{"option", "update", "cow", `{"a":"b"}`}, input: `option update cow {"a":"b"}`},
+		"vip-cli bugs":             {want: []string{"option", "list", `--search=a b`}, input: `option list --search="a b"`},
+		"proper quoting":           {want: []string{"option", "list", `--search=a b`}, input: `"option" "list" "--search=a b"`},
+		"proper quoting w/nesting": {want: []string{"option", "list", `--search="a b"`}, input: `"option" "list" "--search=\"a b\""`},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			got := getCleanWpCliArgumentArray(tc.input)
+
+			if !reflect.DeepEqual(tc.want, got) {
+				t.Fatalf("testing '%v' getCleanWpCliArgumentArray(\"%v\") expected: %v, got: %v", name, tc.input, tc.want, got)
+			}
+		})
+	}
+}