Skip to content

Commit

Permalink
Forbid reading response BODY after release (#2983)
Browse files Browse the repository at this point in the history
* Disable reading response BODY after release

* Add changelog

* Better coverage

* Change wording
  • Loading branch information
asvetlov authored May 7, 2018
1 parent fab961e commit e0378bd
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGES/2983.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Forbid reading response BODY after release
6 changes: 4 additions & 2 deletions aiohttp/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -362,8 +362,6 @@ async def _request(self, method, url, *,
resp.close()
raise TooManyRedirects(
history[0].request_info, tuple(history))
else:
resp.release()

# For 301 and 302, mimic IE, now changed in RFC
# https://github.com/kennethreitz/requests/pull/269
Expand All @@ -381,6 +379,10 @@ async def _request(self, method, url, *,
if r_url is None:
# see github.com/aio-libs/aiohttp/issues/2022
break
else:
# reading from correct redirection
# response is forbidden
resp.release()

try:
r_url = URL(
Expand Down
12 changes: 9 additions & 3 deletions aiohttp/client_reqrep.py
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,7 @@ class ClientResponse(HeadersMixin):
# setted up by ClientRequest after ClientResponse object creation
# post-init stage allows to not change ctor signature
_closed = True # to allow __del__ for non-initialized properly response
_released = False

def __init__(self, method, url, *,
writer, continue100, timer,
Expand Down Expand Up @@ -795,6 +796,8 @@ def closed(self):
return self._closed

def close(self):
if not self._released:
self._notify_content()
if self._closed:
return

Expand All @@ -806,9 +809,10 @@ def close(self):
self._connection.close()
self._connection = None
self._cleanup_writer()
self._notify_content()

def release(self):
if not self._released:
self._notify_content()
if self._closed:
return noop()

Expand All @@ -818,7 +822,6 @@ def release(self):
self._connection = None

self._cleanup_writer()
self._notify_content()
return noop()

def raise_for_status(self):
Expand All @@ -838,9 +841,10 @@ def _cleanup_writer(self):

def _notify_content(self):
content = self.content
if content and content.exception() is None and not content.is_eof():
if content and content.exception() is None:
content.set_exception(
ClientConnectionError('Connection closed'))
self._released = True

async def wait_for_close(self):
if self._writer is not None:
Expand All @@ -860,6 +864,8 @@ async def read(self):
except BaseException:
self.close()
raise
elif self._released:
raise ClientConnectionError('Connection closed')

return self._body

Expand Down
49 changes: 49 additions & 0 deletions tests/test_client_functional.py
Original file line number Diff line number Diff line change
Expand Up @@ -2549,3 +2549,52 @@ async def gen():

resp = await client.post('/', data=gen())
assert resp.status == 200


async def test_read_from_closed_response(aiohttp_client):
async def handler(request):
return web.Response(body=b'data')

app = web.Application()
app.add_routes([web.get('/', handler)])

client = await aiohttp_client(app)

async with client.get('/') as resp:
assert resp.status == 200

with pytest.raises(aiohttp.ClientConnectionError):
await resp.read()


async def test_read_from_closed_response2(aiohttp_client):
async def handler(request):
return web.Response(body=b'data')

app = web.Application()
app.add_routes([web.get('/', handler)])

client = await aiohttp_client(app)

async with client.get('/') as resp:
assert resp.status == 200
await resp.read()

with pytest.raises(aiohttp.ClientConnectionError):
await resp.read()


async def test_read_from_closed_content(aiohttp_client):
async def handler(request):
return web.Response(body=b'data')

app = web.Application()
app.add_routes([web.get('/', handler)])

client = await aiohttp_client(app)

async with client.get('/') as resp:
assert resp.status == 200

with pytest.raises(aiohttp.ClientConnectionError):
await resp.content.readline()

0 comments on commit e0378bd

Please sign in to comment.