laurentS/slowapi

Wrong parsing of limit_value when having more than one decorator

Closed this issue · 1 comments

Describe the bug
If only one decorator is used, the limitation works well, but if two decorators are used, the parsing is wrong!

To Reproduce
From the documentation :

from fastapi import FastAPI
from slowapi import Limiter, _rate_limit_exceeded_handler
from slowapi.util import get_remote_address
from slowapi.errors import RateLimitExceeded

limiter = Limiter(key_func=get_remote_address)
app = FastAPI()
app.state.limiter = limiter
app.add_exception_handler(RateLimitExceeded, _rate_limit_exceeded_handler)

# Note: the route decorator must be above the limit decorator, not below it
@app.get("/home")
@limiter.limit("5/minute")
async def homepage(request: Request):
    return PlainTextResponse("test")

@app.get("/mars")
@limiter.limit("5/minute")
async def homepage(request: Request, response: Response):
    return {"key": "value"}

I have this weird behavior:

The accepted requests are not 5 requests per minute, but 2!

INFO:     Shutting down
INFO:     Waiting for application shutdown.
INFO:     Application shutdown complete.
INFO:     Finished server process [16844]
INFO:     Started server process [14380]
INFO:     Waiting for application startup.
INFO:     Application startup complete.
INFO:     127.0.0.1:7786 - "GET /home HTTP/1.1" 200 OK
INFO:     127.0.0.1:7786 - "GET /home HTTP/1.1" 200 OK
INFO:     127.0.0.1:7786 - "GET /home HTTP/1.1" 429 Too Many Requests

And then if I comment out the /mars endpoint for example, I got:

INFO:     Shutting down
INFO:     Waiting for application shutdown.
INFO:     Application shutdown complete.
INFO:     Finished server process [14380]
INFO:     Started server process [27128]
INFO:     Waiting for application startup.
INFO:     Application startup complete.
INFO:     127.0.0.1:8104 - "GET /home HTTP/1.1" 200 OK
INFO:     127.0.0.1:8104 - "GET /home HTTP/1.1" 200 OK
INFO:     127.0.0.1:8104 - "GET /home HTTP/1.1" 200 OK
INFO:     127.0.0.1:8104 - "GET /home HTTP/1.1" 200 OK
INFO:     127.0.0.1:8104 - "GET /home HTTP/1.1" 200 OK
INFO:     127.0.0.1:8104 - "GET /home HTTP/1.1" 429 Too Many Requests

Expected behavior
I expected that parsing limit_value "5/minute" should work as expected 5 requests per minute

Screenshots
image

Your app (please complete the following information):

  • FastAPI=0.109.0
  • slowapi version=0.1.9

I found out that this is because the same function name for the endpoint "/mars" in the document.

@app.get("/home")
@limiter.limit("5/minute")
async def homepage(request: Request):
    return PlainTextResponse("test")

@app.get("/mars")
@limiter.limit("5/minute")
async def homepage2(request: Request, response: Response):
    return {"key": "value"}

The problem is solved by changing the function name of the end point "/mars" to a name other than "homepage".

I can see that in https://github.com/laurentS/slowapi/blob/master/docs/index.md#fastapi, the code are correct. But in https://slowapi.readthedocs.io/en/latest/ it is wrong.