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

Client can not handle hostnames with 63 bytes when a port is given in the url #1044

Closed
MirkoDziadzka opened this issue Aug 4, 2016 · 5 comments
Labels

Comments

@MirkoDziadzka
Copy link

MirkoDziadzka commented Aug 4, 2016

Long story short

The http client does have some problem, when the last part of the hostname becomes too long (but in the valid length of an DNS part)

To reproduce, choose a hostname with 60 bytes in the last part of DNS, add a port and try to access the url:

https://host-12345678901234567890123456789012345678901234567890-name:8888/

Note that the hostname is 60 octets, but hostname with port is 65 octets

Expected behaviour

should work with all valid hostnames (a part can have a length of 63 octets)

Actual behaviour

does not work when the last part of a hostname together with the optional port exceeds 63 octets.

Steps to reproduce

Run the following script. It should print "can not connect". Instead it raises

UnicodeError: label too long
UnicodeError: encoding with 'idna' codec failed (UnicodeError: label too long)
ValueError: URL has an invalid label.

import asyncio
import aiohttp

url = "https://host-12345678901234567890123456789012345678901234567890-name:8888/"
#url = "https://fooooo:8888/"

async def test_bug(session):
    try:
        async with session.get(url) as resp:
            pass
    except aiohttp.errors.ClientOSError:
        print("can not connect")

loop = asyncio.get_event_loop()
with aiohttp.ClientSession(loop=loop) as session:
    loop.run_until_complete(test_bug(session))

Your environment

MacOS 10.9, Python 3.5
using aiohttp-0.21.5

@MirkoDziadzka
Copy link
Author

The following patch agains 0.21.5 fixes the problem for me.

diff --git a/build-env/lib/python3.5/site-packages/aiohttp/client_reqrep.py b/build-env/lib/python3.5/site-packages/aiohttp/client_reqrep.py
index 68d0ea5..14e2640 100644
--- a/build-env/lib/python3.5/site-packages/aiohttp/client_reqrep.py
+++ b/build-env/lib/python3.5/site-packages/aiohttp/client_reqrep.py
@@ -117,7 +117,7 @@ class ClientRequest:

         # check domain idna encoding
         try:
-            netloc = netloc.encode('idna').decode('utf-8')
+            # netloc = netloc.encode('idna').decode('utf-8')
             host = host.encode('idna').decode('utf-8')
         except UnicodeError:
             raise ValueError('URL has an invalid label.')

I really do not understand why the netloc is checked here, as it can contain the port. IDNA encoding should probably only be executed on the hostname part without the port.

@MirkoDziadzka MirkoDziadzka changed the title http client can not handle hostnames with 63 bytes when a port is given in the url Client can not handle hostnames with 63 bytes when a port is given in the url Aug 4, 2016
@asvetlov
Copy link
Member

asvetlov commented Aug 4, 2016

Good catch!
But let me postpone the fix for a while: I want to finish https://github.com/asvetlov/yarl first.
The library should address all these shits with URL encoding, quoting etc.

@asvetlov asvetlov added the good first issue Good for newcomers label Aug 14, 2016
@asvetlov
Copy link
Member

Well, I don't know when yarl will be ready but a fix for this particular case is trivial.
Please don't hesitate to make a Pull Request.

Taekyoon added a commit to Taekyoon/aiohttp that referenced this issue Aug 15, 2016
… the url aio-libs#1044

I handled by reconstructing url string.
And I used function make_netlog in client_reqrep.py
Taekyoon added a commit to Taekyoon/aiohttp that referenced this issue Aug 15, 2016
… the url aio-libs#1044

I handled by reconstructing url string.
And I used function make_netlog in client_reqrep.py
Taekyoon added a commit to Taekyoon/aiohttp that referenced this issue Aug 15, 2016
… the url aio-libs#1044

#last Message##############################
I handled by reconstructing url string.
And I used function make_netlog in client_reqrep.py
#########################################

I added missing test functions.
It would verify by these methods
Taekyoon added a commit to Taekyoon/aiohttp that referenced this issue Aug 15, 2016
… the url aio-libs#1044

#last Message##############################
I handled by reconstructing url string.
And I used function make_netlog in client_reqrep.py
#########################################

I added missing test functions.
It would verify by these methods
asvetlov pushed a commit that referenced this issue Aug 25, 2016
* Implement proxy support for ClientSession.ws_connect
#1025 Maybe.. Done..
I didn't check by Test mock....
But you said this would beeee OK... Hopefully ^^

This is my first commit.

* Client can not handle hostnames with 63 bytes when a port is given in the url #1044

I handled by reconstructing url string.
And I used function make_netlog in client_reqrep.py

* Client can not handle hostnames with 63 bytes when a port is given in the url #1044

I handled by reconstructing url string.
And I used function make_netlog in client_reqrep.py

* Client can not handle hostnames with 63 bytes when a port is given in the url #1044

#last Message##############################
I handled by reconstructing url string.
And I used function make_netlog in client_reqrep.py
#########################################

I added missing test functions.
It would verify by these methods

* Client can not handle hostnames with 63 bytes when a port is given in the url #1044

#last Message##############################
I handled by reconstructing url string.
And I used function make_netlog in client_reqrep.py
#########################################

I added missing test functions.
It would verify by these methods

* Fix the code format.
They all checked by flake8

* Fix the code format. @ client_reqrep.py
They all checked by flake8
@asvetlov
Copy link
Member

Fixed by #1088

@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.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants