diff --git a/oapi_validate.go b/oapi_validate.go index f48ad69..8381d5f 100644 --- a/oapi_validate.go +++ b/oapi_validate.go @@ -132,10 +132,30 @@ func ValidateRequestFromContext(ctx echo.Context, router routers.Router, options 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 would drain the original + // request's body and put a fresh reader (and, with KeepValidatedValue, + // the validated value, plus an updated ContentLength when defaults are + // applied) back onto the clone only, leaving the handler a drained + // body. Validating the live request means everything openapi3filter + // writes lands on the request the handler sees. + // + // RawPath is trimmed too, so percent-encoded path parameters under the + // prefix still route correctly. + // + // The deferred restore runs before this function returns, and so + // before the next handler, which therefore still sees the original + // path. Note that an AuthenticationFunc reading + // GetEchoContext(ctx).Request().URL.Path during validation sees the + // trimmed 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/oapi_validate_test.go b/oapi_validate_test.go index 14a8fed..ca2c4fa 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" @@ -190,7 +191,6 @@ func TestOapiRequestValidator(t *testing.T) { e.GET("/protected_resource", func(c echo.Context) error { called = true return c.NoContent(http.StatusNoContent) - }) // Call a protected function to which we have access @@ -453,6 +453,10 @@ func TestOapiRequestValidatorWithPrefix(t *testing.T) { called := false + type testBody struct { + Msg string + } + // Register handler under the prefixed path (as echo sees it) e.GET("/api/resource", func(c echo.Context) error { called = true @@ -460,6 +464,16 @@ func TestOapiRequestValidatorWithPrefix(t *testing.T) { assert.Equal(t, "/api/resource", c.Request().URL.Path) return c.NoContent(http.StatusOK) }) + e.POST("/api/resource", func(c echo.Context) error { + called = true + // The original request path should be preserved for the handler + assert.Equal(t, "/api/resource", c.Request().URL.Path) + req := c.Request() + data, err := io.ReadAll(req.Body) + require.NoError(t, err, "Reading POST body") + assert.Equal(t, "{\"Msg\":\"testbody\"}", string(data)) + return c.NoContent(http.StatusOK) + }) // A request to /api/resource should validate against /resource in the spec { @@ -469,6 +483,16 @@ func TestOapiRequestValidatorWithPrefix(t *testing.T) { called = false } + // A POST request to /api/resource should validate against /resource in the spec and have a specific body + { + rec := doPost(t, e, "http://test.hostname/api/resource", testBody{ + Msg: "testbody", + }) + assert.Equal(t, http.StatusOK, rec.Code) + assert.True(t, called, "Handler should have been called") + called = false + } + // A request with query params should also validate correctly { rec := doGet(t, e, "http://test.hostname/api/resource?id=50") @@ -485,6 +509,277 @@ func TestOapiRequestValidatorWithPrefix(t *testing.T) { } } +// prefixSpec is used by the Options.Prefix tests below. It has no servers, so +// no Host validation is performed. +const prefixSpec = ` +openapi: "3.0.0" +info: + title: prefix + version: 1.0.0 +paths: + /thing: + post: + operationId: createThing + security: + - BearerAuth: [] + requestBody: + required: true + content: + application/json: + schema: + type: object + required: [name] + properties: + name: + type: string + responses: + '204': + description: ok + /defaulted: + post: + operationId: createDefaulted + requestBody: + required: true + content: + application/json: + schema: + type: object + properties: + name: + type: string + count: + type: integer + default: 7 + responses: + '204': + description: ok + /items/{id}: + get: + operationId: getItem + parameters: + - name: id + in: path + required: true + schema: + type: string + pattern: '^[a-z]/[a-z]$' + responses: + '204': + description: ok +components: + securitySchemes: + BearerAuth: + type: http + scheme: bearer +` + +func loadPrefixSpec(t *testing.T) *openapi3.T { + t.Helper() + spec, err := openapi3.NewLoader().LoadFromData([]byte(prefixSpec)) + require.NoError(t, err, "Error initializing OpenAPI spec") + return spec +} + +// TestPrefixWithSecurityKeepsRequestBody covers issue #49: with a Prefix and +// a security requirement, openapi3filter reads the body for the +// AuthenticationFunc and for body validation, and the handler must still get +// the complete body, a matching ContentLength and the original path. +func TestPrefixWithSecurityKeepsRequestBody(t *testing.T) { + spec := loadPrefixSpec(t) + e := echo.New() + + const body = `{"name":"x"}` + + authCalled := false + e.Use(OapiRequestValidatorWithOptions(spec, &Options{ + Prefix: "/api", + Options: openapi3filter.Options{ + AuthenticationFunc: func(ctx context.Context, input *openapi3filter.AuthenticationInput) error { + authCalled = true + // The prefix is trimmed in place on the live request during + // validation, so the echo request seen here has the trimmed path. + assert.Equal(t, "/thing", GetEchoContext(ctx).Request().URL.Path) + + // The AuthenticationFunc can read the body too. + data, err := io.ReadAll(input.RequestValidationInput.Request.Body) + require.NoError(t, err) + assert.Equal(t, body, string(data)) + + if input.RequestValidationInput.Request.Header.Get("Authorization") != "Bearer token" { + return echo.ErrUnauthorized + } + return nil + }, + }, + })) + + var ( + called bool + gotBody []byte + gotLen int64 + gotPath string + ) + e.POST("/api/thing", func(c echo.Context) error { + called = true + req := c.Request() + gotPath = req.URL.Path + gotLen = req.ContentLength + var err error + gotBody, err = io.ReadAll(req.Body) + require.NoError(t, err) + return c.NoContent(http.StatusNoContent) + }) + + newRequest := func() *http.Request { + r := httptest.NewRequest(http.MethodPost, "/api/thing", strings.NewReader(body)) + r.Header.Set("Content-Type", "application/json") + return r + } + + // Authorized: the handler sees the exact body, its length and the + // original, untrimmed path. + { + r := newRequest() + r.Header.Set("Authorization", "Bearer token") + rec := httptest.NewRecorder() + e.ServeHTTP(rec, r) + + assert.Equal(t, http.StatusNoContent, rec.Code) + assert.True(t, authCalled, "AuthenticationFunc should have been called") + assert.True(t, called, "Handler should have been called") + assert.Equal(t, body, string(gotBody)) + assert.Equal(t, int64(len(body)), gotLen) + assert.Equal(t, "/api/thing", gotPath) + assert.Equal(t, "/api/thing", r.URL.Path) + } + + // Unauthorized: the handler must not run, and the path is still restored. + { + authCalled, called = false, false + r := newRequest() + rec := httptest.NewRecorder() + e.ServeHTTP(rec, r) + + assert.Equal(t, http.StatusUnauthorized, rec.Code) + assert.True(t, authCalled, "AuthenticationFunc should have been called") + assert.False(t, called, "Handler should not have been called") + assert.Equal(t, "/api/thing", r.URL.Path) + } +} + +// TestPrefixKeepsRewrittenBody checks that when openapi3filter applies schema +// defaults and rewrites the body, the handler gets the rewritten body with a +// ContentLength that matches it. +func TestPrefixKeepsRewrittenBody(t *testing.T) { + spec := loadPrefixSpec(t) + e := echo.New() + e.Use(OapiRequestValidatorWithOptions(spec, &Options{Prefix: "/api"})) + + var ( + called bool + gotBody []byte + gotLen int64 + ) + e.POST("/api/defaulted", func(c echo.Context) error { + called = true + gotLen = c.Request().ContentLength + var err error + gotBody, err = io.ReadAll(c.Request().Body) + require.NoError(t, err) + return c.NoContent(http.StatusNoContent) + }) + + const body = `{"name":"x"}` + r := httptest.NewRequest(http.MethodPost, "/api/defaulted", strings.NewReader(body)) + r.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + e.ServeHTTP(rec, r) + + assert.Equal(t, http.StatusNoContent, rec.Code) + assert.True(t, called, "Handler should have been called") + assert.JSONEq(t, `{"name":"x","count":7}`, string(gotBody)) + assert.Equal(t, int64(len(gotBody)), gotLen, "ContentLength should match the rewritten body") + assert.NotEqual(t, int64(len(body)), gotLen, "ContentLength should not be the original length") +} + +// TestPrefixKeepValidatedValue checks that the body wrapper set by +// openapi3filter.Options.KeepValidatedValue reaches the handler. +func TestPrefixKeepValidatedValue(t *testing.T) { + spec := loadPrefixSpec(t) + e := echo.New() + e.Use(OapiRequestValidatorWithOptions(spec, &Options{ + Prefix: "/api", + Options: openapi3filter.Options{ + KeepValidatedValue: true, + }, + })) + + called := false + e.POST("/api/defaulted", func(c echo.Context) error { + called = true + vv, ok := c.Request().Body.(interface{ ValidatedValue() any }) + if assert.True(t, ok, "request body should implement ValidatedValue(), got %T", c.Request().Body) { + value, isMap := vv.ValidatedValue().(map[string]any) + if assert.True(t, isMap, "validated value should be a map, got %T", vv.ValidatedValue()) { + assert.Equal(t, "x", value["name"]) + assert.Contains(t, value, "count", "validated value should include the default") + } + } + data, err := io.ReadAll(c.Request().Body) + require.NoError(t, err) + assert.JSONEq(t, `{"name":"x","count":7}`, string(data)) + return c.NoContent(http.StatusNoContent) + }) + + r := httptest.NewRequest(http.MethodPost, "/api/defaulted", strings.NewReader(`{"name":"x"}`)) + r.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + e.ServeHTTP(rec, r) + + assert.Equal(t, http.StatusNoContent, rec.Code) + assert.True(t, called, "Handler should have been called") +} + +// TestPrefixEncodedPathParams checks that a percent-encoded path parameter +// still routes under a Prefix: URL.RawPath must be trimmed along with +// URL.Path, and both restored for the handler. +func TestPrefixEncodedPathParams(t *testing.T) { + spec := loadPrefixSpec(t) + e := echo.New() + e.Use(OapiRequestValidatorWithOptions(spec, &Options{Prefix: "/api"})) + + var ( + called bool + gotPath string + gotRawPath string + ) + e.GET("/api/items/:id", func(c echo.Context) error { + called = true + gotPath = c.Request().URL.Path + gotRawPath = c.Request().URL.RawPath + return c.NoContent(http.StatusNoContent) + }) + + // %2F decodes to "/", so the parameter is "a/b", which matches the + // pattern. Without trimming RawPath, the router would see the decoded + // path /items/a/b and find no route. + { + rec := doGet(t, e, "http://test.hostname/api/items/a%2Fb") + assert.Equal(t, http.StatusNoContent, rec.Code) + assert.True(t, called, "Handler should have been called") + assert.Equal(t, "/api/items/a/b", gotPath) + assert.Equal(t, "/api/items/a%2Fb", gotRawPath) + called = false + } + + // A value that does not match the pattern is still rejected. + { + rec := doGet(t, e, "http://test.hostname/api/items/ab") + assert.Equal(t, http.StatusBadRequest, rec.Code) + assert.False(t, 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") @@ -566,7 +861,6 @@ func TestEncodedPathParams(t *testing.T) { } func TestGetSkipperFromOptions(t *testing.T) { - options := new(Options) assert.NotNil(t, getSkipperFromOptions(options))