From 1192e6f787e791deaa3d6a2d6ed825f6abe194f3 Mon Sep 17 00:00:00 2001 From: "Gerasimos (Makis) Maropoulos" Date: Tue, 18 Aug 2020 05:42:48 +0300 Subject: [PATCH] fix https://github.com/kataras/iris/issues/1594 --- HISTORY.md | 4 ++- core/router/api_builder.go | 9 +++++++ core/router/handler.go | 27 ++++++++++++++------ iris.go | 6 ++--- middleware/logger/config.go | 29 +++++++++++++-------- middleware/logger/logger.go | 50 +++++++++++++++++++++---------------- 6 files changed, 83 insertions(+), 42 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index e653e25b..1dba7d51 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -359,6 +359,8 @@ Response: Other Improvements: +- Fix [#1594](https://github.com/kataras/iris/issues/1594) and add a new `PathAfterHandler` which can be set to true to enable the old behavior (not recommended though). + - New [apps](https://github.com/kataras/iris/tree/master/apps) subpackage. [Example of usage](https://github.com/kataras/iris/tree/master/_examples/routing/subdomains/redirect/multi-instances). ![apps image example](https://user-images.githubusercontent.com/22900943/90459288-8a54f400-e109-11ea-8dea-20631975c9fc.png) @@ -403,7 +405,7 @@ func main() { - `Context.OnCloseErr` and `Context.OnConnectionCloseErr` - to call a function of `func() error` instead of an `iris.Handler` when request is closed or manually canceled. -- `Party.UseError(...Handler)` - to register handlers to run before the `OnErrorCode/OnAnyErrorCode` ones. +- `Party.UseError(...Handler)` - to register handlers to run before any http errors (e.g. before `OnErrorCode/OnAnyErrorCode` or default error codes when no handler is responsible to handle a specific http status code). - `Party.UseRouter(...Handler)` - to register handlers before the main router, useful on handlers that should control whether the router itself should ran or not. Independently of the incoming request's method and path values. These handlers will be executed ALWAYS against ALL incoming matched requests. Example of use-case: CORS. diff --git a/core/router/api_builder.go b/core/router/api_builder.go index 53d72d4b..ef2678a8 100644 --- a/core/router/api_builder.go +++ b/core/router/api_builder.go @@ -952,6 +952,15 @@ func (api *APIBuilder) UseRouter(handlers ...context.Handler) { } } +// GetDefaultErrorMiddleware returns the application's error pre handlers +// registered through `UseError` for the default error handlers. +// This is used when no matching error handlers registered +// for a specific status code but `UseError` is called to register a middleware, +// so the default error handler should make use of those middleware now. +func (api *APIBuilder) GetDefaultErrorMiddleware() context.Handlers { + return api.middlewareErrorCode +} + // UseError upserts one or more handlers that will be fired, // as middleware, before any error handler registered through `On(Any)ErrorCode`. // See `OnErrorCode` too. diff --git a/core/router/handler.go b/core/router/handler.go index 8532b2dc..1c90f974 100644 --- a/core/router/handler.go +++ b/core/router/handler.go @@ -48,8 +48,9 @@ type routerHandler struct { trees []*trie errorTrees []*trie - hosts bool // true if at least one route contains a Subdomain. - errorHosts bool // true if error handlers are registered to at least one Subdomain. + hosts bool // true if at least one route contains a Subdomain. + errorHosts bool // true if error handlers are registered to at least one Subdomain. + errorDefaultHandlers context.Handlers // the main handler(s) for default error code handlers, when not registered directly by the end-developer. } var _ RequestHandler = (*routerHandler)(nil) @@ -122,10 +123,26 @@ type RoutesProvider interface { // api builder // Read `UseRouter` for more. // The map can be altered before router built. GetRouterFilters() map[Party]*Filter + // GetDefaultErrorMiddleware should return + // the default error handler middleares. + GetDefaultErrorMiddleware() context.Handlers +} + +func defaultErrorHandler(ctx *context.Context) { + if err := ctx.GetErr(); err != nil { + ctx.WriteString(err.Error()) + } else { + ctx.WriteString(context.StatusText(ctx.GetStatusCode())) + } } func (h *routerHandler) Build(provider RoutesProvider) error { h.trees = h.trees[0:0] // reset, inneed when rebuilding. + + // set the default error code handler, will be fired on error codes + // that are not handled by a specific handler (On(Any)ErrorCode). + h.errorDefaultHandlers = append(provider.GetDefaultErrorMiddleware(), defaultErrorHandler) + rp := errgroup.New("Routes Builder") registeredRoutes := provider.GetRoutes() @@ -560,11 +577,7 @@ func (h *routerHandler) FireErrorCode(ctx *context.Context) { // not error handler found, // see if failed with a stored error, and if so // then render it, otherwise write a default message. - if err := ctx.GetErr(); err != nil { - ctx.WriteString(err.Error()) - } else { - ctx.WriteString(context.StatusText(statusCode)) - } + ctx.Do(h.errorDefaultHandlers) } func (h *routerHandler) subdomainAndPathAndMethodExists(ctx *context.Context, t *trie, method, path string) bool { diff --git a/iris.go b/iris.go index b99efb95..b66b6fdf 100644 --- a/iris.go +++ b/iris.go @@ -134,9 +134,9 @@ func New() *Application { // The return instance recovers on panics and logs the incoming http requests too. func Default() *Application { app := New() - app.Use(recover.New()) - app.Use(requestLogger.New()) - app.Use(Compression) + app.UseRouter(recover.New()) + app.UseRouter(requestLogger.New()) + app.UseRouter(Compression) app.defaultMode = true diff --git a/middleware/logger/config.go b/middleware/logger/config.go index cf6ef601..6b8d2901 100644 --- a/middleware/logger/config.go +++ b/middleware/logger/config.go @@ -26,9 +26,17 @@ type Config struct { // Defaults to true. Method bool // Path displays the request path (bool). + // See `Query` and `PathAfterHandler` too. // // Defaults to true. Path bool + // PathAfterHandler displays the request path + // which may be set and modified + // after the handler chain is executed. + // See `Query` too. + // + // Defaults to false. + PathAfterHandler bool // Query will append the URL Query to the Path. // Path should be true too. @@ -86,16 +94,17 @@ type Config struct { // LogFunc and Skippers to nil as well. func DefaultConfig() Config { return Config{ - Status: true, - IP: true, - Method: true, - Path: true, - Query: false, - Columns: false, - LogFunc: nil, - LogFuncCtx: nil, - Skippers: nil, - skip: nil, + Status: true, + IP: true, + Method: true, + Path: true, + PathAfterHandler: false, + Query: false, + Columns: false, + LogFunc: nil, + LogFuncCtx: nil, + Skippers: nil, + skip: nil, } } diff --git a/middleware/logger/logger.go b/middleware/logger/logger.go index af61b639..4469bfd4 100644 --- a/middleware/logger/logger.go +++ b/middleware/logger/logger.go @@ -24,6 +24,7 @@ type requestLoggerMiddleware struct { // This is for the http requests. // // Receives an optional configuation. +// Usage: app.UseRouter(logger.New()). func New(cfg ...Config) context.Handler { c := DefaultConfig() if len(cfg) > 0 { @@ -35,6 +36,13 @@ func New(cfg ...Config) context.Handler { return l.ServeHTTP } +func (l *requestLoggerMiddleware) getPath(ctx *context.Context) string { + if l.config.Query { + return ctx.Request().URL.RequestURI() + } + return ctx.Path() +} + // Serve serves the middleware func (l *requestLoggerMiddleware) ServeHTTP(ctx *context.Context) { // skip logs and serve the main request immediately @@ -51,16 +59,7 @@ func (l *requestLoggerMiddleware) ServeHTTP(ctx *context.Context) { var startTime, endTime time.Time startTime = time.Now() - ctx.Next() - - // no time.Since in order to format it well after - endTime = time.Now() - latency = endTime.Sub(startTime) - - if l.config.Status { - status = strconv.Itoa(ctx.GetStatusCode()) - } - + // Before Next. if l.config.IP { ip = ctx.RemoteAddr() } @@ -70,11 +69,21 @@ func (l *requestLoggerMiddleware) ServeHTTP(ctx *context.Context) { } if l.config.Path { - if l.config.Query { - path = ctx.Request().URL.RequestURI() - } else { - path = ctx.Path() - } + path = l.getPath(ctx) + } + + ctx.Next() + + // no time.Since in order to format it well after + endTime = time.Now() + latency = endTime.Sub(startTime) + + if l.config.PathAfterHandler /* we don't care if Path is disabled */ { + path = l.getPath(ctx) + } + + if l.config.Status { + status = strconv.Itoa(ctx.GetStatusCode()) } var message interface{} @@ -125,12 +134,11 @@ func (l *requestLoggerMiddleware) ServeHTTP(ctx *context.Context) { line += fmt.Sprintf(" %v", headerMessage) } - // if context.StatusCodeNotSuccessful(ctx.GetStatusCode()) { - // ctx.Application().Logger().Warn(line) - // } else { - ctx.Application().Logger().Info(line) - // } - + if context.StatusCodeNotSuccessful(ctx.GetStatusCode()) { + ctx.Application().Logger().Warn(line) + } else { + ctx.Application().Logger().Info(line) + } } // Columnize formats the given arguments as columns and returns the formatted output,