Pin the body cap's order in every page route group (closes #93)
check / check (push) Successful in 3m28s
check / check (push) Successful in 3m28s
A route test now posts an oversized body with no session or CSRF token to a POST route in each page route group that has one, and requires 413 with no CSRF cookie. Before, only the login form pinned the cap ahead of CSRF; reordering the /hooks or /hook groups failed nothing. The MaxBodySize doc comment says other methods pass uncapped on purpose, the middleware test comment names the helper it describes, and NewRouterForTest says why its hand-built Server is enough. The README already described the cap's position correctly. Model: opus-5-5
This commit is contained in:
@@ -600,7 +600,10 @@ func bodyLimitedMethod(method string) bool {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// MaxBodySize returns middleware that limits the size of
|
// MaxBodySize returns middleware that limits the size of
|
||||||
// POST/PUT/PATCH request bodies to maxBytes. It must be registered
|
// POST/PUT/PATCH request bodies to maxBytes. A request with any other
|
||||||
|
// method passes through uncapped, deliberately: no route behind it
|
||||||
|
// reads a body on GET, HEAD or DELETE. A handler that starts to needs
|
||||||
|
// its method added to bodyLimitedMethod first. It must be registered
|
||||||
// before any middleware that parses the body — notably CSRF, which
|
// before any middleware that parses the body — notably CSRF, which
|
||||||
// calls r.PostFormValue — so that form parsing happens under this
|
// calls r.PostFormValue — so that form parsing happens under this
|
||||||
// cap rather than net/http's 10 MB default.
|
// cap rather than net/http's 10 MB default.
|
||||||
|
|||||||
@@ -730,10 +730,8 @@ func TestNoCache_SetsHeaders(t *testing.T) {
|
|||||||
|
|
||||||
const testBodyLimit int64 = 64
|
const testBodyLimit int64 = 64
|
||||||
|
|
||||||
// maxBodySizeHandler wraps a sentinel handler in MaxBodySize with
|
// maxBodySizeResult is what runMaxBodySize's sentinel handler saw,
|
||||||
// testBodyLimit. The sentinel records whether it ran and how much of
|
// together with the response.
|
||||||
// the body it managed to read, so tests can distinguish "never
|
|
||||||
// reached" from "reached but truncated".
|
|
||||||
type maxBodySizeResult struct {
|
type maxBodySizeResult struct {
|
||||||
called bool
|
called bool
|
||||||
read int
|
read int
|
||||||
@@ -741,6 +739,10 @@ type maxBodySizeResult struct {
|
|||||||
response *httptest.ResponseRecorder
|
response *httptest.ResponseRecorder
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// runMaxBodySize wraps a sentinel handler in MaxBodySize with
|
||||||
|
// testBodyLimit and serves req through it. The sentinel records
|
||||||
|
// whether it ran and how much of the body it managed to read, so
|
||||||
|
// tests can distinguish "never reached" from "reached but truncated".
|
||||||
func runMaxBodySize(
|
func runMaxBodySize(
|
||||||
t *testing.T,
|
t *testing.T,
|
||||||
req *http.Request,
|
req *http.Request,
|
||||||
|
|||||||
@@ -39,6 +39,13 @@ func SentryClientOptionsForTest(
|
|||||||
// and the HTTP listener. Tests use it so that route-group middleware
|
// and the HTTP listener. Tests use it so that route-group middleware
|
||||||
// registration order is exercised exactly as it ships, rather than
|
// registration order is exercised exactly as it ships, rather than
|
||||||
// against a hand-rebuilt chain that could drift from routes.go.
|
// against a hand-rebuilt chain that could drift from routes.go.
|
||||||
|
//
|
||||||
|
// The Server itself is built by hand rather than through New, because
|
||||||
|
// New registers the fx hooks that start the listener. That is enough
|
||||||
|
// only while SetupRoutes reads no fields beyond mw, h, params.Config
|
||||||
|
// and sentryEnabled: a field it starts reading must also be set here
|
||||||
|
// and in the two probe variants below, or these tests run with it
|
||||||
|
// zero.
|
||||||
func NewRouterForTest(
|
func NewRouterForTest(
|
||||||
log *slog.Logger,
|
log *slog.Logger,
|
||||||
cfg *config.Config,
|
cfg *config.Config,
|
||||||
|
|||||||
@@ -531,6 +531,44 @@ func TestStaticServesOnlyGetAndHead(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// --- every page route group ---
|
||||||
|
|
||||||
|
// TestPageRouteGroups_OversizeBody_RejectedBeforeCSRF pins the body
|
||||||
|
// cap ahead of CSRF and RequireAuth in every page route group that
|
||||||
|
// has a POST route. The requests carry no session and no CSRF token,
|
||||||
|
// so if either ran first the answer would be a 403 or a redirect to
|
||||||
|
// the login page rather than 413, and CSRF would issue its cookie
|
||||||
|
// (see TestPagesLogin_UnderLimit_NoToken_CSRFRejects). The user and
|
||||||
|
// webhook in the paths need not exist: nothing after the cap runs.
|
||||||
|
func TestPageRouteGroups_OversizeBody_RejectedBeforeCSRF(
|
||||||
|
t *testing.T,
|
||||||
|
) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
env := newTestEnv(t)
|
||||||
|
|
||||||
|
form := url.Values{}
|
||||||
|
form.Set("name", oversizeValue())
|
||||||
|
|
||||||
|
for _, path := range []string{
|
||||||
|
"/pages/login",
|
||||||
|
"/user/nobody/password",
|
||||||
|
"/hooks/new",
|
||||||
|
"/hook/nonexistent/edit",
|
||||||
|
} {
|
||||||
|
w := env.post(path, form, nil)
|
||||||
|
|
||||||
|
assert.Equal(
|
||||||
|
t, http.StatusRequestEntityTooLarge, w.Code, path,
|
||||||
|
)
|
||||||
|
assert.False(
|
||||||
|
t, csrfCookieSet(w),
|
||||||
|
"CSRF middleware must not run for an oversized body to %s",
|
||||||
|
path,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// --- /pages group ---
|
// --- /pages group ---
|
||||||
|
|
||||||
// TestPagesLogin_OversizeBody_RejectedBeforeCSRF proves the cap runs
|
// TestPagesLogin_OversizeBody_RejectedBeforeCSRF proves the cap runs
|
||||||
|
|||||||
Reference in New Issue
Block a user