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

With or without trailing slash will be matched to different endpoints #2411

Closed
ZSSVE opened this issue Mar 15, 2022 · 4 comments
Closed

With or without trailing slash will be matched to different endpoints #2411

ZSSVE opened this issue Mar 15, 2022 · 4 comments

Comments

@ZSSVE
Copy link

ZSSVE commented Mar 15, 2022

Describe the bug

Hi, during our upgrade of Sanic from 19.9.0 to a higher one at 21.9.3, we ran into a seemly behavior change for path routing.

A clear and concise description of what the bug is, make sure to paste any exceptions and tracebacks.

With two GET endpoints

  1. /test/
  2. /test/< name>

In Sanic 19.9.0,

  • curl 'http://127.0.0.1:8000/test/' will be routed to endpoint 1

In Sanic 21.9.3,

  • curl 'http://127.0.0.1:8000/test/' will be routed to endpoint 2 with empty string as parameter

In both Sanic 19.9.0 and 21.9.3, curl 'http://127.0.0.1:8000/test' resolves to endpoint 1, expectedly.

Code snippet
Relevant source code, make sure to remove what is not necessary.

from sanic import Sanic
from sanic.response import text

app = Sanic("MyHelloWorldApp")

@app.get("/test/")
async def hello_test(request):
    return text("Hey test")

@app.get("/test/<name>")
async def hello_test_name(request, name):
    return text("Hello, " + name)


if __name__ == "__main__":
    app.run("localhost", 8000)

Expected behavior
A clear and concise description of what you expected to happen.

This backward incompatibility does not seem to be documented.

Environment (please complete the following information):

  • OS: [e.g. iOS] big sur
  • Version [e.g. 0.8.3] 21.9.3

Additional context
Add any other context about the problem here.

We'll appreciate helps on a way to keep the old behavior for a smooth transition.

@Tronic
Copy link
Member

Tronic commented Mar 15, 2022

This will be mostly fixed in the next release, with the parameters no longer matching empty strings by default. I recommend always setting Sanic("yourapp", strict_slashes=True) to avoid such issues with all Sanic versions.

@ZSSVE
Copy link
Author

ZSSVE commented Mar 16, 2022

This will be mostly fixed in the next release, with the parameters no longer matching empty strings by default. I recommend always setting Sanic("yourapp", strict_slashes=True) to avoid such issues with all Sanic versions.

Thank you for the reply! Since in pre-21.3 version, both /test/ and /test seem to route to the endpoint without parameter (hello_test), we might have existing callers using both. Just adding strict_slashes=True may break some of the callers. One way we are thinking about temporarily is to set strict_slashes=True and add a pair of routes (with and without trailing slash) for all our endpoints, but we are unsure if there are other subtle differences from the pre-21.3 behavior.

@ashleysommer
Copy link
Member

Yeah, this was a regression (or unintended change in behavior) in v21.3, caused by the introduction of the new Router, it was not corrected until recently. As @Tronic said, this will revert to the pre-21.3 behavior in the next release (v22.3), and you will not need to add strict_slashes=True.

@Tronic
Copy link
Member

Tronic commented Mar 16, 2022

Closing as duplicate of #2391 #2384 #2239

PR fixing the regression: sanic-org/sanic-routing#58
An open issue with relevant discussion: sanic-org/sanic-guide#92

@Tronic Tronic closed this as completed Mar 16, 2022
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

3 participants