fix(ASGI mounts): Prevent accidental scope overrides by mounted ASGI apps#3945
fix(ASGI mounts): Prevent accidental scope overrides by mounted ASGI apps#3945provinzkraut merged 2 commits intomainfrom
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3945 +/- ##
=======================================
Coverage 98.34% 98.34%
=======================================
Files 347 347
Lines 15727 15743 +16
Branches 1738 1740 +2
=======================================
+ Hits 15467 15483 +16
Misses 124 124
Partials 136 136 ☔ View full report in Codecov by Sentry. |
|
dumb question maybe but what would happen if you mount a litestar app in a litestar app, won't you end up with the same issue you're fixing for other asgi apps ? |
True, didn't think of that case as it's not something we really advertise and I can't immediately think of a use case.
If we want to handle this one case in particular, we could just "restore" the original |
|
The use case to is pretty much the same as this one: you mount something you have no control on ? Realistically we'll have more Litestar users mounting FastAPI for the time being but you never know ;) This was just a small remark, every asgi app is a squatter of the same namespace, but I'm all for squatts generally speaking so probably leaving this as is is a good option if ths copy_scope let the user solve the issue, it gives Litestar another edge other probably don't have. |
Co-authored-by: Jacob Coffee <jacob@z7x.org>
|
|
Documentation preview will be available shortly at https://litestar-org.github.io/litestar-docs-preview/3945 |
When mounting ASGI apps, there's no guarantee they won't overwrite some key in the
scopethat we rely on, e.g.scope["app"], which is what caused #3934.To prevent this, I've implemented two things:
appkey, but the more specificlitestar_appkey. I've also added aLitestar.from_scopemethod, which can be used to safely access the current app from the sopecopy_scopeto the ASGI route handler, which, when set toTruewill copy the scope before calling into the mounted ASGI app. This should make things behave more as expected, since it truly give the called app its own environment without causing any side-effects. Since this change might break some things, I've left it with a default ofNone, which does not copy the scope, but will issue a warning if the mounted app modified it, enabling users to decide how to deal with that situationFixes #3934