🔥 feat: add MatchedRoute for middleware look-ahead - #4601
Conversation
Expose the next non-middleware matching route without advancing the handler chain so global middleware can inspect Path/Name for RBAC. Closes gofiber#2195
Walkthrough
ChangesMatchedRoute API
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ctx.go`:
- Around line 405-409: Clear c.matchedRoute in DefaultReq.Method() whenever
changing c.methodInt, alongside resetting firstMatchIndex. Add a regression test
that obtains MatchedRoute(), overrides the method through Request().Method(...),
then verifies the subsequent MatchedRoute() does not return the previously
cached route.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8abea0f3-6eb5-4de2-9435-179b07dee224
📒 Files selected for processing (4)
ctx.goctx_interface_gen.goctx_test.godocs/api/ctx.md
| if c.matchedRoute != nil { | ||
| return c.matchedRoute | ||
| } | ||
| if c.methodInt == -1 || c.app == nil { | ||
| return nil |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect method mutation and existing cache invalidation.
rg -n -A45 -B4 'func \(c \*DefaultCtx\) Method\(' ctx.go
rg -n -C4 'matchedRoute\s*=|firstMatchIndex\s*=' ctx.go
# Find existing MatchedRoute coverage to extend.
rg -n -C5 'MatchedRoute\(' ctx_test.goRepository: gofiber/fiber
Length of output: 151
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate relevant files without relying on exact repository root layout.
printf 'Git root: '; git rev-parse --show-toplevel 2>/dev/null || pwd
printf 'ctx.go candidates: '; fd -a '^ctx\.go$' . 2>/dev/null || find . -name ctx.go -type f
printf '\nSearch DefaultCtx.Method and MatchedRoute symbols:\n'
rg -n -C5 'func \(c \*DefaultCtx\) Method\(' . || true
rg -n -C4 'func \(c \*DefaultCtx\) MatchedRoute' . || true
rg -n -C3 'matchedRoute\s*=|firstMatchIndex\s*=' . || true
rg -n -C5 'MatchedRoute\(' . || trueRepository: gofiber/fiber
Length of output: 8640
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Relevant ctx.go sections:\n'
sed -n '394,445p' ctx.go
sed -n '260,330p' ctx.go
printf '\nReq.go Method implementation:\n'
sed -n '910,955p' req.go
printf '\nContext reset methods around Reset:\n'
sed -n '760,815p' ctx.go
printf '\nInspect request method override and MethodInt behavior:\n'
rg -n -C4 'func \(c \*DefaultCtx\) MethodInt\(\)|func UpdateRequestMethod|Request.*Method|MethodInt|HeaderRequestMethod|c\.methodInt|c\.Req.*Header|SetMethod\(' req.go ctx.go router.goRepository: gofiber/fiber
Length of output: 17239
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Route matching helpers and firstMatchIndex write:\n'
sed -n '720,770p' router.go
rg -n -C6 'firstMatchIndex = ' router.go
printf '\nRead-only behavioral model for cached MatchedRoute after method override:\n'
python3 - <<'PY'
import copy
class Tree:
def __init__(self, routes_by_path):
self.routes_by_path = routes_by_path
class App:
def __init__(self):
self.treeIndex = {
0: Tree({"same": "GET_route"}),
1: Tree({"same": "POST_route"}),
}
class Ctx:
def __init__(self):
self.app = App()
self.treePathHash = 0
self.methodInt = 0
self.firstMatchIndex = 0
self.matchedRoute = "GET_route"
def matcheRoute(ctx):
if ctx.matchedRoute is not None:
return ctx.matchedRoute
if ctx.methodInt == -1 or ctx.app is None:
return None
return ctx.app.treeIndex[ctx.methodInt].routes_by_path["same"]
def method_override(ctx, method_int):
ctx.methodInt = method_int
ctx.firstMatchIndex = -1
ctx = Ctx()
before = matcheRoute(ctx)
first_after_method = ctx.firstMatchIndex
method_override(ctx, 1)
second = matcheRoute(ctx)
lookup_after_method = ctx.firstMatchIndex
print({"before": before, "firstMatchIndex_after_method": first_after_method, "after_method": second, "firstMatchIndex_after_second_call": lookup_after_method})
PYRepository: gofiber/fiber
Length of output: 2381
Invalidate matchedRoute when DefaultReq.Method changes methodInt.
MatchedRoute() returns c.matchedRoute before inspecting c.methodInt. DefaultReq.Method() updates c.methodInt and clears firstMatchIndex, but it keeps the old c.matchedRoute cached. Middleware that calls c.Request().Method("POST") can return the previous GET matched route on the first MatchedRoute() call after the override. Clear c.matchedRoute in DefaultReq.Method() and add a regression test covering MatchedRoute(), Request().Method(...), and another MatchedRoute() call.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ctx.go` around lines 405 - 409, Clear c.matchedRoute in DefaultReq.Method()
whenever changing c.methodInt, alongside resetting firstMatchIndex. Add a
regression test that obtains MatchedRoute(), overrides the method through
Request().Method(...), then verifies the subsequent MatchedRoute() does not
return the previously cached route.
There was a problem hiding this comment.
Pull request overview
Adds a new Ctx API (MatchedRoute()) to let middleware inspect the next matching non-middleware endpoint route (path/name) before calling Next(), addressing the common “route name is empty in pre-handler middleware” use case (RBAC, logging, etc.).
Changes:
- Introduces
MatchedRoute()onCtx/DefaultCtxwith look-ahead logic over the route tree, including a fast path whenSkipUnmatchedRoutespre-resolves an endpoint index. - Adds caching + invalidation hooks for path normalization / rewrites to avoid recomputing the look-ahead.
- Adds tests for middleware usage, not-found behavior, and
SkipUnmatchedRoutes, plus API docs updates.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| docs/api/ctx.md | Documents MatchedRoute() and clarifies Route() semantics in middleware. |
| ctx.go | Implements DefaultCtx.MatchedRoute() and caching/invalidation behavior. |
| ctx_test.go | Adds coverage for middleware look-ahead, not-found, and SkipUnmatchedRoutes behavior. |
| ctx_interface_gen.go | Extends the generated Ctx interface to include MatchedRoute(). |
Files not reviewed (1)
- ctx_interface_gen.go: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // Already on a non-middleware endpoint. | ||
| if c.route != nil && !c.route.use && !c.route.mount { | ||
| return c.route | ||
| } | ||
| if c.matchedRoute != nil { | ||
| return c.matchedRoute | ||
| } | ||
| if c.methodInt == -1 || c.app == nil { | ||
| return nil | ||
| } |
| tree := c.app.treeIndex[c.methodInt].lookup(c.treePathHash) | ||
| detectionPath := utils.UnsafeString(c.detectionPath) | ||
| path := utils.UnsafeString(c.path) | ||
| pathSlashes := c.pathSlashCount(c.app) | ||
| // Use a scratch params buffer so look-ahead does not clobber c.values. | ||
| var scratch [maxParams]string | ||
|
|
||
| for i := c.indexRoute + 1; i < len(tree); i++ { | ||
| route := tree[i] | ||
| if route.mount || route.use { | ||
| continue | ||
| } | ||
| if route.prefixRejects(pathHeadWord(detectionPath)) { | ||
| continue | ||
| } |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4601 +/- ##
==========================================
- Coverage 93.47% 93.44% -0.04%
==========================================
Files 140 140
Lines 14983 15017 +34
==========================================
+ Hits 14006 14032 +26
- Misses 608 612 +4
- Partials 369 373 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@copilot Fix the code for all comments in this review thread. When a review comment includes a suggested change, apply the suggestion exactly. Do not make changes beyond what is described in the linked review thread. |
|
@RubenPari Thank you for your contributions. However, in the future, please refrain from submitting multiple pull requests (you have 10 open right now) simultaneously. Reviewing each request requires a significant amount of time from our team. |
Summary
Adds
c.MatchedRoute()so global middleware can inspect the next non-middleware matching route (path/name) beforec.Next(), without advancing the handler chain.This unblocks RBAC and similar middleware patterns that currently see an empty
c.Route().Namebefore the endpoint runs (#2195). Builds on the approach sketched in theadd-MatchedRoute-method-to-contextbranch, adapted to the currenttreeIndexrouter andSkipUnmatchedRouteslook-ahead.Changes
MatchedRoute()onCtx/DefaultCtx(cached, path-rewrite invalidation)c.valuesfirstMatchIndexis setRoute/MatchedRouteLinked issue
Closes #2195
Checklist
make generatemake formatmake lint— 0 issues