net/http: show offset in pattern parsing error

Track the offset in the pattern string being parsed so we can show it
in the error message.

Change-Id: I495b99378d866f359f45974ffc33587e2c1e366d
Reviewed-on: https://go-review.googlesource.com/c/go/+/529123
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

diff --git a/src/net/http/pattern.go b/src/net/http/pattern.go
index eca179180faf81a1ae7f97fe841b88de204c9153..2993aeccb92d26ac8f79bbbc633e0502bb276529 100644 (file)
--- a/src/net/http/pattern.go
+++ b/src/net/http/pattern.go
@@ -80,31 +80,42 @@ type segment struct {
 // The "{$}" and "{name...}" wildcard must occur at the end of PATH.
 // PATH may end with a '/'.
 // Wildcard names in a path must be distinct.
-func parsePattern(s string) (*pattern, error) {
+func parsePattern(s string) (_ *pattern, err error) {
        if len(s) == 0 {
                return nil, errors.New("empty pattern")
        }
-       // TODO(jba): record the rune offset in s to provide more information in errors.
+       off := 0 // offset into string
+       defer func() {
+               if err != nil {
+                       err = fmt.Errorf("at offset %d: %w", off, err)
+               }
+       }()
+
        method, rest, found := strings.Cut(s, " ")
        if !found {
                rest = method
                method = ""
        }
        if method != "" && !validMethod(method) {
-               return nil, fmt.Errorf("net/http: invalid method %q", method)
+               return nil, fmt.Errorf("invalid method %q", method)
        }
        p := &pattern{str: s, method: method}
 
+       if found {
+               off = len(method) + 1
+       }
        i := strings.IndexByte(rest, '/')
        if i < 0 {
                return nil, errors.New("host/path missing /")
        }
        p.host = rest[:i]
        rest = rest[i:]
-       if strings.IndexByte(p.host, '{') >= 0 {
+       if j := strings.IndexByte(p.host, '{'); j >= 0 {
+               off += j
                return nil, errors.New("host contains '{' (missing initial '/'?)")
        }
        // At this point, rest is the path.
+       off += i
 
        // An unclean path with a method that is not CONNECT can never match,
        // because paths are cleaned before matching.
@@ -116,6 +127,7 @@ func parsePattern(s string) (*pattern, error) {
        for len(rest) > 0 {
                // Invariant: rest[0] == '/'.
                rest = rest[1:]
+               off = len(s) - len(rest)
                if len(rest) == 0 {
                        // Trailing slash.
                        p.segments = append(p.segments, segment{wild: true, multi: true})

diff --git a/src/net/http/pattern_test.go b/src/net/http/pattern_test.go
index f67a2b5135f422622a1e4cbab5baa4c9b94155d3..e71cba863242de29c3af8c48d1347697eaade6f2 100644 (file)
--- a/src/net/http/pattern_test.go
+++ b/src/net/http/pattern_test.go
@@ -108,22 +108,24 @@ func TestParsePatternError(t *testing.T) {
                contains string
        }{
                {"", "empty pattern"},
-               {"A=B /", "invalid method"},
-               {" ", "missing /"},
-               {"/{w}x", "bad wildcard segment"},
-               {"/x{w}", "bad wildcard segment"},
-               {"/{wx", "bad wildcard segment"},
-               {"/{a$}", "bad wildcard name"},
-               {"/{}", "empty wildcard"},
-               {"/{...}", "empty wildcard"},
-               {"/{$...}", "bad wildcard"},
-               {"/{$}/", "{$} not at end"},
-               {"/{$}/x", "{$} not at end"},
-               {"/{a...}/", "not at end"},
-               {"/{a...}/x", "not at end"},
-               {"{a}/b", "missing initial '/'"},
-               {"/a/{x}/b/{x...}", "duplicate wildcard name"},
-               {"GET //", "unclean path"},
+               {"A=B /", "at offset 0: invalid method"},
+               {" ", "at offset 1: host/path missing /"},
+               {"/{w}x", "at offset 1: bad wildcard segment"},
+               {"/x{w}", "at offset 1: bad wildcard segment"},
+               {"/{wx", "at offset 1: bad wildcard segment"},
+               {"/{a$}", "at offset 1: bad wildcard name"},
+               {"/{}", "at offset 1: empty wildcard"},
+               {"POST a.com/x/{}/y", "at offset 13: empty wildcard"},
+               {"/{...}", "at offset 1: empty wildcard"},
+               {"/{$...}", "at offset 1: bad wildcard"},
+               {"/{$}/", "at offset 1: {$} not at end"},
+               {"/{$}/x", "at offset 1: {$} not at end"},
+               {"/abc/{$}/x", "at offset 5: {$} not at end"},
+               {"/{a...}/", "at offset 1: {...} wildcard not at end"},
+               {"/{a...}/x", "at offset 1: {...} wildcard not at end"},
+               {"{a}/b", "at offset 0: host contains '{' (missing initial '/'?)"},
+               {"/a/{x}/b/{x...}", "at offset 9: duplicate wildcard name"},
+               {"GET //", "at offset 4: non-CONNECT pattern with unclean path"},
        } {
                _, err := parsePattern(test.in)
                if err == nil || !strings.Contains(err.Error(), test.contains) {

diff --git a/src/net/http/server_test.go b/src/net/http/server_test.go
index 0c361c7d66a957aebc7aeab4549fe57fb4167004..a96d87656e757b2c62a16c383b15ba4f2ac1fd35 100644 (file)
--- a/src/net/http/server_test.go
+++ b/src/net/http/server_test.go
@@ -131,7 +131,7 @@ func TestRegisterErr(t *testing.T) {
                {"", h, "invalid pattern"},
                {"/", nil, "nil handler"},
                {"/", HandlerFunc(nil), "nil handler"},
-               {"/{x", h, `parsing "/\{x": bad wildcard segment`},
+               {"/{x", h, `parsing "/\{x": at offset 1: bad wildcard segment`},
                {"/a", h, `conflicts with pattern.* \(registered at .*/server_test.go:\d+`},
        } {
                t.Run(fmt.Sprintf("%s:%#v", test.pattern, test.handler), func(t *testing.T) {