oapi-codegen icon indicating copy to clipboard operation
oapi-codegen copied to clipboard

Improvement : oapi validation middleware should only validate path/route/endpoint that included in oapi spec file.

Open bandirsen opened this issue 5 years ago • 3 comments

In current implementation, oapi validation middleware validate all route/path of the server application including path/route that wasn't included in oapi spec file,

I have a case when my application is serving oapi request but also serve static files from a webroot (I'm using echo framework)

e := echo.New()
e.Static('/', "path_to_static_files_dir")
...
e.Use(OapiRequestValidator(swagger))   

The middleware will reject request to static files such as http://host/index.html or http://host/javascript/any.js or http://host/image/any.png etc and send 400 error "Path was not found",

In my opinion those requests should not validated by the middleware, since was not oapi end-point. I knew we could use echo middleware Skipper, but it will be tedious job if we have many static files/folder in webroot directory.

My suggestion is if the middleware can not find a matching route from a request, it should letting the upstream application code to handle it.

Anyone agree with me ?

bandirsen avatar Oct 14 '20 17:10 bandirsen

Did you ever find a workaround for this? My use case is metrics endpoints. I'm trying to use the echo Prometheus middleware, but the validator keeps blocking /metrics requests. I kind of sort of worked around it by writing a custom Skipper function. That feels like it could get cumbersome with more and more exceptions.

	validatorOptions.Skipper = func(ctx echo.Context) bool {
		if ctx.Path() == "/metrics" {
			return true
		}

		return false
	}
	e.Use(middleware.OapiRequestValidatorWithOptions(swagger, validatorOptions))

iggy avatar Nov 14 '20 20:11 iggy

Using e.Group solved similar issue for me

e := echo.New()
noCodegen := e.Group("")
// Add non-codegen handlers here

codegen := e.Group("")
// Add codegen handlers and middleware here.

Full example:

// Add non-codegen handlers here
noCodegen := e.Group("")
noCodegen.Static("/fonts", "fonts")
noCodegen.Static("/css", "css")

// Add codegen handlers and middleware here
codegen := e.Group("")

// Instead of setting swagger.Servers to nil, set it to the base path similar to router
// https://github.com/deepmap/oapi-codegen/issues/806
swagger.Servers = []*openapi3.Server{{URL: "/api", Description: "Needed to match path"}}
// Use our validation middleware to check all requests against the OpenAPI schema.
codegen.Use(middleware.OapiRequestValidator(swagger))

// We now register our store above as the handler for the interface
api.RegisterHandlersWithBaseURL(codegen, handler, "/api")

Proposed in https://github.com/deepmap/oapi-codegen/pull/552#issuecomment-1102784902.

aliakseiz avatar Aug 24 '23 09:08 aliakseiz

In case of chi, it generates a function called HandlerWithOptions. The second param it accepts is a ChiServerOptions type which can have a slice of middlewares. Adding OapiRequestValidator to the slice only validates paths in the spec.

So maybe something similar can be implemented in echo.

pratyushbarik avatar Oct 06 '23 06:10 pratyushbarik