Repository: pallets/werkzeug (python)

Issue:
app.route mixes up parameters passed when multiple regex patterns are used
We recently upgraded from Flask v1.1.2 to Flask v2.2.2 and were surprised when many of our project unit tests started failing because Flask v2.2.2 is not passing the correct parameters to a route function.  This worked correctly in the previous v1.1.2 which we were using but no longer works properly in v2.2.2.

The following python code example of a route which passes 3 parameters with regex expressions will replicate the bug:

```python
ALLOWED_METHODS = ["GET", "HEAD"]
DATE_PATTERN = "((\d{8}T\d{6}([.,]\d{1,3})?)|(\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}([.,]\d{1,3})?))Z"
# Note that 2 date formats are supported using this date pattern
# if you want to see what this regex does, paste it into https://regexper.com/

@app.route('/<regex("\d+"):source_id>/'
        + '<regex("' + DATE_PATTERN + '"):start_time>/'
        + '<regex("' + DATE_PATTERN + '"):end_time>', methods=ALLOWED_METHODS)
def clipslist_id(source_id, start_time, end_time):
    print("clipslist_id parameters source_id {} start_time {} and end time {}".format(source_id, start_time, end_time))
    print("Valid parameters {}".format(end_time > start_time))
    return
```

Call the app.get function and pass in a uri such one of the following (two date formats are supported):

```
/22/2023-02-16T23:36:36.266Z/2023-02-16T23:46:36.266Z
/22/20230217T002724.998Z/20230217T003724.998Z
```

Note that in both of the example uri's above the 3rd datetime parameter value is 10 minutes after the 2nd datetime parameter value
When the uri gets routed to clipslist_id the function receives 3 parameters, but the 3rd parameter is not correct.  Instead of passing through the correct 3rd parameter the route code passes the 2nd parameter twice, once as the 2nd parameter, and again as the 3rd parameter.

The three parameters received by the function should have had the values for: source_id, start_time, and end_time
Instead the three parameters that the function received were: source_id, start_time, start_time
The end_time value was not passed to the function

Environment:
Python 3.8.10 (default, Nov 14 2022, 12:59:47)
Flask 2.2.2

Distributor ID:	Ubuntu
Description:	Ubuntu 20.04.5 LTS
Release:	20.04
Codename:	focal
5.15.0-58-generic


Pull request:
Allow the router to cope with nested groups
The previous, regex table, version of the router wrapped each converter's regex in a named capturing group which was then extracted from the match and passed to the converter as the value with the name matching the rule variable name. This meant that any nested groups within that named group were ignored.

The new, state machine, version of the router couldn't use this approach as the variable names are likely unique and hence the same rule with different variable names would have seperate states, rather than the same. Therefore the state machine version simply took the groups in order and passed them to the converters in order.

The requirement that the groups be in order means that nested groups cannot be used, as the nested groups are not ignored and in fact result in more group matches than there are converters.

As Werkzeug/Flask did support nested groups (although not directly as they aren't used in Werkzeug/Flask) it makes sense to restore this support. To do so I've used the previous approach with a generic name `__werkzeug_#` that is unlikely to conflict with any user defined name. Then only matching only the named groups (with the `__werkzeug_#` prefix) are passed to the converters in the order given by the name i.e. `__werkzeug_0` for the first converter in the part, `__werkzeug_1` for the second etc... This numbering is required as the match groupdict() order is not guaranteed anywhere.

<!--
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 #2590

<!--
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 84757d0c..707cf8c9 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -21,6 +21,8 @@ Unreleased
     :issue:`2550`
 -   A URL converter's ``part_isolating`` defaults to ``False`` if its ``regex`` contains
     a ``/``. :issue:`2582`
+-   A custom converter's regex can have capturing groups without breaking the router.
+    :pr:`2596`
 -   The reloader can pick up arguments to ``python`` like ``-X dev``, and does not
     require heuristics to determine how to reload the command. Only available
     on Python >= 3.10. :issue:`2589`
@@ -28,6 +30,7 @@ Unreleased
     Watchdog to 2.3.0. :issue:`2603`
 
 
+
 Version 2.2.3
 -------------
 
diff --git a/src/werkzeug/routing/matcher.py b/src/werkzeug/routing/matcher.py
index 05370c3e..1a2b7596 100644
--- a/src/werkzeug/routing/matcher.py
+++ b/src/werkzeug/routing/matcher.py
@@ -127,13 +127,21 @@ class StateMachineMatcher:
                     remaining = []
                 match = re.compile(test_part.content).match(target)
                 if match is not None:
-                    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()
+                        suffix = match.groups()[-1]
                         if suffix == "/":
                             remaining = [""]
+
+                    converter_groups = sorted(
+                        match.groupdict().items(), key=lambda entry: entry[0]
+                    )
+                    groups = [
+                        value
+                        for key, value in converter_groups
+                        if key[:11] == "__werkzeug_"
+                    ]
                     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 c9600842..9298102e 100644
--- a/src/werkzeug/routing/rules.py
+++ b/src/werkzeug/routing/rules.py
@@ -588,6 +588,7 @@ class Rule(RuleFactory):
         argument_weights = []
         static_weights: t.List[t.Tuple[int, int]] = []
         final = False
+        convertor_number = 0
 
         pos = 0
         while pos < len(rule):
@@ -614,7 +615,8 @@ class Rule(RuleFactory):
                 self.arguments.add(data["variable"])
                 if not convobj.part_isolating:
                     final = True
-                content += f"({convobj.regex})"
+                content += f"(?P<__werkzeug_{convertor_number}>{convobj.regex})"
+                convertor_number += 1
                 argument_weights.append(convobj.weight)
                 self._trace.append((True, data["variable"]))
 
@@ -643,6 +645,7 @@ class Rule(RuleFactory):
                     argument_weights = []
                     static_weights = []
                     final = False
+                    convertor_number = 0
 
             pos = match.end()
 


Test patch:
diff --git a/tests/test_routing.py b/tests/test_routing.py
index 735bd07e..a0ffc145 100644
--- a/tests/test_routing.py
+++ b/tests/test_routing.py
@@ -778,6 +778,35 @@ def test_converter_with_tuples():
     assert kwargs["foo"] == ("qwert", "yuiop")
 
 
+def test_nested_regex_groups():
+    """
+    Regression test for https://github.com/pallets/werkzeug/issues/2590
+    """
+
+    class RegexConverter(r.BaseConverter):
+        def __init__(self, url_map, *items):
+            super().__init__(url_map)
+            self.part_isolating = False
+            self.regex = items[0]
+
+    # This is a regex pattern with nested groups
+    DATE_PATTERN = r"((\d{8}T\d{6}([.,]\d{1,3})?)|(\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}([.,]\d{1,3})?))Z"  # noqa: B950
+
+    map = r.Map(
+        [
+            r.Rule(
+                f"/<regex('{DATE_PATTERN}'):start>/<regex('{DATE_PATTERN}'):end>/",
+                endpoint="handler",
+            )
+        ],
+        converters={"regex": RegexConverter},
+    )
+    a = map.bind("example.org", "/")
+    route, kwargs = a.match("/2023-02-16T23:36:36.266Z/2023-02-16T23:46:36.266Z/")
+    assert kwargs["start"] == "2023-02-16T23:36:36.266Z"
+    assert kwargs["end"] == "2023-02-16T23:46:36.266Z"
+
+
 def test_anyconverter():
     m = r.Map(
         [