werkzeug icon indicating copy to clipboard operation
werkzeug copied to clipboard

`part_isolating` not mentioned in Changes

Open encukou opened this issue 1 year ago • 3 comments

Hello, and thank you for your work on Werkzeug!


Since 2.2.0, custom URL variable converters that can match slashes need to set part_isolating = False. The docs were uptated, but in the Changes list this is mentioned only as “Add a new faster matching router based on a state machine. #2433”. It took some digging to find out why my app was breaking.


Reproducer (sorry if it isn't idiomatic, I use Flask on top):

from werkzeug.routing import BaseConverter

class TwoPartConverter(BaseConverter):

    #part_isolating = False   # add this line to fix on 2.2.0

    regex = r"(?:[a-z]+/[a-z]+)"

    def __init__(self, url_map):
        super().__init__(url_map)

    def to_python(self, value):
        return value.split('/')

    def to_url(self, value):
        return '/'.join(value)

from werkzeug.routing import Map, Rule

url_map = Map([
    Rule("/<twopart:test>/", endpoint="end"),
], converters={'twopart': TwoPartConverter})

urls = url_map.bind("example.com", "/")

print(urls.match('/category/item/'))

With Werkzeug 2.1.2:

('end', {'test': ['category', 'item']})

With Werkzeug 2.2.0:

Traceback (most recent call last):
  File "/tmp/repro.py", line 28, in <module>
    print(urls.match('/category/item/'))
  File "/tmp/fff/__venv__/lib64/python3.10/site-packages/werkzeug/routing/map.py", line 624, in match
    raise NotFound() from None
werkzeug.exceptions.NotFound: 404 Not Found: The requested URL was not found on the server. If you entered the URL manually please check your spelling and try again.

Environment:

  • Python version: 3.10.6
  • Werkzeug version: see above

encukou avatar Sep 06 '22 15:09 encukou

@pgjones Can we assume that if / appears in the regex, part_isolating should be set to False by default? That would cover the likely majority of cases.

davidism avatar Sep 06 '22 16:09 davidism

Yes, I think that would be fine. I'm not sure when/where we'd check in the code though?

pgjones avatar Sep 06 '22 19:09 pgjones

Something like this should work:

def __init_subclass__(cls, **kwargs):
    super().__init_subclass__(**kwargs)

    if "part_isolating" not in cls.__dict__:
        cls.part_isolating = "/" not in cls.regex

That way you can still set it manually for more complex cases.

davidism avatar Sep 06 '22 20:09 davidism