Repository: pallets/werkzeug (python)

Issue:
path converter doesn't redirect to trailing slash
It seems to me that the automatic redirect for URLs with trailing shlashes (like `/user` → `/user/`) described in [Flask docs](https://flask.palletsprojects.com/en/2.2.x/quickstart/#unique-urls-redirection-behavior) and [Werkzeug docs](https://werkzeug.palletsprojects.com/en/2.2.x/routing/#rule-format) does not work if you use PathConverter in an URL definition.

I have this straightforward Flask application to reproduce the problem:
```
from flask import Flask, Response

app = Flask(__name__)

@app.route('/')
@app.route('/<path:username>/')
def main(username=None):
    return Response(f"It works! Hello, {username}")
```
In werkzeug 2.1.2, it works as expected and if you send a request to `/user`, it redirects you via HTTP/308 to `/user/`. The log is:
```
* Running on http://127.0.0.1:5000 (Press CTRL+C to quit)
127.0.0.1 - - [12/Oct/2022 14:47:37] "GET / HTTP/1.1" 200 -
127.0.0.1 - - [12/Oct/2022 14:47:45] "GET /user HTTP/1.1" 308 -
127.0.0.1 - - [12/Oct/2022 14:47:45] "GET /user/ HTTP/1.1" 200 -
```
But when I use werkzeug 2.2.0, the same request gives me HTTP/404:
```
 * Running on http://127.0.0.1:5000 (Press CTRL+C to quit)
127.0.0.1 - - [12/Oct/2022 14:48:43] "GET /fooo HTTP/1.1" 404 -
```
I've tried a bisection of all the commits between those versions to see whether this was an intentional change or not and find that the difference has been introduced in: https://github.com/pallets/werkzeug/commit/5a7c0dec029d296a5e8131c8db51ce19e28c0c58

The workaround for this is to define both possible URLs for a single endpoint but it's not pretty. Also, the difference might cause problems if you still support Python 3.6 because pip installs older Flask and werkzeug for Python 3.6 where the redirect works fine and newer packages for Python 3.7+ where it does not.

Is this behavior intentional and if so, could you please add it to the documentation?

Pull request:
Fix strict slash redirects for final RuleParts
Final RuleParts, for example ones with a path converter will consume the remaining request target trying to match. This would raise NotFound if the rule required a trailing slash and the request target was missing one. Now, with this fix, it will redirect instead.

This is implemented via a suffix group and marker to final RuleParts. The suffix group will optionally allow for the trailing slash to be present, and if missing (and required) will trigger a redirect response.

Note that rules like `/<path:path>` and `/<path:path>/` are not distinguishable as the former matches the latter. Hence the strict_slashes False behavior for other rules does not apply.

<!--
Before opening a PR, open a ticket describing the issue or feature the
PR will address. Follow the steps in CONTRIBUTING.rst.

Replace this comment with a description of the change. Describe how it
addresses the linked ticket.
-->

<!--
Link to relevant issues or previous PRs, one per line. Use "fixes" to
automatically close an issue.
-->

- fixes #2533

<!--
Ensure each step in CONTRIBUTING.rst is complete by adding an "x" to
each box below.

If only docs were changed, these aren't relevant and can be removed.
-->

Checklist:

- [x] Add tests that demonstrate the correct behavior of the change. Tests should fail without the change.
- [x] Add or update relevant docs, in the docs folder and in code.
- [x] Add an entry in `CHANGES.rst` summarizing the change and linking to the issue.
- [x] Add `.. versionchanged::` entries in any relevant code docs.
- [x] Run `pre-commit` hooks and fix any issues.
- [x] Run `pytest` and `tox`, no tests failed.


Patch:
diff --git a/CHANGES.rst b/CHANGES.rst
index 161b14fa..86d3a63e 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -5,6 +5,8 @@ Version 2.2.3
 
 Unreleased
 
+-   Ensure that URL rules using path converters will redirect with strict slashes when
+    the trailing slash is missing. :issue:`2533`
 -   Type signature for ``get_json`` specifies that return type is not optional when
     ``silent=False``. :issue:`2508`
 -   ``parse_content_range_header`` returns ``None`` for a value like ``bytes */-1``
@@ -16,6 +18,7 @@ Unreleased
 -   Fix handling of header extended parameters such that they are no longer quoted.
     :issue:`2529`
 
+
 Version 2.2.2
 -------------
 
diff --git a/src/werkzeug/routing/matcher.py b/src/werkzeug/routing/matcher.py
index d22b05a5..05370c3e 100644
--- a/src/werkzeug/routing/matcher.py
+++ b/src/werkzeug/routing/matcher.py
@@ -127,7 +127,14 @@ class StateMachineMatcher:
                     remaining = []
                 match = re.compile(test_part.content).match(target)
                 if match is not None:
-                    rv = _match(new_state, remaining, values + list(match.groups()))
+                    groups = list(match.groups())
+                    if test_part.suffixed:
+                        # If a part_isolating=False part has a slash suffix, remove the
+                        # suffix from the match and check for the slash redirect next.
+                        suffix = groups.pop()
+                        if suffix == "/":
+                            remaining = [""]
+                    rv = _match(new_state, remaining, values + groups)
                     if rv is not None:
                         return rv
 
diff --git a/src/werkzeug/routing/rules.py b/src/werkzeug/routing/rules.py
index a61717ad..7b37890a 100644
--- a/src/werkzeug/routing/rules.py
+++ b/src/werkzeug/routing/rules.py
@@ -36,6 +36,7 @@ class RulePart:
     content: str
     final: bool
     static: bool
+    suffixed: bool
     weight: Weighting
 
 
@@ -631,7 +632,11 @@ class Rule(RuleFactory):
                         argument_weights,
                     )
                     yield RulePart(
-                        content=content, final=final, static=static, weight=weight
+                        content=content,
+                        final=final,
+                        static=static,
+                        suffixed=False,
+                        weight=weight,
                     )
                     content = ""
                     static = True
@@ -641,6 +646,12 @@ class Rule(RuleFactory):
 
             pos = match.end()
 
+        suffixed = False
+        if final and content[-1] == "/":
+            # If a converter is part_isolating=False (matches slashes) and ends with a
+            # slash, augment the regex to support slash redirects.
+            suffixed = True
+            content = content[:-1] + "(?<!/)(/?)"
         if not static:
             content += r"\Z"
         weight = Weighting(
@@ -649,7 +660,17 @@ class Rule(RuleFactory):
             -len(argument_weights),
             argument_weights,
         )
-        yield RulePart(content=content, final=final, static=static, weight=weight)
+        yield RulePart(
+            content=content,
+            final=final,
+            static=static,
+            suffixed=suffixed,
+            weight=weight,
+        )
+        if suffixed:
+            yield RulePart(
+                content="", final=False, static=True, suffixed=False, weight=weight
+            )
 
     def compile(self) -> None:
         """Compiles the regular expression and stores it."""
@@ -665,7 +686,11 @@ class Rule(RuleFactory):
         if domain_rule == "":
             self._parts = [
                 RulePart(
-                    content="", final=False, static=True, weight=Weighting(0, [], 0, [])
+                    content="",
+                    final=False,
+                    static=True,
+                    suffixed=False,
+                    weight=Weighting(0, [], 0, []),
                 )
             ]
         else:


Test patch:
diff --git a/tests/test_routing.py b/tests/test_routing.py
index 15d25a7c..bfb191c3 100644
--- a/tests/test_routing.py
+++ b/tests/test_routing.py
@@ -163,6 +163,7 @@ def test_strict_slashes_redirect():
             r.Rule("/bar/", endpoint="get", methods=["GET"]),
             r.Rule("/bar", endpoint="post", methods=["POST"]),
             r.Rule("/foo/", endpoint="foo", methods=["POST"]),
+            r.Rule("/<path:var>/", endpoint="path", methods=["GET"]),
         ]
     )
     adapter = map.bind("example.org", "/")
@@ -170,6 +171,7 @@ def test_strict_slashes_redirect():
     # Check if the actual routes works
     assert adapter.match("/bar/", method="GET") == ("get", {})
     assert adapter.match("/bar", method="POST") == ("post", {})
+    assert adapter.match("/abc/", method="GET") == ("path", {"var": "abc"})
 
     # Check if exceptions are correct
     pytest.raises(r.RequestRedirect, adapter.match, "/bar", method="GET")
@@ -177,6 +179,9 @@ def test_strict_slashes_redirect():
     with pytest.raises(r.RequestRedirect) as error_info:
         adapter.match("/foo", method="POST")
     assert error_info.value.code == 308
+    with pytest.raises(r.RequestRedirect) as error_info:
+        adapter.match("/abc", method="GET")
+    assert error_info.value.new_url == "http://example.org/abc/"
 
     # Check differently defined order
     map = r.Map(
@@ -1434,6 +1439,9 @@ def test_strict_slashes_false():
         [
             r.Rule("/path1", endpoint="leaf_path", strict_slashes=False),
             r.Rule("/path2/", endpoint="branch_path", strict_slashes=False),
+            r.Rule(
+                "/<path:path>", endpoint="leaf_path_converter", strict_slashes=False
+            ),
         ],
     )
 
@@ -1443,6 +1451,14 @@ def test_strict_slashes_false():
     assert adapter.match("/path1/", method="GET") == ("leaf_path", {})
     assert adapter.match("/path2", method="GET") == ("branch_path", {})
     assert adapter.match("/path2/", method="GET") == ("branch_path", {})
+    assert adapter.match("/any", method="GET") == (
+        "leaf_path_converter",
+        {"path": "any"},
+    )
+    assert adapter.match("/any/", method="GET") == (
+        "leaf_path_converter",
+        {"path": "any/"},
+    )
 
 
 def test_invalid_rule():