Allow fieldSelectors to match arbitrary values

staging/src/k8s.io/apimachinery/pkg/fields/selector.go

@@ -17,6 +17,7 @@ limitations under the License.
 package fields
 
 import (
+	"bytes"
 	"fmt"
 	"sort"
 	"strings"
@@ -90,7 +91,7 @@ func (t *hasTerm) Requirements() Requirements {
 }
 
 func (t *hasTerm) String() string {
-	return fmt.Sprintf("%v=%v", t.field, t.value)
+	return fmt.Sprintf("%v=%v", t.field, EscapeValue(t.value))
 }
 
 type notHasTerm struct {
@@ -126,7 +127,7 @@ func (t *notHasTerm) Requirements() Requirements {
 }
 
 func (t *notHasTerm) String() string {
-	return fmt.Sprintf("%v!=%v", t.field, t.value)
+	return fmt.Sprintf("%v!=%v", t.field, EscapeValue(t.value))
 }
 
 type andTerm []Selector
@@ -212,6 +213,81 @@ func SelectorFromSet(ls Set) Selector {
 	return andTerm(items)
 }
 
+// valueEscaper prefixes \,= characters with a backslash
+var valueEscaper = strings.NewReplacer(
+	// escape \ characters
+	`\`, `\\`,
+	// then escape , and = characters to allow unambiguous parsing of the value in a fieldSelector
+	`,`, `\,`,
+	`=`, `\=`,
+)
+
+// Escapes an arbitrary literal string for use as a fieldSelector value
+func EscapeValue(s string) string {
+	return valueEscaper.Replace(s)
+}
+
+// InvalidEscapeSequence indicates an error occurred unescaping a field selector
+type InvalidEscapeSequence struct {
+	sequence string
+}
+
+func (i InvalidEscapeSequence) Error() string {
+	return fmt.Sprintf("invalid field selector: invalid escape sequence: %s", i.sequence)
+}
+
+// UnescapedRune indicates an error occurred unescaping a field selector
+type UnescapedRune struct {
+	r rune
+}
+
+func (i UnescapedRune) Error() string {
+	return fmt.Sprintf("invalid field selector: unescaped character in value: %v", i.r)
+}
+
+// Unescapes a fieldSelector value and returns the original literal value.
+// May return the original string if it contains no escaped or special characters.
+func UnescapeValue(s string) (string, error) {
+	// if there's no escaping or special characters, just return to avoid allocation
+	if !strings.ContainsAny(s, `\,=`) {
+		return s, nil
+	}
+
+	v := bytes.NewBuffer(make([]byte, 0, len(s)))
+	inSlash := false
+	for _, c := range s {
+		if inSlash {
+			switch c {
+			case '\\', ',', '=':
+				// omit the \ for recognized escape sequences
+				v.WriteRune(c)
+			default:
+				// error on unrecognized escape sequences
+				return "", InvalidEscapeSequence{sequence: string([]rune{'\\', c})}
+			}
+			inSlash = false
+			continue
+		}
+
+		switch c {
+		case '\\':
+			inSlash = true
+		case ',', '=':
+			// unescaped , and = characters are not allowed in field selector values
+			return "", UnescapedRune{r: c}
+		default:
+			v.WriteRune(c)
+		}
+	}
+
+	// Ending with a single backslash is an invalid sequence
+	if inSlash {
+		return "", InvalidEscapeSequence{sequence: "\\"}
+	}
+
+	return v.String(), nil
+}
+
 // ParseSelectorOrDie takes a string representing a selector and returns an
 // object suitable for matching, or panic when an error occur.
 func ParseSelectorOrDie(s string) Selector {
@@ -239,29 +315,83 @@ func ParseAndTransformSelector(selector string, fn TransformFunc) (Selector, err
 // Function to transform selectors.
 type TransformFunc func(field, value string) (newField, newValue string, err error)
 
-func try(selectorPiece, op string) (lhs, rhs string, ok bool) {
+// splitTerms returns the comma-separated terms contained in the given fieldSelector.
-	pieces := strings.Split(selectorPiece, op)
+// Backslash-escaped commas are treated as data instead of delimiters, and are included in the returned terms, with the leading backslash preserved.
-	if len(pieces) == 2 {
+func splitTerms(fieldSelector string) []string {
-		return pieces[0], pieces[1], true
+	if len(fieldSelector) == 0 {
+		return nil
 	}
-	return "", "", false
+
+	terms := make([]string, 0, 1)
+	startIndex := 0
+	inSlash := false
+	for i, c := range fieldSelector {
+		switch {
+		case inSlash:
+			inSlash = false
+		case c == '\\':
+			inSlash = true
+		case c == ',':
+			terms = append(terms, fieldSelector[startIndex:i])
+			startIndex = i + 1
+		}
+	}
+
+	terms = append(terms, fieldSelector[startIndex:])
+
+	return terms
+}
+
+const (
+	notEqualOperator    = "!="
+	doubleEqualOperator = "=="
+	equalOperator       = "="
+)
+
+// termOperators holds the recognized operators supported in fieldSelectors.
+// doubleEqualOperator and equal are equivalent, but doubleEqualOperator is checked first
+// to avoid leaving a leading = character on the rhs value.
+var termOperators = []string{notEqualOperator, doubleEqualOperator, equalOperator}
+
+// splitTerm returns the lhs, operator, and rhs parsed from the given term, along with an indicator of whether the parse was successful.
+// no escaping of special characters is supported in the lhs value, so the first occurance of a recognized operator is used as the split point.
+// the literal rhs is returned, and the caller is responsible for applying any desired unescaping.
+func splitTerm(term string) (lhs, op, rhs string, ok bool) {
+	for i := range term {
+		remaining := term[i:]
+		for _, op := range termOperators {
+			if strings.HasPrefix(remaining, op) {
+				return term[0:i], op, term[i+len(op):], true
+			}
+		}
+	}
+	return "", "", "", false
 }
 
 func parseSelector(selector string, fn TransformFunc) (Selector, error) {
-	parts := strings.Split(selector, ",")
+	parts := splitTerms(selector)
 	sort.StringSlice(parts).Sort()
 	var items []Selector
 	for _, part := range parts {
 		if part == "" {
 			continue
 		}
-		if lhs, rhs, ok := try(part, "!="); ok {
+		lhs, op, rhs, ok := splitTerm(part)
-			items = append(items, &notHasTerm{field: lhs, value: rhs})
+		if !ok {
-		} else if lhs, rhs, ok := try(part, "=="); ok {
+			return nil, fmt.Errorf("invalid selector: '%s'; can't understand '%s'", selector, part)
-			items = append(items, &hasTerm{field: lhs, value: rhs})
+		}
-		} else if lhs, rhs, ok := try(part, "="); ok {
+		unescapedRHS, err := UnescapeValue(rhs)
-			items = append(items, &hasTerm{field: lhs, value: rhs})
+		if err != nil {
-		} else {
+			return nil, err
+		}
+		switch op {
+		case notEqualOperator:
+			items = append(items, &notHasTerm{field: lhs, value: unescapedRHS})
+		case doubleEqualOperator:
+			items = append(items, &hasTerm{field: lhs, value: unescapedRHS})
+		case equalOperator:
+			items = append(items, &hasTerm{field: lhs, value: unescapedRHS})
+		default:
 			return nil, fmt.Errorf("invalid selector: '%s'; can't understand '%s'", selector, part)
 		}
 	}

staging/src/k8s.io/apimachinery/pkg/fields/selector_test.go

@@ -17,18 +17,134 @@ limitations under the License.
 package fields
 
 import (
+	"reflect"
 	"testing"
 )
 
+func TestSplitTerms(t *testing.T) {
+	testcases := map[string][]string{
+		// Simple selectors
+		`a`:                            {`a`},
+		`a=avalue`:                     {`a=avalue`},
+		`a=avalue,b=bvalue`:            {`a=avalue`, `b=bvalue`},
+		`a=avalue,b==bvalue,c!=cvalue`: {`a=avalue`, `b==bvalue`, `c!=cvalue`},
+
+		// Empty terms
+		``:     nil,
+		`a=a,`: {`a=a`, ``},
+		`,a=a`: {``, `a=a`},
+
+		// Escaped values
+		`k=\,,k2=v2`:   {`k=\,`, `k2=v2`},   // escaped comma in value
+		`k=\\,k2=v2`:   {`k=\\`, `k2=v2`},   // escaped backslash, unescaped comma
+		`k=\\\,,k2=v2`: {`k=\\\,`, `k2=v2`}, // escaped backslash and comma
+		`k=\a\b\`:      {`k=\a\b\`},         // non-escape sequences
+		`k=\`:          {`k=\`},             // orphan backslash
+
+		// Multi-byte
+		`함=수,목=록`: {`함=수`, `목=록`},
+	}
+
+	for selector, expectedTerms := range testcases {
+		if terms := splitTerms(selector); !reflect.DeepEqual(terms, expectedTerms) {
+			t.Errorf("splitSelectors(`%s`): Expected\n%#v\ngot\n%#v", selector, expectedTerms, terms)
+		}
+	}
+}
+
+func TestSplitTerm(t *testing.T) {
+	testcases := map[string]struct {
+		lhs string
+		op  string
+		rhs string
+		ok  bool
+	}{
+		// Simple terms
+		`a=value`:  {lhs: `a`, op: `=`, rhs: `value`, ok: true},
+		`b==value`: {lhs: `b`, op: `==`, rhs: `value`, ok: true},
+		`c!=value`: {lhs: `c`, op: `!=`, rhs: `value`, ok: true},
+
+		// Empty or invalid terms
+		``:  {lhs: ``, op: ``, rhs: ``, ok: false},
+		`a`: {lhs: ``, op: ``, rhs: ``, ok: false},
+
+		// Escaped values
+		`k=\,`:          {lhs: `k`, op: `=`, rhs: `\,`, ok: true},
+		`k=\=`:          {lhs: `k`, op: `=`, rhs: `\=`, ok: true},
+		`k=\\\a\b\=\,\`: {lhs: `k`, op: `=`, rhs: `\\\a\b\=\,\`, ok: true},
+
+		// Multi-byte
+		`함=수`: {lhs: `함`, op: `=`, rhs: `수`, ok: true},
+	}
+
+	for term, expected := range testcases {
+		lhs, op, rhs, ok := splitTerm(term)
+		if lhs != expected.lhs || op != expected.op || rhs != expected.rhs || ok != expected.ok {
+			t.Errorf(
+				"splitTerm(`%s`): Expected\n%s,%s,%s,%v\nGot\n%s,%s,%s,%v",
+				term,
+				expected.lhs, expected.op, expected.rhs, expected.ok,
+				lhs, op, rhs, ok,
+			)
+		}
+	}
+}
+
+func TestEscapeValue(t *testing.T) {
+	// map values to their normalized escaped values
+	testcases := map[string]string{
+		``:      ``,
+		`a`:     `a`,
+		`=`:     `\=`,
+		`,`:     `\,`,
+		`\`:     `\\`,
+		`\=\,\`: `\\\=\\\,\\`,
+	}
+
+	for unescapedValue, escapedValue := range testcases {
+		actualEscaped := EscapeValue(unescapedValue)
+		if actualEscaped != escapedValue {
+			t.Errorf("EscapeValue(%s): expected %s, got %s", unescapedValue, escapedValue, actualEscaped)
+		}
+
+		actualUnescaped, err := UnescapeValue(escapedValue)
+		if err != nil {
+			t.Errorf("UnescapeValue(%s): unexpected error %v", escapedValue, err)
+		}
+		if actualUnescaped != unescapedValue {
+			t.Errorf("UnescapeValue(%s): expected %s, got %s", escapedValue, unescapedValue, actualUnescaped)
+		}
+	}
+
+	// test invalid escape sequences
+	invalidTestcases := []string{
+		`\`,   // orphan slash is invalid
+		`\\\`, // orphan slash is invalid
+		`\a`,  // unrecognized escape sequence is invalid
+	}
+	for _, invalidValue := range invalidTestcases {
+		_, err := UnescapeValue(invalidValue)
+		if _, ok := err.(InvalidEscapeSequence); !ok || err == nil {
+			t.Errorf("UnescapeValue(%s): expected invalid escape sequence error, got %#v", invalidValue, err)
+		}
+	}
+}
+
 func TestSelectorParse(t *testing.T) {
 	testGoodStrings := []string{
 		"x=a,y=b,z=c",
 		"",
 		"x!=a,y=b",
+		`x=a||y\=b`,
+		`x=a\=\=b`,
 	}
 	testBadStrings := []string{
 		"x=a||y=b",
 		"x==a==b",
+		"x=a,b",
+		"x in (a)",
+		"x in (a,b,c)",
+		"x",
 	}
 	for _, test := range testGoodStrings {
 		lq, err := ParseSelector(test)
@@ -99,16 +215,18 @@ func TestSelectorMatches(t *testing.T) {
 	expectNoMatch(t, "x=y,z=w", Set{"x": "w", "z": "w"})
 	expectNoMatch(t, "x!=y,z!=w", Set{"x": "z", "z": "w"})
 
-	labelset := Set{
+	fieldset := Set{
-		"foo": "bar",
+		"foo":     "bar",
-		"baz": "blah",
+		"baz":     "blah",
+		"complex": `=value\,\`,
 	}
-	expectMatch(t, "foo=bar", labelset)
+	expectMatch(t, "foo=bar", fieldset)
-	expectMatch(t, "baz=blah", labelset)
+	expectMatch(t, "baz=blah", fieldset)
-	expectMatch(t, "foo=bar,baz=blah", labelset)
+	expectMatch(t, "foo=bar,baz=blah", fieldset)
-	expectNoMatch(t, "foo=blah", labelset)
+	expectMatch(t, `foo=bar,baz=blah,complex=\=value\\\,\\`, fieldset)
-	expectNoMatch(t, "baz=bar", labelset)
+	expectNoMatch(t, "foo=blah", fieldset)
-	expectNoMatch(t, "foo=bar,foobar=bar,baz=blah", labelset)
+	expectNoMatch(t, "baz=bar", fieldset)
+	expectNoMatch(t, "foo=bar,foobar=bar,baz=blah", fieldset)
 }
 
 func TestOneTermEqualSelector(t *testing.T) {