From 1cc316463d6fa0dac8c5b8d5a2502c9323de888d Mon Sep 17 00:00:00 2001 From: Illia Puzanov Date: Fri, 26 Jan 2024 20:14:20 +0000 Subject: [PATCH 1/2] fix(url): fix host extra trailing slash issue --- src/url.rs | 36 ++++++++++++++++----- tests/validators/test_url.py | 61 ++++++++++++++++++++++++++---------- 2 files changed, 72 insertions(+), 25 deletions(-) diff --git a/src/url.rs b/src/url.rs index b381e247d..9d71d62fa 100644 --- a/src/url.rs +++ b/src/url.rs @@ -177,15 +177,25 @@ impl PyUrl { }; let mut url = format!("{scheme}://{url_host}"); if let Some(path) = path { - url.push('/'); - url.push_str(path); + if !url.ends_with('/') { + url.push('/'); + } + if path.starts_with('/') { + url.push_str(path.trim_start_matches('/')); + } else { + url.push_str(path); + } } if let Some(query) = query { - url.push('?'); + if !query.starts_with('?') { + url.push('?'); + } url.push_str(query); } if let Some(fragment) = fragment { - url.push('#'); + if !fragment.starts_with('#') { + url.push('#'); + } url.push_str(fragment); } cls.call1((url,)) @@ -405,15 +415,25 @@ impl PyMultiHostUrl { }; if let Some(path) = path { - url.push('/'); - url.push_str(path); + if !url.ends_with('/') { + url.push('/'); + } + if path.starts_with('/') { + url.push_str(path.trim_start_matches('/')); + } else { + url.push_str(path); + } } if let Some(query) = query { - url.push('?'); + if !query.starts_with('?') { + url.push('?'); + } url.push_str(query); } if let Some(fragment) = fragment { - url.push('#'); + if !fragment.starts_with('#') { + url.push('#'); + } url.push_str(fragment); } cls.call1((url,)) diff --git a/tests/validators/test_url.py b/tests/validators/test_url.py index 421f5e683..9524ac468 100644 --- a/tests/validators/test_url.py +++ b/tests/validators/test_url.py @@ -10,9 +10,8 @@ from ..conftest import Err, PyAndJson -def test_url_ok(py_and_json: PyAndJson): - v = py_and_json(core_schema.url_schema()) - url = v.validate_test('https://example.com/foo/bar?baz=qux#quux') +def assert_example_url(url: Url): + # example URL in question 'https://example.com/foo/bar?baz=qux#quux' assert isinstance(url, Url) assert str(url) == 'https://example.com/foo/bar?baz=qux#quux' @@ -30,23 +29,51 @@ def test_url_ok(py_and_json: PyAndJson): assert url.port == 443 +def test_url_ok(py_and_json: PyAndJson): + v = py_and_json(core_schema.url_schema()) + url = v.validate_test('https://example.com/foo/bar?baz=qux#quux') + + assert_example_url(url) + + def test_url_from_constructor_ok(): url = Url('https://example.com/foo/bar?baz=qux#quux') - assert isinstance(url, Url) - assert str(url) == 'https://example.com/foo/bar?baz=qux#quux' - assert repr(url) == "Url('https://example.com/foo/bar?baz=qux#quux')" - assert url.unicode_string() == 'https://example.com/foo/bar?baz=qux#quux' - assert url.scheme == 'https' - assert url.host == 'example.com' - assert url.unicode_host() == 'example.com' - assert url.path == '/foo/bar' - assert url.query == 'baz=qux' - assert url.query_params() == [('baz', 'qux')] - assert url.fragment == 'quux' - assert url.username is None - assert url.password is None - assert url.port == 443 + assert_example_url(url) + + +def test_url_from_build_ok(): + # 1) no host trailing slash/no path leading slash in the input + url = Url.build(scheme='https', host='example.com', path='foo/bar', query='baz=qux', fragment='quux') + assert_example_url(url) + + # 2) no host trailing slash/with path leading slash in the input + url = Url.build(scheme='https', host='example.com', path='/foo/bar', query='baz=qux', fragment='quux') + assert_example_url(url) + + # 3) with host trailing slash/no path leading slash in the input + url = Url.build(scheme='https', host='example.com/', path='foo/bar', query='baz=qux', fragment='quux') + assert_example_url(url) + + # 4) with host trailing slash/with path leading slash in the input + url = Url.build(scheme='https', host='example.com/', path='/foo/bar', query='baz=qux', fragment='quux') + assert_example_url(url) + + # 5) query no leading question mark + url = Url.build(scheme='https', host='example.com', path='foo/bar', query='baz=qux', fragment='quux') + assert_example_url(url) + + # 6) query with leading question mark + url = Url.build(scheme='https', host='example.com', path='foo/bar', query='?baz=qux', fragment='quux') + assert_example_url(url) + + # 7) fragment no leading hash + url = Url.build(scheme='https', host='example.com', path='foo/bar', query='baz=qux', fragment='quux') + assert_example_url(url) + + # 8) fragment with leading hash + url = Url.build(scheme='https', host='example.com', path='foo/bar', query='baz=qux', fragment='#quux') + assert_example_url(url) @pytest.fixture(scope='module', name='url_validator') From 6ef41b709eff144b6177a66f04a074f9d022299e Mon Sep 17 00:00:00 2001 From: Illia Puzanov Date: Mon, 29 Jan 2024 22:34:47 +0000 Subject: [PATCH 2/2] chore(url): add test for MultiHostUrl method with extra characters --- tests/validators/test_url.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/validators/test_url.py b/tests/validators/test_url.py index 9524ac468..ffe3021c0 100644 --- a/tests/validators/test_url.py +++ b/tests/validators/test_url.py @@ -1259,6 +1259,20 @@ def test_multi_url_build() -> None: assert url == MultiHostUrl('postgresql://testuser:testpassword@127.0.0.1:5432/database?sslmode=require#test') assert str(url) == 'postgresql://testuser:testpassword@127.0.0.1:5432/database?sslmode=require#test' + # assert that `build` builds correctly with leading slash in path, leading question mark in query and leading hash in fragment + url = MultiHostUrl.build( + scheme='postgresql', + username='testuser', + password='testpassword', + host='127.0.0.1', + port=5432, + path='/database', + query='?sslmode=require', + fragment='#test', + ) + assert url == MultiHostUrl('postgresql://testuser:testpassword@127.0.0.1:5432/database?sslmode=require#test') + assert str(url) == 'postgresql://testuser:testpassword@127.0.0.1:5432/database?sslmode=require#test' + @pytest.mark.parametrize('field', ['host', 'password', 'username', 'port']) def test_multi_url_build_hosts_set_with_single_value(field) -> None: