Repository: encode/httpx
Pull request #1080 (2020-07-23 15:05:02)

URL.port becomes Optional[int]
This PR changes `URL.port` from `int` to `Optional[int]`.

Previously any non http/https URL without an explicit port component would raise an error if `url.port` was accessed.
Now `url.port` is optional, and only returns any explicitly included port in the URL, without also trying to use a scheme-default if one is not present.

This is important because...

1. URL is more generically useful. Eg. `URL('ftp://www.example.com').port == None` which makes more sense than `KeyError("ftp")`.
2. This is a precursor to #954, since the only point we're now enforcing http/https schemes on URLs is strictly inside `_transport_for_url`, rather than at any point in the codebase that accesses `url.port`.

(Yes, it's a little bit fiddly)


Patch:
diff --git a/httpx/_client.py b/httpx/_client.py
index e7d018b..6aa576f 100644
--- a/httpx/_client.py
+++ b/httpx/_client.py
@@ -543,10 +543,10 @@ class Client(BaseClient):
         enforce_http_url(url)
 
         if self._proxies and not should_not_be_proxied(url):
-            is_default_port = (url.scheme == "http" and url.port == 80) or (
-                url.scheme == "https" and url.port == 443
-            )
-            hostname = f"{url.host}:{url.port}"
+            default_port = {"http": 80, "https": 443}[url.scheme]
+            is_default_port = url.port is None or url.port == default_port
+            port = url.port or default_port
+            hostname = f"{url.host}:{port}"
             proxy_keys = (
                 f"{url.scheme}://{hostname}",
                 f"{url.scheme}://{url.host}" if is_default_port else None,
@@ -1076,10 +1076,10 @@ class AsyncClient(BaseClient):
         enforce_http_url(url)
 
         if self._proxies and not should_not_be_proxied(url):
-            is_default_port = (url.scheme == "http" and url.port == 80) or (
-                url.scheme == "https" and url.port == 443
-            )
-            hostname = f"{url.host}:{url.port}"
+            default_port = {"http": 80, "https": 443}[url.scheme]
+            is_default_port = url.port is None or url.port == default_port
+            port = url.port or default_port
+            hostname = f"{url.host}:{port}"
             proxy_keys = (
                 f"{url.scheme}://{hostname}",
                 f"{url.scheme}://{url.host}" if is_default_port else None,
diff --git a/httpx/_models.py b/httpx/_models.py
index 73db360..785bf47 100644
--- a/httpx/_models.py
+++ b/httpx/_models.py
@@ -105,11 +105,9 @@ class URL:
         return self._uri_reference.host or ""
 
     @property
-    def port(self) -> int:
+    def port(self) -> typing.Optional[int]:
         port = self._uri_reference.port
-        if port is None:
-            return {"https": 443, "http": 80}[self.scheme]
-        return int(port)
+        return int(port) if port else None
 
     @property
     def path(self) -> str:
@@ -131,7 +129,7 @@ class URL:
         return self._uri_reference.fragment or ""
 
     @property
-    def raw(self) -> typing.Tuple[bytes, bytes, int, bytes]:
+    def raw(self) -> typing.Tuple[bytes, bytes, typing.Optional[int], bytes]:
         return (
             self.scheme.encode("ascii"),
             self.host.encode("ascii"),
@@ -167,7 +165,7 @@ class URL:
             or "port" in kwargs
         ):
             host = kwargs.pop("host", self.host)
-            port = kwargs.pop("port", None if self.is_relative_url else self.port)
+            port = kwargs.pop("port", self.port)
             username = kwargs.pop("username", self.username)
             password = kwargs.pop("password", self.password)
 
diff --git a/httpx/_utils.py b/httpx/_utils.py
index b43f38e..5550365 100644
--- a/httpx/_utils.py
+++ b/httpx/_utils.py
@@ -273,12 +273,20 @@ def enforce_http_url(url: "URL") -> None:
         raise InvalidURL('URL scheme must be "http" or "https".')
 
 
+def port_or_default(url: "URL") -> typing.Optional[int]:
+    if url.port is not None:
+        return url.port
+    return {"http": 80, "https": 443}.get(url.scheme)
+
+
 def same_origin(url: "URL", other: "URL") -> bool:
     """
     Return 'True' if the given URLs share the same origin.
     """
     return (
-        url.scheme == other.scheme and url.host == other.host and url.port == other.port
+        url.scheme == other.scheme
+        and url.host == other.host
+        and port_or_default(url) == port_or_default(other)
     )
 
 


Test patch:
diff --git a/tests/client/test_proxies.py b/tests/client/test_proxies.py
index f981948..f5af90c 100644
--- a/tests/client/test_proxies.py
+++ b/tests/client/test_proxies.py
@@ -4,6 +4,18 @@ import pytest
 import httpx
 
 
+def url_to_origin(url: str):
+    """
+    Given a URL string, return the origin in the raw tuple format that
+    `httpcore` uses for it's representation.
+    """
+    DEFAULT_PORTS = {b"http": 80, b"https": 443}
+    scheme, host, explicit_port = httpx.URL(url).raw[:3]
+    default_port = DEFAULT_PORTS[scheme]
+    port = default_port if explicit_port is None else explicit_port
+    return scheme, host, port
+
+
 @pytest.mark.parametrize(
     ["proxies", "expected_proxies"],
     [
@@ -27,7 +39,7 @@ def test_proxies_parameter(proxies, expected_proxies):
         assert proxy_key in client._proxies
         proxy = client._proxies[proxy_key]
         assert isinstance(proxy, httpcore.AsyncHTTPProxy)
-        assert proxy.proxy_origin == httpx.URL(url).raw[:3]
+        assert proxy.proxy_origin == url_to_origin(url)
 
     assert len(expected_proxies) == len(client._proxies)
 
@@ -85,7 +97,7 @@ def test_transport_for_request(url, proxies, expected):
         assert transport is client._transport
     else:
         assert isinstance(transport, httpcore.AsyncHTTPProxy)
-        assert transport.proxy_origin == httpx.URL(expected).raw[:3]
+        assert transport.proxy_origin == url_to_origin(expected)
 
 
 @pytest.mark.asyncio
@@ -131,4 +143,4 @@ def test_proxies_environ(monkeypatch, client_class, url, env, expected):
     if expected is None:
         assert transport == client._transport
     else:
-        assert transport.proxy_origin == httpx.URL(expected).raw[:3]
+        assert transport.proxy_origin == url_to_origin(expected)
diff --git a/tests/client/test_redirects.py b/tests/client/test_redirects.py
index 30b6f6a..61e600c 100644
--- a/tests/client/test_redirects.py
+++ b/tests/client/test_redirects.py
@@ -242,7 +242,7 @@ async def test_malformed_redirect():
     client = AsyncClient(transport=AsyncMockTransport())
     response = await client.get("http://example.org/malformed_redirect")
     assert response.status_code == codes.OK
-    assert response.url == URL("https://example.org/")
+    assert response.url == URL("https://example.org:443/")
     assert len(response.history) == 1
 
 
diff --git a/tests/models/test_requests.py b/tests/models/test_requests.py
index 7fd61a1..e360ea0 100644
--- a/tests/models/test_requests.py
+++ b/tests/models/test_requests.py
@@ -102,11 +102,11 @@ def test_url():
     url = "http://example.org"
     request = httpx.Request("GET", url)
     assert request.url.scheme == "http"
-    assert request.url.port == 80
+    assert request.url.port is None
     assert request.url.full_path == "/"
 
     url = "https://example.org/abc?foo=bar"
     request = httpx.Request("GET", url)
     assert request.url.scheme == "https"
-    assert request.url.port == 443
+    assert request.url.port is None
     assert request.url.full_path == "/abc?foo=bar"
diff --git a/tests/models/test_url.py b/tests/models/test_url.py
index daa84e0..6c0fdac 100644
--- a/tests/models/test_url.py
+++ b/tests/models/test_url.py
@@ -18,9 +18,9 @@ from httpx import URL
             "http://xn--knigsgchen-b4a3dun.de",
             "xn--knigsgchen-b4a3dun.de",
             "http",
-            80,
+            None,
         ),
-        ("https://faß.de", "https://xn--fa-hia.de", "xn--fa-hia.de", "https", 443),
+        ("https://faß.de", "https://xn--fa-hia.de", "xn--fa-hia.de", "https", None),
         (
             "https://βόλος.com:443",
             "https://xn--nxasmm1c.com:443",
diff --git a/tests/test_config.py b/tests/test_config.py
index 46d154c..0c2c2f7 100644
--- a/tests/test_config.py
+++ b/tests/test_config.py
@@ -214,7 +214,7 @@ def test_ssl_config_support_for_keylog_file(tmpdir, monkeypatch):  # pragma: noc
         ("https://example.com", "https://example.com", {}, "DEFAULT"),
         (
             "https://user:pass@example.com",
-            "https://example.com:443",
+            "https://example.com",
             {"proxy-authorization": "Basic dXNlcjpwYXNz"},
             "DEFAULT",
         ),