From 53f0eebf35b448be4da2d4c8f72f3d4ea5622902 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Mon, 7 Oct 2019 21:51:21 +0300 Subject: [PATCH] [3.6] Reset the sock_read timeout when data arrives (#4142) (#4143) The sock_read timeout should apply *per read operation*, but currently applies cumulatively across all reads. Reschedule the timeout each time data is received and add tests to validate that the timeout doesn't interfere with overall reading and correctly catches reads that take too long. Co-authored-by: Martijn Pieters --- CHANGES/3808.bugfix | 1 + CONTRIBUTORS.txt | 1 + aiohttp/client_proto.py | 2 ++ tests/test_client_functional.py | 50 +++++++++++++++++++++++++++++++++ 4 files changed, 54 insertions(+) create mode 100644 CHANGES/3808.bugfix diff --git a/CHANGES/3808.bugfix b/CHANGES/3808.bugfix new file mode 100644 index 00000000000..4691f241375 --- /dev/null +++ b/CHANGES/3808.bugfix @@ -0,0 +1 @@ +Reset the ``sock_read`` timeout each time data is received for a ``aiohttp.client`` response. \ No newline at end of file diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index 5c63704704e..c845df4c83b 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -160,6 +160,7 @@ Manuel Miranda Marat Sharafutdinov Marco Paolini Mariano Anaya +Martijn Pieters Martin Melka Martin Richard Mathias Fröjdman diff --git a/aiohttp/client_proto.py b/aiohttp/client_proto.py index 87db71a0e6e..a44e6454234 100644 --- a/aiohttp/client_proto.py +++ b/aiohttp/client_proto.py @@ -176,6 +176,8 @@ def _on_read_timeout(self) -> None: self._payload.set_exception(exc) def data_received(self, data: bytes) -> None: + self._reschedule_timeout() + if not data: return diff --git a/tests/test_client_functional.py b/tests/test_client_functional.py index 417e1fb646c..d860663a58d 100644 --- a/tests/test_client_functional.py +++ b/tests/test_client_functional.py @@ -598,6 +598,56 @@ async def handler(request): await client.get('/') +async def test_read_timeout_between_chunks(aiohttp_client, mocker) -> None: + mocker.patch('aiohttp.helpers.ceil').side_effect = ceil + + async def handler(request): + resp = aiohttp.web.StreamResponse() + await resp.prepare(request) + # write data 4 times, with pauses. Total time 0.4 seconds. + for _ in range(4): + await asyncio.sleep(0.1) + await resp.write(b'data\n') + return resp + + app = web.Application() + app.add_routes([web.get('/', handler)]) + + # A timeout of 0.2 seconds should apply per read. + timeout = aiohttp.ClientTimeout(sock_read=0.2) + client = await aiohttp_client(app, timeout=timeout) + + res = b'' + async with await client.get('/') as resp: + res += await resp.read() + + assert res == b'data\n' * 4 + + +async def test_read_timeout_on_reading_chunks(aiohttp_client, mocker) -> None: + mocker.patch('aiohttp.helpers.ceil').side_effect = ceil + + async def handler(request): + resp = aiohttp.web.StreamResponse() + await resp.prepare(request) + await resp.write(b'data\n') + await asyncio.sleep(1) + await resp.write(b'data\n') + return resp + + app = web.Application() + app.add_routes([web.get('/', handler)]) + + # A timeout of 0.2 seconds should apply per read. + timeout = aiohttp.ClientTimeout(sock_read=0.2) + client = await aiohttp_client(app, timeout=timeout) + + async with await client.get('/') as resp: + assert (await resp.content.read(5)) == b'data\n' + with pytest.raises(asyncio.TimeoutError): + await resp.content.read() + + async def test_timeout_on_reading_data(aiohttp_client, mocker) -> None: loop = asyncio.get_event_loop()