From 3d616e8c6d65e5617f5a918d72fb1514c9c7144e Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Fri, 11 Mar 2022 12:34:55 -0700 Subject: [PATCH] requestbody: Return HTTP 413 (fix #4558) --- modules/caddyhttp/errors.go | 3 +++ modules/caddyhttp/replacer.go | 8 ++------ modules/caddyhttp/requestbody/requestbody.go | 18 +++++++++++++++++- modules/caddyhttp/server.go | 4 +++- 4 files changed, 25 insertions(+), 8 deletions(-) diff --git a/modules/caddyhttp/errors.go b/modules/caddyhttp/errors.go index 85dc3dfbec0..9d1cf470bfd 100644 --- a/modules/caddyhttp/errors.go +++ b/modules/caddyhttp/errors.go @@ -82,6 +82,9 @@ func (e HandlerError) Error() string { return strings.TrimSpace(s) } +// Unwrap returns the underlying error value. See the `errors` package for info. +func (e HandlerError) Unwrap() error { return e.Err } + // randString returns a string of n random characters. // It is not even remotely secure OR a proper distribution. // But it's good enough for some things. It excludes certain diff --git a/modules/caddyhttp/replacer.go b/modules/caddyhttp/replacer.go index 26f7e0bec5e..d3fa89186a1 100644 --- a/modules/caddyhttp/replacer.go +++ b/modules/caddyhttp/replacer.go @@ -162,12 +162,8 @@ func addHTTPVarsToReplacer(repl *caddy.Replacer, req *http.Request, w http.Respo // read the request body into a buffer (can't pool because we // don't know its lifetime and would have to make a copy anyway) buf := new(bytes.Buffer) - _, err := io.Copy(buf, req.Body) - if err != nil { - return "", true - } - // replace real body with buffered data - req.Body = io.NopCloser(buf) + _, _ = io.Copy(buf, req.Body) // can't handle error, so just ignore it + req.Body = io.NopCloser(buf) // replace real body with buffered data return buf.String(), true // original request, before any internal changes diff --git a/modules/caddyhttp/requestbody/requestbody.go b/modules/caddyhttp/requestbody/requestbody.go index 76cd27433c4..dfc0fd92891 100644 --- a/modules/caddyhttp/requestbody/requestbody.go +++ b/modules/caddyhttp/requestbody/requestbody.go @@ -15,6 +15,7 @@ package requestbody import ( + "io" "net/http" "github.com/caddyserver/caddy/v2" @@ -28,6 +29,7 @@ func init() { // RequestBody is a middleware for manipulating the request body. type RequestBody struct { // The maximum number of bytes to allow reading from the body by a later handler. + // If more bytes are read, an error with HTTP status 413 is returned. MaxSize int64 `json:"max_size,omitempty"` } @@ -44,10 +46,24 @@ func (rb RequestBody) ServeHTTP(w http.ResponseWriter, r *http.Request, next cad return next.ServeHTTP(w, r) } if rb.MaxSize > 0 { - r.Body = http.MaxBytesReader(w, r.Body, rb.MaxSize) + r.Body = errorWrapper{http.MaxBytesReader(w, r.Body, rb.MaxSize)} } return next.ServeHTTP(w, r) } +// errorWrapper wraps errors that are returned from Read() +// so that they can be associated with a proper status code. +type errorWrapper struct { + io.ReadCloser +} + +func (ew errorWrapper) Read(p []byte) (n int, err error) { + n, err = ew.ReadCloser.Read(p) + if err != nil && err.Error() == "http: request body too large" { + err = caddyhttp.Error(http.StatusRequestEntityTooLarge, err) + } + return +} + // Interface guard var _ caddyhttp.MiddlewareHandler = (*RequestBody)(nil) diff --git a/modules/caddyhttp/server.go b/modules/caddyhttp/server.go index 3d2118469a6..6ab77dfc909 100644 --- a/modules/caddyhttp/server.go +++ b/modules/caddyhttp/server.go @@ -17,6 +17,7 @@ package caddyhttp import ( "context" "encoding/json" + "errors" "fmt" "net" "net/http" @@ -600,7 +601,8 @@ func PrepareRequest(r *http.Request, repl *caddy.Replacer, w http.ResponseWriter // If err is a HandlerError, the returned values will // have richer information. func errLogValues(err error) (status int, msg string, fields []zapcore.Field) { - if handlerErr, ok := err.(HandlerError); ok { + var handlerErr HandlerError + if errors.As(err, &handlerErr) { status = handlerErr.StatusCode if handlerErr.Err == nil { msg = err.Error()