Skip to content

🎯 Fix URL Parameter Loss When Request.URL.Path is Rewritten by Middleware #1

Description

@madalynerlge2

📝 Description

When a middleware modifies r.URL.Path (for example, to perform path rewriting, strip prefixes, or normalize URLs) before routing to a sub-router or a handler, the URL parameters parsed by chi are lost. Consequently, calling chi.URLParam(r, "key") returns an empty string.

This happens because chi caches the routing path in the routing context (chi.RouteContext(r.Context()).RoutePath) during the initial routing phase. If a middleware updates r.URL.Path but the routing context's internal RoutePath is not updated or synchronized, subsequent routing decisions and parameter extractions are performed against stale path data, leading to mismatched or empty URL parameters.

🎯 Acceptance Criteria

  • When r.URL.Path is mutated by a middleware, subsequent routing steps (such as sub-routers or inline handlers) must correctly match patterns against the updated path.
  • chi.URLParam(r, "paramName") must successfully return the correct parameter values extracted from the rewritten path.
  • The fix must support nested routing and wildcards (e.g., /*, /users/{id}) after a path rewrite.
  • The solution must not break backward compatibility for existing middlewares that do not modify r.URL.Path.
  • No performance regressions in the hot path of the router's multiplexer (Mux.ServeHTTP).

🛠️ Technical Specifications & Context

The issue lies in how chi tracks the current routing path within its context object.

Key Files & Structs:

  • context.go: Contains the Context struct which holds RoutePath, RouteMethod, and URLParams.
  • mux.go: Contains the routing logic, specifically Mux.ServeHTTP and Mux.routeHTTP.

Root Cause & Suggested Fix:

  1. chi initializes a RouteContext and sets rctx.RoutePath = r.URL.Path (or r.URL.RawPath if configured) at the start of routing.
  2. If a middleware rewrites r.URL.Path and calls next.ServeHTTP(w, r), the RouteContext still contains the original RoutePath.
  3. When the sub-router or next handler attempts to match the route or extract parameters, it references rctx.RoutePath instead of the updated r.URL.Path.

Proposed Solution:
In mux.go (specifically where sub-routers are dispatched or inside the routing loop), detect if r.URL.Path has diverged from rctx.RoutePath. If it has, update rctx.RoutePath to match the new r.URL.Path before proceeding with matching and parameter extraction.

Alternatively, ensure that RouteContext provides a mechanism to resync or automatically detect path updates during routing transitions.

// Example check to insert in routing/sub-routing logic:
if rctx.RoutePath != r.URL.Path {
    rctx.RoutePath = r.URL.Path
}

🧪 Verification & Testing

To verify the fix, add a test case in mux_test.go (or a dedicated test file):

  1. Test Case Setup:

    • Create a main router with a middleware that rewrites the path (e.g., rewrites /old/{id} to /new/{id}).
    • Mount a sub-router or define a route for /new/{id}.
    • In the handler for /new/{id}, assert that chi.URLParam(r, "id") returns the correct value.
  2. Sample Test Structure:

    func TestMiddlewarePathRewriteURLParams(t *testing.T) {
        r := chi.NewRouter()
        
        // Middleware that rewrites the path
        r.Use(func(next http.Handler) http.Handler {
            return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
                if req.URL.Path == "/legacy/123" {
                    req.URL.Path = "/users/123"
                }
                next.ServeHTTP(w, req)
            })
        })
    
        r.Get("/users/{id}", func(w http.ResponseWriter, req *http.Request) {
            id := chi.URLParam(req, "id")
            if id != "123" {
                t.Errorf("expected URL param 'id' to be '123', got '%s'", id)
            }
            w.Write([]byte("ok"))
        })
    
        ts := httptest.NewServer(r)
        defer ts.Close()
    
        res, err := http.Get(ts.URL + "/legacy/123")
        if err != nil {
            t.Fatal(err)
        }
        if res.StatusCode != http.StatusOK {
            t.Errorf("expected status 200, got %d", res.StatusCode)
        }
    }

Opire Bounty


This repo is using Opire - what does it mean? 👇
💵 Everyone can add rewards for this issue commenting /reward 100 (replace 100 with the amount).
🕵️‍♂️ If someone starts working on this issue to earn the rewards, they can comment /try to let everyone know!
🙌 And when they open the PR, they can comment /claim #1 either in the PR description or in a PR's comment.

🪙 Also, everyone can tip any user commenting /tip 20 @madalynerlge2 (replace 20 with the amount, and @madalynerlge2 with the user to tip).

📖 If you want to learn more, check out our documentation.

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions