Rewrite utils.parse_host to detect more invalid addresses.

The method now uses parsing methods from urllib to better split provided URLs.
Addresses containing query strings, parameters, passwords or fragments no longer fail silently.
SSH addresses containing paths are no longer accepted.

Signed-off-by: Joffrey F <joffrey@docker.com>

docker/transport/sshconn.py

@@ -1,10 +1,7 @@
-import urllib.parse
-
 import paramiko
 import requests.adapters
 import six
 
-
 from .. import constants
 
 if six.PY3:
@@ -82,7 +79,7 @@ class SSHAdapter(requests.adapters.HTTPAdapter):
         self.ssh_client = paramiko.SSHClient()
         self.ssh_client.load_system_host_keys()
 
-        parsed = urllib.parse.urlparse(base_url)
+        parsed = six.moves.urllib_parse.urlparse(base_url)
         self.ssh_client.connect(
             parsed.hostname, parsed.port, parsed.username,
         )

docker/utils/utils.py

@@ -1,10 +1,11 @@
 import base64
+import json
 import os
 import os.path
-import json
 import shlex
-from distutils.version import StrictVersion
+import string
 from datetime import datetime
+from distutils.version import StrictVersion
 
 import six
 
@@ -13,11 +14,12 @@ from .. import tls
 
 if six.PY2:
     from urllib import splitnport
+    from urlparse import urlparse
 else:
-    from urllib.parse import splitnport
+    from urllib.parse import splitnport, urlparse
 
 DEFAULT_HTTP_HOST = "127.0.0.1"
-DEFAULT_UNIX_SOCKET = "http+unix://var/run/docker.sock"
+DEFAULT_UNIX_SOCKET = "http+unix:///var/run/docker.sock"
 DEFAULT_NPIPE = 'npipe:////./pipe/docker_engine'
 
 BYTE_UNITS = {
@@ -212,81 +214,93 @@ def parse_repository_tag(repo_name):
     return repo_name, None
 
 
-# Based on utils.go:ParseHost http://tinyurl.com/nkahcfh
-# fd:// protocol unsupported (for obvious reasons)
-# Added support for http and https
-# Protocol translation: tcp -> http, unix -> http+unix
 def parse_host(addr, is_win32=False, tls=False):
-    proto = "http+unix"
-    port = None
     path = ''
+    port = None
+    host = None
 
+    # Sensible defaults
     if not addr and is_win32:
-        addr = DEFAULT_NPIPE
+        return DEFAULT_NPIPE
-
     if not addr or addr.strip() == 'unix://':
         return DEFAULT_UNIX_SOCKET
 
     addr = addr.strip()
-    if addr.startswith('http://'):
-        addr = addr.replace('http://', 'tcp://')
-    if addr.startswith('http+unix://'):
-        addr = addr.replace('http+unix://', 'unix://')
 
-    if addr == 'tcp://':
+    parsed_url = urlparse(addr)
+    proto = parsed_url.scheme
+    if not proto or any([x not in string.ascii_letters + '+' for x in proto]):
+        # https://bugs.python.org/issue754016
+        parsed_url = urlparse('//' + addr, 'tcp')
+        proto = 'tcp'
+
+    if proto == 'fd':
+        raise errors.DockerException('fd protocol is not implemented')
+
+    # These protos are valid aliases for our library but not for the
+    # official spec
+    if proto == 'http' or proto == 'https':
+        tls = proto == 'https'
+        proto = 'tcp'
+    elif proto == 'http+unix':
+        proto = 'unix'
+
+    if proto not in ('tcp', 'unix', 'npipe', 'ssh'):
         raise errors.DockerException(
-            "Invalid bind address format: {0}".format(addr)
+            "Invalid bind address protocol: {}".format(addr)
+        )
+
+    if proto == 'tcp' and not parsed_url.netloc:
+        # "tcp://" is exceptionally disallowed by convention;
+        # omitting a hostname for other protocols is fine
+        raise errors.DockerException(
+            'Invalid bind address format: {}'.format(addr)
+        )
+
+    if any([
+        parsed_url.params, parsed_url.query, parsed_url.fragment,
+        parsed_url.password
+    ]):
+        raise errors.DockerException(
+            'Invalid bind address format: {}'.format(addr)
+        )
+
+    if parsed_url.path and proto == 'ssh':
+        raise errors.DockerException(
+            'Invalid bind address format: no path allowed for this protocol:'
+            ' {}'.format(addr)
         )
-    elif addr.startswith('unix://'):
-        addr = addr[7:]
-    elif addr.startswith('tcp://'):
-        proto = 'http{0}'.format('s' if tls else '')
-        addr = addr[6:]
-    elif addr.startswith('https://'):
-        proto = "https"
-        addr = addr[8:]
-    elif addr.startswith('npipe://'):
-        proto = 'npipe'
-        addr = addr[8:]
-    elif addr.startswith('fd://'):
-        raise errors.DockerException("fd protocol is not implemented")
-    elif addr.startswith('ssh://'):
-        proto = 'ssh'
-        addr = addr[6:]
     else:
-        if "://" in addr:
+        path = parsed_url.path
-            raise errors.DockerException(
+        if proto == 'unix' and parsed_url.hostname is not None:
-                "Invalid bind address protocol: {0}".format(addr)
+            # For legacy reasons, we consider unix://path
-            )
+            # to be valid and equivalent to unix:///path
-        proto = "https" if tls else "http"
+            path = '/'.join((parsed_url.hostname, path))
-
-    if proto in ("http", "https", "ssh"):
-        address_parts = addr.split('/', 1)
-        host = address_parts[0]
-        if len(address_parts) == 2:
-            path = '/' + address_parts[1]
-        host, port = splitnport(host)
 
+    if proto in ('tcp', 'ssh'):
+        # parsed_url.hostname strips brackets from IPv6 addresses,
+        # which can be problematic hence our use of splitnport() instead.
+        host, port = splitnport(parsed_url.netloc)
         if port is None or port < 0:
-            if proto == 'ssh':
+            if proto != 'ssh':
-                port = 22
-            else:
                 raise errors.DockerException(
-                    "Invalid port: {0}".format(addr)
+                    'Invalid bind address format: port is required:'
+                    ' {}'.format(addr)
                 )
+            port = 22
 
         if not host:
             host = DEFAULT_HTTP_HOST
-    else:
-        host = addr
 
-    if proto in ("http", "https") and port == -1:
+    # Rewrite schemes to fit library internals (requests adapters)
-        raise errors.DockerException(
+    if proto == 'tcp':
-            "Bind address needs a port: {0}".format(addr))
+        proto = 'http{}'.format('s' if tls else '')
+    elif proto == 'unix':
+        proto = 'http+unix'
 
-    if proto == "http+unix" or proto == 'npipe':
+    if proto in ('http+unix', 'npipe'):
-        return "{0}://{1}".format(proto, host).rstrip('/')
+        return "{}://{}".format(proto, path).rstrip('/')
-    return "{0}://{1}:{2}{3}".format(proto, host, port, path).rstrip('/')
+    return '{0}://{1}:{2}{3}'.format(proto, host, port, path).rstrip('/')
 
 
 def parse_devices(devices):

tests/unit/utils_test.py

@@ -272,6 +272,11 @@ class ParseHostTest(unittest.TestCase):
             'tcp://',
             'udp://127.0.0.1',
             'udp://127.0.0.1:2375',
+            'ssh://:22/path',
+            'tcp://netloc:3333/path?q=1',
+            'unix:///sock/path#fragment',
+            'https://netloc:3333/path;params',
+            'ssh://:clearpassword@host:22',
         ]
 
         valid_hosts = {
@@ -281,7 +286,7 @@ class ParseHostTest(unittest.TestCase):
             'http://:7777': 'http://127.0.0.1:7777',
             'https://kokia.jp:2375': 'https://kokia.jp:2375',
             'unix:///var/run/docker.sock': 'http+unix:///var/run/docker.sock',
-            'unix://': 'http+unix://var/run/docker.sock',
+            'unix://': 'http+unix:///var/run/docker.sock',
             '12.234.45.127:2375/docker/engine': (
                 'http://12.234.45.127:2375/docker/engine'
             ),
@@ -294,6 +299,9 @@ class ParseHostTest(unittest.TestCase):
             '[fd12::82d1]:2375/docker/engine': (
                 'http://[fd12::82d1]:2375/docker/engine'
             ),
+            'ssh://': 'ssh://127.0.0.1:22',
+            'ssh://user@localhost:22': 'ssh://user@localhost:22',
+            'ssh://user@remote': 'ssh://user@remote:22',
         }
 
         for host in invalid_hosts:
@@ -304,7 +312,7 @@ class ParseHostTest(unittest.TestCase):
             assert parse_host(host, None) == expected
 
     def test_parse_host_empty_value(self):
-        unix_socket = 'http+unix://var/run/docker.sock'
+        unix_socket = 'http+unix:///var/run/docker.sock'
         npipe = 'npipe:////./pipe/docker_engine'
 
         for val in [None, '']: