From 0cdaaba4b87da8a72c0591ee301bb50df94a6371 Mon Sep 17 00:00:00 2001 From: Ngo The Trung Date: Fri, 4 Nov 2016 08:25:49 +0800 Subject: [PATCH] Add maxrequestbody directive (#1163) --- caddyhttp/caddyhttp.go | 1 + caddyhttp/caddyhttp_test.go | 2 +- caddyhttp/httpserver/plugin.go | 1 + caddyhttp/httpserver/replacer.go | 4 +- caddyhttp/httpserver/server.go | 82 ++++++++ caddyhttp/httpserver/siteconfig.go | 10 + caddyhttp/maxrequestbody/maxrequestbody.go | 178 ++++++++++++++++++ .../maxrequestbody/maxrequestbody_test.go | 158 ++++++++++++++++ caddyhttp/proxy/proxy.go | 4 + 9 files changed, 438 insertions(+), 2 deletions(-) create mode 100644 caddyhttp/maxrequestbody/maxrequestbody.go create mode 100644 caddyhttp/maxrequestbody/maxrequestbody_test.go diff --git a/caddyhttp/caddyhttp.go b/caddyhttp/caddyhttp.go index 3f8009fd4..f400443a2 100644 --- a/caddyhttp/caddyhttp.go +++ b/caddyhttp/caddyhttp.go @@ -17,6 +17,7 @@ import ( _ "github.com/mholt/caddy/caddyhttp/internalsrv" _ "github.com/mholt/caddy/caddyhttp/log" _ "github.com/mholt/caddy/caddyhttp/markdown" + _ "github.com/mholt/caddy/caddyhttp/maxrequestbody" _ "github.com/mholt/caddy/caddyhttp/mime" _ "github.com/mholt/caddy/caddyhttp/pprof" _ "github.com/mholt/caddy/caddyhttp/proxy" diff --git a/caddyhttp/caddyhttp_test.go b/caddyhttp/caddyhttp_test.go index 24a2749e7..fa9f7edf3 100644 --- a/caddyhttp/caddyhttp_test.go +++ b/caddyhttp/caddyhttp_test.go @@ -11,7 +11,7 @@ import ( // ensure that the standard plugins are in fact plugged in // and registered properly; this is a quick/naive way to do it. func TestStandardPlugins(t *testing.T) { - numStandardPlugins := 27 // importing caddyhttp plugs in this many plugins + numStandardPlugins := 28 // importing caddyhttp plugs in this many plugins s := caddy.DescribePlugins() if got, want := strings.Count(s, "\n"), numStandardPlugins+5; got != want { t.Errorf("Expected all standard plugins to be plugged in, got:\n%s", s) diff --git a/caddyhttp/httpserver/plugin.go b/caddyhttp/httpserver/plugin.go index 63e05f191..8105a57a9 100644 --- a/caddyhttp/httpserver/plugin.go +++ b/caddyhttp/httpserver/plugin.go @@ -393,6 +393,7 @@ var directives = []string{ // primitive actions that set up the fundamental vitals of each config "root", "bind", + "maxrequestbody", "tls", // services/utilities, or other directives that don't necessarily inject handlers diff --git a/caddyhttp/httpserver/replacer.go b/caddyhttp/httpserver/replacer.go index 02ad54fcb..b112e8354 100644 --- a/caddyhttp/httpserver/replacer.go +++ b/caddyhttp/httpserver/replacer.go @@ -268,7 +268,9 @@ func (r *replacer) getSubstitution(key string) string { } _, err := ioutil.ReadAll(r.request.Body) if err != nil { - return r.emptyValue + if _, ok := err.(MaxBytesExceeded); ok { + return r.emptyValue + } } return requestReplacer.Replace(r.requestBody.String()) case "{status}": diff --git a/caddyhttp/httpserver/server.go b/caddyhttp/httpserver/server.go index f1fa16215..c435b0a9a 100644 --- a/caddyhttp/httpserver/server.go +++ b/caddyhttp/httpserver/server.go @@ -4,6 +4,7 @@ package httpserver import ( "crypto/tls" "fmt" + "io" "log" "net" "net/http" @@ -265,6 +266,19 @@ func (s *Server) serveHTTP(w http.ResponseWriter, r *http.Request) (int, error) } } + // Apply the path-based request body size limit + // The error returned by MaxBytesReader is meant to be handled + // by whichever middleware/plugin that receives it when calling + // .Read() or a similar method on the request body + if r.Body != nil { + for _, pathlimit := range vhost.MaxRequestBodySizes { + if Path(r.URL.Path).Matches(pathlimit.Path) { + r.Body = MaxBytesReader(w, r.Body, pathlimit.Limit) + break + } + } + } + return vhost.middlewareChain.ServeHTTP(w, r) } @@ -393,6 +407,74 @@ func (ln tcpKeepAliveListener) File() (*os.File, error) { return ln.TCPListener.File() } +// MaxBytesExceeded is the error type returned by MaxBytesReader +// when the request body exceeds the limit imposed +type MaxBytesExceeded struct{} + +func (err MaxBytesExceeded) Error() string { + return "http: request body too large" +} + +// MaxBytesReader and its associated methods are borrowed from the +// Go Standard library (comments intact). The only difference is that +// it returns a MaxBytesExceeded error instead of a generic error message +// when the request body has exceeded the requested limit +func MaxBytesReader(w http.ResponseWriter, r io.ReadCloser, n int64) io.ReadCloser { + return &maxBytesReader{w: w, r: r, n: n} +} + +type maxBytesReader struct { + w http.ResponseWriter + r io.ReadCloser // underlying reader + n int64 // max bytes remaining + err error // sticky error +} + +func (l *maxBytesReader) Read(p []byte) (n int, err error) { + if l.err != nil { + return 0, l.err + } + if len(p) == 0 { + return 0, nil + } + // If they asked for a 32KB byte read but only 5 bytes are + // remaining, no need to read 32KB. 6 bytes will answer the + // question of the whether we hit the limit or go past it. + if int64(len(p)) > l.n+1 { + p = p[:l.n+1] + } + n, err = l.r.Read(p) + + if int64(n) <= l.n { + l.n -= int64(n) + l.err = err + return n, err + } + + n = int(l.n) + l.n = 0 + + // The server code and client code both use + // maxBytesReader. This "requestTooLarge" check is + // only used by the server code. To prevent binaries + // which only using the HTTP Client code (such as + // cmd/go) from also linking in the HTTP server, don't + // use a static type assertion to the server + // "*response" type. Check this interface instead: + type requestTooLarger interface { + requestTooLarge() + } + if res, ok := l.w.(requestTooLarger); ok { + res.requestTooLarge() + } + l.err = MaxBytesExceeded{} + return n, l.err +} + +func (l *maxBytesReader) Close() error { + return l.r.Close() +} + // DefaultErrorFunc responds to an HTTP request with a simple description // of the specified HTTP status code. func DefaultErrorFunc(w http.ResponseWriter, r *http.Request, status int) { diff --git a/caddyhttp/httpserver/siteconfig.go b/caddyhttp/httpserver/siteconfig.go index 6c683d0d7..2980d3770 100644 --- a/caddyhttp/httpserver/siteconfig.go +++ b/caddyhttp/httpserver/siteconfig.go @@ -30,6 +30,16 @@ type SiteConfig struct { // standardized way of loading files from disk // for a request. HiddenFiles []string + + // Max amount of bytes a request can send on a given path + MaxRequestBodySizes []PathLimit +} + +// PathLimit is a mapping from a site's path to its corresponding +// maximum request body size (in bytes) +type PathLimit struct { + Path string + Limit int64 } // AddMiddleware adds a middleware to a site's middleware stack. diff --git a/caddyhttp/maxrequestbody/maxrequestbody.go b/caddyhttp/maxrequestbody/maxrequestbody.go new file mode 100644 index 000000000..8b5fef464 --- /dev/null +++ b/caddyhttp/maxrequestbody/maxrequestbody.go @@ -0,0 +1,178 @@ +package maxrequestbody + +import ( + "errors" + "sort" + "strconv" + "strings" + + "github.com/mholt/caddy" + "github.com/mholt/caddy/caddyhttp/httpserver" +) + +const ( + serverType = "http" + pluginName = "maxrequestbody" +) + +func init() { + caddy.RegisterPlugin(pluginName, caddy.Plugin{ + ServerType: serverType, + Action: setupMaxRequestBody, + }) +} + +// pathLimitUnparsed is a PathLimit before it's parsed +type pathLimitUnparsed struct { + Path string + Limit string +} + +func setupMaxRequestBody(c *caddy.Controller) error { + config := httpserver.GetConfig(c) + + if !c.Next() { + return c.ArgErr() + } + + args := c.RemainingArgs() + argList := []pathLimitUnparsed{} + + switch len(args) { + case 0: + // Format: { ... } + for c.NextBlock() { + path := c.Val() + if !c.NextArg() { + // Uneven pairing of path/limit + return c.ArgErr() + } + argList = append(argList, pathLimitUnparsed{ + Path: path, + Limit: c.Val(), + }) + } + case 1: + // Format: + argList = []pathLimitUnparsed{{ + Path: "/", + Limit: args[0], + }} + case 2: + // Format: + argList = []pathLimitUnparsed{{ + Path: args[0], + Limit: args[1], + }} + default: + return c.ArgErr() + } + + pathLimit, err := parseArguments(argList) + if err != nil { + return c.ArgErr() + } + + SortPathLimits(pathLimit) + + config.MaxRequestBodySizes = pathLimit + + return nil +} + +func parseArguments(args []pathLimitUnparsed) ([]httpserver.PathLimit, error) { + pathLimit := []httpserver.PathLimit{} + + for _, pair := range args { + size := parseSize(pair.Limit) + if size < 1 { // also disallow size = 0 + return pathLimit, errors.New("Parse failed") + } + pathLimit = addPathLimit(pathLimit, pair.Path, size) + } + return pathLimit, nil +} + +var validUnits = []struct { + symbol string + multiplier int64 +}{ + {"KB", 1024}, + {"MB", 1024 * 1024}, + {"GB", 1024 * 1024 * 1024}, + {"B", 1}, + {"", 1}, // defaulting to "B" +} + +// parseSize parses the given string as size limit +// Size are positive numbers followed by a unit (case insensitive) +// Allowed units: "B" (bytes), "KB" (kilo), "MB" (mega), "GB" (giga) +// If the unit is omitted, "b" is assumed +// Returns the parsed size in bytes, or -1 if cannot parse +func parseSize(sizeStr string) int64 { + sizeStr = strings.ToUpper(sizeStr) + + for _, unit := range validUnits { + if strings.HasSuffix(sizeStr, unit.symbol) { + size, err := strconv.ParseInt(sizeStr[0:len(sizeStr)-len(unit.symbol)], 10, 64) + if err != nil { + return -1 + } + return size * unit.multiplier + } + } + + // Unreachable code + return -1 +} + +// addPathLimit appends the path-to-request body limit mapping to pathLimit +// Slashes are checked and added to path if necessary. Duplicates are ignored. +func addPathLimit(pathLimit []httpserver.PathLimit, path string, limit int64) []httpserver.PathLimit { + // Enforces preceding slash + if path[0] != '/' { + path = "/" + path + } + + // Use the last value if there are duplicates + for i, p := range pathLimit { + if p.Path == path { + pathLimit[i].Limit = limit + return pathLimit + } + } + + return append(pathLimit, httpserver.PathLimit{Path: path, Limit: limit}) +} + +// SortPathLimits sort pathLimits by their paths length, longest first +func SortPathLimits(pathLimits []httpserver.PathLimit) { + sorter := &pathLimitSorter{ + pathLimits: pathLimits, + by: LengthDescending, + } + sort.Sort(sorter) +} + +// structs and methods implementing the sorting interfaces for PathLimit +type pathLimitSorter struct { + pathLimits []httpserver.PathLimit + by func(p1, p2 *httpserver.PathLimit) bool +} + +func (s *pathLimitSorter) Len() int { + return len(s.pathLimits) +} + +func (s *pathLimitSorter) Swap(i, j int) { + s.pathLimits[i], s.pathLimits[j] = s.pathLimits[j], s.pathLimits[i] +} + +func (s *pathLimitSorter) Less(i, j int) bool { + return s.by(&s.pathLimits[i], &s.pathLimits[j]) +} + +// LengthDescending is the comparator for SortPathLimits +func LengthDescending(p1, p2 *httpserver.PathLimit) bool { + return len(p1.Path) > len(p2.Path) +} diff --git a/caddyhttp/maxrequestbody/maxrequestbody_test.go b/caddyhttp/maxrequestbody/maxrequestbody_test.go new file mode 100644 index 000000000..7d2c31e78 --- /dev/null +++ b/caddyhttp/maxrequestbody/maxrequestbody_test.go @@ -0,0 +1,158 @@ +package maxrequestbody + +import ( + "reflect" + "testing" + + "github.com/mholt/caddy" + "github.com/mholt/caddy/caddyhttp/httpserver" +) + +const ( + KB = 1024 + MB = 1024 * 1024 + GB = 1024 * 1024 * 1024 +) + +func TestSetupMaxRequestBody(t *testing.T) { + cases := []struct { + input string + hasError bool + }{ + // Format: { ... } + {input: "maxrequestbody / 20MB", hasError: false}, + // Format: + {input: "maxrequestbody 999KB", hasError: false}, + // Format: { ... } + {input: "maxrequestbody { /images 50MB /upload 10MB\n/test 10KB }", hasError: false}, + + // Wrong formats + {input: "maxrequestbody typo { /images 50MB }", hasError: true}, + {input: "maxrequestbody 999MB /home 20KB", hasError: true}, + } + for caseNum, c := range cases { + controller := caddy.NewTestController("", c.input) + err := setupMaxRequestBody(controller) + + if c.hasError && (err == nil) { + t.Errorf("Expecting error for case %v but none encountered", caseNum) + } + if !c.hasError && (err != nil) { + t.Errorf("Expecting no error for case %v but encountered %v", caseNum, err) + } + } +} + +func TestParseArguments(t *testing.T) { + cases := []struct { + arguments []pathLimitUnparsed + expected []httpserver.PathLimit + hasError bool + }{ + // Parse errors + {arguments: []pathLimitUnparsed{{"/", "123.5"}}, expected: []httpserver.PathLimit{}, hasError: true}, + {arguments: []pathLimitUnparsed{{"/", "200LB"}}, expected: []httpserver.PathLimit{}, hasError: true}, + {arguments: []pathLimitUnparsed{{"/", "path:999MB"}}, expected: []httpserver.PathLimit{}, hasError: true}, + {arguments: []pathLimitUnparsed{{"/", "1_234_567"}}, expected: []httpserver.PathLimit{}, hasError: true}, + {arguments: []pathLimitUnparsed{{"/", "0MB"}}, expected: []httpserver.PathLimit{}, hasError: true}, + + // Valid results + {arguments: []pathLimitUnparsed{}, expected: []httpserver.PathLimit{}, hasError: false}, + { + arguments: []pathLimitUnparsed{{"/", "100"}}, + expected: []httpserver.PathLimit{{Path: "/", Limit: 100}}, + hasError: false, + }, + { + arguments: []pathLimitUnparsed{{"/", "100KB"}}, + expected: []httpserver.PathLimit{{Path: "/", Limit: 100 * KB}}, + hasError: false, + }, + { + arguments: []pathLimitUnparsed{{"/", "100MB"}}, + expected: []httpserver.PathLimit{{Path: "/", Limit: 100 * MB}}, + hasError: false, + }, + { + arguments: []pathLimitUnparsed{{"/", "100GB"}}, + expected: []httpserver.PathLimit{{Path: "/", Limit: 100 * GB}}, + hasError: false, + }, + { + arguments: []pathLimitUnparsed{{"index", "100"}}, + expected: []httpserver.PathLimit{{Path: "/index", Limit: 100}}, + hasError: false, + }, + { + arguments: []pathLimitUnparsed{{"/home", "100MB"}, {"/upload/images", "500GB"}}, + expected: []httpserver.PathLimit{ + {Path: "/home", Limit: 100 * MB}, + {Path: "/upload/images", Limit: 500 * GB}, + }, + hasError: false}, + { + arguments: []pathLimitUnparsed{{"/", "999"}, {"/home", "12345MB"}}, + expected: []httpserver.PathLimit{ + {Path: "/", Limit: 999}, + {Path: "/home", Limit: 12345 * MB}, + }, + hasError: false, + }, + + // Duplicates + { + arguments: []pathLimitUnparsed{{"/home", "999"}, {"/home", "12345MB"}}, + expected: []httpserver.PathLimit{ + {Path: "/home", Limit: 12345 * MB}, + }, + hasError: false, + }, + } + + for caseNum, c := range cases { + output, err := parseArguments(c.arguments) + if c.hasError && (err == nil) { + t.Errorf("Expecting error for case %v but none encountered", caseNum) + } + if !c.hasError && (err != nil) { + t.Errorf("Expecting no error for case %v but encountered %v", caseNum, err) + } + + if !reflect.DeepEqual(c.expected, output) { + t.Errorf("Case %v is expecting: %v, actual %v", caseNum, c.expected, output) + } + } +} + +func TestSortPathLimits(t *testing.T) { + cases := []struct { + arguments []httpserver.PathLimit + expected []httpserver.PathLimit + }{ + // Parse errors + {arguments: []httpserver.PathLimit{}, expected: []httpserver.PathLimit{}}, + { + arguments: []httpserver.PathLimit{{Path: "/index", Limit: 100}}, + expected: []httpserver.PathLimit{{Path: "/index", Limit: 100}}, + }, + { + arguments: []httpserver.PathLimit{ + {Path: "/static", Limit: 1}, + {Path: "/static/images", Limit: 100}, + {Path: "/index", Limit: 200}, + }, + expected: []httpserver.PathLimit{ + {Path: "/static/images", Limit: 100}, + {Path: "/static", Limit: 1}, + {Path: "/index", Limit: 200}}, + }, + } + + for caseNum, c := range cases { + output := append([]httpserver.PathLimit{}, c.arguments...) + SortPathLimits(output) + if !reflect.DeepEqual(c.expected, output) { + t.Errorf("Case %v is expecting: %v, actual %v", caseNum, c.expected, output) + } + } +} diff --git a/caddyhttp/proxy/proxy.go b/caddyhttp/proxy/proxy.go index bee689285..71c7476b5 100644 --- a/caddyhttp/proxy/proxy.go +++ b/caddyhttp/proxy/proxy.go @@ -174,6 +174,10 @@ func (p Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) { return 0, nil } + if _, ok := backendErr.(httpserver.MaxBytesExceeded); ok { + return http.StatusRequestEntityTooLarge, backendErr + } + // failover; remember this failure for some time if // request failure counting is enabled timeout := host.FailTimeout