Skip to content

Support basic re_path #337

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 23, 2021
Merged

Support basic re_path #337

merged 3 commits into from
May 23, 2021

Conversation

mik-laj
Copy link
Contributor

@mik-laj mik-laj commented May 21, 2021

Helllo.

I had a project that used a very old URL format and unfortunately, this library couldn't find the path.

from django.conf.urls import url

It turned out that this library tries to make various concatenations and matches based on this expression which obviously failed.
https://github.com/p1c2u/openapi-core/blob/512ff6b48d105351c4be730dd78c5d7c00492db1/openapi_core/templating/paths/finders.py#L49

Close: #278

Brest regards,
Kamil Breguła

@codecov
Copy link

codecov bot commented May 22, 2021

Codecov Report

Merging #337 (ff4d852) into master (512ff6b) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #337   +/-   ##
=======================================
  Coverage   95.82%   95.82%           
=======================================
  Files          79       79           
  Lines        1627     1627           
=======================================
  Hits         1559     1559           
  Misses         68       68           
Impacted Files Coverage Δ
openapi_core/contrib/django/requests.py 100.00% <100.00%> (ø)
openapi_core/deserializing/media_types/util.py 100.00% <0.00%> (ø)
...penapi_core/deserializing/media_types/factories.py 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 512ff6b...ff4d852. Read the comment docs.

@@ -36,6 +36,11 @@ def create(cls, request):
else:
route = cls.path_regex.sub(
r'{\1}', request.resolver_match.route)
# Delete start marker and expression marker to allow concatenation.
if route[:1] == "^":
route = route[1:]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I failed to trigger this case into a test environment as there are side effects in testing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@p1c2u Can you help me with it?

Copy link
Collaborator

@p1c2u p1c2u May 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just add ^ to re_path:
re_path('^test/test-regexp/$', lambda d: None)
?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to add ^/ but it didn't work. Stupid mistake. It should work now.

@mik-laj
Copy link
Contributor Author

mik-laj commented May 22, 2021

@p1c2u It is green now.

Copy link
Collaborator

@p1c2u p1c2u left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@p1c2u
Copy link
Collaborator

p1c2u commented May 23, 2021

@mik-laj thanks for the contribution

@p1c2u p1c2u merged commit 8dc3921 into python-openapi:master May 23, 2021
@mik-laj mik-laj deleted the re-path branch May 23, 2021 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect django request validation for re_path urls
2 participants