Fix included router rate limitingfix: handle included routers in default rate limiting

[15054538509]

Fix default rate limiting for FastAPI included routers.

Root cause: FastAPI 0.137+ stores include_router() entries as _IncludedRouter proxies, so app.routes no longer exposes a direct .endpoint for nested routes. slowapi was treating those routes as unmatched and skipping default limits.

Changes:

• recurse through nested router trees when resolving the handler
• add a FastAPI regression test for an included router with default_limits

Validation:

• pytest tests/test_fastapi_extension.py -q
• 60 passed

[tsukumijima]

@15054538509
Thanks for working on this. I tested the PR commit (63cb749d) against FastAPI 0.137.0 / Starlette 1.3.1, and the fix is incomplete for router prefixes applied by include_router() and for nested included routers.

The added test uses APIRouter(prefix="/api"). In that case the original APIRoute.path already contains /api, so walking original_router.routes happens to work. These common variants still bypass the default limit with both SlowAPIMiddleware and SlowAPIASGIMiddleware:

limiter = Limiter(key_func=lambda request: request.client.host, default_limits=["1/minute"])
app = FastAPI()
app.state.limiter = limiter
app.add_exception_handler(RateLimitExceeded, _rate_limit_exceeded_handler)
app.add_middleware(SlowAPIMiddleware)  # same result with SlowAPIASGIMiddleware

router = APIRouter()

@router.get("/t1")
async def endpoint(request: Request):
    return {"ok": True}

app.include_router(router, prefix="/api")

with TestClient(app) as client:
    assert [client.get("/api/t1").status_code for _ in range(2)] == [200, 429]

Actual result on this PR: [200, 200].

A two-level include_router() tree also produces [200, 200]:

inner = APIRouter(prefix="/inner")
# add GET /t1 to inner
outer = APIRouter(prefix="/outer")
outer.include_router(inner)
app.include_router(outer, prefix="/api")
# GET /api/outer/inner/t1 twice -> [200, 200]

The reason is that FastAPI 0.137's _IncludedRouter.matches() returns Match.FULL with an empty child scope, while the combined include prefixes live in effective_candidates(). Recursing through original_router.routes discards the include context, so the child routes are matched against their uncombined paths.

A minimal change that I verified locally is to prefer the effective candidates before falling back to the existing generic route/original-router lookup:

def _get_nested_routes(route: BaseRoute) -> Optional[Iterable[BaseRoute]]:
    effective_candidates = getattr(route, "effective_candidates", None)
    if callable(effective_candidates):
        return effective_candidates()

    nested_routes = getattr(route, "routes", None)
    if nested_routes is not None:
        return nested_routes

    original_router = getattr(route, "original_router", None)
    return getattr(original_router, "routes", None)

With that change, I got [200, 429] for all of these cases with both middleware implementations:

• prefix declared on APIRouter
• prefix passed to app.include_router()
• nested included routers with combined prefixes

There is also a test-environment gap: this repository's current poetry.lock resolves FastAPI 0.89.1 and Starlette 0.22.0. I ran the new test against the parent commit in that locked environment and it already passed ([200, 429]), so it does not prove the FastAPI 0.137 regression is fixed unless CI explicitly installs FastAPI 0.137+ (or the lock file is updated). I suggest changing the regression test to apply the prefix through include_router(), adding the nested case, and ensuring at least one CI job runs them on FastAPI 0.137+.

[Mukller]

Triage note for the included-router cluster (#285, #286, #282): I verified all three functionally on FastAPI 0.141.1 with a uniform probe (default_limits only, SlowAPIMiddleware, decorator-free endpoint — the path where only _find_route_handler can enforce):

app.include_router(sub)            -> second request status
app.include_router(sub, prefix="/sub")

master : 200,200 BYPASSED   | 200,200 BYPASSED     (bug confirmed)
#285   : 200,429 ENFORCED   | 200,429 ENFORCED
#286   : 200,429 ENFORCED   | 200,429 ENFORCED
#282   : 200,429 ENFORCED   | 200,200 BYPASSED    <- THIS PR: prefixed case still leaks

Repro of the remaining gap on this branch — the prefixed include still bypasses default limits:

limiter = Limiter(key_func=get_remote_address, default_limits=["1/minute"])
sub = APIRouter()

@sub.get("/inner")
def inner(): ...

app.add_middleware(SlowAPIMiddleware)
app.include_router(sub, prefix="/sub")   # <- with prefix
# GET /sub/inner x2 -> 200, 200  (expected 200, 429)

Likely cause: on 0.141 the lazy wrapper is _IncludedRouter, which has no .routes attribute; your _get_nested_routes falls back to original_router.routes, but for a prefixed include the nested routes' paths don't carry the prefix, so matches(scope) against the un-prefixed child scope returns NONE and recursion dies there. #285/#286 avoid it by recursing through effective_route_contexts()/_effective_candidates(), which carry per-context state.

Also worth noting: merging child_scope into the nested scope (which this PR does and others don't) is a good idea in principle — the winning candidates just need to reach the right level first.

Everything else about this PR (suite passes locally: 60 passed) is fine; it's only the prefixed-lazy case that needs one more hop.

[15054538509]

@tsukumijima Thanks for the detailed review and for pinpointing the _IncludedRouter behaviour. I have pushed 929a11e addressing all three points:

• _get_nested_routes() now prefers effective_candidates() (your suggested minimal change) before falling back to .routes/original_router.routes, so child routes are matched against their effective (prefixed) paths.
• The regression tests now cover the prefix passed to include_router(prefix=...) and a two-level nested include_router() tree, in addition to the APIRouter(prefix=...) case. Both new cases returned [200, 200] before the change and [200, 429] after it.
• Added a test-latest-fastapi CI job that installs fastapi>=0.137 and runs tests/test_fastapi_extension.py, so the regression tests actually exercise the lazy include path in CI.

Local verification:

• FastAPI 0.137.0 and 0.143.0: all three cases give [200, 429] with both SlowAPIMiddleware and SlowAPIASGIMiddleware, and tests/test_fastapi_extension.py passes.
• Locked environment (FastAPI 0.89.1 / Starlette 0.22.0): full test suite passes, so behaviour is unchanged for FastAPI < 0.137.