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

aiohttp.web.run_app displays wrong URI for IPv6 addresses (e.g. http://::1:1234/) #1139

Closed
rutsky opened this issue Aug 31, 2016 · 12 comments
Closed
Labels

Comments

@rutsky
Copy link
Member

rutsky commented Aug 31, 2016

Use of aiohttp.web with IPv6 addresses produces wrong URI:

$ python -m aiohttp.web -H ::1 -P 1234 module:init_function
2016-08-31 03:13:21,999 asyncio DEBUG: Using selector: EpollSelector
======== Running on http://::1:1234/ ========
(Press CTRL+C to quit)

According to RFC 2732 IPv6 should be included in brackets: http://[::1]:1234/.

@asvetlov
Copy link
Member

Would you create a patch?
With help of ipaddress module it's really trivial task.

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

rutsky commented Aug 31, 2016

Is there a generic way to construct network location part of urlparse/urlsplit result?
Since URL is usually parsed with urlparse/urlsplit I would expect to use urlunparse/urlunsplit to properly construct URL.

Currently I don't see any way of handling IPv6 brackets other than somehow check is string is IPv6 address and append brackets if it is.

Other issue that we allow to use integer representation of IP addresses, like:

$ python -m aiohttp.web -H 2130706433 -P 1234 module:init_function
2016-08-31 15:10:34,376 asyncio DEBUG: Using selector: EpollSelector
======== Running on http://2130706433:1234/ ========
(Press CTRL+C to quit)

and http://2130706433:1234/ is not proper URL, should be http://127.0.0.1:1234.

@asvetlov
Copy link
Member

http://2130706433:1234/ is not forbidden, 2130706433 is a valid hostname. You might push it as an alias into /etc/host for example.
urlunsplit cannot help you: it accepts properly constructed netloc but has no way to specify ip and port. netloc in turn require brackets for ipv6.

So I suggest to use code like this: https://github.com/aio-libs/yarl/blob/master/yarl/__init__.py#L324-L330

@rutsky
Copy link
Member Author

rutsky commented Aug 31, 2016

@asvetlov Yes, 2130706433 is valid host name, but here 2130706433 is integer representation of 127.0.0.1:

In [13]: int(ipaddress.ip_address('127.0.0.1'))
Out[13]: 2130706433

and if we bind on 2130706433 we actually bind on 127.0.0.1 (python -m aiohttp.web -H 2130706433 ... listens on 127.0.0.1), so we need to translate integer representation of IP address to one that accepted in URL.

@asvetlov
Copy link
Member

Oops. We are not supposed to keep such weird functionality.
Feel free to break it.

@ghost
Copy link

ghost commented Sep 10, 2016

@asvetlov so, should we keep this functionality (-h 2130706433 equals 127.0.0.1) or not ? should this issue just include brackets ?

@asvetlov
Copy link
Member

No, we shouldn't.
Just support three forms:

  1. IPv4 aka 127.0.0.1
  2. IPv6 aka [::1]
  3. Hostname (with dots or without) aka localhost or google.com

@ghost
Copy link

ghost commented Sep 13, 2016

@asvetlov looks like it'a valid functionality, it's not weird. I have to explicitly check if hostname is integer representation of ip address

>>> str(ipaddress.IPv4Address(3232235521))
'192.168.0.1'

>>> socket.inet_ntoa(struct.pack("!I", 3232235521))
'192.168.0.1'

@asvetlov
Copy link
Member

asvetlov commented Sep 13, 2016

Just add lines like:

if not isinstance(host, str):
    raise ValueError('host should be str')

We may relax the rule by accepting int objects as well on demand but I really doubt if we will need it.
Anyway relaxing in the future is much easier than restricting via deprecation process.

@ghost
Copy link

ghost commented Sep 14, 2016

@asvetlov

I've started doing something like this, is this too much ? regex is from django
https://github.com/django/django/blob/master/django/core/validators.py#L87

    ipv6 = False
    try:
        ipv6 = ipaddress.ip_address(args.hostname).version == 6
    except ValueError:
        if not re.match(r'[a-z\u00a1-\uffff0-9](?:[a-z\u00a1-\uffff0-9-]{0,61}[a-z\u00a1-\uffff0-9])?',
                        args.hostname):
            arg_parser.error("hostname is not valid ipv4 or ipv6 address")

@asvetlov
Copy link
Member

You don't need any regex check. ip_address should be enough.
Honestly I don't care about non-ascii hostnames or very long strings.

@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