Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Bugfix/anyurl escapes percent sign in url #4461

Closed
wants to merge 4 commits into from
Closed

Bugfix/anyurl escapes percent sign in url #4461

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9983d38
fix re-quote, fix mypy error and add cases to test_build_url
vetedde Sep 1, 2022
ba983c3
add changelog
vetedde Sep 1, 2022
25b6d8a
fix mypy
vetedde Sep 1, 2022
911a84a
Update changes/4458-vetedde.md
vetedde Sep 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changes/4458-vetedde.md
View file
Open in desktop
@@ -0,0 +1 @@
Fix AnyUrl escaping problem by preventing re-quoting an already escaped URL.
4 changes: 4 additions & 0 deletions pydantic/networks.py
View file
Open in desktop
Expand Up @@ -396,6 +396,10 @@ def apply_default_parts(cls, parts: 'Parts') -> 'Parts':

@classmethod
def quote(cls, string: str, safe: str = '') -> str:
pattern = r'^([\w]+|(%\d{2}))+$'
Copy link
Member

Choose a reason for hiding this comment

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

please compile the pattern and then use the match on the compiled pattern.
Take a look at the same implementation in datetime_parse.py

pydantic/pydantic/datetime_parse.py

Line 30 in 32ea885

date_re = re.compile(f'{date_expr}$')

Copy link
Member

Choose a reason for hiding this comment

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

Can you add a comment explaining what this regex is doing?

I also think it could be simplified.

Copy link
Author

Choose a reason for hiding this comment

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

Regex checks that string contains only %xx or A-Za-z0-9 and _ and nothing else.
Pattern work with cases: http://regexr.com/6t6g6

if re.match(pattern, string) is not None:
return string

return quote_plus(string, safe) if cls.quote_plus else quote(string, safe)

def __repr__(self) -> str:
Expand Down
2 changes: 2 additions & 0 deletions tests/test_networks.py
View file
Open in desktop
Expand Up @@ -683,6 +683,8 @@ class Model2(BaseModel):
(dict(scheme='http', user='foo', password='a b', host='example.net'), 'http://foo:a%20b@example.net'),
(dict(scheme='http', host='example.net', query='q=foo bar'), 'http://example.net?q=foo%20bar'),
(dict(scheme='http', host='example.net', path="/m&m's"), 'http://example.net/m%26m%27s'),
(dict(scheme='http', host='localhost', path='/foo bar'), 'http://localhost/foo%20bar'),
(dict(scheme='http', host='localhost', path='/foo%20bar'), 'http://localhost/foo%20bar'),
],
)
def test_build_url(kwargs, expected):
Expand Down