Starlette: Consume request body in middleware is problematic

Created on 30 Apr 2019  路  11Comments  路  Source: encode/starlette

from starlette.applications import Starlette
from starlette.requests import Request
from starlette.responses import PlainTextResponse
from starlette.middleware.base import BaseHTTPMiddleware


class SampleMiddleware(BaseHTTPMiddleware):
    async def dispatch(self, request: Request, call_next):
        _ = await request.form()
        return await call_next(request)


app = Starlette()


@app.route('/test', methods=['POST'])
async def test(request):
    _ = await request.form()  # blocked, because middleware already consumed request body
    return PlainTextResponse('Hello, world!')


app.add_middleware(SampleMiddleware)
$ uvicorn test:app --reload
$ curl -d "a=1" http://127.0.0.1:8000/test
# request is blocked

Most helpful comment

Coming from FastAPI issue referenced above.

@tomchristie I don't understand the issue about consuming the request in a middleware, could you explain this point ? In fact, I have the need (which is current where I work) to log every requests received by my production server. Is there a better place than a middleware to do it and avoid duplicating code in every endpoint ? (I would like something like this https://github.com/Rhumbix/django-request-logging)

For now, I found this workaround (but that's not very pretty):

async def set_body(request: Request, body: bytes):
    async def receive() -> Message:
        return {"type": "http.request", "body": body}

    request._receive = receive

async def get_body(request: Request) -> bytes:
    body = await request.body()
    set_body(request, body)
    return body

but there will always be cases where it can't work (eg. if you stream the request data, then it's just not going to be available anymore later down the line)

I kind of disagree with your example. In fact, stream data is not stored by default, but stream metadata (is the stream closed) are; there will be an understandable error raised if someone try to stream twice, and that is enough imho. That's why if the body is cached, the stream consumption has to be cached too.

All 11 comments

Where is it blocking?

Is there a second request instance involved? (otherwise it should just return, shouldn't it?)
https://github.com/encode/starlette/blob/7eb43757307d4702ee6a1f2739388242c703e47e/starlette/requests.py#L178-L193

"Consume request body in middleware is problematic"

Indeed. Consuming request data in middleware is problematic.
Not just to Starlette, but generally, everywhere.

On the whole you should avoid doing so if at all possible.

There's some work we could do to make it work better, but there will always be cases where it can't work (eg. if you stream the request data, then it's just not going to be available anymore later down the line).

Coming from FastAPI issue referenced above.

@tomchristie I don't understand the issue about consuming the request in a middleware, could you explain this point ? In fact, I have the need (which is current where I work) to log every requests received by my production server. Is there a better place than a middleware to do it and avoid duplicating code in every endpoint ? (I would like something like this https://github.com/Rhumbix/django-request-logging)

For now, I found this workaround (but that's not very pretty):

async def set_body(request: Request, body: bytes):
    async def receive() -> Message:
        return {"type": "http.request", "body": body}

    request._receive = receive

async def get_body(request: Request) -> bytes:
    body = await request.body()
    set_body(request, body)
    return body

but there will always be cases where it can't work (eg. if you stream the request data, then it's just not going to be available anymore later down the line)

I kind of disagree with your example. In fact, stream data is not stored by default, but stream metadata (is the stream closed) are; there will be an understandable error raised if someone try to stream twice, and that is enough imho. That's why if the body is cached, the stream consumption has to be cached too.

There are plenty use cases like @wyfo mention. In my case i'm using JWT Signature to check all the data integrity, so i decode the token and then compare the decoded result with the body + query + path params of the request. I don't know any better way of doing this

I'm working on a WTForms plugin for Starlette and I'm running into a similar issue. What is the recommended way to consume requst.form() in middleware?

Currently, this is one of the better options: https://fastapi.tiangolo.com/advanced/custom-request-and-route/#accessing-the-request-body-in-an-exception-handler but unfortunately there still isn't a great way to do this as far as I'm aware.

Thanks! Is there an equivalent to before_request() in Flask?
https://flask.palletsprojects.com/en/1.1.x/api/#flask.Flask.before_request

@amorey Depending on exactly what you want to do, you can either create a custom middleware that only does things before the request, or you can create a dependency that does whatever setup you need, and include it in the router or endpoint decorator if you don't need access to its return value and don't want to have an unused injected argument.

I think that's the closest you'll get to a direct translation of the before_request function, but I'm not very knowledgeable about flask.

@dmontagu I am using FastAPI and trying to log all the 500 Internal Server error only by using the ServerErrorMiddleware from Starlette using add_middleware in FastAPI.
Is there a way to the request JSON body in this case? It appears to be that I could consume the JSON body in HTTPException and RequestValidationError with add_exception_handler but nothing with ServerErrorMiddleware. Indeed ServerErrorMiddleware has request info but not the JSON Body.

Conversation from Gitter but lmk if anybody have feedbacks

    async def exception_handler(request, exc) -> Response:
        print(exc)
        return PlainTextResponse('Yes! it works!')

    app.add_middleware( ServerErrorMiddleware,  handler=exception_handler )

Okay I found a way to use with starlette-context to retrieve my payload only when I need logging
https://github.com/tomwojcik/starlette-context/blob/e222f739f113b74c2dad772d417d7fcc6f82f0ae/examples/example_with_logger/logger.py

I am writing every request using an incoming middleware with context in starlette-context and retrieving through context.data at HTTPException and ServerErrorMiddleware. This is not required in ValidationError as FastAPI support natively by using exc.body. All this roundabout only when you need to log those failed responses.

You dont need an middleware for logging incoming requests if you dont want to simply use depends in your router

application.include_router(api_router, prefix=API_V1_STR, dependencies=[Depends(log_json)])

is there an update for this?
it seems like this is something a lot of people need for very legit use cases ( mainly logging it seems)

is there a plan to allow consuming the body in a middleware ?
i see the body is cached on the request object _body, is it possible to cache it on the scope so it accessible from everywhere after it is read?

any other solution would also be ok, but i do feel this i needed

@JHBalaji how are you logging the request body to context, are you not running into the same issue when trying to access the body in the incoming request middleware?

Was this page helpful?
0 / 5 - 0 ratings

Related issues

rlewkowicz picture rlewkowicz  路  3Comments

cw-andrews picture cw-andrews  路  4Comments

kouohhashi picture kouohhashi  路  3Comments

Ekuzkamaza picture Ekuzkamaza  路  3Comments

Immortalin picture Immortalin  路  3Comments