Serving static files at '/' conflict with routes

[Tronic]

app.static("/", "./static/")

@app.get("/foo")
async def foo(request):
    return text("OK")

@app.get("/<arg>")
async def bar(request, arg):
    return text(arg)

Given this app, the static route for /foo takes precedence over static files, so that works as expected. However, the dynamic route does not, so trying to access /bar returns 404 (assuming there is no static/bar file).

It would be nice to have static files either

• handled with lower precedence than any handlers, or
• fallback to other routes if file is not found

The second option would better match with Nginx hosting where the server first attempts static files and if not found, proxies to Sanic. The first option is probably easier to implement (ping @ahopkins re: routing) and faster (no slowdown of normal handlers at all, in particular if static files are actually handled by Nginx).

[ahopkins]

I am somewhat surprised by that behavior since I thought that we already were checking static routes first.

Nonetheless, I like the first option better.

But, I think we could even improve upon this at startup time to check to see even if any static routes exist. If there are, then the router simply works as you proposed with precedence. If not, then we can skip the static checking altogether.

[Tronic]

Static routes are indeed checked first but since static files are served as a dynamic route, any other dynamic routes in the application may conflict with them. Making static files their own class in routing with even lower precedence would indeed implement the first option mentioned above.

Or perhaps more generally, implement a precedence setting for all dynamic routes, and give static file serving a below-normal precedence by default.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is incorrect, please respond with an update. Thank you for your contributions.

[ahopkins]

In the new router, this is handled by matching the static files first.

Therefore, in your example:

$ curl localhost:9999/foo                                                                                                                                                                                                                                                                                                                                                                                                                               (env: sanic) 
OK

$ curl localhost:9999/foo.txt                                                                                                                                                                                                                                                                                                                                                                                                                           (env: sanic) 
You got the foo file

$ curl localhost:9999/bar                                                                                                                                                                                                                                                                                                                                                                                                                               (env: sanic) 
⚠️ 404 — Not Found
==================
File not found

---

I do agree that the ultimate solution would be to allow the fallback to continue looking for other routes. I think that we should get there, but not yet. I am going to mark this for a future release.

My thought would be to add an opt-in:

app.static("/", "/tmp", fallback=True)

When that is enabled, we inject code into the Router.find_route_src that will tell the resolver that it should check for file existing (os.path.exists) before returning the static route handler.

[ahopkins]

I am moving this to 21.9.

Use case:

from sanic import Sanic
from sanic.response import text

app = Sanic("app")
app.static("/", "./static/")


@app.get("/foo")
async def foo(request):
    return text("OK")


@app.get("/<bar:int>")
async def bar(request, bar):
    return text(f"{bar=}")


@app.get("/<arg>")
async def arg(request, arg):
    return text(f"{arg=}")

Source:

def find_route(path, method, router, basket, extra):
    parts = tuple(path[1:].split(router.delimiter))
    try:
        group = router.static_routes[parts]
        basket['__raw_path__'] = path
        return group, basket
    except KeyError:
        pass
    num = len(parts)
    
    # node=1 // part=__dynamic__:int
    if num == 1:  # CHECK 1
        try:
            basket['__matches__'][0] = int(parts[0])
        except ValueError:
            pass
        else:
            # Return 1
            return router.dynamic_routes[('<__dynamic__:int>',)][0], basket
    
    # node=2 // part=__dynamic__:str
    if num == 1:  # CHECK 1
        try:
            basket['__matches__'][0] = str(parts[0])
        except ValueError:
            pass
        else:
            # Return 2
            return router.dynamic_routes[('<__dynamic__:str>',)][0], basket
    match = router.matchers[0].match(path)
    if match:
        basket['__params__'] = match.groupdict()
        return router.regex_routes[('<__dynamic__:path>',)][0], basket
    raise NotFound
matchers = [
    re.compile(r'^/(?P<__file_uri__>[^/]?.*?)$'),
]

The resolution is to add fallback=True (as stated above). This would place the static path matcher up at the top (before static routes) with os.path.exists as suggested. I do not currently think this will make it into the upcoming release.

[Tronic]

This affects serving from a subdirectory too:

@app.static("/.well-known/", "/tmp/well-known/")

@app.get("/<path:path>")
def handler(request, path):
    return response.redirect(f"https://{request.server_name}/{path}")

Expected anything that begins with /.well-known/ to be served as a static file, especially if the directory actually exists on filesystem and the file being accessed is there. Instead I get the redirection on any path.

[Tronic]

Actually the above was caused by the trailing slashes on app.static. It turns out that

# Works as intended
app.static("/.well-known", "/tmp/well-known", resource_type="dir")

# No error on server startup, just gives 404 when trying to access anything
app.static("/.well-known", "/tmp/well-known/", resource_type="dir")
app.static("/.well-known/", "/tmp/well-known", resource_type="dir")
app.static("/.well-known/", "/tmp/well-known/", resource_type="dir")

Could we fix this so that it is more tolerant against slashes, and maybe also implicitly set that resource_type when trailing slashes are seen? Ping @ahopkins

[ChihweiLHBird]

How is the status of this issue? Are we still going to work on it?

We want:

1. Handle the trailing slashes issue.
2. Add the fallback option.

right?

[ahopkins]

1. Add resource_type="dir" when there is a trailing slash this
2. fallback this

Can and should be added to LTS

[ChihweiLHBird]

@Tronic @ahopkins Just tested, and the trailing slashes is no longer reproducible in the latest code base. resource_type is also checked here and in the following lines, https://github.com/sanic-org/sanic/blob/4422d0c34d7a5652395de49e3a5db42457245ff0/sanic/mixins/routes.py#L957

So, I think the fallback is the only TODO here.

[ahopkins]

Moving to 23.3 unfortunately. This might require a much deeper dive than I thought to get that fallback working right.