Skip to content

Commit

Permalink
Deprecate URLs without an explicit scheme
Browse files Browse the repository at this point in the history
  • Loading branch information
Ousret authored May 22, 2023
1 parent 4e9060b commit ffa2b63
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 0 deletions.
1 change: 1 addition & 0 deletions docs/v2-migration-guide.rst
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ Here's a short summary of which changes in urllib3 v2.0 are most important:
- Deprecated the ``urllib3[secure]`` extra, will be removed in v2.1.0.
- Deprecated the ``HTTPResponse.getheaders()`` method in favor of ``HTTPResponse.headers``, will be removed in v2.1.0.
- Deprecated the ``HTTPResponse.getheader(name, default)`` method in favor of ``HTTPResponse.headers.get(name, default)``, will be removed in v2.1.0.
- Deprecated URLs without a scheme (ie 'https://') and will be raising an error in a future version of urllib3.
- Changed the default minimum TLS version to TLS 1.2 (previously was TLS 1.0).
- Removed support for verifying certificate hostnames via ``commonName``, now only ``subjectAltName`` is used.
- Removed the default set of TLS ciphers, instead now urllib3 uses the list of ciphers configured by the system.
Expand Down
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ filterwarnings = [
'''default:ssl\.PROTOCOL_TLSv1_2 is deprecated:DeprecationWarning''',
'''default:unclosed .*:ResourceWarning''',
'''default:ssl NPN is deprecated, use ALPN instead:DeprecationWarning''',
'''default:URLs without a scheme''',
]

[tool.isort]
Expand Down
10 changes: 10 additions & 0 deletions src/urllib3/poolmanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,16 @@ def urlopen( # type: ignore[override]
"""
u = parse_url(url)

if u.scheme is None:
warnings.warn(
"URLs without a scheme (ie 'https://') are deprecated and will raise an error "
"in a future version of urllib3. To avoid this DeprecationWarning ensure all URLs "
"start with 'https://' or 'http://'. Read more in this issue: "
"https://github.com/urllib3/urllib3/issues/2920",
category=DeprecationWarning,
stacklevel=2,
)

conn = self.connection_from_host(u.host, port=u.port, scheme=u.scheme)

kw["assert_same_host"] = False
Expand Down
17 changes: 17 additions & 0 deletions test/test_poolmanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,23 @@ def test_http_connection_from_context_case_insensitive(self) -> None:
assert pool is other_pool
assert all(isinstance(key, PoolKey) for key in p.pools.keys())

def test_deprecated_no_scheme(self) -> None:
p = PoolManager()

with pytest.warns(DeprecationWarning) as records:
p.request(method="GET", url="evil.com://good.com")

msg = (
"URLs without a scheme (ie 'https://') are deprecated and will raise an error "
"in a future version of urllib3. To avoid this DeprecationWarning ensure all URLs "
"start with 'https://' or 'http://'. Read more in this issue: "
"https://github.com/urllib3/urllib3/issues/2920"
)

assert len(records) == 1
assert isinstance(records[0].message, DeprecationWarning)
assert records[0].message.args[0] == msg

@patch("urllib3.poolmanager.PoolManager.connection_from_pool_key")
def test_connection_from_context_strict_param(
self, connection_from_pool_key: mock.MagicMock
Expand Down
6 changes: 6 additions & 0 deletions test/with_dummyserver/test_connectionpool.py
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,12 @@ def test_tunnel(self) -> None:
finally:
conn.close()

def test_redirect_relative_url_no_deprecation(self) -> None:
with HTTPConnectionPool(self.host, self.port) as pool:
with warnings.catch_warnings():
warnings.simplefilter("error", DeprecationWarning)
pool.request("GET", "/redirect", fields={"target": "/"})

def test_redirect(self) -> None:
with HTTPConnectionPool(self.host, self.port) as pool:
r = pool.request("GET", "/redirect", fields={"target": "/"}, redirect=False)
Expand Down

0 comments on commit ffa2b63

Please sign in to comment.