svnUriCanonicalize should never fail, even on invalid uris

buildbot · Aug 28, 2014 · c50dd2e · c50dd2e
1 parent 1478a2a
commit c50dd2e
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 43 deletions.
diff --git a/master/buildbot/steps/source/svn.py b/master/buildbot/steps/source/svn.py
@@ -411,7 +411,10 @@ def svnUriCanonicalize(uri):
         (scheme, authority, path, parameters, query, fragment) = urlparse(uri)
         scheme = lower(scheme)
         if authority:
-            userinfo, host, port = server_authority.match(authority).groups()
+            mo = server_authority.match(authority)
+            if not mo:
+                return uri  # give up
+            userinfo, host, port = mo.groups()
             if host[-1] == '.':
                 host = host[:-1]
             authority = lower(host)

diff --git a/master/buildbot/test/unit/test_steps_source_svn.py b/master/buildbot/test/unit/test_steps_source_svn.py
@@ -1826,21 +1826,6 @@ def test_omit_password(self):
         return self.runStep()
 
 
-#
-# svn.SVN.svnUriCanonicalize() test method factory
-#
-# given input string and expected result create a test method that
-# will call svn.SVN.svnUriCanonicalize() with the input and check
-# that expected result is returned
-#
-# @param input: test input
-# @param exp: expected result
-#
-def _makeSUCTest(input, exp):
-    return lambda self: self.assertEqual(
-        svn.SVN.svnUriCanonicalize(input), exp)
-
-
 class TestGetUnversionedFiles(unittest.TestCase):
 
     def test_getUnversionedFiles_does_not_list_externals(self):
@@ -1927,59 +1912,74 @@ def test_getUnversionedFiles_no_item(self):
         unversioned_files = list(svn.SVN.getUnversionedFiles(svn_st_xml, []))
         self.assertEquals(["svn_external_path/unversioned_file"], unversioned_files)
 
-    test_svnUriCanonicalize_empty = _makeSUCTest(
+
+class TestSvnUriCanonicalize(unittest.TestCase):
+    # svn.SVN.svnUriCanonicalize() test method factory
+    #
+    # given input string and expected result create a test method that
+    # will call svn.SVN.svnUriCanonicalize() with the input and check
+    # that expected result is returned
+    #
+    # @param input: test input
+    # @param exp: expected result
+
+    def _makeSUCTest(input, exp):
+        return lambda self: self.assertEqual(
+            svn.SVN.svnUriCanonicalize(input), exp)
+
+    test_empty = _makeSUCTest(
         "", "")
-    test_svnUriCanonicalize_canonical = _makeSUCTest(
+    test_canonical = _makeSUCTest(
         "http://foo.com/bar", "http://foo.com/bar")
-    test_svnUriCanonicalize_lc_scheme = _makeSUCTest(
+    test_lc_scheme = _makeSUCTest(
         "hTtP://foo.com/bar", "http://foo.com/bar")
-    test_svnUriCanonicalize_trailing_dot = _makeSUCTest(
+    test_trailing_dot = _makeSUCTest(
         "http://foo.com./bar", "http://foo.com/bar")
-    test_svnUriCanonicalize_lc_hostname = _makeSUCTest(
+    test_lc_hostname = _makeSUCTest(
         "http://foO.COm/bar", "http://foo.com/bar")
-    test_svnUriCanonicalize_lc_hostname_with_user = _makeSUCTest(
+    test_lc_hostname_with_user = _makeSUCTest(
         "http://Jimmy@fOO.Com/bar", "http://Jimmy@foo.com/bar")
-    test_svnUriCanonicalize_lc_hostname_with_user_pass = _makeSUCTest(
+    test_lc_hostname_with_user_pass = _makeSUCTest(
         "http://Jimmy:Sekrit@fOO.Com/bar", "http://Jimmy:Sekrit@foo.com/bar")
-    test_svnUriCanonicalize_trailing_slash = _makeSUCTest(
+    test_trailing_slash = _makeSUCTest(
         "http://foo.com/bar/", "http://foo.com/bar")
-    test_svnUriCanonicalize_trailing_slash_scheme = _makeSUCTest(
+    test_trailing_slash_scheme = _makeSUCTest(
         "http://", "http://")
-    test_svnUriCanonicalize_trailing_slash_hostname = _makeSUCTest(
+    test_trailing_slash_hostname = _makeSUCTest(
         "http://foo.com/", "http://foo.com")
-    test_svnUriCanonicalize_trailing_double_slash = _makeSUCTest(
+    test_trailing_double_slash = _makeSUCTest(
         "http://foo.com/x//", "http://foo.com/x")
-    test_svnUriCanonicalize_double_slash = _makeSUCTest(
+    test_double_slash = _makeSUCTest(
         "http://foo.com/x//y", "http://foo.com/x/y")
-    test_svnUriCanonicalize_slash = _makeSUCTest(
+    test_slash = _makeSUCTest(
         "/", "/")
-    test_svnUriCanonicalize_dot = _makeSUCTest(
+    test_dot = _makeSUCTest(
         "http://foo.com/x/./y", "http://foo.com/x/y")
-    test_svnUriCanonicalize_dot_dot = _makeSUCTest(
+    test_dot_dot = _makeSUCTest(
         "http://foo.com/x/../y", "http://foo.com/y")
-    test_svnUriCanonicalize_double_dot_dot = _makeSUCTest(
+    test_double_dot_dot = _makeSUCTest(
         "http://foo.com/x/y/../../z", "http://foo.com/z")
-    test_svnUriCanonicalize_dot_dot_root = _makeSUCTest(
+    test_dot_dot_root = _makeSUCTest(
         "http://foo.com/../x/y", "http://foo.com/x/y")
-    test_svnUriCanonicalize_quote_spaces = _makeSUCTest(
+    test_quote_spaces = _makeSUCTest(
         "svn+ssh://user@host:123/My Stuff/file.doc",
         "svn+ssh://user@host:123/My%20Stuff/file.doc")
-    test_svnUriCanonicalize_remove_port_80 = _makeSUCTest(
+    test_remove_port_80 = _makeSUCTest(
         "http://foo.com:80/bar", "http://foo.com/bar")
-    test_svnUriCanonicalize_dont_remove_port_80 = _makeSUCTest(
+    test_dont_remove_port_80 = _makeSUCTest(
         "https://foo.com:80/bar", "https://foo.com:80/bar")  # not http
-    test_svnUriCanonicalize_remove_port_443 = _makeSUCTest(
+    test_remove_port_443 = _makeSUCTest(
         "https://foo.com:443/bar", "https://foo.com/bar")
-    test_svnUriCanonicalize_dont_remove_port_443 = _makeSUCTest(
+    test_dont_remove_port_443 = _makeSUCTest(
         "svn://foo.com:443/bar", "svn://foo.com:443/bar")  # not https
-    test_svnUriCanonicalize_remove_port_3690 = _makeSUCTest(
+    test_remove_port_3690 = _makeSUCTest(
         "svn://foo.com:3690/bar", "svn://foo.com/bar")
-    test_svnUriCanonicalize_dont_remove_port_3690 = _makeSUCTest(
+    test_dont_remove_port_3690 = _makeSUCTest(
         "http://foo.com:3690/bar", "http://foo.com:3690/bar")  # not svn
-    test_svnUriCanonicalize_dont_remove_port_other = _makeSUCTest(
+    test_dont_remove_port_other = _makeSUCTest(
         "https://foo.com:2093/bar", "https://foo.com:2093/bar")
-    test_svnUriCanonicalize_quote_funny_chars = _makeSUCTest(
+    test_quote_funny_chars = _makeSUCTest(
         "http://foo.com/\x10\xe6%", "http://foo.com/%10%E6%25")
-    test_svnUriCanonicalize_overquoted = _makeSUCTest(
+    test_overquoted = _makeSUCTest(
         "http://foo.com/%68%65%6c%6c%6f%20%77%6f%72%6c%64",
         "http://foo.com/hello%20world")