Repository: kythe/kythe
Pull request #4352 (2020-02-04 22:14:12)

fix(tooling): ensure VNameRewriteRule JSON encoding is canonical


Patch:
diff --git a/kythe/go/util/vnameutil/BUILD b/kythe/go/util/vnameutil/BUILD
index 674145c13..d39f4f6fb 100644
--- a/kythe/go/util/vnameutil/BUILD
+++ b/kythe/go/util/vnameutil/BUILD
@@ -25,6 +25,7 @@ go_test(
     deps = [
         "//kythe/proto:storage_go_proto",
         "@com_github_google_go_cmp//cmp:go_default_library",
+        "@com_github_google_go_cmp//cmp/cmpopts:go_default_library",
     ],
 )
 
diff --git a/kythe/go/util/vnameutil/rewrite.go b/kythe/go/util/vnameutil/rewrite.go
index 2e5d099c4..8ffc36e88 100644
--- a/kythe/go/util/vnameutil/rewrite.go
+++ b/kythe/go/util/vnameutil/rewrite.go
@@ -33,7 +33,7 @@ import (
 	spb "kythe.io/kythe/proto/storage_go_proto"
 )
 
-var jsonMarshaler = &jsonpb.Marshaler{OrigName: true}
+var jsonMarshaler = &jsonpb.Marshaler{}
 
 // A Rule associates a regular expression pattern with a VName template.  A
 // Rule can be applied to a string to produce a VName.
@@ -64,7 +64,7 @@ func (r Rule) Apply(input string) (*spb.VName, bool) {
 // ToProto returns an equivalent VNameRewriteRule proto.
 func (r Rule) ToProto() *spb.VNameRewriteRule {
 	return &spb.VNameRewriteRule{
-		Pattern: r.Regexp.String(),
+		Pattern: trimAnchors(r.Regexp.String()),
 		VName: &spb.VName{
 			Corpus:    unfixTemplate(r.VName.Corpus),
 			Root:      unfixTemplate(r.VName.Root),
@@ -143,7 +143,7 @@ func (r Rules) Marshal() ([]byte, error) { return proto.Marshal(r.ToProto()) }
 
 // ConvertRule compiles a VNameRewriteRule proto into a Rule that can be applied to strings.
 func ConvertRule(r *spb.VNameRewriteRule) (Rule, error) {
-	pattern := "^" + strings.TrimSuffix(strings.TrimPrefix(r.Pattern, "^"), "$") + "$"
+	pattern := "^" + trimAnchors(r.Pattern) + "$"
 	re, err := regexp.Compile(pattern)
 	if err != nil {
 		return Rule{}, fmt.Errorf("invalid regular expression: %v", err)
@@ -161,10 +161,17 @@ func ConvertRule(r *spb.VNameRewriteRule) (Rule, error) {
 }
 
 var (
-	fieldRE  = regexp.MustCompile(`@(\w+)@`)
-	markerRE = regexp.MustCompile(`([^$]|^)(\$\$)*\${\w+}`)
+	anchorsRE = regexp.MustCompile(`([^\\]|^)(\\\\)*\$+$`)
+	fieldRE   = regexp.MustCompile(`@(\w+)@`)
+	markerRE  = regexp.MustCompile(`([^$]|^)(\$\$)*\${\w+}`)
 )
 
+func trimAnchors(pattern string) string {
+	return anchorsRE.ReplaceAllStringFunc(strings.TrimPrefix(pattern, "^"), func(r string) string {
+		return strings.TrimSuffix(r, "$")
+	})
+}
+
 // fixTemplate rewrites @x@ markers in the template to the ${x} markers used by
 // the regexp.Expand function, to simplify rewriting.
 func fixTemplate(s string) string {


Test patch:
diff --git a/kythe/go/util/vnameutil/rewrite_test.go b/kythe/go/util/vnameutil/rewrite_test.go
index 5056b7240..42238ef5a 100644
--- a/kythe/go/util/vnameutil/rewrite_test.go
+++ b/kythe/go/util/vnameutil/rewrite_test.go
@@ -25,6 +25,7 @@ import (
 
 	"github.com/golang/protobuf/proto"
 	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 
 	spb "kythe.io/kythe/proto/storage_go_proto"
 )
@@ -100,6 +101,67 @@ func TestParseConsistency(t *testing.T) {
 	}
 }
 
+func TestMarshalJSON(t *testing.T) {
+	tests := []string{
+		"[]",
+		`[{"vname":{"corpus":"a"}}]`,
+		`[{"pattern":"something","vname":{"corpus":"b"}}]`,
+		`[{"pattern":"something\\$","vname":{"corpus":"c"}}]`,
+		`[{"pattern":"\\^\\$","vname":{"corpus":"d"}}]`,
+	}
+
+	for i, test := range tests {
+		t.Run(strconv.Itoa(i), func(t *testing.T) {
+			rules, err := ReadRules(strings.NewReader(test))
+			if err != nil {
+				t.Fatal(err)
+			}
+			t.Logf("Rules: %s", rules)
+
+			rec, err := json.Marshal(rules)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			if diff := cmp.Diff(string(rec), test, splitLines); diff != "" {
+				t.Errorf("Unexpected diff (- found; + expected):\n%s", diff)
+			}
+		})
+	}
+}
+
+func TestTrimAnchors(t *testing.T) {
+	tests := []struct {
+		Pattern  string
+		Expected string
+	}{
+		{"", ""},
+		{"^", ""},
+		{"$", ""},
+		{"^$", ""},
+		{"^a$", "a"},
+		{"^a", "a"},
+		{"a$", "a"},
+		{`\$`, `\$`},
+		{`\^\$`, `\^\$`},
+		{`\^`, `\^`},
+		{`\^^$\$`, `\^^$\$`},
+		{`\\$`, `\\`},
+		{`\\\\$`, `\\\\`},
+		{`\\\$`, `\\\$`},
+		{`^abc[^def$]ghi$`, `abc[^def$]ghi`},
+	}
+
+	for _, test := range tests {
+		test := test
+		t.Run(strconv.Quote(test.Pattern), func(t *testing.T) {
+			if found := trimAnchors(test.Pattern); found != test.Expected {
+				t.Errorf("Found: %q; Expected: %q", found, test.Expected)
+			}
+		})
+	}
+}
+
 func TestRoundtripJSON(t *testing.T) {
 	tests := []string{
 		"[]",
@@ -251,4 +313,5 @@ func (v V) pb() *spb.VName {
 var (
 	transformRegexp = cmp.Transformer("Regexp", func(r *regexp.Regexp) string { return r.String() })
 	compareVNames   = cmp.Comparer(func(a, b *spb.VName) bool { return proto.Equal(a, b) })
+	splitLines      = cmpopts.AcyclicTransformer("Lines", func(s string) []string { return strings.Split(s, "\n") })
 )