Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Contextvars are not reset between requests if the request contains multipart form data #2335

Open
Kludex opened this issue Nov 13, 2023 Discussed in #2312 · 2 comments
Open
Labels
need confirmation This issue needs confirmation.

Comments

@Kludex
Copy link
Sponsor Member

Kludex commented Nov 13, 2023

Discussed in #2312

Originally posted by simon-sk October 26, 2023
Hi Starlette team,

we are experiencing an issue with Starlette where contextvars set in a middleware are leaking between requests if the request contains multi-part form data and the client session is reused.

I constructed a minimal example which sets a context var in a middleware and resets it after the app has been called. It also raises an exception if the var is already set:

import contextvars

from starlette.applications import Starlette
from starlette.middleware import Middleware
from starlette.responses import JSONResponse
from starlette.routing import Route

var = contextvars.ContextVar("var", default=None)


class ASGIMiddleware:
    def __init__(self, app):
        self.app = app

    async def __call__(self, scope, receive, send):
        if scope["type"] != "http":
            return await self.app(scope, receive, send)

        if var.get() is not None:
            raise ValueError(f"var is already set {var.get}")

        token = var.set("TEST")

        await self.app(scope, receive, send)

        var.reset(token)


async def route(request):
    return JSONResponse({"hello": "world"})


app = Starlette(
    middleware=[Middleware(ASGIMiddleware)],
    routes=[Route('/', route, methods=["POST"])]
)


if __name__ == "__main__":
    import uvicorn

    uvicorn.run(
        "app.main_starlette:app",
        host="0.0.0.0",
        port=8001,
    )

Then, I execute a second script which calls this endpoint using a httpx client and includes a files object:

import httpx

if __name__ == "__main__":
    url = "http://localhost:8001/"
    files = {'file': open('./test.yaml', 'rb')}

    with httpx.Client() as client:
        for i in range(2):
            r = client.post(url, timeout=None, files=files)
            assert r.status_code == 200

This results in a server error due to the fact that the variable is already set (sometimes it is necessary to run the client script multiple times as the issue does not always occur).

If I execute the same script without multipart data, it works without issues:

...
r = client.post(url, timeout=None)
...

We have also experienced this issue without sending multipart form data for requests within the same client session but it occurs significantly less often and unfortunately I am not able to reproduce it consistently.

We experienced this issue with Python 3.9 and 3.10, Starlette v0.31.1, and uvicorn v0.23.2.

Important

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar
@Kludex Kludex added the need confirmation This issue needs confirmation. label Nov 13, 2023
@Kludex
Copy link
Sponsor Member Author

Kludex commented Dec 22, 2023

I can't reproduce on the latest versions of the packages, and considering Python 3.9 and 3.10.

If you can create a repository, and show me in the CI how this fails, I can continue helping.

@Kludex Kludex closed this as not planned Won't fix, can't repro, duplicate, stale Dec 22, 2023
@simon-sk
Copy link

@Kludex I created a repo here that reproduces the issue in this action run.

The issue does not always occur on the first request, so I added a loop that sends multiple requests in a row.

@Kludex Kludex reopened this Jan 16, 2024
@Kludex Kludex added good first issue Good for beginners and removed good first issue Good for beginners labels Feb 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need confirmation This issue needs confirmation.
Projects
None yet
Development

No branches or pull requests

2 participants