Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
javierdialpad opened this issue Nov 30, 2023 · 2 comments

Comments

@javierdialpad
Copy link

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

  • Sanic v23.6.0
  • sanic-routing==23.6.0
@ahopkins
Copy link
Member

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants