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

Don't decompress responses for HEAD requests #154

Closed
wants to merge 2 commits into from
Closed

Don't decompress responses for HEAD requests #154

wants to merge 2 commits into from

Conversation

kxepal
Copy link
Member

@kxepal kxepal commented Sep 17, 2014

aiohttp causes error on receiving response against HEAD request with CONTENT-ENCODING header.

Traceback (most recent call last):
  File "/home/kxepal/projects/aiocouchdb/test.py", line 94, in <module>
    loop.run_until_complete(go('http://localhost:5984'))
  File "/usr/local/opt/virtualenv/aiocouchdb/lib/python3.3/site-packages/asyncio/base_events.py", line 208, in run_until_complete
    return future.result()
  File "/usr/local/opt/virtualenv/aiocouchdb/lib/python3.3/site-packages/asyncio/futures.py", line 243, in result
    raise self._exception
  File "/usr/local/opt/virtualenv/aiocouchdb/lib/python3.3/site-packages/asyncio/tasks.py", line 319, in _step
    result = coro.send(value)
  File "/home/kxepal/projects/aiocouchdb/test.py", line 41, in go
    att = yield from doc.att('bar.txt')
  File "/home/kxepal/projects/aiocouchdb/aiocouchdb/document.py", line 58, in attachment
    yield from resp.read()
  File "/home/kxepal/projects/aiocouchdb/aiocouchdb/client.py", line 91, in read
    data.extend((yield from self.content.read()))
  File "/home/kxepal/projects/aiohttp/aiohttp/streams.py", line 382, in read
    return (yield from super().read())
  File "/home/kxepal/projects/aiohttp/aiohttp/streams.py", line 393, in read
    return (yield from super().read())
  File "/home/kxepal/projects/aiohttp/aiohttp/streams.py", line 353, in read
    raise self._exception
  File "/home/kxepal/projects/aiohttp/aiohttp/parsers.py", line 197, in set_parser
    next(p)
  File "/home/kxepal/projects/aiohttp/aiohttp/protocol.py", line 307, in __call__
    out.feed_eof()
  File "/home/kxepal/projects/aiohttp/aiohttp/protocol.py", line 386, in feed_eof
    raise errors.IncompleteRead(0)
aiohttp.errors.IncompleteRead: IncompleteRead(0 bytes read)

This happens because DeflateBuffer is been used if response is compressed and it tries to decompress the received data on feed_eof. Obliviously, it fails because no data is returned back on HEAD request.

@fafhrd91
Copy link
Member

Looks ok, but that seems fix for this specific problem. What if you get empty body for other responses. Should deflatebuffer support empty body in general instead?

@kxepal
Copy link
Member Author

kxepal commented Sep 18, 2014

@fafhrd91 others aren't affected by this change since it depends on response_with_body flag which sets only for HEAD requests: https://github.com/KeepSafe/aiohttp/blob/master/aiohttp/client.py#L615
Unless you'd set it manually to False for others, but even if you did you probably had some significant reasons for that and you have to understand the related behaviour.

@kxepal
Copy link
Member Author

kxepal commented Sep 18, 2014

As in general, for others, this is an error case caused by decoding mechanisms must be applied :

The Content-Encoding entity-header field is used as a modifier to the media-type. When
present, its value indicates what additional content codings have been applied to the 
entity-body, and thus what decoding mechanisms must be applied in order to obtain the 
media-type referenced by the Content-Type header field.

-- http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.11

The error raises not by HTTP protocol routines, but by decodings one.

    The 204 response MUST NOT include a message-body, and thus is always
    terminated by the first empty line after the header fields.

	-- http://tools.ietf.org/html/rfc2616#section-10.2.5
@kxepal
Copy link
Member Author

kxepal commented Sep 29, 2014

@fafhrd91 I got your idea. There is also good old HTTP 204 No Content response which MUST NOT contains payload body, but there is nothing about response headers. And it also fails into the same trap. PR updated with the fix.

@fafhrd91
Copy link
Member

I'm talking about bugs in server software. Right now aiohttp can't handle
zero body response with deflate encoding, I'd prefer to cover all responses
and not just head

On Monday, September 29, 2014, Alexander Shorin [email protected]
wrote:

@fafhrd91 https://github.com/fafhrd91 I got your idea. There is also
good old HTTP 204 No Content response which MUST NOT contains payload
body, but there is nothing about response headers. And it also fails into
the same trap. PR updated with the fix.


Reply to this email directly or view it on GitHub
#154 (comment).

@kxepal
Copy link
Member Author

kxepal commented Sep 29, 2014

Fair enough, agreed. I'll open another PR with proper fix and better name with backreference.

@lock
Copy link

lock bot commented Oct 30, 2019

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.

@lock lock bot added the outdated label Oct 30, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants