Skip to content

Commit

Permalink
Fix cancelled payload send leading to hung connection (#8992) (#9002)
Browse files Browse the repository at this point in the history
(cherry picked from commit 5c0b8e4)
  • Loading branch information
Dreamsorcerer authored Sep 3, 2024
1 parent ba201c4 commit a3fa8d8
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 15 deletions.
1 change: 1 addition & 0 deletions CHANGES/8992.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed client incorrectly reusing a connection when the previous message had not been fully sent -- by :user:`Dreamsorcerer`.
3 changes: 2 additions & 1 deletion aiohttp/client_reqrep.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
35 changes: 21 additions & 14 deletions tests/test_client_functional.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Expand Down Expand Up @@ -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):
Expand All @@ -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(
Expand Down

0 comments on commit a3fa8d8

Please sign in to comment.