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

Problems with HEAD responses #820

Closed
lgelo opened this issue Mar 10, 2016 · 6 comments
Closed

Problems with HEAD responses #820

lgelo opened this issue Mar 10, 2016 · 6 comments

Comments

@lgelo
Copy link
Contributor

lgelo commented Mar 10, 2016

RFC7231 forbids sending body content when responding to HEAD requests. Believing that servers are RFC compliant, aiohttp client is not parsing body of HEAD responses. Content of body stays in buffer and when parsing next response from the same session/connection BadStatusLine is raised. Not all servers are compliant, and unfortunately aiohttp server is one of them. Example code is bellow.

I believe the correct behaviour should be according to Postel's law: server won't send body in response to HEAD and will read and discard (maybe with warning) non-compliant responses.

#!/usr/bin/env python3.5

import asyncio
import aiohttp
from aiohttp import web

async def client(session):
    resp = await session.head('http://127.0.0.1:5000/')
    print("%d: '%s'" % (resp.status, await resp.text()))
    resp.release()

    # so far so good - try again
    resp = await session.get('http://127.0.0.1:5000/')
    print("%d: '%s'" % (resp.status, await resp.text()))
    await resp.release()

async def server(loop):
    app = web.Application()
    handler = app.make_handler()
    srv = await loop.create_server(handler, '127.0.0.1', 5000)
    return srv, handler

loop = asyncio.get_event_loop()
session = aiohttp.ClientSession(loop=loop)

srv, handler = loop.run_until_complete(server(loop))

client = loop.run_until_complete(client(session))
session.close()

loop.stop()
loop.close()
@asvetlov
Copy link
Member

Agree with all your proposals.
Would you make a Pull Request? Or two separate PRs -- one for client and other for server?

@lgelo
Copy link
Contributor Author

lgelo commented Mar 15, 2016

Probably just one PR. Proposed fix for the server (web_reqrep.py):

    def should_send_body(self):
        return (self._req.method.lower() != 'head' and 
               self._status not in [204,304])

    @asyncio.coroutine
    def write_eof(self):
        try:
            body = self._body
            if body is not None and self.should_send_body():
                self.write(body)
        finally:
            self.set_tcp_nodelay(True)
        yield from super().write_eof()

I don't know if it is possible to do something on the client side, because server can set Content-length header to "what it would have been if the request method had been GET" so there's no way to tell how much data (if any) should be read.

@asvetlov
Copy link
Member

User still may write

resp = web.StreamResponse(status=204)
await resp.prepare(request)
resp.write(b'BODY')
return resp

Honestly I not sure if we need to do anything on server side.

@lgelo
Copy link
Contributor Author

lgelo commented Mar 18, 2016

I agree, if they're trying to shoot yourself into leg, there's no way we can prevent it. Real life use case examples are: using HEAD to check existence of resource, cache probing/flushing or bandwidth saving measures for mobile devices. I think we need to do something because at least 404 response is created by server itself outside user's control.

@asvetlov
Copy link
Member

asvetlov commented Jun 2, 2016

Fixed by #838

@asvetlov asvetlov closed this as completed Jun 2, 2016
@lock
Copy link

lock bot commented Oct 29, 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 29, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants