diff --git a/CHANGES/8992.bugfix.rst b/CHANGES/8992.bugfix.rst new file mode 100644 index 00000000000..bc41d5feb81 --- /dev/null +++ b/CHANGES/8992.bugfix.rst @@ -0,0 +1 @@ +Fixed client incorrectly reusing a connection when the previous message had not been fully sent -- by :user:`Dreamsorcerer`. diff --git a/aiohttp/client_reqrep.py b/aiohttp/client_reqrep.py index 933f3275e28..d2c5f16df2b 100644 --- a/aiohttp/client_reqrep.py +++ b/aiohttp/client_reqrep.py @@ -659,7 +659,8 @@ async def write_bytes( set_exception(protocol, reraised_exc, underlying_exc) except asyncio.CancelledError: - await writer.write_eof() + # Body hasn't been fully sent, so connection can't be reused. + conn.close() except Exception as underlying_exc: set_exception( protocol, diff --git a/tests/test_client_functional.py b/tests/test_client_functional.py index c7c31c739b1..95f73d84ec0 100644 --- a/tests/test_client_functional.py +++ b/tests/test_client_functional.py @@ -341,10 +341,11 @@ async def data_gen(): async with client.get("/") as resp: assert 200 == resp.status - # Connection should have been reused + # First connection should have been closed, otherwise server won't know if it + # received the full message. conns = next(iter(client.session.connector._conns.values())) assert len(conns) == 1 - assert conns[0][0] is conn + assert conns[0][0] is not conn async def test_stream_request_on_server_eof_nested(aiohttp_client) -> None: @@ -362,14 +363,21 @@ async def data_gen(): yield b"just data" await asyncio.sleep(0.1) + assert client.session.connector is not None async with client.put("/", data=data_gen()) as resp: + first_conn = next(iter(client.session.connector._acquired)) assert 200 == resp.status - async with client.get("/") as resp: - assert 200 == resp.status + + async with client.get("/") as resp2: + assert 200 == resp2.status # Should be 2 separate connections conns = next(iter(client.session.connector._conns.values())) - assert len(conns) == 2 + assert len(conns) == 1 + + assert first_conn is not None + assert not first_conn.is_connected() + assert first_conn is not conns[0][0] async def test_HTTP_304_WITH_BODY(aiohttp_client) -> None: @@ -3783,9 +3791,10 @@ async def handler(request): assert resp.reason == "x" * 8191 -@pytest.mark.xfail(raises=asyncio.TimeoutError, reason="#7599") -async def test_rejected_upload(aiohttp_client, tmp_path) -> None: - async def ok_handler(request): +async def test_rejected_upload( + aiohttp_client: AiohttpClient, tmp_path: pathlib.Path +) -> None: + async def ok_handler(request: web.Request) -> web.Response: return web.Response() async def not_ok_handler(request): @@ -3802,13 +3811,11 @@ async def not_ok_handler(request): with open(file_path, "rb") as file: data = {"file": file} - async with await client.post("/not_ok", data=data) as resp_not_ok: - assert 400 == resp_not_ok.status + async with client.post("/not_ok", data=data) as resp_not_ok: + assert resp_not_ok.status == 400 - async with await client.get( - "/ok", timeout=aiohttp.ClientTimeout(total=0.01) - ) as resp_ok: - assert 200 == resp_ok.status + async with client.get("/ok", timeout=aiohttp.ClientTimeout(total=1)) as resp_ok: + assert resp_ok.status == 200 @pytest.mark.parametrize(