Consuming the request body multiple times

[antonpirker]

Hello!

There are three high level functions to read the request body: Request.json(), Request.form() and Request.body():
https://github.com/encode/starlette/blob/master/starlette/requests.py#L231-L266

All of them use the function Request.stream() to read the actual data:
https://github.com/encode/starlette/blob/master/starlette/requests.py#L208-L229

All of them cache the request data in a variable called _json, _form, and _body respectively.

Because of this I can call either of .json(), .form() or .body() multiple times and it works because the stream is only consumed once and from subsequent calls served from the cache.

It is also possible to call .json() and then .body() because .json() uses internally .body() to get the data (and therefore the second call to .body() gets the cached data from _body)

But if I call .form() and then .body() I get an RuntimeError: Stream consumed because .form() is calling .stream() directly (and not .body()) and saving the request data to _form.
But.stream()is only checking if there is a _body and, if yes, returning this, but in this case there is no _body so it is trying to read the stream once again, which is not possible and leads to the RuntimeError.

Question:
Is this by design or just a coincidence?

Would it make sense:
A) in stream() to also check for the existence of _form rather then just _body? (having a second if statement similar to this one: https://github.com/encode/starlette/blob/master/starlette/requests.py#L209-L212)
B) or call .body() inside of .form() so the request data is cached in _body?

[antonpirker]

Some context: This leads to this problem in the Sentry Starlette integration: getsentry/sentry-python#1675

[joshwilson-dbx]

This also becomes a problem when one wishes to perform a content hash on form data requests, for say sender verification purposes. Since the content has already been streamed and parsed, there's no way (that I can see) to access the original body bytes for hashing purposes. So now I have to disable request sender verification for form requests for the time being (not good).

My vote would be for option B, since the caching parsed results on _form wouldn't allow access to the original data (if i'm understanding A correctly).

[tedsecretsource]

This is exactly my case. There must be a way to make this work. There must be an original version of the body stored somewhere and if not, could one possibly override the class that is doing the reading and add a class variable that contains the cached version? This has me completely blocked at present :(

And FWIW, await Request.body() is not working for me, even if I call it before .form().

[tedsecretsource]

FWIW, I've resorted to recreating the body because in my case, it is only a form element (technically it's not even that, just a post request with x-www-form-urlencoded) so my verification ought to work (but man, it sure would be great if starlette would just let me grab the request body and save me all of this technical debt…)

[adriangb]

There is no reason why this must work in any specific way. If we just stored the body in case someone wanted it lasted we'd end up doubling the memory usage for all users (most of which don't need that feature). The reason this is not the case with .body() is that we can return a reference to the exact same bytes. With .form() the bytes get parsed and transformed, so we'd be keeping two copies around.

[joshwilson-dbx]

Yes, i agree, that's a fair critique. It would only occur in projects reading .form(), but I could see that being an unexpected deleterious effect for that set of clients.

[tedsecretsource]

I suppose one could write some middleware that runs before all other middleware and saved a copy? I'm brand new to Starlette/FastAPI so I have no idea how hard that is to do but I have to assume it would not be that complicated. Any pointers? I'm sure others would like to use it as well. Thanks.

…

On Fri, Aug 18, 2023, 13:45 Adrian Garcia Badaracco < ***@***.***> wrote: There is no reason why this *must* work in any specific way. If we just stored the body in case someone wanted it lasted we'd end up doubling the memory usage for all users (most of which don't need that feature). The reason this is not the case with .body() is that we can return a reference to the exact same bytes. With .form() the bytes get parsed and transformed, so we'd be keeping two copies around. — Reply to this email directly, view it on GitHub <#1933 (reply in thread)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AG27XGLRMON56YH7NZQJAS3XV5PVFANCNFSM6AAAAAARVC2FJY> . You are receiving this because you commented.Message ID: ***@***.***>

--

____________________________ The information included in this e-mail and any file attached to it, are privileged and confidential and is intended only for use of the addressee. If you are not the intended recipient or the employee or agent responsible for delivering the message to the intended recipient, or you have received this communication in error, please be aware that any dissemination, distribution or duplication of this communication is strictly prohibited. Please notify us immediately and destroy the original without reading it.  Your personal data will be processed by SECRET SOURCE TECHNOLOGY, S. L., address at C/ Bravo Murillo, 34, 35003, Las Palmas de G. C., for the purpose of managing communications made by e-mail due to the services provided, commercial information or activities carried out by the Responsible. You can exercise your rights of access, rectification, deletion, restriction of processing or portability by writing to the file manager SECRET SOURCE TECHNOLOGY, S. L., C/ Bravo Murillo, 34, 35003, Las Palmas de G. C. You have the right to lodge a complaint at the AEPD (www.aepd.es <http://www.aepd.es>). You can consult/request more information on data protection at ***@***.*** ***@***.***>.

[adriangb]

Check out what we do in BaseHTTPMiddleware:

starlette/starlette/middleware/base.py

Line 48 in a8b8856

async def wrapped_receive(self) -> Message:

[joshwilson-dbx]

Thanks for the link. FastAPI uses that for its @middleware decorator. Let me find some time to dive deeper on that one.

[tedsecretsource]

Middlewares aren't the best tool for selective application of behavior.

Indeed. Good catch but given everything else, the dependencies and ecosystem already in place, adding one's own middleware doesn't seem too outrageous. Also, I'm in need of this because Slack requires it for verification (for integrations).

[tedsecretsource]

And FWIW, if you are trying to verify slack requests, I found this StackOverflow answer to be helpful. You'll note there is no need to access request.body(). Also, note that the code in this answer isn't perfect and I needed to tweak it a little to get it to work. Hit me up if you want the details. Thanks to everyone here for taking a look at this and your feedback. Without this kind of interaction, I could never get this stuff working!

[joshwilson-dbx]

That's a helpful link and this is the key hint.

You'll note there is no need to access request.body().

Here's what I'm doing at the moment (I too am addressing slack request verification!).

async def _do_slack_verification(
    request: Request,
    content_type: Annotated[str, Header()],
    x_slack_request_timestamp: Annotated[int, Header()],
    x_slack_signature: Annotated[str, Header()],
    settings: Settings,
):
    # We re-encode the parsed FormData if the request is form content.
    # We have to do this in order to support Form() field dependency injection.
    payload = (
        urllib.parse.urlencode(await request.form())
        if content_type == "application/x-www-form-urlencoded"
        else (await request.body()).decode("utf-8")
    )
    sig_basestring = f"v0:{x_slack_request_timestamp}:{payload}"
    sig_hash = hmac.new(
        settings.slack.apps.bot.signing_secret,
        sig_basestring.encode("utf-8"),
        hashlib.sha256,
    )
    signature = f"v0={sig_hash.hexdigest()}"
    if not hmac.compare_digest(signature, x_slack_signature):
        raise HTTPException(status.HTTP_401_UNAUTHORIZED)

# Add this to a router's dependencies list
SlackVerification = Depends(_do_slack_verification)

I'm using urllib.parse.urlencode() rather than manually reassembling the content with urllib.parse.quote(). I haven't tested this thoroughly yeI, but it seems to be working at the moment.

[wu-clan]

Is there already a solution here?

Forgive me for using form(), body(), json() together in my middleware.

In version 0.32, I have a form-data request, and if I call them after call(request), the stream is already consumed, and if I call them before, then call(request) is subject to the empty body().

[jerivas]

I have middleware that inspects form data, and it subtly breaks all endpoints that also want to consume it later. Endpoints will get an empty dictionary, not even an error.

See demo code

from starlette.applications import Starlette
from starlette.middleware import Middleware
from starlette.middleware.base import BaseHTTPMiddleware
from starlette.responses import PlainTextResponse
from starlette.routing import Route

class FormLoggerMiddleware(BaseHTTPMiddleware):
    async def dispatch(self, request, call_next):
        form = await request.form()  # Reading the form in the middleware prevents it from being read in the endpoint
        print(f"User submitted: {form}")
        return await call_next(request)


async def hello(request):
    form = await request.form()  # This will always be an empty dict because the middleware consumed the form
    return PlainTextResponse(
        f"""
        <form method="POST">
            <input type="text" name="message" />
        </form>
        You said: {form.get('message', '(nothing)')}
        """,
        media_type="text/html",
    )

middleware = [Middleware(FormLoggerMiddleware)]

routes = [
    Route("/", endpoint=hello, methods=["GET", "POST"]),
]

app = Starlette(
    routes=routes,
    middleware=middleware,  # Disable the middleware to fix the endpoint
)

The workaround:

await request.body() # <--- Add this when reading form data in middleware
form = await request.form()

IMO this is a bug and it would be best to have request.form() call request.body() internally as suggested in option B of the original post.

[adriangb]

I agree, but that would be a breaking change. @Kludex should we do it in 1.0?

[joshwilson-dbx]

Still looking forward to being able to do this!

[joshwilson-dbx]

The workaround:

await request.body() # <--- Add this when reading form data in middleware
form = await request.form()

Unfortunately, I was not able to simply add a middleware to "pre-read" the body. The Request received by this new "hack" middleware was not the same object received by the fastapi handler, so the cached _body data was lost.

[adriangb]

See what we do in BaseHTTPMiddleware.