Use str as default path converter by sevdog · Pull Request #9066 · encode/django-rest-framework (original) (raw)

Consider the test viewset UrlPathViewSet.

The url-patterns produced by the router for this are going to be:

# using <path:pk>
^list/(?P<kwarg>[0-9]+)/\\Z
^(?P<pk>.+)/detail/(?P<kwarg>[0-9]+)/\\Z
^(?P<pk>.+)/detail/(?P<kwarg>[0-9]+)/detail/(?P<param>[0-9]+)/\\Z

# using <str:pk>
^list/(?P<kwarg>[0-9]+)/\\Z
^(?P<pk>[^/]+)/detail/(?P<kwarg>[0-9]+)/\\Z
^(?P<pk>[^/]+)/detail/(?P<kwarg>[0-9]+)/detail/(?P<param>[0-9]+)/\\Z

The second pattern with path is ambiguous (since it is also going to match also every URL which also matches the third) while with str it is not.

Since the router follows this sequence to build patterns:

  1. default list view
  2. dynamic list view
  3. default detail view
  4. dynamic detail view

The problem arises when there are both default detail and any action detail. Since the first pattern (prefix/<path:pk>/ -> ^prefix/(?P<pk>.+)/\\Z) is also going to match against the any other detail actions defined (prefix/<path:pk>/action/ -> ^prefix/(?P<pk>.+)/action/\\Z) because the patterns are matched in the order in which they appears in the url configuration and because those patterns overlap.

An other way to avoid such confusion would be to change the order of routes in SimpleRouter.routes by moving the more generics to the bottom of the list (thus the latest to be evaluated/tried). Registering first dynamic actions and then default one should avoid any confusion (referencing the list before the order should be 4, 2, 3, 1).

As final note the path converter may be too greedy and may cause useless queries on the database by trying to access non-exinstent objects, since that regex takes literally everithing that it encounters in the URI.

We may also register a custom path converter for rest-framework to match the pattern [^/.]+