client: fix upload timeouts with sock_read set (#7150)

CHANGES/7149.bugfix

@@ -0,0 +1 @@
+changed ``sock_read`` timeout to start after writing has finished, to avoid read timeouts caused by an unfinished write. -- by :user:`dtrifiro`

aiohttp/client_proto.py

@@ -150,7 +150,6 @@ def set_response_params(
         self._skip_payload = skip_payload
 
         self._read_timeout = read_timeout
-        self._reschedule_timeout()
 
         self._timeout_ceil_threshold = timeout_ceil_threshold
 
@@ -186,6 +185,9 @@ def _reschedule_timeout(self) -> None:
         else:
             self._read_timeout_handle = None
 
+    def start_timeout(self) -> None:
+        self._reschedule_timeout()
+
     def _on_read_timeout(self) -> None:
         exc = ServerTimeoutError("Timeout on reading data from socket")
         self.set_exception(exc)

aiohttp/client_reqrep.py

@@ -606,6 +606,8 @@ async def write_bytes(
                 protocol.set_exception(exc)
         except Exception as exc:
             protocol.set_exception(exc)
+        else:
+            protocol.start_timeout()
         finally:
             self._writer = None
 

tests/test_client_functional.py

@@ -8,6 +8,7 @@
 import pathlib
 import socket
 import ssl
+from typing import AsyncIterator
 from unittest import mock
 
 import pytest
@@ -673,6 +674,25 @@ async def handler(request):
             await resp.content.read()
 
 
+async def test_read_timeout_on_write(aiohttp_client) -> None:
+    async def gen_payload() -> AsyncIterator[str]:
+        # Delay writing to ensure read timeout isn't triggered before writing completes.
+        await asyncio.sleep(0.5)
+        yield b"foo"
+
+    async def handler(request: web.Request) -> web.Response:
+        return web.Response(body=await request.read())
+
+    app = web.Application()
+    app.router.add_put("/", handler)
+
+    timeout = aiohttp.ClientTimeout(total=None, sock_read=0.1)
+    client = await aiohttp_client(app)
+    async with client.put("/", data=gen_payload(), timeout=timeout) as resp:
+        result = await resp.read()  # Should not trigger a read timeout.
+    assert result == b"foo"
+
+
 async def test_timeout_on_reading_data(aiohttp_client, mocker) -> None:
     loop = asyncio.get_event_loop()
 

tests/test_client_proto.py

@@ -107,12 +107,15 @@ async def test_empty_data(loop) -> None:
 async def test_schedule_timeout(loop) -> None:
     proto = ResponseHandler(loop=loop)
     proto.set_response_params(read_timeout=1)
+    assert proto._read_timeout_handle is None
+    proto.start_timeout()
     assert proto._read_timeout_handle is not None
 
 
 async def test_drop_timeout(loop) -> None:
     proto = ResponseHandler(loop=loop)
     proto.set_response_params(read_timeout=1)
+    proto.start_timeout()
     assert proto._read_timeout_handle is not None
     proto._drop_timeout()
     assert proto._read_timeout_handle is None
@@ -121,6 +124,7 @@ async def test_drop_timeout(loop) -> None:
 async def test_reschedule_timeout(loop) -> None:
     proto = ResponseHandler(loop=loop)
     proto.set_response_params(read_timeout=1)
+    proto.start_timeout()
     assert proto._read_timeout_handle is not None
     h = proto._read_timeout_handle
     proto._reschedule_timeout()
@@ -131,6 +135,7 @@ async def test_reschedule_timeout(loop) -> None:
 async def test_eof_received(loop) -> None:
     proto = ResponseHandler(loop=loop)
     proto.set_response_params(read_timeout=1)
+    proto.start_timeout()
     assert proto._read_timeout_handle is not None
     proto.eof_received()
     assert proto._read_timeout_handle is None