From 3cb7cd92380fc4c60d7760787a1731de4202304e Mon Sep 17 00:00:00 2001 From: Nick Pope Date: Thu, 19 Jan 2023 12:25:47 +0000 Subject: [PATCH] Fix PATH_PARAMETER_PATTERN for DRF default value pattern. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Django REST Framework will use `[^/.]+` as the default value pattern in certain cases, e.g. `(?P[^/.]+)`, which doesn't work well with the current regular expression in `PATH_PARAMETER_PATTERN`. ``` ❯ git describe; git rev-parse HEAD 3.14.0-69-g0618fa88 0618fa88e1a8c2cf8a2aab29ef6de66b49e5f7ed ❯ git grep -n '\[^/.\]+' rest_framework/routers.py:143: self._default_value_pattern = '[^/.]+' tests/test_routers.py:304: expected = ['^notes/$', '^notes/(?P[^/.]+)/$'] tests/test_routers.py:319: expected = ['^notes$', '^notes/(?P[^/.]+)$'] ``` The fix inserts `(?:[^/]*?\[\^[^/]*/)?` before the final `[^/]*` in the pattern. This allows matching a slash inside an inverted character set, i.e. `[^...]`, treating it as part of the named group. --- openapi_core/contrib/django/requests.py | 8 +- tests/unit/contrib/django/test_django.py | 101 ++++++++++------------- 2 files changed, 49 insertions(+), 60 deletions(-) diff --git a/openapi_core/contrib/django/requests.py b/openapi_core/contrib/django/requests.py index b894063b..bac713ea 100644 --- a/openapi_core/contrib/django/requests.py +++ b/openapi_core/contrib/django/requests.py @@ -8,7 +8,7 @@ from openapi_core.validation.request.datatypes import RequestParameters -# https://docs.djangoproject.com/en/2.2/topics/http/urls/ +# https://docs.djangoproject.com/en/stable/topics/http/urls/ # # Currently unsupported are : # - nested arguments, e.g.: ^comments/(?:page-(?P\d+)/)?$ @@ -16,9 +16,11 @@ # - multiple named parameters between a single pair of slashes # e.g.: -/edit/ # -# The regex matches everything, except a "/" until "<". Than only the name +# The regex matches everything, except a "/" until "<". Then only the name # is exported, after which it matches ">" and everything until a "/". -PATH_PARAMETER_PATTERN = r"(?:[^\/]*?)<(?:(?:.*?:))*?(\w+)>(?:[^\/]*)" +# A check is made to ensure that "/" is not in an excluded character set such +# as may be found with Django REST Framwork's default value pattern, "[^/.]+". +PATH_PARAMETER_PATTERN = r"(?:[^/]*?)<(?:(?:.*?:))*?(\w+)>(?:(?:[^/]*?\[\^[^/]*/)?[^/]*)" class DjangoOpenAPIRequest: diff --git a/tests/unit/contrib/django/test_django.py b/tests/unit/contrib/django/test_django.py index 8fc5ca02..25003615 100644 --- a/tests/unit/contrib/django/test_django.py +++ b/tests/unit/contrib/django/test_django.py @@ -43,6 +43,7 @@ def django_settings(self): settings.ROOT_URLCONF = ( path("admin/", admin.site.urls), re_path("^test/test-regexp/$", lambda d: None), + re_path("^object/(?P[^/.]+)/action/$", lambda d: None), ) @pytest.fixture @@ -68,27 +69,16 @@ def test_no_resolver(self, request_factory): openapi_request = DjangoOpenAPIRequest(request) - path = {} - query = ImmutableMultiDict( - [ - ("test1", "test2"), - ] - ) - headers = Headers( - { - "Cookie": "", - } - ) - cookies = {} assert openapi_request.parameters == RequestParameters( - path=path, - query=query, - header=headers, - cookie=cookies, + path={}, + query=ImmutableMultiDict([("test1", "test2")]), + header=Headers({"Cookie": ""}), + cookie={}, ) assert openapi_request.method == request.method.lower() assert openapi_request.host_url == request._current_scheme_host assert openapi_request.path == request.path + assert openapi_request.path_pattern is None assert openapi_request.body == "" assert openapi_request.mimetype == request.content_type @@ -96,27 +86,20 @@ def test_simple(self, request_factory): from django.urls import resolve request = request_factory.get("/admin/") - request.resolver_match = resolve("/admin/") + request.resolver_match = resolve(request.path) openapi_request = DjangoOpenAPIRequest(request) - path = {} - query = {} - headers = Headers( - { - "Cookie": "", - } - ) - cookies = {} assert openapi_request.parameters == RequestParameters( - path=path, - query=query, - header=headers, - cookie=cookies, + path={}, + query={}, + header=Headers({"Cookie": ""}), + cookie={}, ) assert openapi_request.method == request.method.lower() assert openapi_request.host_url == request._current_scheme_host assert openapi_request.path == request.path + assert openapi_request.path_pattern == request.path assert openapi_request.body == "" assert openapi_request.mimetype == request.content_type @@ -124,25 +107,15 @@ def test_url_rule(self, request_factory): from django.urls import resolve request = request_factory.get("/admin/auth/group/1/") - request.resolver_match = resolve("/admin/auth/group/1/") + request.resolver_match = resolve(request.path) openapi_request = DjangoOpenAPIRequest(request) - path = { - "object_id": "1", - } - query = {} - headers = Headers( - { - "Cookie": "", - } - ) - cookies = {} assert openapi_request.parameters == RequestParameters( - path=path, - query=query, - header=headers, - cookie=cookies, + path={"object_id": "1"}, + query={}, + header=Headers({"Cookie": ""}), + cookie={}, ) assert openapi_request.method == request.method.lower() assert openapi_request.host_url == request._current_scheme_host @@ -155,27 +128,41 @@ def test_url_regexp_pattern(self, request_factory): from django.urls import resolve request = request_factory.get("/test/test-regexp/") - request.resolver_match = resolve("/test/test-regexp/") + request.resolver_match = resolve(request.path) openapi_request = DjangoOpenAPIRequest(request) - path = {} - query = {} - headers = Headers( - { - "Cookie": "", - } + assert openapi_request.parameters == RequestParameters( + path={}, + query={}, + header=Headers({"Cookie": ""}), + cookie={}, ) - cookies = {} + assert openapi_request.method == request.method.lower() + assert openapi_request.host_url == request._current_scheme_host + assert openapi_request.path == request.path + assert openapi_request.path_pattern == request.path + assert openapi_request.body == "" + assert openapi_request.mimetype == request.content_type + + def test_drf_default_value_pattern(self, request_factory): + from django.urls import resolve + + request = request_factory.get("/object/123/action/") + request.resolver_match = resolve(request.path) + + openapi_request = DjangoOpenAPIRequest(request) + assert openapi_request.parameters == RequestParameters( - path=path, - query=query, - header=headers, - cookie=cookies, + path={"pk": "123"}, + query={}, + header=Headers({"Cookie": ""}), + cookie={}, ) assert openapi_request.method == request.method.lower() assert openapi_request.host_url == request._current_scheme_host - assert openapi_request.path == "/test/test-regexp/" + assert openapi_request.path == request.path + assert openapi_request.path_pattern == "/object/{pk}/action/" assert openapi_request.body == "" assert openapi_request.mimetype == request.content_type