Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Forbid reading response BODY after release #2983

Merged
merged 4 commits into from
May 7, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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()