Falcon: Routes with trailing slash do not work under default API Options

[laurent-chriqui]

Hello,

It seems when using the current default for api.req_options.strip_url_path_trailing_slash which is False, adding a slash at the end of a route doesn't work.

I think I found out what the problem is.

Here is the code I tried:

import falcon

class StaticResource(object):

    def on_get(self, _: falcon.Request, resp: falcon.Response) -> None:
        resp.status = falcon.HTTP_200
        resp.content_type = 'text/plain'
        resp.body = 'static'

def create_app() -> falcon.API:
    api = falcon.API()
    api.req_options.strip_url_path_trailing_slash = False
    api.add_route('/static_route/', StaticResource())
    api.add_route('/static_route2', StaticResource())
    print(api._router._finder_src)
    return api

The finder_src compiled function is :

def find(path, return_values, patterns, converters, params):
    path_len = len(path)
    if path_len > 0:
        if path[0] == 'static_route':
            if path_len == 1:
                return return_values[0]
            return None
        if path[0] == 'static_route2':
            if path_len == 1:
                return return_values[1]
            return None
        return None
    return None

We see that the code generated for both routes is the same.
The path passed to this function when calling uri: "http://127.0.0.1:8000/static_route/" is : ['static_route', ''] which has a length of 2, leading the function to return None and therefore an HTTPError 404.
When calling uri: "http://127.0.0.1:8000/static_route", the function returns the correct route and leads to HTTP 200.

Therefore using the default option leads to an impossibility to have routes ending with a trailing slash and even worse, when using routes ending with a trailing slash, the route without it works...

[open-collective-bot[bot]]

Hi :wave:,

Thanks for using Falcon. The large amount of time and effort needed to
maintain the project and develop new features is not sustainable without
the generous financial support of community members like you.

Please consider helping us secure the future of the Falcon framework with a
one-time or recurring donation.

Thank you for your support!

[nZac]

Hi @laurent-chriqui, thanks for submitting a bug report, we really appreciate when it includes code!

I am not sure I understand what you expect. Note that, req_options.strip_url_path_trailing_slash controls how the Request segments are determined,
not how routing works, they are independent ideas.

Consider a route add_route('/home/', HomeResc()) and add_route('/home/{param}, Dashboard()).

How would you route a request to /home/data vs /home/ with strip_url_path_trailing_slash=False? Should /home/ go to HomeResc or should it go Dashboard? I think there is an argument it should go to Dashboard with param="" and that is what it does. Here is a quick example:

import falcon
import falcon.testing
import falcon.routing


class TestResource:
    def on_get(self, req, resp, **kwargs):
        resp.media = {'params': kwargs}


api = falcon.API()
api.add_route('/home/', TestResource())
api.add_route('/home/{id}', TestResource())

client = falcon.testing.TestClient(api)

api.req_options.strip_url_path_trailing_slash = False
assert client.simulate_get('/home').json['params'] == {}
assert client.simulate_get('/home/').json['params'] == {'id': ''}
assert client.simulate_get('/home/data').json['params'] == {'id': 'data'}

This changes slightly with strip_url_path_trailing_slash=True:

# ... same as above

api.req_options.strip_url_path_trailing_slash = True
assert client.simulate_get('/home').json['params'] == {}
assert client.simulate_get('/home/').json['params'] == {}.  # Note, we don't get `id`
assert client.simulate_get('/home/data').json['params'] == {'id': 'data'}

This might point to us needing to raise when there is a trailing slash passed to add_route since it is ultimately meaningless.

Morale of the story strip_url_path_trailing_slash is about the Request handling not about the routers behavior.

I am going to mark this as needing documentation here. If the other maintainers think there are code level changes for this... or if I am patently wrong (that happens far more often than I like!) please let me know!

Bonus... if you would like it to be different you can change it! 🎉

Bonus 2... If you are actually trying to serve statics, you might want to look at add_static_route. If it doesn't meet your needs, let us know, we might be able to add features!

Bonus 3.... Thanks again for submitting this!

[laurent-chriqui]

Hi,

Thank you for taking this issue into consideration and for exposing the reasoning behind this behavior.

It would seem that changing the option req_options.strip_url_path_trailing_slash would not prevent the developper to define a route ending with a forward slash. I understand the tricky part about the routes such as '/home/{id}' but as you pointed it out, the problem remains when the option is set to True.

In my opinion, if a route ending with a forward slash is explicitly defined ('/home/', it should override a route such as '/home/{id}' and it is up to the developper to not have colliding routes, or to decide that if the parameter is null, it should point to a different ressource and define it using '/home/'.

It would seem to me that going through the hassle of defining a custom router to achieve this is a little overkill...

Again, thank you very much for looking into it and I look forward to hearing your opinion on what I've just written.

[nZac]

'/home/', it should override a route such as '/home/{id}'

That isn't the expected behavior from my perspective. /home/{id} is _more_ specific than /home/ not less so overriding doesn't seem to be a good option.

Regardless, it seems to me you want the forward-slash character to have special meaning for the router, but it doesn't. The / character is just for delineating route parts and we strip the trailing slash off in the default CompiledRouter. note the .strip and .split calls, that is all we are doing to get the parts.

_Routing_ compilation is independent from the _Request_ parsing and req_options.strip_url_path_trailing_slash is about the Request.

Maybe we could not do that .strip call if this flag is set to False, but I don't think we should conflate those two. In my opinion, it would be _more preferable_ to render a warning that trailing / is a no-op. However, the _most preferable_ (in my opinion) is to leave it the way it is.

Maybe we could add this to the CompiledRouterOptions (here) allowing support for trailing slashes? If you wanted to take a stab at that and submit a PR, we could take a look at that implementation.

Thanks again, happy to continue dialoging and thanks for using Falcon!

If you find Falcon useful, consider helping the project but submitting a PR for any of the open issues we have lots of good first issue items that can be picked up or consider financial support!

[laurent-chriqui]

You are right that the documentation should be updated if things are left the way they are because the doc leads us to believe that we have the possibility to define routes ending with a trailing slash (here)

For example, with this option enabled, adding a route for '/foo/bar' implicitly adds a route for '/foo/bar/'. In other words, requests coming in for either path will be sent to the same resource.

[vishalrp]

Hello, we experienced this issue as well. We were on version 1.4.1 and one of the routes was defined with a / in the end. It used to work with and without the slash when queried. However, after upgrading, querying with a / returns a 404.

[vytas7]

@vishalrp You can easily bring back the 1.4.1 behaviour by setting strip_url_path_trailing_slash to True in your application's request options.

It is a much bigger issue when the new default behaviour is desired, i.e., one wants to differentiate between the two routes, with and without slash, e.g., in a case described by the strip_url_path_trailing_slash docs:

However, this behavior can be problematic in certain cases, such as when working with authentication schemes that employ URL-based signatures.

As also demonstrated by @laurent-chriqui , we now simply fail on this premise:

>>> import falcon
>>> import falcon.testing
>>> class Responder:
...     def on_get(self, req, resp):
...         pass
... 
>>> app = falcon.API()
>>> app.add_route('/my/uri/example', Responder())
>>> 
>>> app.add_route('/my/uri/example/', Responder())
>>> client = falcon.testing.TestClient(app)
>>> client.simulate_get('/my/uri/example').status_code
200
>>> client.simulate_get('/my/uri/example/').status_code
404

To summarize, I am convinced this is bug, and a fairly serious one for users that do need to tell whether there was a trailing slash or not. Others can define routes without trailing slashes, and/or enable strip_url_path_trailing_slash as needed.

[vytas7]

Added to the excellent analysis by @laurent-chriqui , we seem to always strip trailing slashes inside CompiledRouter.add_route:

        path = uri_template.strip('/').split('/')

When fixing this, have the special / case in mind (there cannot be a route without a trailing slash for /) though.

Also watch out for conflicts between /home/ and /home/{id}, as also discussed by @nZac .
OTOH I hope the current logic should just work for this /home/ and /home/{id}, as it does for /home/kitchen and /home/{id}, except there would be an empty string in lieu of kitchen:

>>> class Responder:
...     def on_get(self, req, resp, **kwargs):
...         resp.media = {'kwargs': kwargs}
... 
>>> app = falcon.API()
>>> app.add_route('/home/kitchen', Responder())
>>> app.add_route('/home/{id}', Responder())
>>> client = falcon.testing.TestClient(app)
>>> client.simulate_get('/home/kitchen').json
{'kwargs': {}}
>>> client.simulate_get('/home/file001').json
{'kwargs': {'id': 'file001'}}