bug: route without a trailing slash gets routed to route with a trailing slash, when the route is the top level of blueprint (maybe also app)

[artcg]

Version: 21.3.1

Description:

setting two urls in a blueprint with strict slashes for "" and "/" will raise an exception "route already registered" in sanic 21.3.1

In sanic version before 21.3 this would be acceptable

Code to reproduce:

from sanic import Sanic, Blueprint
from sanic.response import text

app = Sanic("test")

bp = Blueprint("test", url_prefix="test")
@bp.get("", strict_slashes=True)
def _(req):
    return text("a")

@bp.get("/", strict_slashes=True)  # causes exception on sanic 21.3
def _(req):
    return text("b")

app.blueprint(bp)
app.run()

potential fix: #2079

[artcg]

Probably needs a bugfix release since routes like /api/example will stop working in 21.3.1 when strict_slashes is set

[ashleysommer]

Hi @argaen
I can see why routes "" and "/" cause the bug, and I understand why you might have a niche requirement to have a route "" with strict_slashes=True, and how your PR fixes that feature.

But I don't understand your second comment, how would a route like "/api/example" be affected by this bug?

[ashleysommer]

And are you sure this is just Blueprints? Wouldn't this also affect app routes?

app = Sanic("test")
@app.get("", strict_slashes=True)
def _(req):
    return text("a")

@app.get("/", strict_slashes=True)  # <- does this cause exception?
def _(req):
    return text("b")

Or is it something to do with how blueprints prepend url_prefix?

[artcg]

Like this example

from sanic import Sanic, Blueprint
from sanic.response import text

app = Sanic("test")

bp = Blueprint("test", url_prefix="test")
@bp.get("", strict_slashes=True)
def _(req):
    return text("a")

app.blueprint(bp)
app.run()

Old:

http://localhost:8000/test -> works

New:

http://localhost:8000/test --> does not work

The problem is the line illustrated in the fix turning "" into "/" when its an empty string

[artcg]

Im not too sure on the logic between the difference between the host root path with the slash or not, like

localhost:8000 vs localhost:8000/

I don't know if in networking terms there is a difference, so can't answer on whether it should affect app or not with no url_prefix

[ashleysommer]

Ok, yep I absolutely see how this is an issue on a blueprint when the route is "" and strict_slashes is True. But I'm still struggling to understand what you mean when you said it would affect a route like "/api/example". Your follow-up example to demonstrate still uses route of "".

Putting that aside, it seems like we need a condition like:

IF route is on a blueprint AND strict_slashes is True AND bp has url_prefix AND route is "" THEN
leave route as ""
ELSE
route = "/"

I will have to do some more testing to get to a correct solution for this.

[artcg]

Ok sounds good, will leave it to sanic core devs to decide about how to handle it ...

Probably a poor example on my end but basically with that I meant if "example" was a blueprint and "/api/example" route was then the example_blueprint.get("")

[artcg]

Feel free to close & replace my PR if its inadequate, I just made it in the case that it would be a quick fix

[ashleysommer]

Thinking about it a bit more:

• If route is "" and route is on the App, change "" to "/" even if strict_slashes is True. (because app-level route of "" is invalid)
• Or, if route is "" and route is on a Blueprint but bp url_prefix is (None or ""), then change "" to "/"
• in all other cases then add "/" to the start of the route if its not there, and if strict_slashes is False, make the route match with trailing slash and without trailing slash.

@ahopkins is the guru for everything regarding the new router, I'll leave it to him.
But thanks for bringing this to our attention!

[ahopkins]

I am looking at this now. Looks like @ashleysommer has the right idea.

Basically, this should work as they should be functionally equivalent:

app = Sanic(__name__)

bp = Blueprint("test", url_prefix="/bar")


@bp.get("", strict_slashes=True)
def bar(req):
    return text("bar")


@bp.get("/", strict_slashes=True)
def Bar(req):
    return text("Bar")


@app.get("/foo", strict_slashes=True)
def foo(request):
    return text("foo")


@app.get("/foo/", strict_slashes=True)
def Foo(request):
    return text("Foo")

[ahopkins]

Running some tests, but I think it is as simple as this:

if not uri.startswith("/") and (uri or hasattr(self, "router")):
    uri = "/" + uri

If there is a uri, we always want it to have a /. No other rules apply.

If uri is falsey (in this case it should only be "", I am fine with standard exceptions if you pass None), we always add it when the route is on the app instance, but never on the blueprint. We do not need to inspect strict_slashes or anything else here. All we care about is whether or not something else will be in front of this eventually or not.

---

Now, why am I using hasattr(self, "router")? I am not sure of the best way to handle this since the method is on the mixin because of circular imports. This seemed to me the most relevant property to test for.