diff --git a/internal/httprule/template.go b/internal/httprule/template.go index fdbf9fb..e964970 100644 --- a/internal/httprule/template.go +++ b/internal/httprule/template.go @@ -228,7 +228,11 @@ func (p *parser) parseLiteral() (string, error) { break } if len(literal) == 0 { - return "", fmt.Errorf("expected literal at %d, found %q", startPos-1, p.tok) + tok := fmt.Sprintf("%q", p.tok) + if p.tok == -1 { + tok = "EOF" + } + return "", fmt.Errorf("expected literal at position %d, found %s", startPos-1, tok) } return string(literal), nil } @@ -245,7 +249,11 @@ func (p *parser) parseIdent() (string, error) { break } if len(ident) == 0 { - return "", fmt.Errorf("expected identifier at %d, found %q", startPos-1, p.tok) + tok := fmt.Sprintf("%q", p.tok) + if p.tok == -1 { + tok = "EOF" + } + return "", fmt.Errorf("expected identifier at position %d, found %s", startPos-1, tok) } return string(ident), nil } @@ -274,7 +282,11 @@ func (p *parser) peekN(n int) rune { func (p *parser) expect(r rune) error { if p.tok != r { - return fmt.Errorf("expected token %q, got %q at position %d", r, p.tok, p.pos-1) + tok := fmt.Sprintf("%q", p.tok) + if p.tok == -1 { + tok = "EOF" + } + return fmt.Errorf("expected token %q at position %d, found %s", r, p.pos, tok) } p.next() return nil @@ -342,12 +354,12 @@ func validate(t Template) error { continue } if s.Kind == SegmentKindMatchMultiple { - return fmt.Errorf("'**' only allowed as the last part of the template") + return fmt.Errorf("'**' only allowed as last part of template") } if s.Kind == SegmentKindVariable { for _, s2 := range s.Variable.Segments { if s2.Kind == SegmentKindMatchMultiple { - return fmt.Errorf("'**' only allowed as the last part of the template") + return fmt.Errorf("'**' only allowed as last part of template") } } } @@ -366,5 +378,25 @@ func validate(t Template) error { } } } + // check for top level expansions + for _, s := range t.Segments { + if s.Kind == SegmentKindMatchSingle { + return fmt.Errorf("'*' must only be used in variables") + } + if s.Kind == SegmentKindMatchMultiple { + return fmt.Errorf("'**' must only be used in variables") + } + } + // check for duplicate variable bindings + seen := make(map[string]struct{}) + for _, s := range t.Segments { + if s.Kind == SegmentKindVariable { + field := s.Variable.FieldPath.String() + if _, ok := seen[s.Variable.FieldPath.String()]; ok { + return fmt.Errorf("variable '%s' bound multiple times", field) + } + seen[field] = struct{}{} + } + } return nil } diff --git a/internal/httprule/template_test.go b/internal/httprule/template_test.go index 9c9e963..46dc9f0 100644 --- a/internal/httprule/template_test.go +++ b/internal/httprule/template_test.go @@ -199,21 +199,25 @@ func Test_ParseTemplate(t *testing.T) { func Test_ParseTemplate_Invalid(t *testing.T) { t.Parallel() - for _, tt := range []string{ - "", - "//", - "/v1:", - "/v1/:", - "/{name=messages/{id}}", - "/**/*", - "v1/messages/*", - "v1/{id}/{id}", + for _, tt := range []struct { + template string + expected string + }{ + {template: "", expected: "expected token '/' at position 0, found EOF"}, + {template: "//", expected: "expected literal at position 1, found '/'"}, + {template: "/v1:", expected: "expected literal at position 3, found EOF"}, + {template: "/v1/:", expected: "expected literal at position 4, found ':'"}, + {template: "/{name=messages/{id}}", expected: "nested variable segment is not allowed"}, + {template: "/**/*", expected: "'**' only allowed as last part of template"}, + {template: "/v1/messages/*", expected: "'*' must only be used in variables"}, + {template: "/v1/{id}/{id}", expected: "variable 'id' bound multiple times"}, } { tt := tt - t.Run(tt, func(t *testing.T) { + t.Run(tt.template, func(t *testing.T) { t.Parallel() - _, err := ParseTemplate(tt) + _, err := ParseTemplate(tt.template) assert.Check(t, err != nil) + assert.ErrorContains(t, err, tt.expected) }) } }