OapiRequestValidator does not stop middleware propagation with gin
I'm using a gin router and passing a middlewarefunc to RegisterHandlersWithOptions. One middleware func is a OapiRequestValidatorWithOptions. That function does not exit properly. If the request fails the security check, it calls c.Abort() but does not return to stop propagation. Instead, it calls c.Next() and begins the next middleware.
I can't provide much code, because it is private, but here is how I mount the oapi-codegen router.
engine.Router = application.RegisterHandlersWithOptions(engine.Router, app, application.GinServerOptions{
ErrorHandler: func(c *gin.Context, err error, statusCode int) {
c.AbortWithStatusJSON(statusCode, gin.H{
"err": err.Error(),
})
},
Middlewares: []application.MiddlewareFunc{
application.MiddlewareFunc(middleware.OapiRequestValidatorWithOptions(
spec,
&middleware.Options{
Options: openapi3filter.Options{
AuthenticationFunc: createTokenVerifier(config.JwtSecret),
},
ErrorHandler: func(c *gin.Context, message string, statusCode int) {
c.AbortWithStatusJSON(statusCode, gin.H{
"err": message,
})
},
},
)),
},
})
The bug
Internally, in oapi-validate.go
func OapiRequestValidatorWithOptions(swagger *openapi3.T, options *Options) gin.HandlerFunc {
router, err := gorillamux.NewRouter(swagger)
if err != nil {
panic(err)
}
return func(c *gin.Context) {
err := ValidateRequestFromContext(c, router, options)
if err != nil {
if options != nil && options.ErrorHandler != nil {
options.ErrorHandler(c, err.Error(), http.StatusBadRequest)
// in case the handler didn't internally call Abort, stop the chain
c.Abort() // This case is hit when security fails
// c.Abort merely writes a status code to the response header. You must also
// call return here, or else c.Next() will call and the next middleware will continue.
// THIS SHOULD RETURN HERE
} else {
// note: i am not sure if this is the best way to handle this
c.AbortWithStatusJSON(http.StatusBadRequest, gin.H{"error": err.Error()})
// THIS SHOULD ALSO RETURN
}
}
c.Next()
}
}
Edit: After playing around with this some, adding the return statements isn't enough. The generated handler code should check if the context was aborted before calling all the middlewarefuncs.
In the generated handler code:
for _, middleware := range siw.HandlerMiddlewares {
middleware(c)
}
It is directly calling the middleware funcs, disregarding the state of the context. Before it calls middleware(c), it should first check to see if the headers were written already or something similar to how gin checks if a context was aborted.
Here is a link to that bugged file https://github.com/deepmap/oapi-codegen/blob/master/pkg/gin-middleware/oapi_validate.go