sanic-org / sanic-routing

Internal handler routing for Sanic beginning with v21.3.
https://sanicframework.org/
MIT License
13 stars 11 forks source link

Expandable routes have more priority than static routes when adding a trailing slash to the path #79

Open javierdialpad opened 9 months ago

javierdialpad commented 9 months ago

Problem

There seems to be an unexpected behavior when mixing paths with trailing slashes and a route with a path/regex parameter. When creating an app with strict_slashes=False, one would expect that static routes will match paths with and without the trailing slash as a top priority. But paths with a trailing slash are actually a "last resource" when looking for a matching route.

Code snippet

from sanic import Sanic, response

app = Sanic("MyApp", strict_slashes=False)

async def hello_world(request, *args, **kwargs):
  return response.text('Hello World!')

async def catch_all(request, path):
  return response.text(f'Caught one: {path}')

app.add_route(hello_world, '/hello/')  # same behavior using '/hello'
app.add_route(catch_all, '/<path:path>')

if __name__ == '__main__':
  app.run(port=8080)

Expected behavior

Because strict_slashes=False, the hello_world route is supposed to match both /hello and /hello/. But the latter is being matched with the catch_all route.

Looks like this is happening due to how the resolve method works rigt now: https://github.com/sanic-org/sanic-routing/blob/main/sanic_routing/router.py#L92; it's looking for the route with the trailing slash only when no other route would match.

Sanic version

ahopkins commented 8 months ago

I am not sure that is likely to change anytime soon. That would be a pretty big change to do it differently. And, part of the reason it is implemented as such is for performance. I'll have to think on this some more, and I'm happy to hear suggestions, but I do not know of a good way to handle this currently without making some sacrifices.

javierdialpad commented 8 months ago

Yeah, I can't think of a way of changing that behavior without adding an extra call to resolve, which i'm guessing are not cheap.