From 03eaeac459f80c96335a609e9700d3f4262950e5 Mon Sep 17 00:00:00 2001 From: Rick Elrod Date: Tue, 4 Oct 2022 15:21:56 -0500 Subject: [PATCH] Better handle IPv6 in util function update_scm_url (#12995) - Firstly -- add a bunch of unit tests for `update_scm_url`, because it previously had none and desperately needed them. - Secondly -- fix #12992 by adding back in IPv6 address brackets if they existed in the first place when the function was called. - Thirdly -- fix a related case where we disallowed IPv6 in URLs that did not include the scheme. Signed-off-by: Rick Elrod --- awx/main/tests/unit/utils/test_common.py | 81 ++++++++++++++++++++++++ awx/main/utils/common.py | 16 ++++- 2 files changed, 94 insertions(+), 3 deletions(-) diff --git a/awx/main/tests/unit/utils/test_common.py b/awx/main/tests/unit/utils/test_common.py index 65d2b5d1f3..8d14d7cf64 100644 --- a/awx/main/tests/unit/utils/test_common.py +++ b/awx/main/tests/unit/utils/test_common.py @@ -194,3 +194,84 @@ def test_extract_ansible_vars(): redacted, var_list = common.extract_ansible_vars(json.dumps(my_dict)) assert var_list == set(['ansible_connetion_setting']) assert redacted == {"foobar": "baz"} + + +@pytest.mark.parametrize( + 'scm_type, url, username, password, check_special_cases, scp_format, expected', + [ + # General/random cases + ('git', '', True, True, True, False, ''), + ('git', 'git://example.com/foo.git', True, True, True, False, 'git://example.com/foo.git'), + ('git', 'http://example.com/foo.git', True, True, True, False, 'http://example.com/foo.git'), + ('git', 'example.com:bar.git', True, True, True, False, 'git+ssh://example.com/bar.git'), + ('git', 'user@example.com:bar.git', True, True, True, False, 'git+ssh://user@example.com/bar.git'), + ('git', '127.0.0.1:bar.git', True, True, True, False, 'git+ssh://127.0.0.1/bar.git'), + ('git', 'git+ssh://127.0.0.1/bar.git', True, True, True, True, '127.0.0.1:bar.git'), + ('git', 'ssh://127.0.0.1:22/bar.git', True, True, True, False, 'ssh://127.0.0.1:22/bar.git'), + ('git', 'ssh://root@127.0.0.1:22/bar.git', True, True, True, False, 'ssh://root@127.0.0.1:22/bar.git'), + ('git', 'some/path', True, True, True, False, 'file:///some/path'), + ('git', '/some/path', True, True, True, False, 'file:///some/path'), + # Invalid URLs - ensure we error properly + ('cvs', 'anything', True, True, True, False, ValueError('Unsupported SCM type "cvs"')), + ('svn', 'anything-without-colon-slash-slash', True, True, True, False, ValueError('Invalid svn URL')), + ('git', 'http://example.com:123invalidport/foo.git', True, True, True, False, ValueError('Invalid git URL')), + ('git', 'git+ssh://127.0.0.1/bar.git', True, True, True, False, ValueError('Unsupported git URL')), + ('git', 'git@example.com:3000:/git/repo.git', True, True, True, False, ValueError('Invalid git URL')), + ('insights', 'git://example.com/foo.git', True, True, True, False, ValueError('Unsupported insights URL')), + ('svn', 'file://example/path', True, True, True, False, ValueError('Unsupported host "example" for file:// URL')), + ('svn', 'svn:///example', True, True, True, False, ValueError('Host is required for svn URL')), + # Username/password cases + ('git', 'https://example@example.com/bar.git', False, True, True, False, 'https://example.com/bar.git'), + ('git', 'https://example@example.com/bar.git', 'user', True, True, False, 'https://user@example.com/bar.git'), + ('git', 'https://example@example.com/bar.git', 'user:pw', True, True, False, 'https://user%3Apw@example.com/bar.git'), + ('git', 'https://example@example.com/bar.git', False, 'pw', True, False, 'https://example.com/bar.git'), + ('git', 'https://some:example@example.com/bar.git', True, False, True, False, 'https://some@example.com/bar.git'), + ('git', 'https://some:example@example.com/bar.git', False, False, True, False, 'https://example.com/bar.git'), + ('git', 'https://example.com/bar.git', 'user', 'pw', True, False, 'https://user:pw@example.com/bar.git'), + ('git', 'https://example@example.com/bar.git', False, 'something', True, False, 'https://example.com/bar.git'), + # Special github/bitbucket cases + ('git', 'notgit@github.com:ansible/awx.git', True, True, True, False, ValueError('Username must be "git" for SSH access to github.com.')), + ( + 'git', + 'notgit@bitbucket.org:does-not-exist/example.git', + True, + True, + True, + False, + ValueError('Username must be "git" for SSH access to bitbucket.org.'), + ), + ( + 'git', + 'notgit@altssh.bitbucket.org:does-not-exist/example.git', + True, + True, + True, + False, + ValueError('Username must be "git" for SSH access to altssh.bitbucket.org.'), + ), + ('git', 'git:password@github.com:ansible/awx.git', True, True, True, False, 'git+ssh://git@github.com/ansible/awx.git'), + # Disabling the special handling should not raise an error + ('git', 'notgit@github.com:ansible/awx.git', True, True, False, False, 'git+ssh://notgit@github.com/ansible/awx.git'), + ('git', 'notgit@bitbucket.org:does-not-exist/example.git', True, True, False, False, 'git+ssh://notgit@bitbucket.org/does-not-exist/example.git'), + ( + 'git', + 'notgit@altssh.bitbucket.org:does-not-exist/example.git', + True, + True, + False, + False, + 'git+ssh://notgit@altssh.bitbucket.org/does-not-exist/example.git', + ), + # awx#12992 - IPv6 + ('git', 'http://[fd00:1234:2345:6789::11]:3000/foo.git', True, True, True, False, 'http://[fd00:1234:2345:6789::11]:3000/foo.git'), + ('git', 'http://foo:bar@[fd00:1234:2345:6789::11]:3000/foo.git', True, True, True, False, 'http://foo:bar@[fd00:1234:2345:6789::11]:3000/foo.git'), + ('git', 'example@[fd00:1234:2345:6789::11]:example/foo.git', True, True, True, False, 'git+ssh://example@[fd00:1234:2345:6789::11]/example/foo.git'), + ], +) +def test_update_scm_url(scm_type, url, username, password, check_special_cases, scp_format, expected): + if isinstance(expected, Exception): + with pytest.raises(type(expected)) as excinfo: + common.update_scm_url(scm_type, url, username, password, check_special_cases, scp_format) + assert str(excinfo.value) == str(expected) + else: + assert common.update_scm_url(scm_type, url, username, password, check_special_cases, scp_format) == expected diff --git a/awx/main/utils/common.py b/awx/main/utils/common.py index 5d06185f78..98b20851c8 100644 --- a/awx/main/utils/common.py +++ b/awx/main/utils/common.py @@ -264,9 +264,15 @@ def update_scm_url(scm_type, url, username=True, password=True, check_special_ca userpass, hostpath = url.split('@', 1) else: userpass, hostpath = '', url - if hostpath.count(':') > 1: + # Handle IPv6 here. In this case, we might have hostpath of: + # [fd00:1234:2345:6789::11]:example/foo.git + if hostpath.startswith('[') and ']:' in hostpath: + host, path = hostpath.split(']:', 1) + host = host + ']' + elif hostpath.count(':') > 1: raise ValueError(_('Invalid %s URL') % scm_type) - host, path = hostpath.split(':', 1) + else: + host, path = hostpath.split(':', 1) # if not path.startswith('/') and not path.startswith('~/'): # path = '~/%s' % path # if path.startswith('/'): @@ -325,7 +331,11 @@ def update_scm_url(scm_type, url, username=True, password=True, check_special_ca netloc = u':'.join([urllib.parse.quote(x, safe='') for x in (netloc_username, netloc_password) if x]) else: netloc = u'' - netloc = u'@'.join(filter(None, [netloc, parts.hostname])) + # urllib.parse strips brackets from IPv6 addresses, so we need to add them back in + hostname = parts.hostname + if hostname and ':' in hostname and '[' in url and ']' in url: + hostname = f'[{hostname}]' + netloc = u'@'.join(filter(None, [netloc, hostname])) if parts.port: netloc = u':'.join([netloc, str(parts.port)]) new_url = urllib.parse.urlunsplit([parts.scheme, netloc, parts.path, parts.query, parts.fragment])