♻️ Refactor logic to handle root_path to keep compatibility with ASGI and compatibility with other non-Starlette-specific libraries like a2wsgi#2400
Conversation
…ing the child scope with its own root_pat, this allows sub-apps (e.g. WSGIMiddleware) to know where it was mounted, and from which path prefix starts the path its sub-app should handle
… spec, respecting root_path
…w about Starlette internals (e.g. the route_root_path extension scope key that was added in
… a root_path to pass the root_path, which is what clients would have to do if the app is mounted.
root_path to keep compatibility with ASGI and compatibility with other non-starlette specific libraries like a2wsgiroot_path to keep compatibility with ASGI and compatibility with other non-Starlette specific libraries like a2wsgi
root_path to keep compatibility with ASGI and compatibility with other non-Starlette specific libraries like a2wsgiroot_path to keep compatibility with ASGI and compatibility with other non-Starlette-specific libraries like a2wsgi
|
Thanks @abersheeran! 🚀 |
|
@tiangolo @abersheeran @Kludex It seems there's a regression in 0.35.0 for mounts that starts with substring, that equals to It works just fine before 0.35.0 in both cases. I created a repository with the bare minimum to demonstrate regression. FastAPI app: https://github.com/Pentusha/fastapi-root-path-issue/blob/main/app/main.py fastapi 0.108.0 & starlette 0.32.0.post1fastapi 0.109.0 & starlette 0.35.1Related discussion: #2495 |
When a shiny app is running under a prefix (root_path in ASGI terms), the static files are not served correctly under pyodide. This is because the ASGI path includes the prefix, and the root_path should be removed. Starlette 0.33 and 0.34 however, did not set root_path correctly, and in those cases, we have to rely on route_path. Related discussions: Kludex/starlette#2400 Kludex/starlette#2361 A related fix we had in Solara: widgetti/solara#413 But this fix also did not seem to work for our situation at https://py.cafe I logged the output of the relevant entries in the scope dict, together with the starlette version: ``` scope 0.32.0 {'path': '/strftime-min.js', 'root_path': '/_app/lib/strftime-0.9.2'} scope 0.33.0 {'path': '/_app/lib/strftime-0.9.2/strftime-min.js', 'root_path': '', 'route_root_path': '/lib/strftime-0.9.2', 'route_path': '/strftime-min.js'} scope 0.34.0 {'path': '/_app/lib/strftime-0.9.2/strftime-min.js', 'root_path': '', 'route_root_path': '/lib/strftime-0.9.2', 'route_path': '/strftime-min.js'} scope 0.35.0 {'path': '/_app/lib/strftime-0.9.2/strftime-min.js', 'root_path': '/_app/lib/strftime-0.9.2'} scope 0.36.0 {'path': '/_app/lib/strftime-0.9.2/strftime-min.js', 'root_path': '/_app/lib/strftime-0.9.2'} scope 0.37.2 {'path': '/_app/lib/strftime-0.9.2/strftime-min.js', 'root_path': '/_app/lib/strftime-0.9.2'} ``` This was using a similar situation as on py.cafe: ```python .... routes = [ Mount('/static', app=app_static), Mount('/_app', app=app_shiny) ] app = Starlette(routes=routes) ``` Which led me to the following fix, making shiny work under pyodide in combination with a prefix with the above mentioned versions of starlette.
When a shiny app is running under a prefix (root_path in ASGI terms), the static files are not served correctly under pyodide. This is because the ASGI path includes the root_path, and the root_path should be removed. Starlette 0.33 and 0.34 however, did not set root_path correctly, and in those cases, we have to rely on route_path. Related discussions: Kludex/starlette#2400 Kludex/starlette#2361 A related fix we had in Solara: widgetti/solara#413 But this fix also did not seem to work for our situation at https://py.cafe I logged the output of the relevant entries in the scope dict, together with the starlette version: ``` starlette 0.32.0 {'path': '/strftime-min.js', 'root_path': '/_app/lib/strftime-0.9.2'} starlette 0.33.0 {'path': '/_app/lib/strftime-0.9.2/strftime-min.js', 'root_path': '', 'route_root_path': '/lib/strftime-0.9.2', 'route_path': '/strftime-min.js'} starlette 0.34.0 {'path': '/_app/lib/strftime-0.9.2/strftime-min.js', 'root_path': '', 'route_root_path': '/lib/strftime-0.9.2', 'route_path': '/strftime-min.js'} starlette 0.35.0 {'path': '/_app/lib/strftime-0.9.2/strftime-min.js', 'root_path': '/_app/lib/strftime-0.9.2'} starlette 0.36.0 {'path': '/_app/lib/strftime-0.9.2/strftime-min.js', 'root_path': '/_app/lib/strftime-0.9.2'} starlette 0.37.2 {'path': '/_app/lib/strftime-0.9.2/strftime-min.js', 'root_path': '/_app/lib/strftime-0.9.2'} ``` This was using a similar situation as on py.cafe: ```python .... routes = [ Mount('/static', app=app_static), Mount('/_app', app=app_shiny) ] app = Starlette(routes=routes) ``` Which led me to the following fix, making shiny work under pyodide in combination with a prefix with the above mentioned versions of starlette.
…spec Before this change, the dispatcher middleware didn't do anything with 'root_path'. It did, however, modify 'path'. The problem with this behavior, is that the child ASGI app has no way to determine what its prefix it. And if it can't determine its prefix, it doesn't know how to construct URLs. The ASGI spec isn't super clear on the expected behavior. However, some resources to review are: * https://asgi.readthedocs.io/en/latest/specs/www.html#wsgi-compatibility * Kludex/starlette#2400 * django/asgiref#229 Based on the above, I believe that the correct behavior is that "root_path" should be updated by the dispatcher middleware but that "path" should not be modified. In addition to the above change, I also updated the tests. And I also added a new test case where the dispatcher middleware is nested inside of itself.
Summary
♻️ Refactor logic to handle
root_pathto keep compatibility with ASGI and compatibility with other non-Starlette-specific libraries like a2wsgi (in the latest version).Description
The ASGI spec specifies that
pathshould include the fullroot_path, which is different from the equivalent in WSGI, that strips the root path equivalent. This was not the case in Starlette up to #2352 that fixed it and was released in Starlette 0.33.0.Nevertheless, by that point, Starlette stopped modifying the
root_pathfor mounted children ASGI apps. This means that children ASGI apps would share the sameroot_pathas the top level Starlette app, when they were actually sub-mounted inside of Starlette, in a path prefix lower than whatever is the path prefix that Starlette was mounted on (the top levelroot_path). Because Starlette stopped modifying and increasing theroot_pathfor child scopes, this means that ASGI apps mounted in Starlette withMountwould not be able to know what their actual path prefix was, to be able to handle their own routing after that prefix.The way that other internal parts of Starlette dealt with it in that PR was by adding a couple of ASGI extension scope keys specific only to Starlette
route_root_pathandroute_path. But that only worked for Starlette-specific components that knew about those (non-standard) scope keys.This PR stops relying on those new scope keys (and removes them) and restores modifying the child
root_scope. But keeps being ASGI compliant with the fix from #2352, keeping the fullpathincluding theroot_path.It also fixes a couple of internals that used to depend on the old behavior.
TestClientThe
TestClientwas also updated, when creating aTestClient, it received aroot_pathparameter without much information of what it would mean and how it would be used internally. In fact, it was not really used by the TestClient internally, it was just added to the ASGI scope as is.This PR refactors that to make the client take the used
root_pathinto account, if it is set, it will be passed to the ASGI apps, that will internally extract/remove it from the paths. But clients communicating with those ASGI apps, if they are indeed mounted at the definedroot_path, would have to communicate with it using theroot_pathprefix. So this PR updates the client to actually require/use that, clients created withroot_pathwould need to be used the same way that clients communicating with those ASGI apps mounted at some prefix path.root_pathI think
root_pathin general is sometimes confusing, as it is about path prefixes that exist in some scenarios, for some parties, e.g. the final clients communicating through a proxy with a path prefix (e.g./api/v2), but not for the other parties, e.g. the ASGI apps that has aroot_path(e.g. the Starlette app that doesn't know anything about/api/v2).It also applies to ASGI apps mounted inside of other things, like other ASGI apps, under a path prefix. For example, a Flask app mounted through a2wsgi
WSGIMiddlewareinside of a Starlette app at/oldapp. In this case, Starlette would know about the path prefix/oldapp, but the Flask app (throughWSGIMiddleware) would not know about/oldapp, so,WSGIMiddlewarewould be the thing in charge of reading that/oldappfrom theroot_pathpassed to it in its ASGI scope (a child scope) and remove that path prefix from the paths passed to Flask.I wrote a lot more about
root_pathwith diagrams and stuff in the FastAPI docs: https://fastapi.tiangolo.com/advanced/behind-a-proxy/Checklist