Repository: cdent/gabbi

Issue:
gabbi can't cope with an ipv6 host argument

If an ipv6 address is passed in host when doing `build_tests` the built url is incorrect. An ipv6 address in a url is supposed to be bracketed: `[::1]`.

Discussion:

Patch:
diff --git a/docs/source/runner.rst b/docs/source/runner.rst
index fe35932..aaa5815 100644
--- a/docs/source/runner.rst
+++ b/docs/source/runner.rst
@@ -32,5 +32,8 @@ or in the target URL::
 The value of prefix will be prepended to the path portion of URLs that
 are not fully qualified.
 
+Anywhere host is used, if it is a raw IPV6 address it should be
+wrapped in ``[`` and ``]``.
+
 If a ``-x`` or ``--failfast`` argument is provided then ``gabbi-run`` will
 exit after the first test failure.
diff --git a/gabbi/runner.py b/gabbi/runner.py
index 463f030..25036a4 100644
--- a/gabbi/runner.py
+++ b/gabbi/runner.py
@@ -67,7 +67,9 @@ def run():
         'target',
         nargs='?', default='stub',
         help='A fully qualified URL (with optional path as prefix) '
-             'to the primary target or a host and port, : separated'
+             'to the primary target or a host and port, : separated. '
+             'If using an IPV6 address for the host in either form, '
+             'wrap it in \'[\' and \']\'.'
     )
     parser.add_argument(
         'prefix',
@@ -98,11 +100,14 @@ def run():
         target = args.target
         prefix = args.prefix
 
-    if ':' in target:
-        host, port = target.split(':')
+    if ':' in target and '[' not in target:
+        host, port = target.rsplit(':', 1)
+    elif ']:' in target:
+        host, port = target.rsplit(':', 1)
     else:
         host = target
         port = None
+    host = host.replace('[', '').replace(']', '')
 
     # Initialize response handlers.
     custom_response_handlers = []
diff --git a/gabbi/utils.py b/gabbi/utils.py
index f433dd2..a0f812e 100644
--- a/gabbi/utils.py
+++ b/gabbi/utils.py
@@ -29,10 +29,17 @@ from six.moves.urllib import parse as urlparse
 def create_url(base_url, host, port=None, prefix='', ssl=False):
     """Given pieces of a path-based url, return a fully qualified url."""
     scheme = 'http'
-    netloc = host
+
+    # A host with : in it at this stage is assumed to be an IPv6
+    # address of some kind (they come in many forms). Port should
+    # already have been stripped off.
+    if ':' in host and not (host.startswith('[') and host.endswith(']')):
+        host = '[%s]' % host
 
     if port and not _port_follows_standard(port, ssl):
         netloc = '%s:%s' % (host, port)
+    else:
+        netloc = host
 
     if ssl:
         scheme = 'https'


Test patch:
diff --git a/gabbi/tests/test_parse_url.py b/gabbi/tests/test_parse_url.py
index 8879cec..43066c7 100644
--- a/gabbi/tests/test_parse_url.py
+++ b/gabbi/tests/test_parse_url.py
@@ -40,21 +40,21 @@ class UrlParseTest(unittest.TestCase):
         return http_case
 
     def test_parse_url(self):
-        host = uuid.uuid4()
+        host = uuid.uuid4().hex
         http_case = self.make_test_case(host)
         parsed_url = http_case._parse_url('/foobar')
 
         self.assertEqual('http://%s:8000/foobar' % host, parsed_url)
 
     def test_parse_prefix(self):
-        host = uuid.uuid4()
+        host = uuid.uuid4().hex
         http_case = self.make_test_case(host, prefix='/noise')
         parsed_url = http_case._parse_url('/foobar')
 
         self.assertEqual('http://%s:8000/noise/foobar' % host, parsed_url)
 
     def test_parse_full(self):
-        host = uuid.uuid4()
+        host = uuid.uuid4().hex
         http_case = self.make_test_case(host)
         parsed_url = http_case._parse_url('http://example.com/house')
 
@@ -102,6 +102,37 @@ class UrlParseTest(unittest.TestCase):
 
         self.assertEqual('https://%s:80/foobar' % host, parsed_url)
 
+    def test_ipv6_url(self):
+        host = '::1'
+        http_case = self.make_test_case(host, port='80', ssl=True)
+        parsed_url = http_case._parse_url('/foobar')
+
+        self.assertEqual('https://[%s]:80/foobar' % host, parsed_url)
+
+    def test_ipv6_full_url(self):
+        host = '::1'
+        http_case = self.make_test_case(host, port='80', ssl=True)
+        parsed_url = http_case._parse_url(
+            'http://[2001:4860:4860::8888]/foobar')
+
+        self.assertEqual('http://[2001:4860:4860::8888]/foobar', parsed_url)
+
+    def test_ipv6_no_double_colon_wacky_ssl(self):
+        host = 'FEDC:BA98:7654:3210:FEDC:BA98:7654:3210'
+        http_case = self.make_test_case(host, port='80', ssl=True)
+        parsed_url = http_case._parse_url('/foobar')
+
+        self.assertEqual(
+            'https://[FEDC:BA98:7654:3210:FEDC:BA98:7654:3210]:80/foobar',
+            parsed_url)
+
+        http_case = self.make_test_case(host, ssl=True)
+        parsed_url = http_case._parse_url('/foobar')
+
+        self.assertEqual(
+            'https://[FEDC:BA98:7654:3210:FEDC:BA98:7654:3210]:8000/foobar',
+            parsed_url)
+
     def test_add_query_params(self):
         host = uuid.uuid4().hex
         # Use a sequence of tuples to ensure order.
@@ -121,7 +152,7 @@ class UrlParseTest(unittest.TestCase):
         self.assertEqual('http://%s:8000/foobar?alpha=beta&x=1&y=2'
                          % host, parsed_url)
 
-    def test_extend_query_params_full_ulr(self):
+    def test_extend_query_params_full_url(self):
         host = 'stub'
         query = OrderedDict([('x', 1), ('y', 2)])
         http_case = self.make_test_case(host, params=query)
diff --git a/gabbi/tests/test_runner.py b/gabbi/tests/test_runner.py
index c0b6925..a5c9ea0 100644
--- a/gabbi/tests/test_runner.py
+++ b/gabbi/tests/test_runner.py
@@ -13,6 +13,7 @@
 """Test that the CLI works as expected
 """
 
+import mock
 import sys
 import unittest
 from uuid import uuid4
@@ -218,6 +219,72 @@ class RunnerTest(unittest.TestCase):
         self._stderr.write(sys.stderr.read())
 
 
+class RunnerHostArgParse(unittest.TestCase):
+
+    @mock.patch('sys.exit')
+    @mock.patch('sys.stdin')
+    @mock.patch('gabbi.driver.test_suite_from_dict')
+    @mock.patch('yaml.safe_load', return_value={})
+    def _test_hostport(self, url_or_host, expected_host,
+                       portmock_yaml, mock_test_suite, mock_read, mock_exit,
+                       provided_prefix=None, expected_port=None,
+                       expected_prefix=None,):
+        sys.argv = ['gabbi-run', url_or_host]
+        if provided_prefix:
+            sys.argv.append(provided_prefix)
+        runner.run()
+
+        mock_test_suite.assert_called_with(
+            unittest.defaultTestLoader, 'input', {}, '.', expected_host,
+            expected_port, None, None, prefix=expected_prefix
+        )
+
+    def test_plain_url(self):
+        self._test_hostport('http://foobar.com:80/news',
+                            'foobar.com',
+                            expected_port='80',
+                            expected_prefix='/news')
+
+    def test_simple_hostport(self):
+        self._test_hostport('foobar.com:999',
+                            'foobar.com',
+                            expected_port='999')
+
+    def test_simple_hostport_with_prefix(self):
+        self._test_hostport('foobar.com:999',
+                            'foobar.com',
+                            provided_prefix='/news',
+                            expected_port='999',
+                            expected_prefix='/news')
+
+    def test_ipv6_url_long(self):
+        self._test_hostport(
+            'http://[FEDC:BA98:7654:3210:FEDC:BA98:7654:3210]:999/news',
+            'FEDC:BA98:7654:3210:FEDC:BA98:7654:3210',
+            expected_port='999',
+            expected_prefix='/news')
+
+    def test_ipv6_url_localhost(self):
+        self._test_hostport(
+            'http://[::1]:999/news',
+            '::1',
+            expected_port='999',
+            expected_prefix='/news')
+
+    def test_ipv6_host_localhost(self):
+        # If a user wants to use the hostport form, then they need
+        # to hack it with the brackets.
+        self._test_hostport(
+            '[::1]',
+            '::1')
+
+    def test_ipv6_hostport_localhost(self):
+        self._test_hostport(
+            '[::1]:999',
+            '::1',
+            expected_port='999')
+
+
 class HTMLResponseHandler(handlers.ResponseHandler):
 
     test_key_suffix = 'html'
diff --git a/gabbi/tests/test_utils.py b/gabbi/tests/test_utils.py
index a69d72b..1754dad 100644
--- a/gabbi/tests/test_utils.py
+++ b/gabbi/tests/test_utils.py
@@ -132,3 +132,29 @@ class CreateURLTest(unittest.TestCase):
         url = utils.create_url('/foo/bar?x=1&y=2', 'test.host.com', ssl=True,
                                port=80)
         self.assertEqual('https://test.host.com:80/foo/bar?x=1&y=2', url)
+
+    def test_create_url_ipv6_ssl(self):
+        url = utils.create_url('/foo/bar?x=1&y=2', '::1', ssl=True)
+        self.assertEqual('https://[::1]/foo/bar?x=1&y=2', url)
+
+    def test_create_url_ipv6_ssl_weird_port(self):
+        url = utils.create_url('/foo/bar?x=1&y=2', '::1', ssl=True, port=80)
+        self.assertEqual('https://[::1]:80/foo/bar?x=1&y=2', url)
+
+    def test_create_url_ipv6_full(self):
+        url = utils.create_url('/foo/bar?x=1&y=2',
+                               '2607:f8b0:4000:801::200e', port=8080)
+        self.assertEqual(
+            'http://[2607:f8b0:4000:801::200e]:8080/foo/bar?x=1&y=2', url)
+
+    def test_create_url_ipv6_already_bracket(self):
+        url = utils.create_url(
+            '/foo/bar?x=1&y=2', '[2607:f8b0:4000:801::200e]', port=999)
+        self.assertEqual(
+            'http://[2607:f8b0:4000:801::200e]:999/foo/bar?x=1&y=2', url)
+
+    def test_create_url_no_double_colon(self):
+        url = utils.create_url(
+            '/foo', 'FEDC:BA98:7654:3210:FEDC:BA98:7654:3210', port=999)
+        self.assertEqual(
+            'http://[FEDC:BA98:7654:3210:FEDC:BA98:7654:3210]:999/foo', url)