Fix client connection header not reflecting connector force_close value (#10003)

Author: bdraco

CHANGES/10003.bugfix.rst

@@ -0,0 +1 @@
+Fixed the HTTP client not considering the connector's ``force_close`` value when setting the ``Connection`` header -- by :user:`bdraco`.

aiohttp/client_reqrep.py

@@ -572,15 +572,6 @@ def update_proxy(
             proxy_headers = CIMultiDict(proxy_headers)
         self.proxy_headers = proxy_headers
 
-    def keep_alive(self) -> bool:
-        if self.version >= HttpVersion11:
-            return self.headers.get(hdrs.CONNECTION) != "close"
-        if self.version == HttpVersion10:
-            # no headers means we close for Http 1.0
-            return self.headers.get(hdrs.CONNECTION) == "keep-alive"
-        # keep alive not supported at all
-        return False
-
     async def write_bytes(
         self, writer: AbstractStreamWriter, conn: "Connection"
     ) -> None:
@@ -678,21 +669,15 @@ async def send(self, conn: "Connection") -> "ClientResponse":
         ):
             self.headers[hdrs.CONTENT_TYPE] = "application/octet-stream"
 
-        # set the connection header
-        connection = self.headers.get(hdrs.CONNECTION)
-        if not connection:
-            if self.keep_alive():
-                if self.version == HttpVersion10:
-                    connection = "keep-alive"
-            else:
-                if self.version == HttpVersion11:
-                    connection = "close"
-
-        if connection is not None:
-            self.headers[hdrs.CONNECTION] = connection
+        v = self.version
+        if hdrs.CONNECTION not in self.headers:
+            if conn._connector.force_close:
+                if v == HttpVersion11:
+                    self.headers[hdrs.CONNECTION] = "close"
+            elif v == HttpVersion10:
+                self.headers[hdrs.CONNECTION] = "keep-alive"
 
         # status + headers
-        v = self.version
         status_line = f"{self.method} {path} HTTP/{v.major}.{v.minor}"
         await writer.write_headers(status_line, self.headers)
         task: Optional["asyncio.Task[None]"]

tests/test_benchmarks_client_request.py

@@ -99,10 +99,16 @@ async def _drain_helper(self) -> None:
         def start_timeout(self) -> None:
             """Swallow start_timeout."""
 
+    class MockConnector:
+
+        def __init__(self) -> None:
+            self.force_close = False
+
     class MockConnection:
         def __init__(self) -> None:
             self.transport = None
             self.protocol = MockProtocol()
+            self._connector = MockConnector()
 
     conn = MockConnection()
 

tests/test_client_request.py

@@ -33,7 +33,7 @@
     _gen_default_accept_encoding,
 )
 from aiohttp.connector import Connection
-from aiohttp.http import HttpVersion
+from aiohttp.http import HttpVersion10, HttpVersion11
 from aiohttp.test_utils import make_mocked_coro
 from aiohttp.typedefs import LooseCookies
 
@@ -156,30 +156,6 @@ def test_version_err(make_request: _RequestMaker) -> None:
         make_request("get", "http://python.org/", version="1.c")
 
 
-def test_keep_alive(make_request: _RequestMaker) -> None:
-    req = make_request("get", "http://python.org/", version=(0, 9))
-    assert not req.keep_alive()
-
-    req = make_request("get", "http://python.org/", version=(1, 0))
-    assert not req.keep_alive()
-
-    req = make_request(
-        "get",
-        "http://python.org/",
-        version=(1, 0),
-        headers={"connection": "keep-alive"},
-    )
-    assert req.keep_alive()
-
-    req = make_request("get", "http://python.org/", version=(1, 1))
-    assert req.keep_alive()
-
-    req = make_request(
-        "get", "http://python.org/", version=(1, 1), headers={"connection": "close"}
-    )
-    assert not req.keep_alive()
-
-
 def test_host_port_default_http(make_request: _RequestMaker) -> None:
     req = make_request("get", "http://python.org/")
     assert req.host == "python.org"
@@ -624,25 +600,31 @@ async def test_connection_header(
     loop: asyncio.AbstractEventLoop, conn: mock.Mock
 ) -> None:
     req = ClientRequest("get", URL("http://python.org"), loop=loop)
-    with mock.patch.object(req, "keep_alive") as m:
-        req.headers.clear()
+    req.headers.clear()
+
+    req.version = HttpVersion11
+    req.headers.clear()
+    with mock.patch.object(conn._connector, "force_close", False):
+        await req.send(conn)
+    assert req.headers.get("CONNECTION") is None
 
-        m.return_value = True
-        req.version = HttpVersion(1, 1)
-        req.headers.clear()
+    req.version = HttpVersion10
+    req.headers.clear()
+    with mock.patch.object(conn._connector, "force_close", False):
         await req.send(conn)
-        assert req.headers.get("CONNECTION") is None
+    assert req.headers.get("CONNECTION") == "keep-alive"
 
-        req.version = HttpVersion(1, 0)
-        req.headers.clear()
+    req.version = HttpVersion11
+    req.headers.clear()
+    with mock.patch.object(conn._connector, "force_close", True):
         await req.send(conn)
-        assert req.headers.get("CONNECTION") == "keep-alive"
+    assert req.headers.get("CONNECTION") == "close"
 
-        m.return_value = False
-        req.version = HttpVersion(1, 1)
-        req.headers.clear()
+    req.version = HttpVersion10
+    req.headers.clear()
+    with mock.patch.object(conn._connector, "force_close", True):
         await req.send(conn)
-        assert req.headers.get("CONNECTION") == "close"
+    assert not req.headers.get("CONNECTION")
 
 
 async def test_no_content_length(

tests/test_web_functional.py

@@ -700,7 +700,6 @@ async def handler(request: web.Request) -> web.Response:
     resp.release()
 
 
-@pytest.mark.xfail
 async def test_http10_keep_alive_default(aiohttp_client: AiohttpClient) -> None:
     async def handler(request: web.Request) -> web.Response:
         return web.Response()