diff --git a/README.md b/README.md index 2d9efe6..97d5096 100644 --- a/README.md +++ b/README.md @@ -68,6 +68,14 @@ server := use(router, mw) // now all HTTP routes will be handled by the middleware, and any requests that are invalid will be rejected ``` +### Keeping the validated request body + +The middleware enables kin-openapi's [`openapi3filter.Options.KeepValidatedValue`](https://pkg.go.dev/github.com/getkin/kin-openapi/openapi3filter#Options) by default. After a request body passes validation, the `r.Body` that your handlers receive is still readable as before, and also has a `ValidatedValue() any` method returning the value that was validated. An [`oapi-codegen`](https://github.com/oapi-codegen/oapi-codegen) strict server can bind that value instead of decoding the body a second time, so the request it handles is the request that was validated (for example, `encoding/json`'s case-insensitive field matching can no longer let `{"role":"user","ROLE":"admin"}` validate as `role: user` but bind as `Role: "admin"`). + +To turn this off, for instance to limit memory retention for very large request bodies, set `Options.DiscardValidatedValue`, which takes precedence over `Options.Options.KeepValidatedValue`. + +Any middleware placed _after_ the validator that replaces `r.Body` (decompression, size limiting, logging wrappers, ...) hides the validated value from the handler, so such middleware belongs _before_ the validator. + ## FAQs ### Which HTTP servers should this work with? diff --git a/oapi_validate.go b/oapi_validate.go index e21e778..8ef3c09 100644 --- a/oapi_validate.go +++ b/oapi_validate.go @@ -111,6 +111,30 @@ type Options struct { // Skipper allows writing a function that runs before any middleware and determines whether the given request should skip any validation middleware Skipper Skipper + + // DiscardValidatedValue turns off openapi3filter.Options.KeepValidatedValue, + // which this middleware enables by default. With it enabled, the request body + // seen by handlers exposes the value that was validated through a + // ValidatedValue() method, so a handler (for example an oapi-codegen strict + // server) can bind that value instead of decoding the body a second time. + // Set this to drop the decoded value after validation, for instance to limit + // memory retention for very large request bodies. + // + // Options.Options.KeepValidatedValue is overridden by this setting. + DiscardValidatedValue bool +} + +// filterOptions returns the openapi3filter.Options to validate requests with. +// It returns a copy of options.Options, so the caller's struct is never +// modified, with KeepValidatedValue set unless DiscardValidatedValue is. +func filterOptions(options *Options) *openapi3filter.Options { + if options == nil { + return &openapi3filter.Options{KeepValidatedValue: true} + } + + opts := options.Options + opts.KeepValidatedValue = !options.DiscardValidatedValue + return &opts } // OapiRequestValidator Creates the middleware to validate that incoming requests match the given OpenAPI 3.x spec, with a default set of configuration. @@ -135,6 +159,8 @@ func OapiRequestValidatorWithOptions(spec *openapi3.T, options *Options) func(ne panic(err) } + filterOpts := filterOptions(options) + return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if options != nil && options.Skipper != nil && options.Skipper(r) { @@ -143,23 +169,23 @@ func OapiRequestValidatorWithOptions(spec *openapi3.T, options *Options) func(ne } if options == nil { - performRequestValidationForErrorHandler(next, w, r, router, options, http.Error) + performRequestValidationForErrorHandler(next, w, r, router, options, filterOpts, http.Error) } else if options.ErrorHandlerWithOpts != nil { - performRequestValidationForErrorHandlerWithOpts(next, w, r, router, options) + performRequestValidationForErrorHandlerWithOpts(next, w, r, router, options, filterOpts) } else if options.ErrorHandler != nil { - performRequestValidationForErrorHandler(next, w, r, router, options, options.ErrorHandler) + performRequestValidationForErrorHandler(next, w, r, router, options, filterOpts, options.ErrorHandler) } else { // NOTE that this shouldn't happen, but let's be sure that we always end up calling the default error handler if no other handler is defined - performRequestValidationForErrorHandler(next, w, r, router, options, http.Error) + performRequestValidationForErrorHandler(next, w, r, router, options, filterOpts, http.Error) } }) } } -func performRequestValidationForErrorHandler(next http.Handler, w http.ResponseWriter, r *http.Request, router routers.Router, options *Options, errorHandler ErrorHandler) { +func performRequestValidationForErrorHandler(next http.Handler, w http.ResponseWriter, r *http.Request, router routers.Router, options *Options, filterOpts *openapi3filter.Options, errorHandler ErrorHandler) { // validate request - statusCode, err := validateRequest(r, router, options) + statusCode, err := validateRequest(r, router, options, filterOpts) if err == nil { // serve next.ServeHTTP(w, r) @@ -222,7 +248,7 @@ func stripPrefix(s, prefix string) string { } // Note that this is an inline-and-modified version of `validateRequest`, with a simplified control flow and providing full access to the `error` for the `ErrorHandlerWithOpts` function. -func performRequestValidationForErrorHandlerWithOpts(next http.Handler, w http.ResponseWriter, r *http.Request, router routers.Router, options *Options) { +func performRequestValidationForErrorHandlerWithOpts(next http.Handler, w http.ResponseWriter, r *http.Request, router routers.Router, options *Options, filterOpts *openapi3filter.Options) { var route *routers.Route var pathParams map[string]string var validationErr error @@ -241,10 +267,7 @@ func performRequestValidationForErrorHandlerWithOpts(next http.Handler, w http.R Request: r, PathParams: pathParams, Route: route, - } - - if options != nil { - requestValidationInput.Options = &options.Options + Options: filterOpts, } validationErr = openapi3filter.ValidateRequest(r.Context(), requestValidationInput) @@ -293,12 +316,12 @@ func performRequestValidationForErrorHandlerWithOpts(next http.Handler, w http.R // validateRequest is called from the middleware above and actually does the work // of validating a request. -func validateRequest(r *http.Request, router routers.Router, options *Options) (int, error) { +func validateRequest(r *http.Request, router routers.Router, options *Options, filterOpts *openapi3filter.Options) (int, error) { var statusCode int var validationErr error withPrefixStripped(r, options, func() { - statusCode, validationErr = doValidateRequest(r, router, options) + statusCode, validationErr = doValidateRequest(r, router, options, filterOpts) }) return statusCode, validationErr @@ -306,7 +329,7 @@ func validateRequest(r *http.Request, router routers.Router, options *Options) ( // doValidateRequest performs the actual validation, called within // withPrefixStripped so the prefix is already removed from r's path. -func doValidateRequest(r *http.Request, router routers.Router, options *Options) (int, error) { +func doValidateRequest(r *http.Request, router routers.Router, options *Options, filterOpts *openapi3filter.Options) (int, error) { // Find route route, pathParams, err := router.FindRoute(r) if err != nil { @@ -323,10 +346,7 @@ func doValidateRequest(r *http.Request, router routers.Router, options *Options) PathParams: pathParams, QueryParams: r.URL.Query(), Route: route, - } - - if options != nil { - requestValidationInput.Options = &options.Options + Options: filterOpts, } if err := openapi3filter.ValidateRequest(r.Context(), requestValidationInput); err != nil { diff --git a/oapi_validate_test.go b/oapi_validate_test.go index c07b17c..bbf8706 100644 --- a/oapi_validate_test.go +++ b/oapi_validate_test.go @@ -1,9 +1,16 @@ package nethttpmiddleware import ( + "bytes" + "context" "fmt" + "io" + "net/http" + "net/http/httptest" + "reflect" "testing" + "github.com/getkin/kin-openapi/openapi3" "github.com/getkin/kin-openapi/openapi3filter" ) @@ -63,3 +70,192 @@ func Test_determineStatusCodeForMultiError(t *testing.T) { } }) } + +const validatedValueSpec = ` +openapi: "3.0.0" +info: + version: 1.0.0 + title: TestServer +paths: + /resource: + post: + operationId: createResource + responses: + '204': + description: No content + requestBody: + required: true + content: + application/json: + schema: + properties: + name: + type: string +` + +// validatedValuer is the interface openapi3filter gives the request body when +// openapi3filter.Options.KeepValidatedValue is set. +type validatedValuer interface { + ValidatedValue() any +} + +// bodySeen records what a handler behind the middleware saw on r.Body. +type bodySeen struct { + called bool + hasValue bool + validatedValue any + body []byte +} + +// postThroughValidator sends a POST with body through the middleware built +// from options (nil means OapiRequestValidator) and records the r.Body the +// handler received. +func postThroughValidator(t *testing.T, options *Options, body string) (*httptest.ResponseRecorder, bodySeen) { + t.Helper() + + spec, err := openapi3.NewLoader().LoadFromData([]byte(validatedValueSpec)) + if err != nil { + t.Fatalf("failed to load spec: %v", err) + } + + var seen bodySeen + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + seen.called = true + if vv, ok := r.Body.(validatedValuer); ok { + seen.hasValue = true + seen.validatedValue = vv.ValidatedValue() + } + data, err := io.ReadAll(r.Body) + if err != nil { + t.Errorf("failed to read request body in handler: %v", err) + } + seen.body = data + w.WriteHeader(http.StatusNoContent) + }) + + var mw func(http.Handler) http.Handler + if options == nil { + mw = OapiRequestValidator(spec) + } else { + mw = OapiRequestValidatorWithOptions(spec, options) + } + + req := httptest.NewRequest(http.MethodPost, "/resource", bytes.NewReader([]byte(body))) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + mw(handler).ServeHTTP(rec, req) + + return rec, seen +} + +func TestKeepValidatedValue(t *testing.T) { + const body = `{"name":"Wilbur"}` + expectedValue := map[string]any{"name": "Wilbur"} + + keptCases := []struct { + name string + options *Options + }{ + { + name: "OapiRequestValidator without options", + options: nil, + }, + { + name: "empty Options", + options: &Options{}, + }, + { + name: "ErrorHandlerWithOpts", + options: &Options{ + ErrorHandlerWithOpts: func(_ context.Context, err error, w http.ResponseWriter, _ *http.Request, opts ErrorHandlerOpts) { + http.Error(w, err.Error(), opts.StatusCode) + }, + }, + }, + } + + for _, tc := range keptCases { + t.Run("keeps the validated value with "+tc.name, func(t *testing.T) { + rec, seen := postThroughValidator(t, tc.options, body) + + if rec.Code != http.StatusNoContent { + t.Fatalf("expected HTTP %d, got %d: %s", http.StatusNoContent, rec.Code, rec.Body.String()) + } + if !seen.called { + t.Fatal("handler was not called") + } + if !seen.hasValue { + t.Fatalf("expected r.Body (%T) to implement ValidatedValue() any", seen.validatedValue) + } + if !reflect.DeepEqual(expectedValue, seen.validatedValue) { + t.Errorf("expected validated value %#v, got %#v", expectedValue, seen.validatedValue) + } + if string(seen.body) != body { + t.Errorf("expected the body to still read %q, got %q", body, string(seen.body)) + } + if tc.options != nil && tc.options.Options.KeepValidatedValue { + t.Error("the caller's Options.Options.KeepValidatedValue was modified") + } + }) + } + + t.Run("DiscardValidatedValue drops the validated value", func(t *testing.T) { + options := &Options{ + DiscardValidatedValue: true, + Options: openapi3filter.Options{ + // overridden by DiscardValidatedValue + KeepValidatedValue: true, + }, + } + rec, seen := postThroughValidator(t, options, body) + + if rec.Code != http.StatusNoContent { + t.Fatalf("expected HTTP %d, got %d: %s", http.StatusNoContent, rec.Code, rec.Body.String()) + } + if !seen.called { + t.Fatal("handler was not called") + } + if seen.hasValue { + t.Error("expected r.Body not to implement ValidatedValue() any") + } + if string(seen.body) != body { + t.Errorf("expected the body to still read %q, got %q", body, string(seen.body)) + } + if !options.Options.KeepValidatedValue { + t.Error("the caller's Options.Options.KeepValidatedValue was modified") + } + }) + + t.Run("ExcludeRequestBody still passes the body through", func(t *testing.T) { + options := &Options{ + Options: openapi3filter.Options{ + ExcludeRequestBody: true, + }, + } + rec, seen := postThroughValidator(t, options, body) + + if rec.Code != http.StatusNoContent { + t.Fatalf("expected HTTP %d, got %d: %s", http.StatusNoContent, rec.Code, rec.Body.String()) + } + if !seen.called { + t.Fatal("handler was not called") + } + if seen.hasValue { + t.Error("expected r.Body not to implement ValidatedValue() any when the body is not validated") + } + if string(seen.body) != body { + t.Errorf("expected the body to still read %q, got %q", body, string(seen.body)) + } + }) + + t.Run("an invalid body is still rejected", func(t *testing.T) { + rec, seen := postThroughValidator(t, nil, `{"name":1}`) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("expected HTTP %d, got %d: %s", http.StatusBadRequest, rec.Code, rec.Body.String()) + } + if seen.called { + t.Error("handler should not have been called") + } + }) +}