Repository: pallets/werkzeug (python)

Issue:
default `part_isolating=False` if the converter regex contains a `/`
Users didn't know about the `Converter.part_isolating` change as part of the router speedup, so custom converters that matched slashes stopped working. It should be possible to detect whether it should be disabled in most cases, so users won't need to know about it.

If a converter's `regex` has a `/`, we can assume it should set `part_isolating = False` by default. This default can still be overridden by setting `part_isolating` manually.

```python
def __init_subclass__(cls):
    super().__init_subclass__()
    if "part_isolating" not in cls.__dict__:
        cls.part_isolating = "/" not in cls.regex
```

This is a little too aggressive, as a regex could have something like `[^/]`, but that's unlikely since that's already something the router itself is handling, and the router will still work if a converter sets it `False` when it doesn't need to.

Originally discussed in https://github.com/pallets/werkzeug/issues/2506#issuecomment-1238375362

Pull request:
default `part_isolating=False` if regex has `/`
fixes #2582 

The default is only set if the new converter class sets `regex` and doesn't set `part_isolating`. This way, a converter can override `part_isolating` if it knows better, and can inherit `regex` and not change `part_isolating` if it was set manually in the parent.

Removed `part_isolating` from all the built-in converters, since it's detected correctly with this new behavior.

I also noticed that the base regex, which `UnicodeConverter` also uses, is `[^/]+` instead of `.+`. It's not clear if the "not slash" is needed, all tests still pass if I change it. I noticed this because "not slash" is the corner case that would make `part_isolating` false unnecessarily. @pgjones will look into whether it's necessary with the new router.

Patch:
diff --git a/CHANGES.rst b/CHANGES.rst
index 1ca61776..13a1f4be 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -10,6 +10,8 @@ Unreleased
 -   ``Request.get_json()`` will raise a ``415 Unsupported Media Type`` error if the
     ``Content-Type`` header is not ``application/json``, instead of a generic 400.
     :issue:`2550`
+-   A URL converter's ``part_isolating`` defaults to ``False`` if its ``regex`` contains
+    a ``/``. :issue:`2582`
 
 
 Version 2.2.3
@@ -84,9 +86,8 @@ Released 2022-07-23
     debug console. :pr:`2439`
 -   Fix compatibility with Python 3.11 by ensuring that ``end_lineno``
     and ``end_col_offset`` are present on AST nodes. :issue:`2425`
--   Add a new faster URL matching router based on a state machine. If a custom converter
-    needs to match a ``/`` it must set the class variable ``part_isolating = False``.
-    :pr:`2433`
+-   Add a new faster matching router based on a state
+    machine. :pr:`2433`
 -   Fix branch leaf path masking branch paths when strict-slashes is
     disabled. :issue:`1074`
 -   Names within options headers are always converted to lowercase. This
diff --git a/docs/utils.rst b/docs/utils.rst
index 6afa4ab3..0d4e3391 100644
--- a/docs/utils.rst
+++ b/docs/utils.rst
@@ -23,8 +23,6 @@ General Helpers
 
 .. autofunction:: send_file
 
-.. autofunction:: send_from_directory
-
 .. autofunction:: import_string
 
 .. autofunction:: find_modules
diff --git a/src/werkzeug/routing/converters.py b/src/werkzeug/routing/converters.py
index bbad29d7..5c16414a 100644
--- a/src/werkzeug/routing/converters.py
+++ b/src/werkzeug/routing/converters.py
@@ -15,12 +15,24 @@ class ValidationError(ValueError):
 
 
 class BaseConverter:
-    """Base class for all converters."""
+    """Base class for all converters.
+
+    .. versionchanged:: 2.3
+        ``part_isolating`` defaults to ``False`` if ``regex`` contains a ``/``.
+    """
 
     regex = "[^/]+"
     weight = 100
     part_isolating = True
 
+    def __init_subclass__(cls, **kwargs: t.Any) -> None:
+        super().__init_subclass__(**kwargs)
+
+        # If the converter isn't inheriting its regex, disable part_isolating by default
+        # if the regex contains a / character.
+        if "regex" in cls.__dict__ and "part_isolating" not in cls.__dict__:
+            cls.part_isolating = "/" not in cls.regex
+
     def __init__(self, map: "Map", *args: t.Any, **kwargs: t.Any) -> None:
         self.map = map
 
@@ -51,8 +63,6 @@ class UnicodeConverter(BaseConverter):
     :param length: the exact length of the string.
     """
 
-    part_isolating = True
-
     def __init__(
         self,
         map: "Map",
@@ -86,8 +96,6 @@ class AnyConverter(BaseConverter):
         Value is validated when building a URL.
     """
 
-    part_isolating = True
-
     def __init__(self, map: "Map", *items: str) -> None:
         super().__init__(map)
         self.items = set(items)
@@ -113,7 +121,6 @@ class PathConverter(BaseConverter):
 
     regex = "[^/].*?"
     weight = 200
-    part_isolating = False
 
 
 class NumberConverter(BaseConverter):
@@ -124,7 +131,6 @@ class NumberConverter(BaseConverter):
 
     weight = 50
     num_convert: t.Callable = int
-    part_isolating = True
 
     def __init__(
         self,
@@ -186,7 +192,6 @@ class IntegerConverter(NumberConverter):
     """
 
     regex = r"\d+"
-    part_isolating = True
 
 
 class FloatConverter(NumberConverter):
@@ -210,7 +215,6 @@ class FloatConverter(NumberConverter):
 
     regex = r"\d+\.\d+"
     num_convert = float
-    part_isolating = True
 
     def __init__(
         self,
@@ -236,7 +240,6 @@ class UUIDConverter(BaseConverter):
         r"[A-Fa-f0-9]{8}-[A-Fa-f0-9]{4}-"
         r"[A-Fa-f0-9]{4}-[A-Fa-f0-9]{4}-[A-Fa-f0-9]{12}"
     )
-    part_isolating = True
 
     def to_python(self, value: str) -> uuid.UUID:
         return uuid.UUID(value)
diff --git a/src/werkzeug/routing/rules.py b/src/werkzeug/routing/rules.py
index a61717ad..b43de607 100644
--- a/src/werkzeug/routing/rules.py
+++ b/src/werkzeug/routing/rules.py
@@ -42,16 +42,16 @@ class RulePart:
 _part_re = re.compile(
     r"""
     (?:
-        (?P<slash>\/)                                 # a slash
+        (?P<slash>/)                                 # a slash
       |
-        (?P<static>[^<\/]+)                           # static rule data
+        (?P<static>[^</]+)                           # static rule data
       |
         (?:
           <
             (?:
               (?P<converter>[a-zA-Z_][a-zA-Z0-9_]*)   # converter name
               (?:\((?P<arguments>.*?)\))?             # converter arguments
-              \:                                      # variable delimiter
+              :                                       # variable delimiter
             )?
             (?P<variable>[a-zA-Z_][a-zA-Z0-9_]*)      # variable name
            >
diff --git a/src/werkzeug/utils.py b/src/werkzeug/utils.py
index 4ef58371..1a5df63a 100644
--- a/src/werkzeug/utils.py
+++ b/src/werkzeug/utils.py
@@ -352,7 +352,7 @@ def send_file(
 
     Never pass file paths provided by a user. The path is assumed to be
     trusted, so a user could craft a path to access a file you didn't
-    intend. Use :func:`send_from_directory` to safely serve user-provided paths.
+    intend.
 
     If the WSGI server sets a ``file_wrapper`` in ``environ``, it is
     used, otherwise Werkzeug's built-in wrapper is used. Alternatively,
@@ -562,10 +562,9 @@ def send_from_directory(
     If the final path does not point to an existing regular file,
     returns a 404 :exc:`~werkzeug.exceptions.NotFound` error.
 
-    :param directory: The directory that ``path`` must be located under. This *must not*
-        be a value provided by the client, otherwise it becomes insecure.
-    :param path: The path to the file to send, relative to ``directory``. This is the
-        part of the path provided by the client, which is checked for security.
+    :param directory: The directory that ``path`` must be located under.
+    :param path: The path to the file to send, relative to
+        ``directory``.
     :param environ: The WSGI environ for the current request.
     :param kwargs: Arguments to pass to :func:`send_file`.
 


Test patch:
diff --git a/tests/test_routing.py b/tests/test_routing.py
index 15d25a7c..a77d89f7 100644
--- a/tests/test_routing.py
+++ b/tests/test_routing.py
@@ -800,6 +800,20 @@ def test_any_converter_build_validates_value() -> None:
     assert str(exc.value) == "'invalid' is not one of 'patient', 'provider'"
 
 
+def test_part_isolating_default() -> None:
+    class TwoConverter(r.BaseConverter):
+        regex = r"\w+/\w+"
+
+        def to_python(self, value: str) -> t.Any:
+            return value.split("/")
+
+    m = r.Map(
+        [r.Rule("/<two:values>/", endpoint="two")], converters={"two": TwoConverter}
+    )
+    a = m.bind("localhost")
+    assert a.match("/a/b/") == ("two", {"values": ["a", "b"]})
+
+
 @pytest.mark.parametrize(
     ("endpoint", "value", "expect"),
     [