Skip to content

Commit

Permalink
fix(tooling): ensure VNameRewriteRule JSON encoding is canonical (#4352)
Browse files Browse the repository at this point in the history
  • Loading branch information
schroederc committed Feb 4, 2020
1 parent e753a99 commit 124e000
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 5 deletions.
1 change: 1 addition & 0 deletions kythe/go/util/vnameutil/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
)

Expand Down
17 changes: 12 additions & 5 deletions kythe/go/util/vnameutil/rewrite.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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)
Expand All @@ -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 {
Expand Down
63 changes: 63 additions & 0 deletions kythe/go/util/vnameutil/rewrite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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{
"[]",
Expand Down Expand Up @@ -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") })
)

0 comments on commit 124e000

Please sign in to comment.