ipv6 client causes errors in twisted.web

[LaMont Jones]

If the client connecting to the twisted (16.0.0-1, still present in upstream 16.3.0), then twisted dies as follows:
2016-07-18 15:40:21 [HTTPChannel,468,::ffff:127.0.0.1] Unhandled Error
        Traceback (most recent call last):
          File "/usr/lib/python3/dist-packages/twisted/web/http.py", line 1657, in lineReceived
            self.allContentReceived()
          File "/usr/lib/python3/dist-packages/twisted/web/http.py", line 1736, in allContentReceived
            req.requestReceived(command, path, version)
          File "/usr/lib/python3/dist-packages/maasserver/webapp.py", line 80, in requestReceived
            command, path, version)
          File "/usr/lib/python3/dist-packages/twisted/web/http.py", line 762, in requestReceived
            self.process()
        --- <exception caught here> ---
          File "/usr/lib/python3/dist-packages/twisted/web/server.py", line 183, in process
            self.render(resrc)
          File "/usr/lib/python3/dist-packages/twisted/web/server.py", line 234, in render
            body = resrc.render(self)
          File "/usr/lib/python3/dist-packages/twisted/web/wsgi.py", line 561, in render
            self._reactor, self._threadpool, self._application, request)
          File "/usr/lib/python3/dist-packages/twisted/web/wsgi.py", line 293, in __init__
            'REMOTE_ADDR': _wsgiString(request.getClientIP()),
          File "/usr/lib/python3/dist-packages/twisted/web/wsgi.py", line 85, in _wsgiString
            return string.decode("iso-8859-1")
        builtins.AttributeError: 'NoneType' object has no attribute 'decode'

In the attached patch, the middle block addresses the above immediate issue, and the other two blocks are my best guess at what the code _should_ read for completeness.

We'll want to get this SRUed into xenial, once it's fixed in wily, as it's required for MAAS 2.1 (which runs on "latest LTS release" == xenial.)

thanks

[LaMont Jones (lamont)]

getRequestHostname() has similar problems, which this addresses:

    def new_getRequestHostname(self):
        host = self.getHeader(b'host')
        if host:
            if host.startswith(b'['):
                if host.find(b']') < host.rfind(b':'):
                    return host[:host.rfind(b':')]
                else:
                    return host
        host = self.getHost().host
        try:
            ip = IPAddress(host.decode("idna"))
        except AddrFormatError:
            # If we could not convert the hostname to an IPAddress, assume that
            # it is a hostname.
            return networkString(host)
        if ip.version == 4:
            return networkString(host)
        else:
            return networkString(b'[' + host + b']')

[LaMont Jones (lamont)]

investigating this more, twisted's handling of ipv6 is fundamentally broken in some bad ways:
1) it allows "2001:db8::1:80" as an IP/Port combination, deciding that what you meant is [2001:db8::1]:80, rather than the default port on 2001:db8::1:80. 2001:db8::1:7f, on the other hand, means "default port on 2001:db8::1:7f.
2) if I have a hostname, it requires me to guess whether that will resolve to A or AAAA records, since I have to tell it which socket to create before I even know the answer (unless I look it up myself.)
2.1) Having said that, TCPClient doesn't even believe in ipv6 at all, as noted above.

[LaMont Jones (lamont)]

With this patch, twisted behaves as expected when given an IPv6 IP address in the standard URL format: scheme://[ip:v6::add:ress]:port/path (port being optional, of course)

[LaMont Jones (lamont)]

What is not addressed by this patch is that twisted assumes that if it is given a hostname, then the user clearly means IPv4. The web client setup is not in the correct order, it should be:
1) resolve the hostname
2) iterate through the available IP addresses (probably preferring ipv6, like the rest of linux), trying each one until success, or end-of-list
2a) create the AF_INET/AF_INET6 socket based on the address family of the current trial IP.
2b) connect.

Today, it creates the socket based on the name it's given (ipv4 address, ipv6 address, or "huh... hostname, let's try ipv4") and then resolves it.

The other option would be to create an AF_INET6 socket, and if it turns out that it's an IPv4 address, change it to the compatible ipv6 address of "::ffff:192.168.1.1" or such. Note that if the app then calls getsockname() on the socket, it will see the ipv6-form, and may be confused.

[Free Ekanayaka (free.ekanayaka)]

Is this issue still there in 16.4.1? Has it been reported upstream?

[Alberto Donato (ack)]

This is still an issue in 17.9.0-2ubuntu0.1 in bionic

[Ubuntu Foundations Team Bug Bot (crichton)]

The attachment "Fixes at least some of the issues." seems to be a patch. If it isn't, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issues please contact him.]