Skip to content

Commit fd7eec8

Browse files
vishrclaude
andcommitted
test: guard group catch-all against 405-masking and route shadowing
Adds regression tests around automatic group catch-all (404) route registration. v5 removed that auto-registration; if it is restored (e.g. PR #2996) these tests ensure it does not bring back the issues that motivated the removal: - wrong HTTP method on an existing group route must still return 405 (with Allow header), not be masked to 404 by the catch-all; - the group catch-all must not shadow concrete sibling routes, root routes, or other sibling groups' routes. All four pass on current master (v5). The 405 test fails against the restore-v4-behavior approach in #2996, pinning down that tradeoff. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent b0a3916 commit fd7eec8

1 file changed

Lines changed: 89 additions & 0 deletions

File tree

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
// SPDX-License-Identifier: MIT
2+
// SPDX-FileCopyrightText: © 2015 LabStack LLC and Echo contributors
3+
4+
package echo
5+
6+
import (
7+
"net/http"
8+
"net/http/httptest"
9+
"testing"
10+
11+
"github.com/stretchr/testify/assert"
12+
)
13+
14+
// These tests guard the behavior re-introduced by restoring automatic group
15+
// catch-all (404) route registration (PR #2996). v5 originally removed that
16+
// registration because the implicit catch-all could:
17+
// 1. mask 405 Method Not Allowed as 404, and
18+
// 2. shadow sibling/root routes.
19+
// Restoring auto-registration must NOT bring those regressions back.
20+
21+
// passthroughMW is a no-op middleware that forces a group to register its
22+
// implicit catch-all routes (a group only auto-registers when it has middleware).
23+
func passthroughMW(next HandlerFunc) HandlerFunc {
24+
return func(c *Context) error { return next(c) }
25+
}
26+
27+
// 1. Wrong method on an existing group route must still return 405 (with Allow
28+
// header), not be swallowed as 404 by the group's catch-all.
29+
func TestGroup_autoCatchAll_wrongMethodStillReturns405(t *testing.T) {
30+
e := New()
31+
g := e.Group("/api", passthroughMW)
32+
g.GET("/users", func(c *Context) error { return c.String(http.StatusOK, "users") })
33+
34+
req := httptest.NewRequest(http.MethodPost, "/api/users", nil)
35+
rec := httptest.NewRecorder()
36+
e.ServeHTTP(rec, req)
37+
38+
assert.Equal(t, http.StatusMethodNotAllowed, rec.Code,
39+
"POST to a GET-only group route must be 405, not masked to 404 by the catch-all")
40+
assert.Contains(t, rec.Header().Get(HeaderAllow), http.MethodGet,
41+
"405 response must advertise allowed methods")
42+
}
43+
44+
// 2. The group's catch-all must not shadow a concrete sibling route under the
45+
// same prefix: a matched route returns its own handler, not the 404 catch-all.
46+
func TestGroup_autoCatchAll_doesNotShadowConcreteSiblingRoute(t *testing.T) {
47+
e := New()
48+
g := e.Group("/api", passthroughMW)
49+
g.GET("/users", func(c *Context) error { return c.String(http.StatusOK, "users") })
50+
g.GET("/health", func(c *Context) error { return c.String(http.StatusOK, "health") })
51+
52+
for path, want := range map[string]string{"/api/users": "users", "/api/health": "health"} {
53+
status, body := request(http.MethodGet, path, e)
54+
assert.Equal(t, http.StatusOK, status, "concrete route %s must win over the group catch-all", path)
55+
assert.Equal(t, want, body, "concrete route %s must run its own handler", path)
56+
}
57+
58+
// Only a genuinely unmatched path under the prefix hits the catch-all (404).
59+
status, _ := request(http.MethodGet, "/api/nope", e)
60+
assert.Equal(t, http.StatusNotFound, status, "unmatched path under the prefix should hit the catch-all 404")
61+
}
62+
63+
// 3. The group's catch-all (prefixed) must not shadow routes outside the group,
64+
// including root-level routes.
65+
func TestGroup_autoCatchAll_doesNotShadowRootRoute(t *testing.T) {
66+
e := New()
67+
e.GET("/health", func(c *Context) error { return c.String(http.StatusOK, "root-health") })
68+
g := e.Group("/api", passthroughMW)
69+
g.GET("/users", func(c *Context) error { return c.String(http.StatusOK, "users") })
70+
71+
status, body := request(http.MethodGet, "/health", e)
72+
assert.Equal(t, http.StatusOK, status, "root route must be unaffected by a group's catch-all")
73+
assert.Equal(t, "root-health", body)
74+
}
75+
76+
// 4. Two sibling groups must not shadow each other's routes via their catch-alls.
77+
func TestGroup_autoCatchAll_siblingGroupsDoNotShadow(t *testing.T) {
78+
e := New()
79+
v1 := e.Group("/api/v1", passthroughMW)
80+
v1.GET("/ping", func(c *Context) error { return c.String(http.StatusOK, "v1") })
81+
v2 := e.Group("/api/v2", passthroughMW)
82+
v2.GET("/ping", func(c *Context) error { return c.String(http.StatusOK, "v2") })
83+
84+
for path, want := range map[string]string{"/api/v1/ping": "v1", "/api/v2/ping": "v2"} {
85+
status, body := request(http.MethodGet, path, e)
86+
assert.Equal(t, http.StatusOK, status, "%s must resolve to its own group handler", path)
87+
assert.Equal(t, want, body)
88+
}
89+
}

0 commit comments

Comments
 (0)