falconry / falcon

The no-magic web data plane API and microservices framework for Python developers, with a focus on reliability, correctness, and performance at scale.
https://falcon.readthedocs.io/en/stable/
Apache License 2.0
9.51k stars 937 forks source link

Doc not up to date or unclear on trailing slash with routing #1929

Closed Stargateur closed 2 years ago

Stargateur commented 3 years ago

The doc say:

Also, note that if the template contains a trailing slash character, it will be stripped in order to normalize the routing logic. https://falcon.readthedocs.io/en/stable/api/routing.html#routing-utilities

https://github.com/falconry/falcon/issues/1544 seem to show the behavior change in 2019, https://github.com/falconry/falcon/pull/1751 seem to have fix a bug or to add the feature.

Currently I use falcon 3.0.1 and /version and /version/ are not considerate the same route contrary to what the doc say, I was really wondering why I did wrong.

import falcon

app = falcon.App()

class Version:
    def on_get(self, req, resp):
        resp.content_type = falcon.MEDIA_JSON
        resp.media = {
            "major": 0,
            "minor": 1,
            "patch": 0
        }
        resp.status = falcon.HTTP_200

app.add_route('/version', Version())

from wsgiref.simple_server import make_server
with make_server('', 8000, app) as httpd:
    print('Serving on port 8000...')
    httpd.serve_forever()
Serving on port 8000...
127.0.0.1 - - [05/Jul/2021 16:22:44] "GET /version HTTP/1.1" 200 36
127.0.0.1 - - [05/Jul/2021 16:22:49] "GET /version/ HTTP/1.1" 404 26

Can you clarify this ? This default is odd specially when it's the contrary to previous version and very few usage case.

Bonus Solution for other like me:

app = falcon.App()
app.req_options.strip_url_path_trailing_slash = True
vytas7 commented 3 years ago

Hi @Stargateur! The default was changed in Falcon 2.0, and as you noticed, there were a couple of severe bugs fixed later.

Re the default value, it's at times hard to find a setting that pleases everyone, that's why these things are configurable using req_options, resp_options, etc. In general, the majority of defaults are tuned to avoid any surprises and to do as little behind the user's back by default. Preserving paths as-is fits that definition, although I do realize that in quite many cases you want to strip the trailing slash.

Re documentation, you are correct that it is indeed out of date in some places :grimacing:

vytas7 commented 3 years ago

In addition, in the time being you can refer to this FAQ item (which is up to date): How does Falcon handle a trailing slash in the request path?

Stargateur commented 3 years ago

the majority of defaults are tuned to avoid any surprises and to do as little behind the user's back by default. Preserving paths as-is fits that definition

I think the most majority of internet does follow the slash and no slash is the same, https://developers.google.com/search/blog/2010/04/to-slash-or-not-to-slash. I'm clearly agree with you about default should avoid surprise but in this case it's a big surprise to me that slash was taken in account in routing. But I'm not a web dev.

That said I'm not here to change this.

In addition, in the time being you can refer to this FAQ item (which is up to date): How does Falcon handle a trailing slash in the request path?

Yeah I find it in the end, it would be nice to have the snipped I write to help user to change the option cause for me (not a python dev) it's very unclear that I must override the value after the initialization of App. Hope I'm clear.

vytas7 commented 3 years ago

Right; the docs clearly need improvement.

Re the default value itself, you might have a point. I wasn't involved in the original change in 2.0, but @kgriffs writes on https://github.com/falconry/falcon/issues/1267:

Based on feedback I've gotten from the community, it appears that people generally expect the trailing slash to be preserved and can run into hard-to-debug issues when it is not, so let's toggle the default.

However, regardless of what is a better option, we might be reluctant to flip this once again.

treharne commented 2 years ago

In case it helps others arriving here before this is addressed:

The wording " Also, note that if the template contains a trailing slash character, it will be stripped in order to normalize the routing logic. " in the docs is correct, but is specific to compile_uri_template, and does not apply to add_route.

The add_route function used in the OP example does not strip trailing slash by default. However, you can change its behaviour using strip_url_path_trailing_slash