From 2802aaf7e7cc99566389bc18569fc4d1aa8e6e93 Mon Sep 17 00:00:00 2001 From: blacksud0 Date: Thu, 20 Aug 2026 16:09:41 +0300 Subject: [PATCH 1/2] fix: trim Options.Prefix in place so the request body survives security validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Request.Clone copies the Body reference. With Prefix set and a security scheme in the spec, openapi3filter's security validation reads the body from the live request and restores a fresh reader onto the clone only, so every POST/PUT/PATCH through a prefixed mount reached its handler with a drained, zero-byte body — even with ExcludeRequestBody enabled, since the read belongs to the security path. Trimming URL.Path (and RawPath, which the clone approach left untrimmed) in place with a deferred restore keeps RequestValidationInput.Request the live request, so the body restore lands where it is needed, and the restore runs before next so handlers still observe the original path — the existing prefix test's path assertion keeps passing. The new regression test fails against the previous clone-based handling and passes with this change. Fixes #16 --- oapi_validate.go | 17 ++++++++--- prefix_body_test.go | 73 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 4 deletions(-) create mode 100644 prefix_body_test.go diff --git a/oapi_validate.go b/oapi_validate.go index 614065b..b44fbec 100644 --- a/oapi_validate.go +++ b/oapi_validate.go @@ -132,10 +132,19 @@ func ValidateRequestFromContext(ctx *echo.Context, router routers.Router, option req := ctx.Request() if options != nil && options.Prefix != "" { - // Clone the request so downstream handlers still see the original path. - clone := req.Clone(req.Context()) - clone.URL.Path = strings.TrimPrefix(clone.URL.Path, options.Prefix) - req = clone + // Trim the prefix in place rather than on a clone: Request.Clone copies + // the Body reference, so openapi3filter's security validation would read + // the original request's body and restore a fresh reader onto the clone + // only, leaving the handler a drained body. The deferred restore runs + // before the handler, which therefore still sees the original path. + path, rawPath := req.URL.Path, req.URL.RawPath + req.URL.Path = strings.TrimPrefix(path, options.Prefix) + if rawPath != "" { + req.URL.RawPath = strings.TrimPrefix(rawPath, options.Prefix) + } + defer func() { + req.URL.Path, req.URL.RawPath = path, rawPath + }() } route, pathParams, err := router.FindRoute(req) diff --git a/prefix_body_test.go b/prefix_body_test.go new file mode 100644 index 0000000..54b0edf --- /dev/null +++ b/prefix_body_test.go @@ -0,0 +1,73 @@ +package echomiddleware + +import ( + "context" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/getkin/kin-openapi/openapi3" + "github.com/getkin/kin-openapi/openapi3filter" + "github.com/labstack/echo/v5" + "github.com/stretchr/testify/require" +) + +func TestPrefixKeepsTheRequestBody(t *testing.T) { + const specYAML = ` +openapi: "3.0.0" +info: + title: t + version: 1.0.0 +paths: + /thing: + post: + operationId: createThing + security: + - BearerAuth: [] + requestBody: + required: true + content: + application/json: + schema: + type: object + responses: + '204': + description: ok +components: + securitySchemes: + BearerAuth: + type: http + scheme: bearer +` + spec, err := openapi3.NewLoader().LoadFromData([]byte(specYAML)) + require.NoError(t, err) + + e := echo.New() + e.Use(OapiRequestValidatorWithOptions(spec, &Options{ + Prefix: "/api", + SilenceServersWarning: true, + Options: openapi3filter.Options{ + ExcludeRequestBody: true, + AuthenticationFunc: func(context.Context, *openapi3filter.AuthenticationInput) error { + return nil + }, + }, + })) + + var got []byte + e.POST("/api/thing", func(c *echo.Context) error { + got, _ = io.ReadAll(c.Request().Body) + return c.NoContent(http.StatusNoContent) + }) + + r := httptest.NewRequest(http.MethodPost, "/api/thing", strings.NewReader(`{"name":"x"}`)) + r.Header.Set("content-type", "application/json") + r.Header.Set("authorization", "Bearer token") + rec := httptest.NewRecorder() + e.ServeHTTP(rec, r) + + require.Equal(t, http.StatusNoContent, rec.Code) + require.NotEmpty(t, got, "the handler received a drained body") +} From efad2649b7799cbbc3408a7a71a6ba0b0bd6119d Mon Sep 17 00:00:00 2001 From: Marcin Romaszewicz Date: Thu, 22 Oct 2026 18:09:26 -0700 Subject: [PATCH 2/2] test: cover body, path and RawPath handling under Options.Prefix Move the prefix body regression test into oapi_validate_test.go next to TestOapiRequestValidatorWithPrefix and broaden it: the handler must get the exact body bytes and the original /api path for plain body validation, for security validation with ExcludeRequestBody, and with KeepValidatedValue, where the body must also expose ValidatedValue(). A new test sends a %2F path parameter under the prefix, which only routes when the prefix is trimmed from URL.RawPath too. All of these fail against the previous req.Clone()-based handling. Co-Authored-By: Claude Opus 5.5 --- oapi_validate_test.go | 197 ++++++++++++++++++++++++++++++++++++++++++ prefix_body_test.go | 73 ---------------- 2 files changed, 197 insertions(+), 73 deletions(-) delete mode 100644 prefix_body_test.go diff --git a/oapi_validate_test.go b/oapi_validate_test.go index dc729a7..83adb10 100644 --- a/oapi_validate_test.go +++ b/oapi_validate_test.go @@ -10,6 +10,7 @@ import ( "net/http" "net/http/httptest" "net/url" + "strings" "testing" "github.com/getkin/kin-openapi/openapi3" @@ -484,6 +485,202 @@ func TestOapiRequestValidatorWithPrefix(t *testing.T) { } } +// newRawJSONPost builds a POST whose body is sent verbatim, so a test can +// compare the exact bytes that reach the handler. +func newRawJSONPost(target, body string) *http.Request { + r := httptest.NewRequest(http.MethodPost, target, strings.NewReader(body)) + r.Header.Set("content-type", "application/json") + return r +} + +// prefixedHandlerResult records what a handler mounted under a prefix saw. +type prefixedHandlerResult struct { + called bool + path string + rawPath string + body []byte + readErr error + // validated is the body's ValidatedValue(), when it has one. + validated any + hasValidated bool +} + +func (r *prefixedHandlerResult) handler(status int) echo.HandlerFunc { + return func(c *echo.Context) error { + req := c.Request() + r.called = true + r.path, r.rawPath = req.URL.Path, req.URL.RawPath + if vv, ok := req.Body.(interface{ ValidatedValue() any }); ok { + r.validated, r.hasValidated = vv.ValidatedValue(), true + } + if req.Body != nil { + r.body, r.readErr = io.ReadAll(req.Body) + } + return c.NoContent(status) + } +} + +// TestOapiRequestValidatorWithPrefixRequestBody checks that, with +// Options.Prefix set, the request body openapi3filter reads during validation +// is restored on the request the handler receives, rather than on a copy of it. +// See https://github.com/oapi-codegen/echo-v5-middleware/issues/16. +func TestOapiRequestValidatorWithPrefixRequestBody(t *testing.T) { + // Not JSON-canonical on purpose: the handler must see these exact bytes, + // not a re-encoding of the decoded value. + const body = `{ "name" : "x" }` + + t.Run("body validation", func(t *testing.T) { + spec, err := openapi3.NewLoader().LoadFromData(testSchema) + require.NoError(t, err) + + e := echo.New() + e.Use(OapiRequestValidatorWithOptions(spec, &Options{ + SilenceServersWarning: true, + Prefix: "/api", + })) + var got prefixedHandlerResult + e.POST("/api/resource", got.handler(http.StatusNoContent)) + + rec := httptest.NewRecorder() + e.ServeHTTP(rec, newRawJSONPost("http://test.hostname/api/resource", body)) + + require.Equal(t, http.StatusNoContent, rec.Code, rec.Body.String()) + require.True(t, got.called, "handler should have been called") + require.NoError(t, got.readErr) + assert.Equal(t, body, string(got.body)) + assert.Equal(t, "/api/resource", got.path) + assert.False(t, got.hasValidated, "body should only expose ValidatedValue() with KeepValidatedValue") + + // A body that doesn't match the schema is still rejected. + got = prefixedHandlerResult{} + rec = httptest.NewRecorder() + e.ServeHTTP(rec, newRawJSONPost("http://test.hostname/api/resource", `{"name": 5}`)) + assert.Equal(t, http.StatusBadRequest, rec.Code) + assert.False(t, got.called, "handler should not have been called") + }) + + t.Run("security validation with ExcludeRequestBody", func(t *testing.T) { + // openapi3filter's security validation reads the body even when body + // validation is excluded, which is how issue #16 was found. + const specYAML = ` +openapi: "3.0.0" +info: + title: t + version: 1.0.0 +paths: + /thing: + post: + operationId: createThing + security: + - BearerAuth: [] + requestBody: + required: true + content: + application/json: + schema: + type: object + responses: + '204': + description: ok +components: + securitySchemes: + BearerAuth: + type: http + scheme: bearer +` + spec, err := openapi3.NewLoader().LoadFromData([]byte(specYAML)) + require.NoError(t, err) + + e := echo.New() + e.Use(OapiRequestValidatorWithOptions(spec, &Options{ + Prefix: "/api", + SilenceServersWarning: true, + Options: openapi3filter.Options{ + ExcludeRequestBody: true, + AuthenticationFunc: func(context.Context, *openapi3filter.AuthenticationInput) error { + return nil + }, + }, + })) + var got prefixedHandlerResult + e.POST("/api/thing", got.handler(http.StatusNoContent)) + + r := newRawJSONPost("/api/thing", body) + r.Header.Set("authorization", "Bearer token") + rec := httptest.NewRecorder() + e.ServeHTTP(rec, r) + + require.Equal(t, http.StatusNoContent, rec.Code, rec.Body.String()) + require.True(t, got.called, "handler should have been called") + require.NoError(t, got.readErr) + assert.Equal(t, body, string(got.body)) + assert.Equal(t, "/api/thing", got.path) + }) + + t.Run("KeepValidatedValue", func(t *testing.T) { + spec, err := openapi3.NewLoader().LoadFromData(testSchema) + require.NoError(t, err) + + e := echo.New() + e.Use(OapiRequestValidatorWithOptions(spec, &Options{ + SilenceServersWarning: true, + Prefix: "/api", + Options: openapi3filter.Options{ + KeepValidatedValue: true, + }, + })) + var got prefixedHandlerResult + e.POST("/api/resource", got.handler(http.StatusNoContent)) + + rec := httptest.NewRecorder() + e.ServeHTTP(rec, newRawJSONPost("http://test.hostname/api/resource", body)) + + require.Equal(t, http.StatusNoContent, rec.Code, rec.Body.String()) + require.True(t, got.called, "handler should have been called") + require.True(t, got.hasValidated, "handler's body should expose ValidatedValue()") + assert.Equal(t, map[string]any{"name": "x"}, got.validated) + require.NoError(t, got.readErr) + assert.Equal(t, body, string(got.body)) + assert.Equal(t, "/api/resource", got.path) + }) +} + +// TestOapiRequestValidatorWithPrefixEncodedPathParams checks that the prefix +// is trimmed from URL.RawPath as well as URL.Path, and both are restored +// before the handler runs. +func TestOapiRequestValidatorWithPrefixEncodedPathParams(t *testing.T) { + spec, err := openapi3.NewLoader().LoadFromData(testSchema) + require.NoError(t, err) + + e := echo.New() + e.Use(OapiRequestValidatorWithOptions(spec, &Options{ + SilenceServersWarning: true, + Prefix: "/api", + })) + var got prefixedHandlerResult + e.GET("/api/resource/maxlength/:param", got.handler(http.StatusNoContent)) + + // %2F decodes to a single "/", which satisfies maxLength: 1. The router + // matches on URL.EscapedPath(), which only uses RawPath while it is a valid + // encoding of Path; if the prefix were trimmed from Path alone, the router + // would see the decoded "//" and find no operation. + { + rec := doGet(t, e, "http://test.hostname/api/resource/maxlength/%2F") + require.Equal(t, http.StatusNoContent, rec.Code, rec.Body.String()) + require.True(t, got.called, "handler should have been called") + assert.Equal(t, "/api/resource/maxlength//", got.path) + assert.Equal(t, "/api/resource/maxlength/%2F", got.rawPath) + } + + // %2B%2B decodes to "++", which exceeds maxLength: 1. + { + got = prefixedHandlerResult{} + rec := doGet(t, e, "http://test.hostname/api/resource/maxlength/%2B%2B") + assert.Equal(t, http.StatusBadRequest, rec.Code) + assert.False(t, got.called, "handler should not have been called") + } +} + func TestEncodedPathParams(t *testing.T) { spec, err := openapi3.NewLoader().LoadFromData(testSchema) require.NoError(t, err, "Error initializing OpenAPI spec") diff --git a/prefix_body_test.go b/prefix_body_test.go deleted file mode 100644 index 54b0edf..0000000 --- a/prefix_body_test.go +++ /dev/null @@ -1,73 +0,0 @@ -package echomiddleware - -import ( - "context" - "io" - "net/http" - "net/http/httptest" - "strings" - "testing" - - "github.com/getkin/kin-openapi/openapi3" - "github.com/getkin/kin-openapi/openapi3filter" - "github.com/labstack/echo/v5" - "github.com/stretchr/testify/require" -) - -func TestPrefixKeepsTheRequestBody(t *testing.T) { - const specYAML = ` -openapi: "3.0.0" -info: - title: t - version: 1.0.0 -paths: - /thing: - post: - operationId: createThing - security: - - BearerAuth: [] - requestBody: - required: true - content: - application/json: - schema: - type: object - responses: - '204': - description: ok -components: - securitySchemes: - BearerAuth: - type: http - scheme: bearer -` - spec, err := openapi3.NewLoader().LoadFromData([]byte(specYAML)) - require.NoError(t, err) - - e := echo.New() - e.Use(OapiRequestValidatorWithOptions(spec, &Options{ - Prefix: "/api", - SilenceServersWarning: true, - Options: openapi3filter.Options{ - ExcludeRequestBody: true, - AuthenticationFunc: func(context.Context, *openapi3filter.AuthenticationInput) error { - return nil - }, - }, - })) - - var got []byte - e.POST("/api/thing", func(c *echo.Context) error { - got, _ = io.ReadAll(c.Request().Body) - return c.NoContent(http.StatusNoContent) - }) - - r := httptest.NewRequest(http.MethodPost, "/api/thing", strings.NewReader(`{"name":"x"}`)) - r.Header.Set("content-type", "application/json") - r.Header.Set("authorization", "Bearer token") - rec := httptest.NewRecorder() - e.ServeHTTP(rec, r) - - require.Equal(t, http.StatusNoContent, rec.Code) - require.NotEmpty(t, got, "the handler received a drained body") -}