Django URL patterns broken in 1.32.0

[nijel]

How do you use Sentry?

Self-hosted/on-premise

Version

1.32.0

Steps to Reproduce

1. Upgrade to 1.32.0
2. Access URL with "translate/<object_path:path>/" path

The urls.py:

https://github.com/WeblateOrg/weblate/blob/03fb51dadc115828b745ca4255be1d5ae0451349/weblate/urls.py#L117-L121

Pattern definition:

https://github.com/WeblateOrg/weblate/blob/03fb51dadc115828b745ca4255be1d5ae0451349/weblate/utils/urls.py#L29-L36

Most likely this is caused by #2432

Expected Result

The event would be captured as /translate/{path}/ as it was before.

Actual Result

Instead, it is captured as /projects/{path}{0,6})/ what is IMHO wrong as there is no repetition of the path argument, the repetition is embedded in it (also, there is extra closing parenthesis).

[sentrivana]

Hey @nijel, thanks for raising this. This is indeed caused by the linked PR reverting the change that caused this regression. (I elaborated on the underlying issue in this comment.) In your case, it's the closing parenthesis in the regex that's throwing our resolver off. (Which shouldn't happen and is a bug.)

We can't revert to the behavior in 1.31.0 since it breaks multiple groups. Fixing this will require more fundamental changes to the resolver -- as I wrote in the linked comment, what we're currently doing is by design a best effort solution. We have to investigate how to do this properly and it'll likely take some time.

[sentrivana]

Related: #1527

[nijel]

Maybe I'm missing something, but why not use ResolverMatch? It is available as request.resolver_match after resolving and has route attribute which contains the actual pattern that matched (translate/<object_path:path>/ in the above case). Processing such string seems much easier than dealing with regex...

[sentrivana]

ResolverMatch could definitely work and would probably be the cleanest way to go about this. I'll look into it.

[sl0thentr0py]

@sentrivana yes please, if we can just use the django stuff directly, let's do that.
This LEGACY_RESOLVER is ported all the way from raven long ago. I just think no one looked at it later.

[sentrivana]

So ResolverMatch didn't prove to be the shiny solution I'd hoped it would be -- we already have access to the original URL pattern in the RavenResolver, so no need for getting it via ResolverMatch. That being said, I did what was suggested here, which is parsing the path directly without turning it into its regex counterpart first, which was the source of the issue here. Added some tests and also tested manually with the custom PathConverter from this issue.