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.

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

Fix AnyUrl.build doesn't do percent encoding (#3061) #4224

Merged
merged 10 commits into from Aug 19, 2022
1 change: 1 addition & 0 deletions changes/3061-FaresAhmedb.md
@@ -0,0 +1 @@
Fix `AnyUrl.build` doesn't do percent encoding
faresbakhit marked this conversation as resolved.
Show resolved Hide resolved
14 changes: 10 additions & 4 deletions pydantic/networks.py
Expand Up @@ -24,6 +24,7 @@
cast,
no_type_check,
)
from urllib.parse import quote, quote_plus

from . import errors
from .utils import Representation, update_not_none
Expand Down Expand Up @@ -153,6 +154,7 @@ def __init__(
path: Optional[str] = None,
query: Optional[str] = None,
fragment: Optional[str] = None,
plus: bool = False,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
plus: bool = False,
quote_plus: bool = False,

might be a clearer name.

Copy link
Member

Choose a reason for hiding this comment

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

Also, is there a good reason not to have quote_plus = True as the default?

Copy link
Member

Choose a reason for hiding this comment

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

Please add quote_plus to stricturl

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, is there a good reason not to have quote_plus = True as the default?

Just conforming to the standard (RFC 3986)

) -> None:
str.__init__(url)
self.scheme = scheme
Expand All @@ -178,6 +180,7 @@ def build(
path: Optional[str] = None,
query: Optional[str] = None,
fragment: Optional[str] = None,
plus: bool = False,
**_kwargs: str,
) -> str:
parts = Parts(
Expand All @@ -192,20 +195,23 @@ def build(
**_kwargs, # type: ignore[misc]
)

do_quote = quote_plus if plus else quote
faresbakhit marked this conversation as resolved.
Show resolved Hide resolved

url = scheme + '://'
if user:
url += user
url += do_quote(user)
if password:
url += ':' + password
url += ':' + do_quote(password)
if user or password:
url += '@'
url += host
if port and ('port' not in cls.hidden_parts or cls.get_default_parts(parts).get('port') != port):
url += ':' + port
if path:
url += path
url += '/'.join(map(do_quote, path.split('/')))
if query:
url += '?' + query
queries = query.split('&')
url += '?' + '&'.join(map(lambda s: '='.join(map(do_quote, s.split('='))), queries))
if fragment:
url += '#' + fragment
return url
Expand Down
6 changes: 5 additions & 1 deletion tests/test_networks.py
Expand Up @@ -370,7 +370,7 @@ class Model(BaseModel):

@pytest.mark.parametrize(
'value',
['file:///foo/bar', 'file://localhost/foo/bar' 'file:////localhost/foo/bar'],
['file:///foo/bar', 'file://localhost/foo/bar', 'file:////localhost/foo/bar'],
)
def test_file_url_success(value):
class Model(BaseModel):
Expand Down Expand Up @@ -559,6 +559,10 @@ class Model2(BaseModel):
(dict(scheme='ws', user='foo', password='x', host='example.net'), 'ws://foo:x@example.net'),
(dict(scheme='ws', host='example.net', query='a=b', fragment='c=d'), 'ws://example.net?a=b#c=d'),
(dict(scheme='http', host='example.net', port='1234'), 'http://example.net:1234'),
(dict(scheme='http', user='foo@bar', host='example.net'), 'http://foo%40bar@example.net'),
(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', plus=True), 'http://example.net?q=foo+bar'),
(dict(scheme='http', host='example.net', path="/m&m's"), 'http://example.net/m%26m%27s'),
],
)
def test_build_url(kwargs, expected):
Expand Down