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

Exception handling doesn't close connection #2827

Closed
brycedrennan opened this Issue Mar 12, 2018 · 1 comment

Comments

Projects
None yet
2 participants
@brycedrennan
Contributor

brycedrennan commented Mar 12, 2018

Long story short

With certain malformed urls, the connection is not properly closed. This causes an "Unclosed connection".

Expected behaviour

Connection cleaned up when there is an exception writing headers.

Actual behaviour

Connection not properly cleaned up.

Steps to reproduce

Here are two examples, one mocked and one using a live site. Both should produce an "Unclosed connection" error.

requires pytest and aresponses

@pytest.mark.asyncio
async def test_invalid_location(aresponses):
    # real example: 'http://www.skinhealthcanada.com/index.cfm?pagepath=Brands&id=66172'
    url = 'http://badwebsite.circleup.com/'
    bad_url = 'http://badwebsite.circleup.com/in\udcaedex.html'

    aresponses.add('badwebsite.circleup.com', aresponses.ANY, 'GET', aresponses.Response(status=302, headers={'Location': bad_url}))
    aresponses.add('badwebsite.circleup.com', aresponses.ANY, 'GET', aresponses.Response())

    async with aiohttp.ClientSession() as session:
        try:
            response = await session.get(bad_url)
        except Exception as e:
            print(repr(e))

    async with aiohttp.ClientSession() as session:
        response = await session.get(bad_url)


@pytest.mark.asyncio
async def test_invalid_location_live():
    url = 'http://www.skinhealthcanada.com/index.cfm?pagepath=Brands&id=66172'
    bad_url = 'http://www.skinhealthcanada.com/index.cfm?pagepath=Brands/Environ\udcae&id=66220'
    async with aiohttp.ClientSession() as session:
        try:
            response = await session.get(bad_url)
        except Exception as e:
            print(repr(e))

    async with aiohttp.ClientSession() as session:
        response = await session.get(bad_url)

Your environment

client

Python 3.6.4
Ubuntu 17.10
virtualenv
aiohttp==3.0.6

possible solution

change aiohttp/client.py lines 328-343 to

                    try:
                        try:
                            resp = req.send(conn)
                        except BaseException:
                            conn.close()
                            raise
                        try:
                            await resp.start(conn, read_until_eof)
                        except BaseException:
                            resp.close()
                            conn.close()
                            raise
                    except ClientError:
                        raise
                    except OSError as exc:
                        raise ClientOSError(*exc.args) from exc
@asvetlov

This comment has been minimized.

Member

asvetlov commented Mar 13, 2018

Looks like you are right.
Would you provide a PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment