Connections are not correctly kept after HEAD requests

[arthurdarcet]

When the client makes a HEAD request, it parses the response as if it was a regular message.

But the HTTP specs has a nice trick: HEAD requests SHOULD respond with the exact same headers than the corresponding GET request. This includes Content-Length or Transfer-Encoding: chunked headers.

This means that when the client receives a HEAD response with a Content-Length: 5 response, it should not expect any body and payload.is_eof() should be True immediately, not after receiving 5 more bytes.

Currently ResponseHandler.should_close is True for HEAD requests with a Content-Length: … because payload.is_eof() is False when it should be True, hence the underlying connection is closed and not re-used when it could be.

This is only inefficient on the client side, not at all a bug, and does not produce any wrong results.

(also, I was mistaken a few minutes ago: returning a Content-Length header with an empty body is perfectly valid, and the allow_head=True parameter of the add_get method works perfectly well, no problem at all on the server side)

reproducer:

#1769 (comment)

[hubo1016]

Does aiohttp client support pipelining? If a client sends more than one requests on the same connection before server returns, this may be a problem if the client mistakenly parses the next respond as the respond body.

[fafhrd91]

client does not support pipelining, issue #1740

[sethmlarson]

Dropping in to say that I was bitten by this bug as well, took a few hours to debug why it was happening.

[webknjaz]

It'd be great if somebody could post a complete reproducer. Or better yet — a test marked with xfail as per https://pganssle-talks.github.io/xfail-lightning.

[sethmlarson]

I have this as a simple reproducer locally FYI:

import aiohttp


async def test_head_request_reuses_connection():
    class SingleTCPConnector(aiohttp.TCPConnector):
        def __init__(self, *args, **kwargs):
            super().__init__(*args, **kwargs)
            self._connections_created = 0

        def _create_connection(self, *args, **kwargs):
            self._connections_created += 1
            if self._connections_created > 1:
                raise ValueError("only one connection!")
            return super()._create_connection(*args, **kwargs)

    async with SingleTCPConnector(limit=1) as connector:
        for _ in range(2):
            async with aiohttp.request("HEAD", "http://example.com", connector=connector) as resp:
                await resp.read()

Fails with HEAD but passes with all other methods.

[webknjaz]

@sethmlarson yeah, it'd be great if you could send that as a PR.