-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Description
Let me just start with saying, that this is not something that was observed during normal Caddy operation. It's a corner case that arose because I'm using caddyhttp.Route
from another Caddy module.
The TL;DR of the issue is that because the middleware
field in caddyhttp.Route
is appended to directly here, then if we ever call ProvisionHandlers
more than once, the pre-compiled handlers will be added to the middleware
field again, instead of reflecting the current Handlers
.
For a more convoluted example, and how I discovered it. What I do is, I have a Caddy module that embeds the caddyhttp.Route
struct and I call Provision
on that embedding to trigger all the JSON magic that creates the Route
object. A little later I look at the Handlers
field, and in some cases I add a Rewrite.Rewrite
handler in front. Then I call ProvisionHandlers
, followed by Compile
. Which I expected would have worked, but because of the pre-compile thing, the Rewrite
handler is not at the top, and never hit because the last Handler
is terminating.
There is an easy fix though, which restores the expected behaviour without affecting Caddy's current behaviour:
diff --git a/modules/caddyhttp/routes.go b/modules/caddyhttp/routes.go
index 6f237149..35e63a55 100644
--- a/modules/caddyhttp/routes.go
+++ b/modules/caddyhttp/routes.go
@@ -159,10 +159,17 @@ func (r *Route) ProvisionHandlers(ctx caddy.Context, metrics *Metrics) error {
r.Handlers = append(r.Handlers, handler.(MiddlewareHandler))
}
+ // Create a new middleware slice so we can call ProvisionHandlers multiple times
+ newMiddleware := []Middleware{}
+
// pre-compile the middleware handler chain
for _, midhandler := range r.Handlers {
- r.middleware = append(r.middleware, wrapMiddleware(ctx, midhandler, metrics))
+ newMiddleware = append(newMiddleware, wrapMiddleware(ctx, midhandler, metrics))
}
+
+ // Assign the new middleware slice to the route
+ r.middleware = newMiddleware
+
return nil
}
I would be more than happy to raise a PR with that small change. I just want to make sure that I haven't misunderstood what is going on.