Conversation
There was a problem hiding this comment.
Hmm... catching HTTPNotFound here may lead to unexpected behavior, imagine the following use case:
app.router.add_route("GET", "/root/resource", handler1)
app.router.add_route("GET", "/root/resource/{tail:.*}", handler2)
def handler1(req):
if not has_permission(req):
raise web.HTTPNotFound() # we don't want to expose this resource
# ....
def handler2(req):
passIn this case application will expose /root/resource/ url (that it is exsist) and also it may be hard to debug such cases.
There was a problem hiding this comment.
Checking that request.match_info is instance of _NotFoundMatchInfo might resolve this issue.
There was a problem hiding this comment.
The code seems to work as is. I have a test case here: https://github.com/sloria/aiohttp_utils/blob/79b38453f924f9a569b6a06f5f3a584d3ecf11da/tests/test_path_norm.py#L116-L121 (setup here: https://github.com/sloria/aiohttp_utils/blob/79b38453f924f9a569b6a06f5f3a584d3ecf11da/tests/test_path_norm.py#L22-L30)
I realized half-way through implementing this that this is already being implemented in aio-libs/aiohttp#362, so right now this is mostly a copy-n-paste of that code. Keeping this for now because it is still useful to have the setup function for reading app config. Once that PR is merged, much of this code can go away.
|
Just wondering: what is the status of this? I was working on similar middleware before I found this PR. I ended up copying much of this implementation into a utility library, aiohttp_utils. If/when this is merged, I can remove much of that code from aiohttp_utils. I can maintain the code there until then. |
|
I've switched to another issues and totally forgot about this one. |
|
I don't think this needs to be a priority. The implementation I have in aiohttp_utils works fine for my purposes. |
Implement
normalize_path_middleware(see #355)