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

Exception handling doesn't close connection #2827

Closed
brycedrennan opened this issue Mar 12, 2018 · 2 comments · Fixed by #2828
Closed

Exception handling doesn't close connection #2827

brycedrennan opened this issue Mar 12, 2018 · 2 comments · Fixed by #2828
Labels

Comments

@brycedrennan
Copy link
Contributor

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
Copy link
Member

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

@jemeraldo
Copy link

jemeraldo commented Apr 29, 2019

Hello. I faced with a similar situation.

import asyncio
import aiohttp

async def main():
    headers = {"User-Agent": None}
    try:
        async with aiohttp.ClientSession(headers=headers) as session:
            async with session.get("https://google.com", proxy="http://80.151.181.110:80") as resp:
                result = await resp.json()
    except Exception as e:
        print(repr(e))

asyncio.run(main())

This code leads to:

TypeError('Cannot serialize non-str key None')
Unclosed connection
client_connection: Connection<ConnectionKey(host='google.com', port=443, is_ssl=True, ssl=None, >proxy=None, proxy_auth=None, proxy_headers_hash=None)>

It's happening only if we pass proxy parameter.
should I close some connection manually in such case?

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

Successfully merging a pull request may close this issue.

3 participants