fix Weighting calculation for path converters and add tests

Fix issue #2924 where Rules with dynamic elements would take priority
over rules with static elements of the same length.

The underlying issue was that the ordering for rules was backwards
with regard to number of argument weights. In order to ensure that
static elements match first the shortest rule must be tested first.

To ensure that a RulePart that contains a path converter does not
incorrectly take priority over other RuleParts that are part of other
Rules that should have priority, RuleParts containing a path converter
are assigned an infinite number of argument weights because they can
consume an arbitrary number of url path elements when matching.

With this change, two consecutive path converters give priority to the
first converter. In general two consecutive path converters with
different names cannot have consistent or predicatable behavior (where
would you cut?). Tests are updated accordingly. Might consider making
back to back path converters an error.

closes #2924

Author: tgbugs

src/werkzeug/routing/rules.py

@@ -623,6 +623,7 @@ def _parse_rule(self, rule: str) -> t.Iterable[RulePart]:
                 self._trace.append((False, data["static"]))
                 content += data["static"] if static else re.escape(data["static"])
 
+            haspath = False
             if data["variable"] is not None:
                 if static:
                     # Switching content to represent regex, hence the need to escape
@@ -640,6 +641,7 @@ def _parse_rule(self, rule: str) -> t.Iterable[RulePart]:
                 convertor_number += 1
                 argument_weights.append(convobj.weight)
                 self._trace.append((True, data["variable"]))
+                haspath = data["converter"] == "path"
 
             if data["slash"] is not None:
                 self._trace.append((False, "/"))
@@ -651,7 +653,7 @@ def _parse_rule(self, rule: str) -> t.Iterable[RulePart]:
                     weight = Weighting(
                         -len(static_weights),
                         static_weights,
-                        -len(argument_weights),
+                        float('+inf') if haspath else len(argument_weights),
                         argument_weights,
                     )
                     yield RulePart(
@@ -681,7 +683,7 @@ def _parse_rule(self, rule: str) -> t.Iterable[RulePart]:
         weight = Weighting(
             -len(static_weights),
             static_weights,
-            -len(argument_weights),
+            float('+inf') if haspath else len(argument_weights),
             argument_weights,
         )
         yield RulePart(

tests/test_routing.py

@@ -381,20 +381,60 @@ def test_greedy():
         [
             r.Rule("/foo", endpoint="foo"),
             r.Rule("/<path:bar>", endpoint="bar"),
-            r.Rule("/<path:bar>/<path:blub>", endpoint="bar"),
+            r.Rule("/<path:bar>/<blub>", endpoint="bar"),
+            r.Rule("/<baz>/static", endpoint="oops")
         ]
     )
     adapter = map.bind("example.org", "/")
 
     assert adapter.match("/foo") == ("foo", {})
     assert adapter.match("/blub") == ("bar", {"bar": "blub"})
     assert adapter.match("/he/he") == ("bar", {"bar": "he", "blub": "he"})
+    assert adapter.match("/he/static") == ("oops", {"baz": "he"})
 
     assert adapter.build("foo", {}) == "/foo"
     assert adapter.build("bar", {"bar": "blub"}) == "/blub"
     assert adapter.build("bar", {"bar": "blub", "blub": "bar"}) == "/blub/bar"
 
 
+def test_greedy_double_paths():
+    # two back to back paths do not have any meaning and should
+    # probably cause an error in _parse_rule
+    map = r.Map(
+        [
+            r.Rule("/foo", endpoint="foo"),
+            r.Rule("/<path:bar>", endpoint="bar"),
+            r.Rule("/<path:bar>/<path:blub>", endpoint="bar"),
+        ]
+    )
+    adapter = map.bind("example.org", "/")
+
+    assert adapter.match("/foo") == ("foo", {})
+    assert adapter.match("/blub") == ("bar", {"bar": "blub"})
+    assert adapter.match("/he/he") == ("bar", {"bar": "he/he"})
+    # this cannot match ("bar", {"bar": "he", "blub": "he"}) without breaking static matching
+    # users should use a rule like "/<path:bar/<blub>" instead
+
+    assert adapter.build("foo", {}) == "/foo"
+    assert adapter.build("bar", {"bar": "blub"}) == "/blub"
+    assert adapter.build("bar", {"bar": "blub", "blub": "bar"}) == "/blub/bar"
+
+
+def test_static_priority():
+    # see https://github.com/pallets/werkzeug/issues/2924
+    map = r.Map(
+        [
+            r.Rule("/<path:dyn2>/<dyn1>", endpoint="file"),
+            r.Rule("/<dyn1>/statn", endpoint="stat"),
+        ],
+    )
+
+    adapter = map.bind("example.org", "/")
+
+    assert adapter.match("/d2/d1", method="GET") ==  ('file', {'dyn2': 'd2', 'dyn1': 'd1'})
+    assert adapter.match("/d1/statn", method="GET") ==  ('stat', {'dyn1': 'd1'})
+
+
 def test_path():
     map = r.Map(
         [