From df896e18c6df627663007f30447b1ae348bca1ce Mon Sep 17 00:00:00 2001 From: David Wortham Date: Sat, 6 Mar 2021 16:43:18 -0800 Subject: [PATCH 1/3] feat!: add validation checks in Jurisdiction constructor --- clarify/jurisdiction.py | 18 ++++++++++++++++ tests/test_jurisdiction.py | 43 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/clarify/jurisdiction.py b/clarify/jurisdiction.py index 8c5ebc8..b788376 100644 --- a/clarify/jurisdiction.py +++ b/clarify/jurisdiction.py @@ -7,6 +7,9 @@ import lxml.html from lxml.cssselect import CSSSelector +CLARITY_RESULTS_HOSTNAME = "results.enr.clarityelections.com" +SUPPORTED_LEVELS = ['state', 'county', 'city'] + class Jurisdiction(object): @@ -23,9 +26,24 @@ def __init__(self, url, level, name=''): level in lowercase ("state" or "county"). """ + if type(url) != str: + raise Exception('Invalid url parameter') + # if url is an HTTP URL to Clarity Election Results + elif len(url) >= 40 and url[0:40] == 'http://' + CLARITY_RESULTS_HOSTNAME + '/': + # Replace HTTP with HTTPS; retain the string after the URL origin + url = 'https://' + CLARITY_RESULTS_HOSTNAME + '/' + url[40] + # if url is not in the allowed list of supported origins + if len(url) < 41 or url[0:41] != 'https://' + CLARITY_RESULTS_HOSTNAME + '/': + raise Exception('Unsupported url origin') self.url = url self.parsed_url = self._parse_url() self.state = self._get_state_from_url() + + if type(level) != str: + raise Exception('Invalid level parameter') + level = level.lower() + if level not in SUPPORTED_LEVELS: + raise Exception('Unsupported level') self.level = level self.name = name self.summary_url = self._get_summary_url() diff --git a/tests/test_jurisdiction.py b/tests/test_jurisdiction.py index a974fb3..862b7e4 100644 --- a/tests/test_jurisdiction.py +++ b/tests/test_jurisdiction.py @@ -166,11 +166,54 @@ def mock_subjurisdiction_redirect_page_meta(page_id): class TestJurisdiction(TestCase): + # Test the constructor def test_construct(self): + """ + A Jurisdiction with a valid, supported URL and level string should create a class instance and not raise an Exception. + """ url = 'https://results.enr.clarityelections.com/KY/15261/30235/en/summary.html' jurisdiction = Jurisdiction(url=url, level='state') self.assertEqual(jurisdiction.url, url) + def test_construct_invalid_url_type(self): + """ + A Jurisdiction with an invalid URL data type should raise an Exception. + """ + with self.assertRaises(Exception): + Jurisdiction(url=None, level='state') + + def test_construct_invalid_url(self): + """ + A Jurisdiction with an invalid URL should raise an Exception. + """ + with self.assertRaises(Exception): + Jurisdiction(url='foo', level='state') + + def test_construct_invalid_hostname(self): + """ + A Jurisdiction with an unsupported hostname in the URL should raise an Exception. + """ + url = 'https://bad.hostname/KY/15261/30235/en/summary.html' + with self.assertRaises(Exception): + Jurisdiction(url=url, level='state') + + def test_construct_no_level(self): + """ + A Jurisdiction with a valid, supported hostname but no level in the URL should raise an Exception. + """ + url = 'https://results.enr.clarityelections.com/KY/15261/30235/en/summary.html' + with self.assertRaises(Exception): + Jurisdiction(url=url) + + def test_construct_invalid_level_str(self): + """ + A Jurisdiction with a valid, supported hostname but an invalid level in the URL should raise an Exception. + """ + url = 'https://results.enr.clarityelections.com/KY/15261/30235/en/summary.html' + with self.assertRaises(Exception): + Jurisdiction(url=url, level='foo') + + @responses.activate def test_get_subjurisdictions_state(self): # subjurisdiction url path is in script tag From b748bb2cdc4c1ed81029e45fcb775e5c6146f493 Mon Sep 17 00:00:00 2001 From: David Wortham Date: Sun, 7 Mar 2021 17:35:03 -0800 Subject: [PATCH 2/3] fix(lib): use TypeError, ValueError instead of Exception; support url conversion to str in Jusidsiction constructor, catching Exceptions; additional unit tests --- clarify/jurisdiction.py | 23 +++++++++---- tests/test_jurisdiction.py | 68 ++++++++++++++++++++++++++++++++------ 2 files changed, 73 insertions(+), 18 deletions(-) diff --git a/clarify/jurisdiction.py b/clarify/jurisdiction.py index b788376..10ce4de 100644 --- a/clarify/jurisdiction.py +++ b/clarify/jurisdiction.py @@ -8,7 +8,7 @@ from lxml.cssselect import CSSSelector CLARITY_RESULTS_HOSTNAME = "results.enr.clarityelections.com" -SUPPORTED_LEVELS = ['state', 'county', 'city'] +SUPPORTED_LEVELS = ['state', 'county', 'city', 'precinct'] class Jurisdiction(object): @@ -23,27 +23,36 @@ def __init__(self, url, level, name=''): """ To create an instance, pass a Clarity results URL for the top-level political jurisdiction (a state, for example), and the corresponding - level in lowercase ("state" or "county"). + level in lowercase ("state", "county", "city", or "precinct"). """ + # Preliminary check for any type which is not string-like + if url == None: + raise TypeError('Invalid url parameter') + try: + # Attempt to convert to str + url = str(url) + except Exception as error: + # If converting to str failed, raise type error + raise TypeError('Invalid url parameter') if type(url) != str: - raise Exception('Invalid url parameter') + raise TypeError('Invalid url parameter') # if url is an HTTP URL to Clarity Election Results - elif len(url) >= 40 and url[0:40] == 'http://' + CLARITY_RESULTS_HOSTNAME + '/': + if len(url) >= 40 and url[0:40] == 'http://' + CLARITY_RESULTS_HOSTNAME + '/': # Replace HTTP with HTTPS; retain the string after the URL origin url = 'https://' + CLARITY_RESULTS_HOSTNAME + '/' + url[40] # if url is not in the allowed list of supported origins if len(url) < 41 or url[0:41] != 'https://' + CLARITY_RESULTS_HOSTNAME + '/': - raise Exception('Unsupported url origin') + raise ValueError('Unsupported url origin') self.url = url self.parsed_url = self._parse_url() self.state = self._get_state_from_url() if type(level) != str: - raise Exception('Invalid level parameter') + raise TypeError('Invalid level parameter') level = level.lower() if level not in SUPPORTED_LEVELS: - raise Exception('Unsupported level') + raise ValueError('Unsupported level') self.level = level self.name = name self.summary_url = self._get_summary_url() diff --git a/tests/test_jurisdiction.py b/tests/test_jurisdiction.py index 862b7e4..f3c877c 100644 --- a/tests/test_jurisdiction.py +++ b/tests/test_jurisdiction.py @@ -145,6 +145,19 @@ COUNTY_URL_RE = re.compile(r'https://results.enr.clarityelections.com/(?P[A-Z]{2})/(?P[A-Za-z\.]+)/(?P\d+)/(?P\d+)/') +# non-string object which has a type(str(Stringlike)) === str +class Stringlike(object): + def __init__(self, url): + self.url = url + def __str__(self): + return self.url + +# non-string object which has a type(str(Stringlike)) !== str +class NotStringlike(object): + def __init__(self, url): + # do something but don't implement __str__ and don't store url + self.noturl = 'noturl' + def mock_county_response_callback(req): m = COUNTY_REDIRECT_URL_RE.match(req.url) assert m is not None @@ -175,26 +188,59 @@ def test_construct(self): jurisdiction = Jurisdiction(url=url, level='state') self.assertEqual(jurisdiction.url, url) - def test_construct_invalid_url_type(self): + def test_construct_valid_county(self): """ - A Jurisdiction with an invalid URL data type should raise an Exception. + A Jurisdiction with a valid, supported URL and level string should create a class instance and not raise an Exception. """ - with self.assertRaises(Exception): - Jurisdiction(url=None, level='state') + url = 'https://results.enr.clarityelections.com/KY/15261/30235/en/summary.html' + jurisdiction = Jurisdiction(url=url, level='county') + self.assertEqual(jurisdiction.url, url) - def test_construct_invalid_url(self): + def test_construct_valid_city(self): """ - A Jurisdiction with an invalid URL should raise an Exception. + A Jurisdiction with a valid, supported URL and level string should create a class instance and not raise an Exception. """ - with self.assertRaises(Exception): - Jurisdiction(url='foo', level='state') + url = 'https://results.enr.clarityelections.com/KY/15261/30235/en/summary.html' + jurisdiction = Jurisdiction(url=url, level='city') + self.assertEqual(jurisdiction.url, url) + + def test_construct_valid_precinct(self): + """ + A Jurisdiction with a valid, supported URL and level string should create a class instance and not raise an Exception. + """ + url = 'https://results.enr.clarityelections.com/KY/15261/30235/en/summary.html' + jurisdiction = Jurisdiction(url=url, level='precinct') + self.assertEqual(jurisdiction.url, url) + + def test_construct_valid_url_stringlike(self): + """ + A Jurisdiction with a valid, supported URL and level string should create a class instance and not raise an Exception. + """ + url = Stringlike('https://results.enr.clarityelections.com/KY/15261/30235/en/summary.html') + jurisdiction = Jurisdiction(url=url, level='state') + self.assertEqual(jurisdiction.url, str(url)) + + def test_construct_invalid_url_notstringlike(self): + """ + A Jurisdiction with a valid, supported URL and level string should raise a TypeError. + """ + url = NotStringlike('https://results.enr.clarityelections.com/KY/15261/30235/en/summary.html') + with self.assertRaises(ValueError): + Jurisdiction(url=url, level='state') + + def test_construct_invalid_url_type(self): + """ + A Jurisdiction with an invalid URL data type should raise an Exception. + """ + with self.assertRaises(TypeError): + Jurisdiction(url=None, level='state') def test_construct_invalid_hostname(self): """ A Jurisdiction with an unsupported hostname in the URL should raise an Exception. """ url = 'https://bad.hostname/KY/15261/30235/en/summary.html' - with self.assertRaises(Exception): + with self.assertRaises(ValueError): Jurisdiction(url=url, level='state') def test_construct_no_level(self): @@ -202,7 +248,7 @@ def test_construct_no_level(self): A Jurisdiction with a valid, supported hostname but no level in the URL should raise an Exception. """ url = 'https://results.enr.clarityelections.com/KY/15261/30235/en/summary.html' - with self.assertRaises(Exception): + with self.assertRaises(TypeError): Jurisdiction(url=url) def test_construct_invalid_level_str(self): @@ -210,7 +256,7 @@ def test_construct_invalid_level_str(self): A Jurisdiction with a valid, supported hostname but an invalid level in the URL should raise an Exception. """ url = 'https://results.enr.clarityelections.com/KY/15261/30235/en/summary.html' - with self.assertRaises(Exception): + with self.assertRaises(ValueError): Jurisdiction(url=url, level='foo') From aa101b4b61993ce9e302b5c188de251201011b31 Mon Sep 17 00:00:00 2001 From: David Wortham Date: Sun, 7 Mar 2021 17:43:37 -0800 Subject: [PATCH 3/3] fix(lib): rethrow TypeError, ValueError if those are raised in Jurisdiction constructor while converting url to str --- clarify/jurisdiction.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/clarify/jurisdiction.py b/clarify/jurisdiction.py index 10ce4de..364aa6f 100644 --- a/clarify/jurisdiction.py +++ b/clarify/jurisdiction.py @@ -32,6 +32,10 @@ def __init__(self, url, level, name=''): try: # Attempt to convert to str url = str(url) + except TypeError as type_error: + raise type_error + except ValueError as value_error: + raise value_error except Exception as error: # If converting to str failed, raise type error raise TypeError('Invalid url parameter')