Skip to content

Unexpected behaviour if caddyhttp.Route is provisioned twice #6551

@jesper-chainalysis

Description

@jesper-chainalysis

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bug 🐞Something isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions