analyze_named_regex_pattern(path) not extracting regex correctly from named group

[joniturmendi]

I've configured DRF with NamespaceVersioning, and when I load the SWAGGER view of my API, I get the following error:
django.core.exceptions.ImproperlyConfigured: "books(/?|/(?P<response_format>[^/]+)$" is not a valid regular expression: missing ), unterminated subpattern at position 5

So after debugging the code, I've found that drf-spectacular modifies the url patterns by detype_patterns(patterns) method in plumbing.py if NamespaceVersioning is enabled in DRF. That method uses the method analyze_named_regex_pattern(path) for extracting the regex inside the named groups. The issue is that this last method is not extracting correctly the regex inside the named group, because giving my original url path:
books(/?|/(?P<response_format>(basic|standard|extended))/?)$
it extracts (basic|standard|extended))/? as the regex inside the named groud, when the correct regex would be just (basic|standard|extended). So that is what is causing the incorrect regex error, because more characters than needed are being replaced.

Anyway, what is the purpose of detype_patterns(patterns) ? I don't want my url paths to be modified. Is there a way to disable this method by a flag or similar?

[tfranzel]

Hi, that seems like a limitation of detype_patterns.

Don't worry, detype_patterns is ONLY used to create mocked requests in order to be able to execute the NamespaceVersioning logic. It is used exclusively for NamespaceVersioning at the time of schema generation. Your actual urls are never changed. It is a hard requirement because we cannot guess a valid mock url otherwise.

As to why this happens. We analyze the regex pattern and replace named groups with generic catch-all groups. Not sure if we are able to support nested expressions like your example. But in any case we should catch that error and warn about it.

[joniturmendi]

Hi, thank you very much for the fast response. I suspected that my actual urls were never changed, but it is a relief to confirm it. So, this possible limitation of detype_patterns is preventing the SWAGGER view from showing. What could be a possible solution or workaround? I cannot change the versioning mode to URLPathVersioning because of my project's requirements.

[tfranzel]

Not sure yet if it is really a limitation but it is possible. I'm unable to provoke the error with

books(/?|/(?P<response_format>(basic|standard|extended))/?)$

I suppose there is some more context missing. The detyping works for this example as far as I can tell. Maybe it is embedded in a bigger context?

Please try to provide more context. Maybe provide a urlpattern example that creates this exception. Also a bit more context on the stackstrace is appreciated. I have a vague idea what could cause it but not sure yet.

[joniturmendi]

Sure, I'll provide more context. I'm using Django 3.2.12, Django REST Framework 3.13.1 and drf-spectacular 0.22.0. In my urls.py I have the following urlpatterns :

urlpatterns = [
    url(r'^admin/', admin.site.urls),
    url(r'^api/v1/', include((urlpatterns_v1, 'api'), namespace='v1')),
    url(r'^api/v2/', include((urlpatterns_v2, 'api'), namespace='v2')),
    url(r'^schema/v1/$', SpectacularAPIView.as_view(api_version='v1'), name='schema-v1'),
    url(r'^schema/v2/$', SpectacularAPIView.as_view(api_version='v2'), name='schema-v2'),
    url(r'^api/v1/$', SpectacularSwaggerView.as_view(url_name='schema-v1'), name='swagger-ui-v1'),
    url(r'^api/v2/$', SpectacularSwaggerView.as_view(url_name='schema-v2'), name='swagger-ui-v2'),
]

The problematic url paths are only in urlpatterns_v2. So here is the beginning of urlpatterns_v2 (all the paths follow the same pattern, so the error happens in all of them):

urlpatterns_v2 = [
    url(r'books%s$' % FORMAT_OPTIONS, views.BooksViewV2.as_view(),
        name='books_list_v2'),
...
]

where FORMAT_OPTIONS is (/?|/(?P<response_format>(basic|standard|extended))/?)
In settings.py I have the following configuration (showing only the relevant ones):

REST_FRAMEWORK = {
    'DEFAULT_SCHEMA_CLASS': 'drf_spectacular.openapi.AutoSchema',
    'DEFAULT_VERSIONING_CLASS': 'rest_framework.versioning.NamespaceVersioning'
}

SPECTACULAR_SETTINGS = {
    'TITLE': 'My title',
    'DESCRIPTION': 'My description',
    'VERSION': get_version_number(),
    'SERVE_INCLUDE_SCHEMA': False,
    'SCHEMA_PATH_PREFIX': r'/api/v[0-9]'
}

The stacktrace of the error is pretty long, but I could provide it if its needed.

[tfranzel]

thx. I will have a look.

I'm not interested in the whole stacktrace, just the last 3-4 entries. I want to know from where in our code the ImproperlyConfigured is raised exactly. That would certainly help

[joniturmendi]

Here it is:

Traceback (most recent call last):
  File "C:\path\to\my\project\.venv\lib\site-packages\rest_framework\views.py", line 506, in dispatch
    response = handler(request, *args, **kwargs)
  File "C:\path\to\my\project\.venv\lib\site-packages\drf_spectacular\views.py", line 70, in get
    return self._get_schema_response(request)
  File "C:\path\to\my\project\.venv\lib\site-packages\drf_spectacular\views.py", line 78, in _get_schema_response
    data=generator.get_schema(request=request, public=self.serve_public),
  File "C:\path\to\my\project\.venv\lib\site-packages\drf_spectacular\generators.py", line 262, in get_schema
    paths=self.parse(request, public),
  File "C:\path\to\my\project\.venv\lib\site-packages\drf_spectacular\generators.py", line 223, in parse
    path = modify_for_versioning(self.inspector.patterns, method, path, view, version)
  File "C:\path\to\my\project\.venv\lib\site-packages\drf_spectacular\plumbing.py", line 895, in modify_for_versioning
    ).resolve(path)
  File "C:\path\to\my\project\.venv\lib\site-packages\django\urls\resolvers.py", line 562, in resolve
    sub_match = pattern.resolve(new_path)
  File "C:\path\to\my\project\.venv\lib\site-packages\django\urls\resolvers.py", line 562, in resolve
    sub_match = pattern.resolve(new_path)
  File "C:\path\to\my\project\.venv\lib\site-packages\django\urls\resolvers.py", line 362, in resolve
    match = self.pattern.match(path)
  File "C:\path\to\my\project\.venv\lib\site-packages\django\urls\resolvers.py", line 159, in match
    if self._is_endpoint and self.regex.pattern.endswith('$')
  File "C:\path\to\my\project\.venv\lib\site-packages\django\urls\resolvers.py", line 105, in __get__
    instance.__dict__['regex'] = instance._compile(pattern)
  File "C:\path\to\my\project\.venv\lib\site-packages\django\urls\resolvers.py", line 198, in _compile
    ) from e
django.core.exceptions.ImproperlyConfigured: "books(/?|/(?P<response_format>[^/]+)$" is not a valid regular expression: missing ), unterminated subpattern at position 5

[joniturmendi]

Here I attach a screenshot of my actual debbug state in analyze_named_regex_pattern(path) just for showing how this function isn't extracting correctly the regex inside the named group for the path books(/?|/(?P<response_format>(basic|standard|extended))/?)$

[tfranzel]

so I investigated this a bit deeper. We do not support NamespaceVersioning with non-trivial unnamed and nested regex patterns. The fix takes care of the the unmatched parenthesis problem, but the issue remains that we have an unnamed non-trivial regex group. So instead of an exception there is caught warning

Error #0: namespace versioning path resolution failed for /api/v2/books{var}. Path will be ignored.

Theoretically this might be fixable but it may be opening pandora's box. Also DRF resolves only the first unnamed group to {var}, which is kind of weird. Your regex is non-trivial in that it is not matched by literally /api/v2/books{var}. Which is what happens when we detype patterns. I have to think about this for a moment.

The fix in the meantime would be to rewrite that regex so that there is no unnamed group going on.

[joniturmendi]

All right, I understand, so I came up with a different regex that doesn't use unnamed non-trivial regex groups but produces the same logic. I think it is cleaner as well:

^api/v2/books/?(?P<response_format>(?<=/)(basic|standard|extended)/?)?$

This one is visualized correctly in the swagger view. Thank you for the support!

[tfranzel]

excellent! I agree it looks nicer.