Enforce uniqueness on custom path converters

[Adam Johnson]

register_converter() allows ​custom path converters to silently replace existing converters. This could lead to surprising behaviour, both when two places accidentally use the same custom name or when replacing a default converter. This could be particularly hard to debug if a third-party package uses a custom converter.

For example, say two modules register path converters named “year”. The first allows 1-4 digits:

class YearConverter:
    regex = r"[0-9]{1,4}"
    ...

register_converter(YearConverter, "year")

Whilst the second requires exactly four digits:

class YearConverter:
    regex = r"[0-9]{4}"
    ...

register_converter(YearConverter, "year")

Whichever module is loaded last will silently overwrite the other’s converter. URLs will then only be interpreted with that converter, leading to fewer URLs being matched than intended. This can be particularly difficult to spot as import order may change accidentally due to other code being rearranged.

See the full example project at ​https://github.com/adamchainz/django-ticket-35090

I propose that support for reusing path converter names be deprecated, eventually raising an exception. I think this should include the default names (int etc.) as replacing them can break URLs from third-party packages.

[Salvo Polizzi]

I've submitted a PR for this bug: ​https://github.com/django/django/pull/17703

[Mariusz Felisiak]

As for it's a cleanup not a bug. If someone registers converters with the same name as builtin converters, they will get what they deserve. What if they do it on purpose?

[Adam Johnson]

If someone registers converters with the same name as builtin converters, they will get what they deserve. What if they do it on purpose?

Even if done on purpose, users can break URLs in other parts of their project or packages unwittingly. If you need a modified int converter, for example, there’s little harm in requiring it to have a different name, but potential harm in allowing silent replacement.

[Mariusz Felisiak]

OK, let's have it.

[Natalia Bidart]

I'm not sure I understand the desired fix: is it to prevent any converter registered name overlap (ie a dynamic check at the time of project load)? or just a check against the Django/default's converters?

Would there be a way to override registered converters?

[Salvo Polizzi]

Replying to Natalia Bidart:

I'm not sure I understand the desired fix: is it to prevent any converter registered name overlap (ie a dynamic check at the time of project load)? or just a check against the Django/default's converters?

I interpreted it as an issue to avoid overlapping among different registered converters, but please correct me if I misunderstood.

Would there be a way to override registered converters?

I honestly don't know if there is a possibility to override

[Adam Johnson]

Natalia, I propose preventing *any* overlap, that is, blocking the overriding of both the default and custom converters. Replacing a converter opens the possibility of “action at a distance” and a hard-to-debug lack of correct URL resolution. Unique names prevent that.

[Salvo Polizzi]

I don't know why javascript tests are failing. If anyone can review the PR, please give me an advice on how to fix that problem.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 6e1ece7:

Fixed #35090 -- Deprecated registering URL converters with the same name.

[GitHub <noreply@…>]

In 0e84e70:

Refs #35090 -- Fixed urlpatterns.tests.SimplifiedURLTests when run in reverse.