Repository: falconry/falcon

Issue:
Routes with trailing slash do not work under default API Options
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:
```python
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 :
```python
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...

Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/falconry/falcon/pull/1751?src=pr&el=h1) Report
> Merging [#1751](https://codecov.io/gh/falconry/falcon/pull/1751?src=pr&el=desc) into [master](https://codecov.io/gh/falconry/falcon/commit/113eea113844058816ed745d8dc9cb197e75cb95&el=desc) will **not change** coverage.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/falconry/falcon/pull/1751/graphs/tree.svg?width=650&height=150&src=pr&token=zOtkfNQ3h6)](https://codecov.io/gh/falconry/falcon/pull/1751?src=pr&el=tree)

```diff
@@            Coverage Diff            @@
##            master     #1751   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           55        55           
  Lines         5037      5037           
  Branches       823       823           
=========================================
  Hits          5037      5037           
```


| [Impacted Files](https://codecov.io/gh/falconry/falcon/pull/1751?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [falcon/routing/compiled.py](https://codecov.io/gh/falconry/falcon/pull/1751/diff?src=pr&el=tree#diff-ZmFsY29uL3JvdXRpbmcvY29tcGlsZWQucHk=) | `100.00% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/falconry/falcon/pull/1751?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/falconry/falcon/pull/1751?src=pr&el=footer). Last update [113eea1...f084fb7](https://codecov.io/gh/falconry/falcon/pull/1751?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

CaselIT: Regarding the question, the problem I see is that we cannot assume that an user has set the options before adding the routes. Also the options may be dynamically updated.
Taking this into consideration, I don't think a warning should be raised, we should just document, maybe with a warning, the behaviour of the router wrt that request option.
vytas7: Ah, good point wrt the configuration order @CaselIT !
[`RequestOptions.strip_url_path_trailing_slash`](https://falcon.readthedocs.io/en/latest/api/app.html#falcon.RequestOptions.strip_url_path_trailing_slash) is by default `False`, so if the user hasn't set that to `True` yet, we obviously cannot do anything about it. But if the user has already overridden to `True`, I am inclined to emit a warning though, that might be helpful in debugging... :thinking:
CaselIT: The documentation looks good 👍
vytas7: @kgriffs good point; I have now added a dedicated test to better exercise this scenario.

Patch:
diff --git a/docs/_newsfragments/1544.breakingchange.rst b/docs/_newsfragments/1544.breakingchange.rst
new file mode 100644
index 0000000..8728bd5
--- /dev/null
+++ b/docs/_newsfragments/1544.breakingchange.rst
@@ -0,0 +1,6 @@
+If :attr:`RequestOptions.strip_url_path_trailing_slash
+<falcon.RequestOptions.strip_url_path_trailing_slash>` is enabled, routes
+should now be added without a trailing slash. Previously, the trailing slash
+was always removed as a side effect of a bug regardless of the
+:attr:`~falcon.RequestOptions.strip_url_path_trailing_slash` option value.
+See also: :ref:`trailing_slash_in_path`
diff --git a/docs/_newsfragments/1544.bugfix.rst b/docs/_newsfragments/1544.bugfix.rst
new file mode 100644
index 0000000..8810830
--- /dev/null
+++ b/docs/_newsfragments/1544.bugfix.rst
@@ -0,0 +1,6 @@
+Previously, the default :class:`CompiledRouter <falcon.routing.CompiledRouter>`
+was erroneously stripping trailing slashes from URI templates.
+This has been fixed so that it is now possible to add two different routes for
+a path with and without a trailing forward slash (see also:
+:attr:`RequestOptions.strip_url_path_trailing_slash
+<falcon.RequestOptions.strip_url_path_trailing_slash>`).
diff --git a/docs/_newsfragments/1594.bugfix.rst b/docs/_newsfragments/1594.bugfix.rst
index 814c057..2ff0112 100644
--- a/docs/_newsfragments/1594.bugfix.rst
+++ b/docs/_newsfragments/1594.bugfix.rst
@@ -1,3 +1,3 @@
 :meth:`falcon.uri.decode` and :meth:`falcon.uri.parse_query_string` no longer
 explode quadratically for a large number of percent-encoded characters. The
-time complexity of these utility functions is now always close to *O*(*n*).
+time complexity of these utility functions is now always close to *O*\(*n*).
diff --git a/docs/user/faq.rst b/docs/user/faq.rst
index 0988d96..941689e 100644
--- a/docs/user/faq.rst
+++ b/docs/user/faq.rst
@@ -478,6 +478,8 @@ being fully defined by the WSGI spec (PEP-3333). This is discussed in the
 reference documentation for :attr:`~falcon.Request.stream`, and a workaround
 is provided in the form of :attr:`~falcon.Request.bounded_stream`.
 
+.. _trailing_slash_in_path:
+
 How does Falcon handle a trailing slash in the request path?
 ------------------------------------------------------------
 If your app sets :attr:`~falcon.RequestOptions.strip_url_path_trailing_slash` to
@@ -491,6 +493,16 @@ 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.
 
+.. warning::
+
+    If :attr:`~falcon.RequestOptions.strip_url_path_trailing_slash` is enabled,
+    adding a route with a trailing slash will effectively make it unreachable
+    from normal routing (theoretically, it may still be matched by rewriting
+    the request path in middleware).
+
+    In this case, routes should be added without a trailing slash (obviously
+    except the root path ``'/'``), such as ``'/foo/bar'`` in the example above.
+
 .. note::
 
     Starting with version 2.0, the default for the
diff --git a/falcon/app.py b/falcon/app.py
index 9c376d5..f3d5a25 100644
--- a/falcon/app.py
+++ b/falcon/app.py
@@ -456,6 +456,13 @@ class App:
 
                 (See also: :meth:`~.App.add_sink`)
 
+                Warning:
+                    If :attr:`~falcon.RequestOptions.strip_url_path_trailing_slash`
+                    is enabled, `uri_template` should be provided without a
+                    trailing slash.
+
+                    (See also: :ref:`trailing_slash_in_path`)
+
             resource (instance): Object which represents a REST
                 resource. Falcon will pass GET requests to ``on_get()``,
                 PUT requests to ``on_put()``, etc. If any HTTP methods are not
diff --git a/falcon/routing/compiled.py b/falcon/routing/compiled.py
index b319884..9bbcb77 100644
--- a/falcon/routing/compiled.py
+++ b/falcon/routing/compiled.py
@@ -198,7 +198,7 @@ class CompiledRouter:
         if re.search(r'\s', _FIELD_PATTERN.sub('{FIELD}', uri_template)):
             raise ValueError('URI templates may not include whitespace.')
 
-        path = uri_template.strip('/').split('/')
+        path = uri_template.lstrip('/').split('/')
 
         used_names = set()
         for segment in path:


Test patch:
diff --git a/tests/test_httperror.py b/tests/test_httperror.py
index 75adc65..55f620f 100644
--- a/tests/test_httperror.py
+++ b/tests/test_httperror.py
@@ -605,7 +605,7 @@ class TestHTTPError:
 
     def test_405_without_body_with_extra_headers_double_check(self, client):
         client.app.add_route(
-            '/405/', MethodNotAllowedResourceWithHeadersWithAccept()
+            '/405', MethodNotAllowedResourceWithHeadersWithAccept()
         )
 
         response = client.simulate_request(path='/405')
diff --git a/tests/test_uri_templates.py b/tests/test_uri_templates.py
index 661edb4..8916f1d 100644
--- a/tests/test_uri_templates.py
+++ b/tests/test_uri_templates.py
@@ -82,6 +82,18 @@ class FileDetailsResource:
         self.called = True
 
 
+class KitchenSinkResource:
+    def __init__(self):
+        self.call_count = 0
+        self.kwargs = None
+        self.uri_template = None
+
+    def on_get(self, req, resp, **kwargs):
+        self.call_count += 1
+        self.kwargs = kwargs
+        self.uri_template = req.uri_template
+
+
 class ResourceWithSuffixRoutes:
     def __init__(self):
         self.get_called = False
@@ -349,21 +361,22 @@ def test_converter_custom(client, resource, uri_template, path, expected):
 def test_single_trailing_slash(client):
     resource1 = IDResource()
     client.app.add_route('/1/{id}/', resource1)
-    result = client.simulate_get('/1/123')
+    assert client.simulate_get('/1/123').status_code == 404
+    result = client.simulate_get('/1/123/')
     assert result.status == falcon.HTTP_200
     assert resource1.called
     assert resource1.id == '123'
-    assert resource1.req.path == '/1/123'
+    assert resource1.req.path == '/1/123/'
 
     resource2 = IDResource()
     client.app.add_route('/2/{id}/', resource2)
-    result = client.simulate_get('/2/123/')
+    result = client.simulate_get('/2/123')
     assert result.status == falcon.HTTP_404
     assert not resource2.called
     assert resource2.id is None
 
     resource3 = IDResource()
-    client.app.add_route('/3/{id}/', resource3)
+    client.app.add_route('/3/{id}', resource3)
     client.app.req_options.strip_url_path_trailing_slash = True
     result = client.simulate_get('/3/123/')
     assert result.status == falcon.HTTP_200
@@ -475,6 +488,34 @@ def test_adding_suffix_routes(client):
     assert resource_with_suffix_routes.collection_id == 'foo321'
 
 
+@pytest.mark.parametrize('reverse', [True, False])
+def test_with_and_without_trailing_slash(client, reverse):
+    routes = [
+        ('/kitchen', KitchenSinkResource()),
+        ('/kitchen/', KitchenSinkResource()),
+        ('/kitchen/{item}', KitchenSinkResource()),
+        ('/kitchen/{item}/', KitchenSinkResource()),
+        ('/kitchen/sink', KitchenSinkResource()),
+        ('/kitchen/sink/', KitchenSinkResource()),
+    ]
+    if reverse:
+        routes.reverse()
+
+    for route in routes:
+        client.app.add_route(*route)
+
+    for uri_template, resource in routes:
+        item = None
+        if '{item}' in uri_template:
+            item = 'kettle' if uri_template.endswith('/') else 'teapot'
+        resp = client.simulate_get(uri_template.replace('{item}', item or ''))
+
+        assert resp.status_code == 200
+        assert resource.call_count == 1
+        assert resource.kwargs.get('item') == item
+        assert resource.uri_template == uri_template
+
+
 def test_custom_error_on_suffix_route_not_found(client):
     resource_with_suffix_routes = ResourceWithSuffixRoutes()
     with pytest.raises(SuffixedMethodNotFoundError):