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

[rutsky]

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]

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

[rutsky]

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]

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]

@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]

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

[ghost]

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

[asvetlov]

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]

@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]

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]

@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]

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]

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.