Regression in URL Calculation of ServerApp

[jhamet93]

Description

I am unsure this is intentional but the calculation of the url field in the configuration file of the ServerApp changed between 1.8 -> 1.11.*. We noticed this doing an upgrade which broke some existing functionality on our end.

In 1.8, the calculation of this value was:

@property
def connection_url(self):
    ip = self.ip if self.ip else 'localhost'
    return self.get_url(ip=ip, path=self.base_url)

def get_url(self, ip=None, path=None, token=None):
        """Build a url for the application with reasonable defaults."""
        if not ip:
            ip = self.ip if self.ip else 'localhost'
        if not path:
            path = self.default_url
        # Build query string.
        if token:
            token = urllib.parse.urlencode({'token': token})
        # Build the URL Parts to dump.
        urlparts = urllib.parse.ParseResult(
            scheme='https' if self.certfile else 'http',
            netloc="{ip}:{port}".format(ip=ip, port=self.port),
            path=path,
            params=None,
            query=token,
            fragment=None
        )
        return urlparts.geturl()

In 1.11 the calculation is:

@property
    def connection_url(self):
        urlparts = self._get_urlparts(path=self.base_url)
        return urlparts.geturl()

def _get_urlparts(self, path=None, include_token=False):
        """Constructs a urllib named tuple, ParseResult,
        with default values set by server config.
        The returned tuple can be manipulated using the `_replace` method.
        """
        if self.sock:
            scheme = "http+unix"
            netloc = urlencode_unix_socket_path(self.sock)
        else:
            # Handle nonexplicit hostname.
            if self.ip in ("", "0.0.0.0", "::"):
                ip = "%s" % socket.gethostname()
            else:
                ip = "[{}]".format(self.ip) if ":" in self.ip else self.ip
            netloc = "{ip}:{port}".format(ip=ip, port=self.port)
            if self.certfile:
                scheme = "https"
            else:
                scheme = "http"
        if not path:
            path = self.default_url
        query = None
        if include_token:
            if self.token:  # Don't log full token if it came from config
                token = self.token if self._token_generated else "..."
                query = urllib.parse.urlencode({"token": token})
        # Build the URL Parts to dump.
        urlparts = urllib.parse.ParseResult(
            scheme=scheme,
            netloc=netloc,
            path=path,
            params=None,
            query=query,
            fragment=None,
        )
        return urlparts

The main condition I want to point out is related to the value of self.ip. In the earlier version, if the value was False such as an empty string (""), the calculation resolves the ip to localhost. In the later version, if the value was False such as an empty string (""), the calculation would resolve the ip to socket.gethostname() which in our case running this server in a K8's cluster, took on a different value than localhost. This overall results in a different value of the url field in the ServerApp configuration.

If this is a truly a regression, I am happy to submit a request to fix the issue at hand. Otherwise please ignore and resolve the issue.

[Zsailer]

This looks like a regression to me. Good catch, @jhamet93, and thank you for reporting!

I am happy to submit a request to fix the issue at hand

This would be awesome, thanks!

[vidartf]

Closing as fixed in #761