router.add_static is greedy

[redsquirrel]

🐞 Describe the bug

The first argument for add_static matches on any path that starts with that string. For instance, if you specify "h", then add_static will match on "h" or "help" or "hoover".

💡 To Reproduce

Run this code:

from aiohttp import web

async def handle(request):
    name = request.match_info.get('name', "Anonymous")
    text = "Hello, " + name
    return web.Response(text=text)

app = web.Application()

router = app.router

router.add_static("/h", "./")

router.add_routes([web.get('/', handle),
                web.get('/{name}', handle)])

if __name__ == '__main__':
    web.run_app(app)

Add a text file in the current working directory named test.txt and put some text in there.

Then navigate to the following URLs to demonstrate the issue:

• http://0.0.0.0:8080/
• http://0.0.0.0:8080/groot
• http://0.0.0.0:8080/h/test.txt
• http://0.0.0.0:8080/hoover

The last link unexpectedly returns a 404.

💡 Expected behavior

I would have expected http://0.0.0.0:8080/hoover to have similar behavior to http://0.0.0.0:8080/groot, and render "Hello, hoover".

📋 Your version of the Python

3.8

📋 Your version of the aiohttp/yarl/multidict distributions

aiohttp 3.7.2

[asvetlov]

The order of routes is important, the search is stopped on the first match.
Is the static route more ready than the plain or dynamic one?

[redsquirrel]

@asvetlov I'm not sure what you mean by "more ready".

[alex-eri]

router.add_static("/h/", "./")

[redsquirrel]

@alex-eri That has no effect due to this logic where any trailing slash is removed: https://github.com/aio-libs/aiohttp/blob/master/aiohttp/web_urldispatcher.py#L1097

[danking]

I also encountered this. It's rather surprising that router.add_static("/tutorials/", ...) would match /tutorials-landing.html.

It seems like the fix should be in StaticResource.resolve:

    async def resolve(self, request: Request) -> _Resolve:
        path = request.rel_url.raw_path
        method = request.method
        allowed_methods = set(self._routes)
        if not path.startswith(self._prefix + "/"):
            return None, set()
        # ....

That seems safe and correct considering that the resolved filename implicitly assumes that the character following the prefix is irrelevant:

        match_dict = {"filename": _unquote_path(path[len(self._prefix) + 1 :])}