[WIP] Handle scope["root_path"] and scope["path"] as per ASGI spec #1774
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
scope["root_path"]andscope["path"]differs from ASGI spec #1336Rationale
This is an exploratory work as how to solve issue #1336. The issue details the problem quite well but in a nutshell:
Mountrouter alongside theroot_pathoption.pathis the remainder of the whole path striped fromroot_path. As pointed out here, this is not how the ASGI spec was imagined:pathshould containroot_path.Mountrouters, by adding the router prefix onroot_pathand strippingpathuntil we match the single route:https://github.com/encode/starlette/blob/e3de71f1433568a3f7ac3dc69ef86cf4f72f5651/starlette/routing.py#L378-L390
Notice that we define an
app_root_pathto keep track of the originalroot_path.The way I understand it, we should keep both
pathandroot_pathuntouched. It's a shame of course because we won't be able to handleMountelegantly as we do now.Current work status
First of all, I added a unit test to reproduce the issue.
Then, I've tried a first approach to solve it which consists of two things:
pathandroot_pathdirectly, use internal variablescurrent_pathandcurrent_root_path(admittedly, this is a very uninspired naming) to allow our recursive approachroot_pathfrom thepathso we can match properly, both inRouteandMountThis solves the specific problem but breaks other parts of the code. Obviously, every parts of the routing which makes use of the
pathwill need to account for thisroot_path, e.g. inWebSocketRouteor inRouter, in the part where we make slash redirects.An idea I had to tackle this would be to have a common
prepare_scopewhich could be called by the Route classes to prepare the path, add the "internal" variables, while keeping the originalpathandroot_pathuntouched.Before going further, I would be interested in your views and opinions about this so we can take the right path.
Cheers!