diff --git a/docs/services/routing.md b/docs/services/routing.md index 9454eee..de727ac 100644 --- a/docs/services/routing.md +++ b/docs/services/routing.md @@ -50,6 +50,19 @@ func showPost(w http.ResponseWriter, r *http.Request) { } ``` +## Overlapping constrained routes + +Laravel matches routes in the order `routes.php` declares them and checks `->where()` constraints while it matches. Two routes can therefore share a shape that only their constraints keep apart, for example `GET /shelves/items/{id}` with a numeric `id` and `GET /shelves/{shelfId}/follow` with a numeric `shelfId`. Both patterns match `/shelves/items/follow`, and neither is more specific than the other, so the standard library ServeMux refuses to register them together. + +surf accepts such routes. When the application starts, it groups every set of routes that ServeMux would refuse side by side into an overlap family and registers the family once, under a generated pattern with a wildcard wherever its routes differ. A request that reaches the family is tried against its routes in registration order. The first route whose literal segments and constraints match runs, with its own path values, middleware, body limit and panic recovery. Declare overlapping routes in the order the PHP file declares them. + +- **No match:** a request whose path no route of the family matches gets the same bare 404 as any unknown path, for every method. +- **Wrong method:** a request whose path a route matches, but not with its method, gets the 405 and `Allow` header that ServeMux gives for the same routes without the overlap. +- **More specific routes** stay outside the family: a literal `GET /shelves/items/similar` keeps answering its own path. +- **Route table:** `surf.Router.Routes` and `route:list` list every route of a family on its own, with its own pattern and middleware. + +A family supports literal segments and single-segment `{name}` wildcards only. These shapes still stop the start-up with a route conflict: two routes of one method that differ only in parameter names, a `{name...}` or `{$}` wildcard or a trailing slash in a family, and a more general route of the same method, such as `GET /shelves/{a}/{b}`, that would take a family member's requests. + ## Auth groups An auth group is a group whose middleware list names a guard. The plugin above turns a [bouncer](../../modules/bouncer/README.md) JWT guard into named middleware and returns it from `pact.HasMiddleware`: diff --git a/modules/surf/README.md b/modules/surf/README.md index d6eaf2d..319f11e 100644 --- a/modules/surf/README.md +++ b/modules/surf/README.md @@ -12,6 +12,7 @@ surf turns the routes that plugins declare through `pact.HasRoutes` into one `ht - Laravel-style route groups: `surf.Router.Group` with a path prefix and middleware list (`surf.Use` builds the list), `surf.Router.Get`, `surf.Router.Post`, `surf.Router.Put`, `surf.Router.Patch` and `surf.Router.Delete` (the same methods exist on each `surf.Group`), with Go 1.22+ path patterns such as `/posts/{id}`. - Path constraints: `surf.Router.Where` (regex, anchored to the whole segment) and `surf.Router.WhereIn` (allow-list) apply to the last declared route; a request that fails a constraint gets a 404. `surf.IntParam` reads a positive integer path value. +- Overlapping constrained routes: two routes that ServeMux would refuse side by side, such as `/shelves/items/{id}` and `/shelves/{shelfId}/follow`, boot as one overlap family when their constraints keep them apart. Requests are tried against the family's routes in registration order, as Laravel does, and each route keeps its own path values and middleware. - Named middleware from plugins (`pact.HasMiddleware`), parameterized middleware used as `name:param` (`pact.HasMiddlewareFactories`) and house middleware for the JSON envelope and error handling (`pact.HasHouseMiddleware`). Duplicate or unknown names fail boot. - Raw groups (`surf.Router.GroupRaw`) for routes that must not be wrapped in house middleware, such as webhooks or file streams: house middleware is refused there, the default body limit is skipped and a panic returns a bare 500. - Built-in middleware names: `throttle:` or `throttle:,`, `body.limit:`, `locale.from-principal`, plus `backend` (the admin guard) when the admin is enabled. @@ -91,6 +92,8 @@ func listPosts(w http.ResponseWriter, r *http.Request) { wire.WriteJSON(w, http func addComment(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusCreated) } ``` +Routes ported from a `routes.php` sometimes overlap: `GET /shelves/items/{id}` with `id` numeric and `GET /shelves/{shelfId}/follow` with `shelfId` numeric both match `/shelves/items/follow` as patterns, so ServeMux alone would refuse to register them. surf registers such routes as an overlap family and tries them in registration order, checking literal segments and `Where` constraints, so declare them in the same order as the PHP file. A request no member matches gets the bare 404. Overlapping routes must use literal segments and single-segment `{name}` wildcards; two routes of one method with the same shape, a `{name...}` wildcard in a family, or a more general route that would take a member's requests still fail at boot. `surf.Router.Routes` and `route:list` keep listing every route on its own. + The generated application `main` wires surf in with `surf.ServeCommand` and `surf.RouteListCommand`; tests can call `surf.Assemble(app, plugins)` and drive the returned handler with `net/http/httptest`. ## API reference diff --git a/modules/surf/overlap.go b/modules/surf/overlap.go new file mode 100644 index 0000000..e5e50a3 --- /dev/null +++ b/modules/surf/overlap.go @@ -0,0 +1,525 @@ +package surf + +import ( + "fmt" + "net/http" + "net/url" + "sort" + "strings" +) + +// Overlapping constrained routes. +// +// Laravel matches routes in registration order and checks ->where() +// constraints while matching, so `GET /shelves/items/{id}` (id numeric) and +// `GET /shelves/{shelfId}/follow` live side by side: no request satisfies +// both. ServeMux only looks at the patterns and refuses the pair at +// registration because neither is more specific than the other. +// +// compile therefore groups such routes into overlap families (the transitive +// closure of "ServeMux refuses these two") and registers each family once, +// under a generated method-less pattern with a wildcard wherever the members +// differ. The family handler walks the members in registration order and runs +// the first whose literal segments and constraints match, with its own path +// values and its own fully wrapped handler. Everything outside a family +// registers exactly as before. + +// pathSeg is one segment of a simple route path: a literal or a +// single-segment wildcard. +type pathSeg struct { + raw string // segment text as written in the route + lit string // unescaped literal; empty for a wildcard + param string // wildcard name; empty for a literal +} + +// overlapFamily is one group of routes registered behind a single pattern. +type overlapFamily struct { + members []int // indexes into Router.routes, in registration order + pattern string // generated method-less ServeMux pattern + wild []string // per segment: generated wildcard name, or "" for a shared literal +} + +const overlapProbeSegment = "surf~overlap~probe" + +var noopHandler = http.NotFoundHandler() + +// parseSimplePath splits a route path into literal and single-segment +// wildcard segments. It reports false for every other shape: the root, a +// trailing-slash subtree, an empty segment, a {name...} or {$} wildcard, a +// host, or a malformed escape. +func parseSimplePath(path string) ([]pathSeg, bool) { + if !strings.HasPrefix(path, "/") || path == "/" || strings.HasSuffix(path, "/") { + return nil, false + } + parts := strings.Split(path[1:], "/") + segs := make([]pathSeg, len(parts)) + for i, part := range parts { + if part == "" { + return nil, false + } + if strings.HasPrefix(part, "{") && strings.HasSuffix(part, "}") { + name := part[1 : len(part)-1] + if !isWildcardName(name) { + return nil, false + } + segs[i] = pathSeg{raw: part, param: name} + continue + } + if strings.ContainsAny(part, "{}") { + return nil, false + } + lit, err := url.PathUnescape(part) + if err != nil { + return nil, false + } + segs[i] = pathSeg{raw: part, lit: lit} + } + return segs, true +} + +func isWildcardName(name string) bool { + if name == "" { + return false + } + for i, c := range name { + switch { + case c == '_', c >= 'a' && c <= 'z', c >= 'A' && c <= 'Z': + case i > 0 && c >= '0' && c <= '9': + default: + return false + } + } + return true +} + +// tryHandle registers pattern on mux and reports whether ServeMux accepted it. +func tryHandle(mux *http.ServeMux, pattern string, h http.Handler) (ok bool) { + defer func() { + if recover() != nil { + ok = false + } + }() + mux.Handle(pattern, h) + return true +} + +// muxConflict asks ServeMux itself whether two valid patterns can share a mux. +func muxConflict(a, b string) bool { + if !tryHandle(http.NewServeMux(), a, noopHandler) || !tryHandle(http.NewServeMux(), b, noopHandler) { + return false + } + mux := http.NewServeMux() + mux.Handle(a, noopHandler) + return !tryHandle(mux, b, noopHandler) +} + +// mayOverlap is a cheap pre-filter: two simple paths with a different segment +// count or two different literals at one position can never match the same +// request. Anything else goes to muxConflict. +func mayOverlap(a, b string) bool { + sa, okA := parseSimplePath(a) + sb, okB := parseSimplePath(b) + if !okA || !okB { + return true + } + if len(sa) != len(sb) { + return false + } + for i := range sa { + if sa[i].param == "" && sb[i].param == "" && sa[i].lit != sb[i].lit { + return false + } + } + return true +} + +func routePattern(rt route) string { return rt.method + " " + rt.path } + +func routeConflict(a, b route) bool { + return a.method == b.method && mayOverlap(a.path, b.path) && muxConflict(routePattern(a), routePattern(b)) +} + +type unionFind []int + +func newUnionFind(n int) unionFind { + u := make(unionFind, n) + for i := range u { + u[i] = i + } + return u +} + +func (u unionFind) find(i int) int { + for u[i] != i { + u[i] = u[u[i]] + i = u[i] + } + return i +} + +// union joins two sets and reports whether they were separate. +func (u unionFind) union(a, b int) bool { + ra, rb := u.find(a), u.find(b) + if ra == rb { + return false + } + if ra < rb { + u[rb] = ra + } else { + u[ra] = rb + } + return true +} + +func (u unionFind) groups() [][]int { + byRoot := map[int][]int{} + var roots []int + for i := range u { + root := u.find(i) + if _, ok := byRoot[root]; !ok { + roots = append(roots, root) + } + byRoot[root] = append(byRoot[root], i) + } + sort.Ints(roots) + var out [][]int + for _, root := range roots { + if members := byRoot[root]; len(members) > 1 { + out = append(out, members) + } + } + return out +} + +// overlapFamilies finds every overlap family in routes. A route whose +// pattern ServeMux rejects for another reason than a conflict is left alone +// so handleRoute reports it as before. +func overlapFamilies(routes []route) ([]overlapFamily, error) { + uf := newUnionFind(len(routes)) + probe := http.NewServeMux() + var outside []int // routes the probe mux refused + for j, rt := range routes { + pat := routePattern(rt) + if !tryHandle(http.NewServeMux(), pat, noopHandler) { + continue + } + if tryHandle(probe, pat, noopHandler) { + for _, i := range outside { + if routeConflict(routes[i], rt) { + uf.union(i, j) + } + } + continue + } + for i := 0; i < j; i++ { + if routeConflict(routes[i], rt) { + uf.union(i, j) + } + } + outside = append(outside, j) + } + if len(uf.groups()) == 0 { + return nil, nil + } + // A family's generated pattern may itself overlap another route or + // another family's pattern; fold those in until nothing changes. + for { + groups := uf.groups() + fams := make([]overlapFamily, len(groups)) + for fi, members := range groups { + fam, err := generaliseFamily(routes, members) + if err != nil { + return nil, err + } + fams[fi] = fam + } + changed := false + for _, fam := range fams { + root := uf.find(fam.members[0]) + for k, rt := range routes { + if uf.find(k) == root || inAnyFamily(uf, fams, k) { + continue + } + if mayOverlap(fam.pattern, rt.path) && muxConflict(fam.pattern, routePattern(rt)) { + changed = uf.union(fam.members[0], k) || changed + } + } + } + for a := 0; a < len(fams); a++ { + for b := a + 1; b < len(fams); b++ { + if mayOverlap(fams[a].pattern, fams[b].pattern) && muxConflict(fams[a].pattern, fams[b].pattern) { + changed = uf.union(fams[a].members[0], fams[b].members[0]) || changed + } + } + } + if !changed { + return fams, nil + } + } +} + +func inAnyFamily(uf unionFind, fams []overlapFamily, k int) bool { + root := uf.find(k) + for _, fam := range fams { + if uf.find(fam.members[0]) == root { + return true + } + } + return false +} + +func describeRoute(rt route) string { + return fmt.Sprintf("%s %s (plugin %q)", rt.method, rt.path, rt.pluginID) +} + +// generaliseFamily validates the member shapes and builds the shared pattern. +func generaliseFamily(routes []route, members []int) (overlapFamily, error) { + parsed := make([][]pathSeg, len(members)) + for mi, idx := range members { + segs, ok := parseSimplePath(routes[idx].path) + if !ok { + other := members[0] + if other == idx { + other = members[1] + } + return overlapFamily{}, fmt.Errorf("surf: route conflict for %s and %s: overlapping routes must use literal and single-segment {name} wildcards only", + describeRoute(routes[idx]), describeRoute(routes[other])) + } + parsed[mi] = segs + } + n := len(parsed[0]) + for mi := 1; mi < len(parsed); mi++ { + if len(parsed[mi]) != n { + return overlapFamily{}, fmt.Errorf("surf: route conflict for %s and %s: overlapping routes differ in segment count", + describeRoute(routes[members[mi]]), describeRoute(routes[members[0]])) + } + } + // Two routes of one method with the same shape differ only in parameter + // names: the later one could never be reached by registration order. + for a := 0; a < len(members); a++ { + for b := a + 1; b < len(members); b++ { + if routes[members[a]].method == routes[members[b]].method && sameShape(parsed[a], parsed[b]) { + return overlapFamily{}, fmt.Errorf("surf: route conflict for %s and %s: the routes have the same shape", + describeRoute(routes[members[b]]), describeRoute(routes[members[a]])) + } + } + } + wild := make([]string, n) + parts := make([]string, n) + for i := 0; i < n; i++ { + shared := true + for mi := range parsed { + s := parsed[mi][i] + if s.param != "" || s.lit != parsed[0][i].lit { + shared = false + break + } + } + if shared { + parts[i] = parsed[0][i].raw + continue + } + wild[i] = fmt.Sprintf("surfOverlap%d", i) + parts[i] = "{" + wild[i] + "}" + } + return overlapFamily{ + members: append([]int(nil), members...), + pattern: "/" + strings.Join(parts, "/"), + wild: wild, + }, nil +} + +func sameShape(a, b []pathSeg) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if (a[i].param == "") != (b[i].param == "") { + return false + } + if a[i].param == "" && a[i].lit != b[i].lit { + return false + } + } + return true +} + +// familyMember is one route behind a family pattern. +type familyMember struct { + method string + pattern string + segs []pathSeg + constraints []Constraint + handler http.Handler +} + +func (m familyMember) pathMatches(vals []string) bool { + for i, s := range m.segs { + if s.param == "" && s.lit != vals[i] { + return false + } + } + return true +} + +func (m familyMember) constraintsMatch(vals []string) bool { + for _, c := range m.constraints { + value := "" + for i, s := range m.segs { + if s.param == c.param { + value = vals[i] + break + } + } + if !c.match(value) { + return false + } + } + return true +} + +// methodMatches mirrors ServeMux: a GET route also answers HEAD. +func methodMatches(routeMethod, reqMethod string) bool { + return routeMethod == reqMethod || (routeMethod == http.MethodGet && reqMethod == http.MethodHead) +} + +// familyDispatch is the handler registered under a family pattern. +type familyDispatch struct { + pattern string + wild []string + lits []string + members []familyMember + methods []string // every method in the router's table, for Allow + mux *http.ServeMux +} + +func (f *familyDispatch) ServeHTTP(w http.ResponseWriter, r *http.Request) { + vals := make([]string, len(f.wild)) + for i, name := range f.wild { + if name == "" { + vals[i] = f.lits[i] + } else { + vals[i] = r.PathValue(name) + } + } + methodSeen := false + for _, m := range f.members { + if !methodMatches(m.method, r.Method) || !m.pathMatches(vals) { + continue + } + methodSeen = true + if !m.constraintsMatch(vals) { + continue + } + for i, s := range m.segs { + if s.param != "" { + r.SetPathValue(s.param, vals[i]) + } + } + r.Pattern = m.pattern + m.handler.ServeHTTP(w, r) + return + } + if methodSeen { + http.NotFound(w, r) + return + } + allow := f.allowed(r, vals) + if len(allow) == 0 { + http.NotFound(w, r) + return + } + w.Header().Set("Allow", strings.Join(allow, ", ")) + http.Error(w, http.StatusText(http.StatusMethodNotAllowed), http.StatusMethodNotAllowed) +} + +// allowed lists the methods ServeMux would put in Allow for this path if the +// table held the members on their own patterns: any other route the mux +// matches for the method, plus members whose literal segments match. +func (f *familyDispatch) allowed(r *http.Request, vals []string) []string { + set := map[string]bool{} + for _, method := range f.methods { + _, pat := f.mux.Handler(&http.Request{Method: method, URL: r.URL, Host: r.Host}) + switch pat { + case "": + case f.pattern: + for _, m := range f.members { + if m.method == method && m.pathMatches(vals) { + set[method] = true + break + } + } + default: + set[method] = true + } + } + if set[http.MethodGet] { + set[http.MethodHead] = true + } + out := make([]string, 0, len(set)) + for method := range set { + out = append(out, method) + } + sort.Strings(out) + return out +} + +// registerOverlapFamilies registers each family pattern on mux, then checks +// that no other route takes a member's requests away from the family. +func registerOverlapFamilies(mux *http.ServeMux, routes []route, handlers []http.Handler, fams []overlapFamily) error { + if len(fams) == 0 { + return nil + } + methodSet := map[string]bool{} + for _, rt := range routes { + methodSet[rt.method] = true + } + methods := make([]string, 0, len(methodSet)) + for method := range methodSet { + methods = append(methods, method) + } + sort.Strings(methods) + for _, fam := range fams { + d := &familyDispatch{pattern: fam.pattern, wild: fam.wild, methods: methods, mux: mux} + d.lits = make([]string, len(fam.wild)) + for _, idx := range fam.members { + rt := routes[idx] + segs, _ := parseSimplePath(rt.path) + for i, s := range segs { + if fam.wild[i] == "" { + d.lits[i] = s.lit + } + } + d.members = append(d.members, familyMember{ + method: rt.method, + pattern: routePattern(rt), + segs: segs, + constraints: rt.constraints, + handler: handlers[idx], + }) + } + if !tryHandle(mux, fam.pattern, d) { + first := routes[fam.members[0]] + return fmt.Errorf("surf: route conflict for %s: the overlap pattern %s cannot be registered", describeRoute(first), fam.pattern) + } + } + for _, fam := range fams { + for _, idx := range fam.members { + rt := routes[idx] + segs, _ := parseSimplePath(rt.path) + parts := make([]string, len(segs)) + for i, s := range segs { + if s.param != "" { + parts[i] = overlapProbeSegment + } else { + parts[i] = s.raw + } + } + probe := &http.Request{Method: rt.method, URL: &url.URL{Path: "/" + strings.Join(parts, "/")}} + if _, pat := mux.Handler(probe); pat != fam.pattern { + return fmt.Errorf("surf: route conflict for %s: route %s takes its requests before the overlapping routes are tried", describeRoute(rt), pat) + } + } + } + return nil +} diff --git a/modules/surf/overlap_test.go b/modules/surf/overlap_test.go new file mode 100644 index 0000000..bb86311 --- /dev/null +++ b/modules/surf/overlap_test.go @@ -0,0 +1,370 @@ +package surf + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +type overlapRoute struct { + name string + method string + path string + where map[string]string +} + +// overlapTable mirrors the overlapping shapes an application may port from +// routes.php: a literal-segment route beside a numeric one, a three-way +// family, two methods sharing one generated pattern, and a literal route +// that stays more specific than every family member. +var overlapTable = []overlapRoute{ + {name: "token-follow", method: "GET", path: "/shelves/token/{token}/follow"}, + {name: "token-follow-post", method: "POST", path: "/shelves/token/{token}/follow"}, + {name: "shelf-follow", method: "GET", path: "/shelves/{shelfId}/follow", where: map[string]string{"shelfId": "[0-9]+"}}, + {name: "shelf-follow-post", method: "POST", path: "/shelves/{shelfId}/follow", where: map[string]string{"shelfId": "[0-9]+"}}, + {name: "shelf-unfollow", method: "DELETE", path: "/shelves/{shelfId}/follow", where: map[string]string{"shelfId": "[0-9]+"}}, + {name: "item-similar", method: "GET", path: "/shelves/items/similar"}, + {name: "item-show", method: "GET", path: "/shelves/items/{id}", where: map[string]string{"id": "[0-9]+"}}, + {name: "item-update", method: "PUT", path: "/shelves/items/{id}", where: map[string]string{"id": "[0-9]+"}}, + {name: "item-delete", method: "DELETE", path: "/shelves/items/{id}", where: map[string]string{"id": "[0-9]+"}}, + {name: "shelf-items", method: "GET", path: "/shelves/{shelfId}/items", where: map[string]string{"shelfId": "[0-9]+"}}, + {name: "shelf-item", method: "GET", path: "/shelves/{shelfId}/items/{itemId}", where: map[string]string{"shelfId": "[0-9]+", "itemId": "[0-9]+"}}, +} + +func overlapParams(path string) []string { + var out []string + for _, seg := range strings.Split(path, "/") { + if strings.HasPrefix(seg, "{") && strings.HasSuffix(seg, "}") { + out = append(out, strings.Trim(seg, "{}")) + } + } + return out +} + +func overlapHandler(rt overlapRoute) http.HandlerFunc { + params := overlapParams(rt.path) + return func(w http.ResponseWriter, r *http.Request) { + parts := []string{rt.name} + for _, p := range params { + parts = append(parts, p+"="+r.PathValue(p)) + } + _, _ = w.Write([]byte(strings.Join(parts, " "))) + } +} + +func markMiddleware(name string) func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Add("X-Mw", name) + next.ServeHTTP(w, r) + }) + } +} + +func buildOverlapRouter(t *testing.T, table []overlapRoute) *Router { + t.Helper() + r := New(nil) + r.BindPlugin("acme.shelves") + for _, rt := range table { + mw := "mw." + rt.name + if err := r.RegisterMiddleware("acme.shelves", mw, markMiddleware(rt.name)); err != nil { + t.Fatal(err) + } + h := overlapHandler(rt) + switch rt.method { + case "GET": + r.Get(rt.path, h, mw) + case "POST": + r.Post(rt.path, h, mw) + case "PUT": + r.Put(rt.path, h, mw) + case "PATCH": + r.Patch(rt.path, h, mw) + case "DELETE": + r.Delete(rt.path, h, mw) + default: + t.Fatalf("method %s", rt.method) + } + for param, re := range rt.where { + r.Where(param, re) + } + } + return r +} + +func compileOverlap(t *testing.T, table []overlapRoute) http.Handler { + t.Helper() + h, err := buildOverlapRouter(t, table).compile() + if err != nil { + t.Fatalf("compile: %v", err) + } + return h +} + +// referenceMux registers the table on a plain ServeMux, without the named +// conflicting partners, as the router would have done before overlap support. +func referenceMux(t *testing.T, table []overlapRoute, drop ...string) *http.ServeMux { + t.Helper() + skip := map[string]bool{} + for _, d := range drop { + skip[d] = true + } + mux := http.NewServeMux() + for _, rt := range table { + if skip[rt.name] { + continue + } + mux.Handle(rt.method+" "+rt.path, overlapHandler(rt)) + } + return mux +} + +func serve(h http.Handler, method, path string) *httptest.ResponseRecorder { + rec := httptest.NewRecorder() + h.ServeHTTP(rec, httptest.NewRequest(method, path, nil)) + return rec +} + +func TestOverlappingConstrainedRoutes(t *testing.T) { + t.Run("boot", func(t *testing.T) { + r := buildOverlapRouter(t, overlapTable) + if _, err := r.compile(); err != nil { + t.Fatalf("compile: %v", err) + } + fams, err := overlapFamilies(r.routes) + if err != nil { + t.Fatal(err) + } + // One four-segment GET family and one three-segment family holding + // the GET and DELETE members (both methods generalise to one shape). + if len(fams) != 2 { + t.Fatalf("families = %d, want 2: %+v", len(fams), fams) + } + got := map[string]int{} + for _, f := range fams { + got[f.pattern] = len(f.members) + } + if got["/shelves/{surfOverlap1}/{surfOverlap2}"] != 5 || got["/shelves/{surfOverlap1}/{surfOverlap2}/{surfOverlap3}"] != 2 { + t.Fatalf("family patterns = %v", got) + } + }) + + h := compileOverlap(t, overlapTable) + + t.Run("dispatch", func(t *testing.T) { + cases := []struct{ method, path, want string }{ + {"GET", "/shelves/token/abc/follow", "token-follow token=abc"}, + {"GET", "/shelves/token/5/follow", "token-follow token=5"}, + {"POST", "/shelves/token/abc/follow", "token-follow-post token=abc"}, + {"GET", "/shelves/5/items/6", "shelf-item shelfId=5 itemId=6"}, + {"GET", "/shelves/5/follow", "shelf-follow shelfId=5"}, + {"POST", "/shelves/5/follow", "shelf-follow-post shelfId=5"}, + {"DELETE", "/shelves/5/follow", "shelf-unfollow shelfId=5"}, + {"GET", "/shelves/items/7", "item-show id=7"}, + {"PUT", "/shelves/items/7", "item-update id=7"}, + {"DELETE", "/shelves/items/7", "item-delete id=7"}, + {"GET", "/shelves/5/items", "shelf-items shelfId=5"}, + {"GET", "/shelves/items/similar", "item-similar"}, + } + for _, c := range cases { + rec := serve(h, c.method, c.path) + if rec.Code != http.StatusOK || rec.Body.String() != c.want { + t.Errorf("%s %s = %d %q, want 200 %q", c.method, c.path, rec.Code, rec.Body.String(), c.want) + } + } + if rec := serve(h, "HEAD", "/shelves/items/7"); rec.Code != http.StatusOK { + t.Errorf("HEAD /shelves/items/7 = %d, want 200 (GET answers HEAD)", rec.Code) + } + }) + + t.Run("constraint-404", func(t *testing.T) { + for _, c := range []struct{ method, path string }{ + {"GET", "/shelves/items/follow"}, + {"GET", "/shelves/0x1/items"}, + {"GET", "/shelves/abc/items/1"}, + {"GET", "/shelves/5/items/x"}, + {"DELETE", "/shelves/items/similar"}, + {"DELETE", "/shelves/abc/follow"}, + } { + rec := serve(h, c.method, c.path) + if rec.Code != http.StatusNotFound || rec.Body.String() != "404 page not found\n" || + rec.Header().Get("Content-Type") != "text/plain; charset=utf-8" { + t.Errorf("%s %s = %d %q %q, want the bare 404", c.method, c.path, rec.Code, rec.Header().Get("Content-Type"), rec.Body.String()) + } + } + }) + + t.Run("method-405", func(t *testing.T) { + cases := []struct { + method, path string + drop []string + }{ + {"POST", "/shelves/items/7", []string{"shelf-follow", "shelf-unfollow", "shelf-items", "shelf-item"}}, + {"PATCH", "/shelves/5/follow", []string{"item-show", "item-delete", "shelf-item"}}, + {"PUT", "/shelves/5/items", []string{"item-show", "item-delete", "shelf-item"}}, + {"POST", "/shelves/5/items/6", []string{"token-follow", "item-show", "item-delete"}}, + {"DELETE", "/shelves/token/abc/follow", []string{"shelf-item", "item-show", "item-delete"}}, + } + for _, c := range cases { + got := serve(h, c.method, c.path) + want := serve(referenceMux(t, overlapTable, c.drop...), c.method, c.path) + if want.Code != http.StatusMethodNotAllowed { + t.Fatalf("reference %s %s = %d, the case must be a 405", c.method, c.path, want.Code) + } + if got.Code != want.Code || got.Header().Get("Allow") != want.Header().Get("Allow") || got.Body.String() != want.Body.String() { + t.Errorf("%s %s = %d Allow %q %q, want %d Allow %q %q", c.method, c.path, + got.Code, got.Header().Get("Allow"), got.Body.String(), + want.Code, want.Header().Get("Allow"), want.Body.String()) + } + } + }) + + t.Run("unknown-method-404", func(t *testing.T) { + for _, c := range []struct { + method, path string + drop []string + }{ + {"POST", "/shelves/abc/def", []string{"item-show", "item-delete", "shelf-item"}}, + {"PATCH", "/shelves/abc/def/ghi", []string{"token-follow", "item-show", "item-delete"}}, + {"GET", "/shelves/abc/def", []string{"item-show", "item-delete", "shelf-item"}}, + } { + got := serve(h, c.method, c.path) + want := serve(referenceMux(t, overlapTable, c.drop...), c.method, c.path) + if got.Code != http.StatusNotFound || want.Code != http.StatusNotFound || got.Header().Get("Allow") != "" { + t.Errorf("%s %s = %d (reference %d) Allow %q, want 404 without Allow", c.method, c.path, got.Code, want.Code, got.Header().Get("Allow")) + } + } + }) + + t.Run("middleware", func(t *testing.T) { + for _, c := range []struct{ method, path, own string }{ + {"GET", "/shelves/items/7", "item-show"}, + {"GET", "/shelves/5/follow", "shelf-follow"}, + {"GET", "/shelves/5/items", "shelf-items"}, + {"DELETE", "/shelves/5/follow", "shelf-unfollow"}, + {"GET", "/shelves/token/t/follow", "token-follow"}, + } { + rec := serve(h, c.method, c.path) + if got := rec.Header().Values("X-Mw"); len(got) != 1 || got[0] != c.own { + t.Errorf("%s %s middleware = %v, want only %s", c.method, c.path, got, c.own) + } + } + }) + + t.Run("route-table", func(t *testing.T) { + r := buildOverlapRouter(t, overlapTable) + if _, err := r.compile(); err != nil { + t.Fatal(err) + } + infos := r.Routes() + if len(infos) != len(overlapTable) { + t.Fatalf("Routes() = %d entries, want %d", len(infos), len(overlapTable)) + } + for i, rt := range overlapTable { + if infos[i].Method != rt.method || infos[i].Pattern != rt.path || len(infos[i].Middleware) != 1 || infos[i].Middleware[0] != "mw."+rt.name { + t.Errorf("Routes()[%d] = %+v, want %s %s [mw.%s]", i, infos[i], rt.method, rt.path, rt.name) + } + if strings.Contains(infos[i].Pattern, "surfOverlap") { + t.Errorf("Routes() leaks a family pattern: %s", infos[i].Pattern) + } + } + }) + + t.Run("unsupported", func(t *testing.T) { + _, err := buildOverlapRouter(t, []overlapRoute{ + {name: "files", method: "GET", path: "/files/{rest...}"}, + {name: "readme", method: "GET", path: "/{dir}/readme", where: map[string]string{"dir": "[a-z]+"}}, + }).compile() + if err == nil || !strings.Contains(err.Error(), "/files/{rest...}") || !strings.Contains(err.Error(), "/{dir}/readme") { + t.Fatalf("err = %v, want a compile error naming both routes", err) + } + }) + + t.Run("same-shape", func(t *testing.T) { + _, err := buildOverlapRouter(t, []overlapRoute{ + {name: "by-id", method: "GET", path: "/shelves/{id}", where: map[string]string{"id": "[0-9]+"}}, + {name: "by-slug", method: "GET", path: "/shelves/{slug}", where: map[string]string{"slug": "[a-z]+"}}, + }).compile() + if err == nil || !strings.Contains(err.Error(), "route conflict") || !strings.Contains(err.Error(), "same shape") { + t.Fatalf("err = %v, want a same-shape conflict", err) + } + }) + + t.Run("shadowed", func(t *testing.T) { + _, err := buildOverlapRouter(t, []overlapRoute{ + {name: "any", method: "GET", path: "/shelves/{a}/{b}"}, + {name: "item-show", method: "GET", path: "/shelves/items/{id}", where: map[string]string{"id": "[0-9]+"}}, + {name: "shelf-follow", method: "GET", path: "/shelves/{shelfId}/follow", where: map[string]string{"shelfId": "[0-9]+"}}, + }).compile() + if err == nil || !strings.Contains(err.Error(), "takes its requests") { + t.Fatalf("err = %v, want a shadowing error", err) + } + }) + + t.Run("pattern-merge", func(t *testing.T) { + // The POST route conflicts with no member, only with the family's + // generated method-less pattern, so it joins the family. + table := []overlapRoute{ + {name: "item-show", method: "GET", path: "/shelves/items/{id}", where: map[string]string{"id": "[0-9]+"}}, + {name: "shelf-follow", method: "GET", path: "/shelves/{shelfId}/follow", where: map[string]string{"shelfId": "[0-9]+"}}, + {name: "any-similar", method: "POST", path: "/{kind}/items/similar"}, + } + h := compileOverlap(t, table) + for _, c := range []struct { + method, path string + code int + want string + }{ + {"POST", "/shelves/items/similar", 200, "any-similar kind=shelves"}, + {"POST", "/racks/items/similar", 200, "any-similar kind=racks"}, + {"GET", "/shelves/items/4", 200, "item-show id=4"}, + {"GET", "/shelves/4/follow", 200, "shelf-follow shelfId=4"}, + } { + rec := serve(h, c.method, c.path) + if rec.Code != c.code || rec.Body.String() != c.want { + t.Errorf("%s %s = %d %q, want %d %q", c.method, c.path, rec.Code, rec.Body.String(), c.code, c.want) + } + } + }) + + t.Run("no-family", func(t *testing.T) { + fams, err := overlapFamilies(buildOverlapRouter(t, []overlapRoute{ + {name: "a", method: "GET", path: "/shelves/{id}"}, + {name: "b", method: "GET", path: "/shelves/{id}/items"}, + {name: "c", method: "POST", path: "/shelves/{id}"}, + }).routes) + if err != nil || len(fams) != 0 { + t.Fatalf("families = %v, %v; want none", fams, err) + } + }) +} + +func TestParseSimplePath(t *testing.T) { + for _, c := range []struct { + path string + ok bool + }{ + {"/a/{b}/c", true}, + {"/a%20b/{_x1}", true}, + {"/", false}, + {"/a/", false}, + {"/a//b", false}, + {"/a/{b...}", false}, + {"/a/{$}", false}, + {"/a/x{b}", false}, + {"/a/{1b}", false}, + {"/a/%zz", false}, + {"host/a", false}, + } { + if _, ok := parseSimplePath(c.path); ok != c.ok { + t.Errorf("parseSimplePath(%q) ok = %v, want %v", c.path, ok, c.ok) + } + } + if segs, _ := parseSimplePath("/a%20b/{x}"); segs[0].lit != "a b" || segs[0].raw != "a%20b" || segs[1].param != "x" { + t.Errorf("segments = %+v", segs) + } + if muxConflict("GET /a/{", "GET /a/b") { + t.Error("an invalid pattern is not a conflict") + } +} diff --git a/modules/surf/router.go b/modules/surf/router.go index 93df5ff..f0c67b2 100644 --- a/modules/surf/router.go +++ b/modules/surf/router.go @@ -343,16 +343,38 @@ func (r *Router) compile() (http.Handler, error) { if r.compileErr != nil { return nil, r.compileErr } - mux := http.NewServeMux() - for _, rt := range r.routes { + handlers := make([]http.Handler, len(r.routes)) + for i, rt := range r.routes { h, err := r.wrap(rt) if err != nil { return nil, err } - if err := handleRoute(mux, rt, h); err != nil { + handlers[i] = h + } + // Routes that ServeMux refuses side by side but Where constraints keep + // apart register as overlap families (overlap.go); the rest as before. + families, err := overlapFamilies(r.routes) + if err != nil { + return nil, err + } + inFamily := make(map[int]bool) + for _, fam := range families { + for _, idx := range fam.members { + inFamily[idx] = true + } + } + mux := http.NewServeMux() + for i, rt := range r.routes { + if inFamily[i] { + continue + } + if err := handleRoute(mux, rt, handlers[i]); err != nil { return nil, err } } + if err := registerOverlapFamilies(mux, r.routes, handlers, families); err != nil { + return nil, err + } return pathScopedCORS(r.corsCfg, mux), nil }