Auditing non-ASGI state.

[lovelydinosaur]

There's a bunch of places in Starlette that we store state in the ASGI scope, that's outside of the ASGI spec.

I'm... okay-ish about this, but we need to be careful about it.

We might want to consider namespacing these someday, not sure. But at a minimum we ought to make sure we've clearly identified where we're doing this at the moment. I think these are what we've got at the moment.

Middleware

• session
• auth
• user

General purpose

• app - The application instance.
• state - General purpose state on the request.

Routing state

• endpoint - Could arguably be the route instead. Although unclear what we'd want to surmounted routes.
• path_params
• app_root_path - I introduced this in Include 'root_path' when returning URLs from request.url_for #699. And just... yuck. Needs to not exist really.
• router - Urg. We shouldn't really need this, since we could instead access it via request.app.router.

Templating

• extensions / http.response.template - Bit different - not part of the scope, but an ASGI messaging extension.

Am I missing anything?

[Kludex]

There's another one proposed by #1351 which solves a current bug.

[lovelydinosaur]

Ah yeah - thanks for the prompt @Kludex!

My assessment of that is that actually we should be doing something simpler, which is reverting #1147.

I'd obviously made the wrong call on that.

The intent was "okay, let's limit the path against which session cookies are applied. Our server might be submounted to a given path, and we shouldn't cookie to / in that case." - Which is reasonable enough idea if it only applied to where the main app was mounted, but as it happens actually just breaks all sorts of cases for us. Because we allow for submounted apps too.

A better resolution here would be...

• Revert Make session cookie use ASGI root path #1147
• Optionally, support a configurable path="/" on SessionMiddleware

[euri10]

templates

[lovelydinosaur]

lemons

[lovelydinosaur]

Oh right. You mean this...

• extensions, http.response.template

Which we set here...

https://github.com/encode/starlette/blob/38185f39de3f92b2781826b6e0d07ca52b6c9b77/starlette/templating.py#L37-L48

And has a halfway though-out ASGI proposal. django/asgiref#135

Yup good spot!

[euri10]

yes this place, I should have been more explicit, no lemons _)

which makes me think we can access response.template and .context with starlette TestClient but this is not possible in httpx, not sure it's important but to keep in mind if you drop the TestClient in favor of using httpx

[lovelydinosaur]

not sure it's important but to keep in mind if you drop the TestClient in favor of using httpx

Exactly, yes. Spot on. 🌟

[lovelydinosaur]

Righty. Added that one in - thanks!

[adriangb]

Could some of these be moved into an ASGI extension?

[lovelydinosaur]

Yeah. Needs some careful thinking through, but sounds like a good plan.

(Even if it's just a "here's space where we mess with some of our own stuff under that")

[adriangb]

Yep I think both approaches are needed: be mindful of what we put in there but also put it in the place the spec says.

[adriangb]

General thought: once we've rounded up all of our uses of storing stuff in the ASGI scope, we could try to create a codified enumeration via a TypedDict or similar so that in the future we don't have to track things down again. And maybe some way to make it easier to set/get stuff (to avoid repeating if "extensions" in scope: ...; if "starlette" in extensions: ...)

[adriangb]

I might take a stab at this. @tomchristie any idea how you want to treat backwards compatibility here?

• We could hack together some dict subclass that issues a warning when accessing certain keys and grabs them from scope["extensions"]["starlette"] instead.
• We could just set the key in both places for now with no deprecation warning.
• We could just do a breaking change, not sure how much stuff out there is relying on these values.

[florimondmanca]

There are a few instances of accessing Starlette scope values out there, including in libraries...

(Maximum count in parentheses - Not call are related to Starlette)

• (14) scope["app"] https://grep.app/search?q=scope%5B%22app%22%5D
• (5) scope["endpoint"] - https://grep.app/search?q=scope%5B%22endpoint%22%5D
• (5) scope["router"] https://grep.app/search?q=scope%5B%22router%22%5D
• (3) scope["route"] https://grep.app/search?q=scope%5B%22route%22%5D

Outside of Starlette and related frameworks, occurrences seem to focus on monitoring tools, that inspect the scope to get information about the current HTTP call.

I guess a hacked dict that raises deprecation warnings would be nicer.

[br3ndonland]

In the "routing state" category, I also noticed router, just in one spot:
https://github.com/encode/starlette/blob/6ad5a6a97f4382459090b4459f1dafdd8ad03c15/starlette/routing.py#L651-L652

[lovelydinosaur]

Urg. Yes. Good catch, thanks.
It'd be neater if were were just accessing that via request.app.router really.