From 64f0173948e9b87a8f898d451ef35daefedf2c50 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Sun, 12 Jan 2020 13:39:32 -0700 Subject: [PATCH] http: Fix subroutes, ensure that next handlers can still be called --- modules/caddyhttp/caddyhttp.go | 6 +++--- modules/caddyhttp/routes.go | 5 +++-- modules/caddyhttp/subroute.go | 6 +++--- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/modules/caddyhttp/caddyhttp.go b/modules/caddyhttp/caddyhttp.go index 389a2d2b..73c48635 100644 --- a/modules/caddyhttp/caddyhttp.go +++ b/modules/caddyhttp/caddyhttp.go @@ -172,7 +172,7 @@ func (app *App) Provision(ctx caddy.Context) error { } // pre-compile the handler chain, and be sure to wrap it in our // route handler so that important security checks are done, etc. - primaryRoute = srv.Routes.Compile() + primaryRoute = srv.Routes.Compile(emptyHandler) } srv.primaryHandlerChain = srv.wrapPrimaryRoute(primaryRoute) @@ -181,7 +181,7 @@ func (app *App) Provision(ctx caddy.Context) error { if err != nil { return fmt.Errorf("server %s: setting up server error handling routes: %v", srvName, err) } - srv.errorHandlerChain = srv.Errors.Routes.Compile() + srv.errorHandlerChain = srv.Errors.Routes.Compile(emptyHandler) } } @@ -546,7 +546,7 @@ func (app *App) automaticHTTPS() error { tlsApp: tlsApp, // required to solve HTTP challenge logger: app.logger.Named("log"), errorLogger: app.logger.Named("log.error"), - primaryHandlerChain: redirRoutes.Compile(), + primaryHandlerChain: redirRoutes.Compile(emptyHandler), } } } diff --git a/modules/caddyhttp/routes.go b/modules/caddyhttp/routes.go index 4ae9bcd0..431d1a5e 100644 --- a/modules/caddyhttp/routes.go +++ b/modules/caddyhttp/routes.go @@ -147,12 +147,12 @@ func (routes RouteList) Provision(ctx caddy.Context) error { // Compile prepares a middleware chain from the route list. // This should only be done once: after all the routes have // been provisioned, and before serving requests. -func (routes RouteList) Compile() Handler { +func (routes RouteList) Compile(next Handler) Handler { var mid []Middleware for _, route := range routes { mid = append(mid, wrapRoute(route)) } - stack := emptyHandler + stack := next for i := len(mid) - 1; i >= 0; i-- { stack = mid[i](stack) } @@ -167,6 +167,7 @@ func (routes RouteList) Compile() Handler { func wrapRoute(route Route) Middleware { return func(next Handler) Handler { return HandlerFunc(func(rw http.ResponseWriter, req *http.Request) error { + // TODO: Update this comment, it seems we've moved the copy into the handler? // copy the next handler (it's an interface, so it's just // a very lightweight copy of a pointer); this is important // because this is a closure to the func below, which diff --git a/modules/caddyhttp/subroute.go b/modules/caddyhttp/subroute.go index a3248738..2e80d88d 100644 --- a/modules/caddyhttp/subroute.go +++ b/modules/caddyhttp/subroute.go @@ -69,12 +69,12 @@ func (sr *Subroute) Provision(ctx caddy.Context) error { return nil } -func (sr *Subroute) ServeHTTP(w http.ResponseWriter, r *http.Request, _ Handler) error { - subroute := sr.Routes.Compile() +func (sr *Subroute) ServeHTTP(w http.ResponseWriter, r *http.Request, next Handler) error { + subroute := sr.Routes.Compile(next) err := subroute.ServeHTTP(w, r) if err != nil && sr.Errors != nil { r = sr.Errors.WithError(r, err) - errRoute := sr.Errors.Routes.Compile() + errRoute := sr.Errors.Routes.Compile(next) return errRoute.ServeHTTP(w, r) } return err