diff --git a/caddytest/integration/handler_test.go b/caddytest/integration/handler_test.go index afc700b02bd..4b39d83c0bc 100644 --- a/caddytest/integration/handler_test.go +++ b/caddytest/integration/handler_test.go @@ -57,3 +57,30 @@ func TestRespondWithJSON(t *testing.T) { t.Errorf("expected Content-Type to be application/json, but was %s", res.Header.Get("Content-Type")) } } + +func TestRequestBodyPlaceholderRespectsMaxSize(t *testing.T) { + tester := caddytest.NewTester(t) + tester.InitServer(` + { + skip_install_trust + admin localhost:2999 + http_port 9080 + https_port 9443 + grace_period 1ns + } + http://localhost:9080 { + request_body { + max_size 10 + } + respond "{{placeholder \"http.request.body\"}}" + templates + } + `, "caddyfile") + + req, err := http.NewRequest(http.MethodPost, "http://localhost:9080/", bytes.NewBufferString("abcdefghijklm")) + if err != nil { + t.Fatalf("creating request: %v", err) + } + res := tester.AssertResponseCode(req, http.StatusRequestEntityTooLarge) + res.Body.Close() +} diff --git a/modules/caddyhttp/replacer.go b/modules/caddyhttp/replacer.go index 623a6ef4bf4..a661197fe7d 100644 --- a/modules/caddyhttp/replacer.go +++ b/modules/caddyhttp/replacer.go @@ -26,6 +26,7 @@ import ( "encoding/asn1" "encoding/base64" "encoding/pem" + "errors" "fmt" "io" "net" @@ -218,31 +219,21 @@ func addHTTPVarsToReplacer(repl *caddy.Replacer, req *http.Request, w http.Respo if req.Body == nil { return "", true } - // normally net/http will close the body for us, but since we - // are replacing it with a fake one, we have to ensure we close - // the real body ourselves when we're done - defer req.Body.Close() - // 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) - _, _ = 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 + body, err := readRequestBodyForPlaceholder(req) + if err != nil { + return err, true + } + return string(body), true case "http.request.body_base64": if req.Body == nil { return "", true } - // normally net/http will close the body for us, but since we - // are replacing it with a fake one, we have to ensure we close - // the real body ourselves when we're done - defer req.Body.Close() - // 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) - _, _ = 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 base64.StdEncoding.EncodeToString(buf.Bytes()), true + body, err := readRequestBodyForPlaceholder(req) + if err != nil { + return err, true + } + return base64.StdEncoding.EncodeToString(body), true // original request, before any internal changes case "http.request.orig_method": @@ -403,6 +394,28 @@ func addHTTPVarsToReplacer(repl *caddy.Replacer, req *http.Request, w http.Respo repl.Map(httpVars) } +func readRequestBodyForPlaceholder(req *http.Request) ([]byte, error) { + // normally net/http will close the body for us, but since we + // are replacing it with a fake one, we have to ensure we close + // the real body ourselves when we're done + defer req.Body.Close() + + // 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 { + var mbe *http.MaxBytesError + if errors.As(err, &mbe) { + err = Error(http.StatusRequestEntityTooLarge, err) + } + return nil, err + } + + req.Body = io.NopCloser(bytes.NewReader(buf.Bytes())) + return buf.Bytes(), nil +} + func getReqTLSReplacement(req *http.Request, key string) (any, bool) { if req == nil || req.TLS == nil { return nil, false diff --git a/modules/caddyhttp/templates/tplcontext.go b/modules/caddyhttp/templates/tplcontext.go index ee553e7a560..b68e7dc2fbe 100644 --- a/modules/caddyhttp/templates/tplcontext.go +++ b/modules/caddyhttp/templates/tplcontext.go @@ -250,7 +250,7 @@ func (c *TemplateContext) executeTemplateInBuffer(tplName string, buf *bytes.Buf return c.tpl.Execute(buf, c) } -func (c TemplateContext) funcPlaceholder(name string) string { +func (c TemplateContext) funcPlaceholder(name string) (string, error) { repl := c.Req.Context().Value(caddy.ReplacerCtxKey).(*caddy.Replacer) // For safety, we don't want to allow the file placeholder in @@ -258,8 +258,11 @@ func (c TemplateContext) funcPlaceholder(name string) string { // if the template contents were not trusted. repl = repl.WithoutFile() - value, _ := repl.GetString(name) - return value + value, _ := repl.Get(name) + if err, ok := value.(error); ok { + return "", err + } + return caddy.ToString(value), nil } func (TemplateContext) funcEnv(varName string) string { diff --git a/modules/caddyhttp/vars.go b/modules/caddyhttp/vars.go index 6c17fe9bb7f..27c0161837e 100644 --- a/modules/caddyhttp/vars.go +++ b/modules/caddyhttp/vars.go @@ -198,7 +198,7 @@ func (m VarsMatcher) MatchWithError(r *http.Request) (bool, error) { case fmt.Stringer: varStr = vv.String() case error: - varStr = vv.Error() + return false, vv case nil: varStr = "" default: @@ -335,7 +335,7 @@ func (m MatchVarsRE) MatchWithError(r *http.Request) (bool, error) { case fmt.Stringer: varStr = vv.String() case error: - varStr = vv.Error() + return false, vv case nil: varStr = "" default: diff --git a/modules/caddyhttp/vars_test.go b/modules/caddyhttp/vars_test.go index 7cbe583e5ef..183c9bd9495 100644 --- a/modules/caddyhttp/vars_test.go +++ b/modules/caddyhttp/vars_test.go @@ -16,6 +16,7 @@ package caddyhttp import ( "context" + "errors" "net/http" "net/http/httptest" "testing" @@ -157,3 +158,43 @@ func TestMatchVarsREDoesNotExpandResolvedValues(t *testing.T) { }) } } + +func TestVarsMatchersPropagateErrors(t *testing.T) { + placeholderErr := Error(http.StatusRequestEntityTooLarge, errors.New("request body too large")) + req, _ := newVarsTestRequest(t, "", nil, map[string]any{"body": placeholderErr}) + + for _, tc := range []struct { + name string + match RequestMatcherWithError + }{ + { + name: "vars", + match: VarsMatcher{"body": []string{"anything"}}, + }, + { + name: "vars_regexp", + match: MatchVarsRE{"body": &MatchRegexp{Pattern: ".*"}}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + if provisioner, ok := tc.match.(caddy.Provisioner); ok { + if err := provisioner.Provision(caddy.Context{}); err != nil { + t.Fatalf("Provision() error = %v", err) + } + } + + _, err := tc.match.MatchWithError(req) + if err == nil { + t.Fatal("MatchWithError() error = nil, want placeholder error") + } + + var handlerErr HandlerError + if !errors.As(err, &handlerErr) { + t.Fatalf("MatchWithError() error = %T, want HandlerError", err) + } + if handlerErr.StatusCode != http.StatusRequestEntityTooLarge { + t.Fatalf("MatchWithError() status = %d, want %d", handlerErr.StatusCode, http.StatusRequestEntityTooLarge) + } + }) + } +}