Merge pull request #553 from okfn/553-compare_domains-wrong-url

compare_domains in lib/datapreview crashes with malformed URLs

ckan/lib/datapreview.py

@@ -22,10 +22,14 @@ def compare_domains(urls):
     for url in urls:
         # all urls are interpreted as absolute urls,
         # except for urls that start with a /
-        if not urlparse.urlparse(url).scheme and not url.startswith('/'):
-            url = '//' + url
-        parsed = urlparse.urlparse(url.lower(), 'http')
-        domain = (parsed.scheme, parsed.hostname, parsed.port)
+        try:
+            if not urlparse.urlparse(url).scheme and not url.startswith('/'):
+                url = '//' + url
+            parsed = urlparse.urlparse(url.lower(), 'http')
+            domain = (parsed.scheme, parsed.hostname, parsed.port)
+        except ValueError:
+            # URL is so messed up that even urlparse can't stand it
+            return False
 
         if not first_domain:
             first_domain = domain

ckan/tests/lib/test_datapreview.py

@@ -1,20 +1,28 @@
+# -*- coding: utf-8 -*-
 import ckan.lib.datapreview as datapreview
 
+
 class TestDataPreview():
     def test_compare_domains(self):
         ''' see https://en.wikipedia.org/wiki/Same_origin_policy
         '''
         comp = datapreview.compare_domains
-        assert comp(['http://www.okfn.org', 'http://www.okfn.org']) == True
-        assert comp(['http://www.okfn.org', 'http://www.okfn.org', 'http://www.okfn.org']) == True
-        assert comp(['http://www.OKFN.org', 'http://www.okfn.org', 'http://www.okfn.org/test/foo.html']) == True
-        assert comp(['http://okfn.org', 'http://okfn.org']) == True
-        assert comp(['www.okfn.org', 'http://www.okfn.org']) == True
-        assert comp(['//www.okfn.org', 'http://www.okfn.org']) == True
+        assert comp(['http://www.okfn.org', 'http://www.okfn.org']) is True
+        assert comp(['http://www.okfn.org', 'http://www.okfn.org', 'http://www.okfn.org']) is True
+        assert comp(['http://www.OKFN.org', 'http://www.okfn.org', 'http://www.okfn.org/test/foo.html']) is True
+        assert comp(['http://okfn.org', 'http://okfn.org']) is True
+        assert comp(['www.okfn.org', 'http://www.okfn.org']) is True
+        assert comp(['//www.okfn.org', 'http://www.okfn.org']) is True
+
+        assert comp(['http://www.okfn.org', 'https://www.okfn.org']) is False
+        assert comp(['http://www.okfn.org:80', 'http://www.okfn.org:81']) is False
+        assert comp(['http://www.okfn.org', 'http://www.okfn.de']) is False
+        assert comp(['http://de.okfn.org', 'http://www.okfn.org']) is False
+
+        assert comp(['http://de.okfn.org', 'http:www.foo.com']) is False
 
-        assert comp(['http://www.okfn.org', 'https://www.okfn.org']) == False
-        assert comp(['http://www.okfn.org:80', 'http://www.okfn.org:81']) == False
-        assert comp(['http://www.okfn.org', 'http://www.okfn.de']) == False
-        assert comp(['http://de.okfn.org', 'http://www.okfn.org']) == False
+        assert comp(['httpö://wöwöwö.ckan.dö', 'www.ckän.örg']) is False
+        assert comp(['www.ckän.örg', 'www.ckän.örg']) is True
 
-        assert comp(['http://de.okfn.org', 'http:www.foo.com']) == False
+        # Wrong URL. Makes urlparse choke
+        assert comp(['http://Server=cda3; Service=sde:sqlserver:cda3; Database=NationalDatasets; User=sde; Version=sde.DEFAULT', 'http://www.okf.org']) is False