From 5186071948529beb5c1da68f23c47403fa972160 Mon Sep 17 00:00:00 2001 From: Alexander Cecile <35971201+AlexanderCecile@users.noreply.github.com> Date: Mon, 17 Aug 2020 19:52:11 -0400 Subject: [PATCH 01/17] Introduce the requests library. Replace the combination of urllib, beautifulsoup and lxml with the requests library. --- requirements.txt | 4 +--- testUpdateHostsFile.py | 41 ---------------------------------------- updateHostsFile.py | 43 ++++-------------------------------------- 3 files changed, 5 insertions(+), 83 deletions(-) diff --git a/requirements.txt b/requirements.txt index 45685c115..f2293605c 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,3 +1 @@ -lxml>=4.2.4,<=5.0 -beautifulsoup4>=4.6.1,<=5.0 -flake8>=3.8,<=4.0 +requests diff --git a/testUpdateHostsFile.py b/testUpdateHostsFile.py index 334aa0868..bf9fcff5f 100644 --- a/testUpdateHostsFile.py +++ b/testUpdateHostsFile.py @@ -1615,47 +1615,6 @@ class DomainToIDNA(Base): self.assertEqual(actual, expected) -class GetFileByUrl(BaseStdout): - @mock.patch("updateHostsFile.urlopen", side_effect=mock_url_open) - def test_read_url(self, _): - url = b"www.google.com" - - expected = "www.google.com" - actual = get_file_by_url(url, delay=0) - - self.assertEqual(actual, expected) - - @mock.patch("updateHostsFile.urlopen", side_effect=mock_url_open_fail) - def test_read_url_fail(self, _): - url = b"www.google.com" - self.assertIsNone(get_file_by_url(url, delay=0)) - - expected = "Problem getting file:" - output = sys.stdout.getvalue() - - self.assertIn(expected, output) - - @mock.patch("updateHostsFile.urlopen", side_effect=mock_url_open_read_fail) - def test_read_url_read_fail(self, _): - url = b"www.google.com" - self.assertIsNone(get_file_by_url(url, delay=0)) - - expected = "Problem getting file:" - output = sys.stdout.getvalue() - - self.assertIn(expected, output) - - @mock.patch("updateHostsFile.urlopen", side_effect=mock_url_open_decode_fail) - def test_read_url_decode_fail(self, _): - url = b"www.google.com" - self.assertIsNone(get_file_by_url(url, delay=0)) - - expected = "Problem getting file:" - output = sys.stdout.getvalue() - - self.assertIn(expected, output) - - class TestWriteData(Base): def test_write_basic(self): f = BytesIO() diff --git a/updateHostsFile.py b/updateHostsFile.py index 9437d40d7..3e2b00185 100644 --- a/updateHostsFile.py +++ b/updateHostsFile.py @@ -21,15 +21,12 @@ import tempfile import time from glob import glob -import lxml # noqa: F401 -from bs4 import BeautifulSoup +import requests # Detecting Python 3 for version-dependent implementations PY3 = sys.version_info >= (3, 0) -if PY3: - from urllib.request import urlopen -else: +if not PY3: raise Exception("We do not support Python 2 anymore.") # Syntactic sugar for "sudo" command in UNIX / Linux @@ -1469,40 +1466,8 @@ def maybe_copy_example_file(file_path): shutil.copyfile(example_file_path, file_path) -def get_file_by_url(url, retries=3, delay=10): - """ - Get a file data located at a particular URL. - - Parameters - ---------- - url : str - The URL at which to access the data. - - Returns - ------- - url_data : str or None - The data retrieved at that URL from the file. Returns None if the - attempted retrieval is unsuccessful. - - Note - ---- - - BeautifulSoup is used in this case to avoid having to search in which - format we have to encode or decode data before parsing it to UTF-8. - """ - - while retries: - try: - with urlopen(url) as f: - soup = BeautifulSoup(f.read(), "lxml").get_text() - return "\n".join(list(map(domain_to_idna, soup.split("\n")))) - except Exception as e: - if 'failure in name resolution' in str(e): - print('No internet connection! Retrying in {} seconds'.format(delay)) - time.sleep(delay) - retries -= 1 - continue - break - print("Problem getting file: ", url) +def get_file_by_url(url, params, **kwargs): + return requests.get(url=url, params=params, **kwargs).text def write_data(f, data): From 79bd7d4122077bbd5fd9721b7c528596664bdf97 Mon Sep 17 00:00:00 2001 From: Alexander Cecile <35971201+AlexanderCecile@users.noreply.github.com> Date: Tue, 18 Aug 2020 19:36:47 -0400 Subject: [PATCH 02/17] Tell requests to detect encoding. Changed the get_file_by_url function to infer/guess the encoding of the content we receive. --- updateHostsFile.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/updateHostsFile.py b/updateHostsFile.py index 3e2b00185..b8d64a29c 100644 --- a/updateHostsFile.py +++ b/updateHostsFile.py @@ -1467,7 +1467,9 @@ def maybe_copy_example_file(file_path): def get_file_by_url(url, params, **kwargs): - return requests.get(url=url, params=params, **kwargs).text + req = requests.get(url=url, params=params, **kwargs) + req.encoding = req.apparent_encoding + return req.text def write_data(f, data): From 140c0bd29ef27cb90f5597e3821901e52773c760 Mon Sep 17 00:00:00 2001 From: Alexander Cecile <35971201+AlexanderCecile@users.noreply.github.com> Date: Thu, 20 Aug 2020 16:15:37 -0400 Subject: [PATCH 03/17] Use domain_to_idna in get_file_by_url --- updateHostsFile.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/updateHostsFile.py b/updateHostsFile.py index b8d64a29c..ad31a246b 100644 --- a/updateHostsFile.py +++ b/updateHostsFile.py @@ -1466,10 +1466,11 @@ def maybe_copy_example_file(file_path): shutil.copyfile(example_file_path, file_path) -def get_file_by_url(url, params, **kwargs): +def get_file_by_url(url, params=None, **kwargs): req = requests.get(url=url, params=params, **kwargs) req.encoding = req.apparent_encoding - return req.text + res_text = "\n".join([domain_to_idna(line) for line in req.text.splitlines()]) + return res_text def write_data(f, data): From 4c8cab12ef2073206918c19e5234e9b40a373220 Mon Sep 17 00:00:00 2001 From: Alexander Cecile <35971201+AlexanderCecile@users.noreply.github.com> Date: Thu, 20 Aug 2020 16:29:37 -0400 Subject: [PATCH 04/17] Re-add flake8 to requirements.txt --- requirements.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/requirements.txt b/requirements.txt index f2293605c..c1f8b4058 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1 +1,2 @@ requests +flake8>=3.8,<=4.0 From 44f41c317b08af936d3428298aac808e0e406fa4 Mon Sep 17 00:00:00 2001 From: Alexander Cecile <35971201+AlexanderCecile@users.noreply.github.com> Date: Thu, 20 Aug 2020 17:01:11 -0400 Subject: [PATCH 05/17] Document get_file_by_url --- updateHostsFile.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/updateHostsFile.py b/updateHostsFile.py index ad31a246b..81d1da820 100644 --- a/updateHostsFile.py +++ b/updateHostsFile.py @@ -1467,6 +1467,21 @@ def maybe_copy_example_file(file_path): def get_file_by_url(url, params=None, **kwargs): + """ + Retrieve the contents of the hosts file at a certain URL, then pass it through domain_to_idna(). + + Simple wrapper around the requests.get() function, uses the same parameters. + + Parameters + ---------- + url + params + kwargs + + Returns + ------- + content: str + """ req = requests.get(url=url, params=params, **kwargs) req.encoding = req.apparent_encoding res_text = "\n".join([domain_to_idna(line) for line in req.text.splitlines()]) From 4fefecf2e3c50e4f6f810c3fb1fa342c166568b3 Mon Sep 17 00:00:00 2001 From: Alexander Cecile <35971201+AlexanderCecile@users.noreply.github.com> Date: Thu, 20 Aug 2020 18:42:31 -0400 Subject: [PATCH 06/17] Document get_file_by_url --- updateHostsFile.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/updateHostsFile.py b/updateHostsFile.py index 81d1da820..85aa25d74 100644 --- a/updateHostsFile.py +++ b/updateHostsFile.py @@ -1468,9 +1468,9 @@ def maybe_copy_example_file(file_path): def get_file_by_url(url, params=None, **kwargs): """ - Retrieve the contents of the hosts file at a certain URL, then pass it through domain_to_idna(). + Retrieve the contents of the hosts file at the URL, then pass it through domain_to_idna(). - Simple wrapper around the requests.get() function, uses the same parameters. + Parameters are passed to the requests.get() function. Parameters ---------- From beca76acfe5f6cee15d67866b47119a143d8e874 Mon Sep 17 00:00:00 2001 From: Alexander Cecile <35971201+AlexanderCecile@users.noreply.github.com> Date: Thu, 20 Aug 2020 20:00:36 -0400 Subject: [PATCH 07/17] Tweak get_file_by_url --- updateHostsFile.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/updateHostsFile.py b/updateHostsFile.py index 85aa25d74..cace08485 100644 --- a/updateHostsFile.py +++ b/updateHostsFile.py @@ -1482,9 +1482,10 @@ def get_file_by_url(url, params=None, **kwargs): ------- content: str """ + req = requests.get(url=url, params=params, **kwargs) req.encoding = req.apparent_encoding - res_text = "\n".join([domain_to_idna(line) for line in req.text.splitlines()]) + res_text = "\n".join([domain_to_idna(line) for line in req.text.splitlines()]) + "\n" return res_text From 9380fe534ee6dc27adc6ecb1d6828669f603c52e Mon Sep 17 00:00:00 2001 From: Alexander Cecile <35971201+AlexanderCecile@users.noreply.github.com> Date: Fri, 21 Aug 2020 14:44:27 -0400 Subject: [PATCH 08/17] Tweak output formatting of get_file_by_url --- updateHostsFile.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/updateHostsFile.py b/updateHostsFile.py index cace08485..cee883237 100644 --- a/updateHostsFile.py +++ b/updateHostsFile.py @@ -1482,10 +1482,10 @@ def get_file_by_url(url, params=None, **kwargs): ------- content: str """ - + req = requests.get(url=url, params=params, **kwargs) req.encoding = req.apparent_encoding - res_text = "\n".join([domain_to_idna(line) for line in req.text.splitlines()]) + "\n" + res_text = "\n".join([domain_to_idna(line) for line in req.text.split("\n")]) return res_text From aa9da1ee0dec056062124e4914a7ed0ed73220ee Mon Sep 17 00:00:00 2001 From: Alexander Cecile <35971201+AlexanderCecile@users.noreply.github.com> Date: Fri, 21 Aug 2020 17:30:47 -0400 Subject: [PATCH 09/17] Remove unused import in tests --- testUpdateHostsFile.py | 1 - 1 file changed, 1 deletion(-) diff --git a/testUpdateHostsFile.py b/testUpdateHostsFile.py index bf9fcff5f..4cb70bc57 100644 --- a/testUpdateHostsFile.py +++ b/testUpdateHostsFile.py @@ -27,7 +27,6 @@ from updateHostsFile import ( flush_dns_cache, gather_custom_exclusions, get_defaults, - get_file_by_url, is_valid_domain_format, matches_exclusions, move_hosts_file_into_place, From 083db6955e03e516b00f78b866c53fcfe937367e Mon Sep 17 00:00:00 2001 From: Alexander Cecile <35971201+AlexanderCecile@users.noreply.github.com> Date: Tue, 25 Aug 2020 18:28:21 -0400 Subject: [PATCH 10/17] Implement error handling and improve documentation in get_file_by_url --- updateHostsFile.py | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/updateHostsFile.py b/updateHostsFile.py index cee883237..2a4bce297 100644 --- a/updateHostsFile.py +++ b/updateHostsFile.py @@ -1474,19 +1474,37 @@ def get_file_by_url(url, params=None, **kwargs): Parameters ---------- - url - params - kwargs + url : str or bytes + URL for the new Request object. + params : + Dictionary, list of tuples or bytes to send in the query string for the Request. + kwargs : + Optional arguments that request takes. Returns ------- - content: str + url_data : str or None + The data retrieved at that URL from the file. Returns None if the + attempted retrieval is unsuccessful. """ - req = requests.get(url=url, params=params, **kwargs) + try: + req = requests.get(url=url, params=params, **kwargs) + except requests.exceptions.RequestException: + print("Error retrieving data from {}".format(url)) + return None + req.encoding = req.apparent_encoding - res_text = "\n".join([domain_to_idna(line) for line in req.text.split("\n")]) - return res_text + + try: + res_text = req.text + except UnicodeDecodeError: + print("Decoding error when retrieving data from {}".format(url)) + return None + + res = "\n".join([domain_to_idna(line) for line in res_text.split("\n")]) + + return res def write_data(f, data): From 26b2ab9e5a4a43fccc87973216cb8402f9b68542 Mon Sep 17 00:00:00 2001 From: Alexander Cecile <35971201+AlexanderCecile@users.noreply.github.com> Date: Tue, 25 Aug 2020 20:40:30 -0400 Subject: [PATCH 11/17] Add basic tests for get_file_by_url --- testUpdateHostsFile.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/testUpdateHostsFile.py b/testUpdateHostsFile.py index 4cb70bc57..3d80691f3 100644 --- a/testUpdateHostsFile.py +++ b/testUpdateHostsFile.py @@ -17,6 +17,8 @@ import unittest import unittest.mock as mock from io import BytesIO, StringIO +import requests + import updateHostsFile from updateHostsFile import ( Colors, @@ -27,6 +29,7 @@ from updateHostsFile import ( flush_dns_cache, gather_custom_exclusions, get_defaults, + get_file_by_url, is_valid_domain_format, matches_exclusions, move_hosts_file_into_place, @@ -1614,6 +1617,32 @@ class DomainToIDNA(Base): self.assertEqual(actual, expected) +class GetFileByUrl(BaseStdout): + def test_basic(self): + raw_resp_content = "hello, ".encode("ascii") + "world".encode("utf-8") + resp_obj = requests.Response() + resp_obj.__setstate__({"_content": raw_resp_content}) + + expected = "hello, world" + + with mock.patch("requests.get", return_value=resp_obj): + actual = get_file_by_url("www.test-url.com") + + self.assertEqual(expected, actual) + + def test_with_idna(self): + raw_resp_content = b"www.huala\xc3\xb1e.cl" + resp_obj = requests.Response() + resp_obj.__setstate__({"_content": raw_resp_content}) + + expected = "www.xn--hualae-0wa.cl" + + with mock.patch("requests.get", return_value=resp_obj): + actual = get_file_by_url("www.test-url.com") + + self.assertEqual(expected, actual) + + class TestWriteData(Base): def test_write_basic(self): f = BytesIO() From 06483cca1db93a0e07350ce32bfc614fc4b86346 Mon Sep 17 00:00:00 2001 From: Alexander Cecile <35971201+AlexanderCecile@users.noreply.github.com> Date: Tue, 25 Aug 2020 20:48:35 -0400 Subject: [PATCH 12/17] Update error handling in get_file_by_url I don't believe the the .text could actually raise that exception. Oops. --- updateHostsFile.py | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/updateHostsFile.py b/updateHostsFile.py index 2a4bce297..60fd80d7a 100644 --- a/updateHostsFile.py +++ b/updateHostsFile.py @@ -1495,16 +1495,8 @@ def get_file_by_url(url, params=None, **kwargs): return None req.encoding = req.apparent_encoding - - try: - res_text = req.text - except UnicodeDecodeError: - print("Decoding error when retrieving data from {}".format(url)) - return None - - res = "\n".join([domain_to_idna(line) for line in res_text.split("\n")]) - - return res + res_text = "\n".join([domain_to_idna(line) for line in req.text.split("\n")]) + return res_text def write_data(f, data): From 1019c6ae00cedb2db4b59063aae4d23cfa1e8044 Mon Sep 17 00:00:00 2001 From: Alexander Cecile <35971201+AlexanderCecile@users.noreply.github.com> Date: Thu, 27 Aug 2020 21:37:54 -0400 Subject: [PATCH 13/17] Improve the error raised when the new dependency is missing --- updateHostsFile.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/updateHostsFile.py b/updateHostsFile.py index 60fd80d7a..2c50b4c37 100644 --- a/updateHostsFile.py +++ b/updateHostsFile.py @@ -21,14 +21,20 @@ import tempfile import time from glob import glob -import requests - # Detecting Python 3 for version-dependent implementations PY3 = sys.version_info >= (3, 0) if not PY3: raise Exception("We do not support Python 2 anymore.") + +try: + import requests +except ModuleNotFoundError: # noqa: F821 + raise ModuleNotFoundError("This project's dependencies have changed. The Requests library (" # noqa: F821 + "https://requests.readthedocs.io/en/master/) is now required.") + + # Syntactic sugar for "sudo" command in UNIX / Linux if platform.system() == "OpenBSD": SUDO = ["/usr/bin/doas"] From 59ddd34d0edce556289bb9bdf8f1cabe1e41014b Mon Sep 17 00:00:00 2001 From: Alexander Cecile <35971201+AlexanderCecile@users.noreply.github.com> Date: Fri, 28 Aug 2020 01:51:14 -0400 Subject: [PATCH 14/17] Changed dependency-related exception to be compatible with Python versions < 3.6 --- updateHostsFile.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/updateHostsFile.py b/updateHostsFile.py index 2c50b4c37..6a2ec9d87 100644 --- a/updateHostsFile.py +++ b/updateHostsFile.py @@ -30,9 +30,9 @@ if not PY3: try: import requests -except ModuleNotFoundError: # noqa: F821 - raise ModuleNotFoundError("This project's dependencies have changed. The Requests library (" # noqa: F821 - "https://requests.readthedocs.io/en/master/) is now required.") +except ImportError: + raise ImportError("This project's dependencies have changed. The Requests library (" + "https://requests.readthedocs.io/en/master/) is now required.") # Syntactic sugar for "sudo" command in UNIX / Linux From ad356812442c5a49b836c84f3a6fe39ce357bb85 Mon Sep 17 00:00:00 2001 From: Alexander Cecile <35971201+AlexanderCecile@users.noreply.github.com> Date: Fri, 28 Aug 2020 03:17:06 -0400 Subject: [PATCH 15/17] Improve tests for get_file_by_url --- testUpdateHostsFile.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/testUpdateHostsFile.py b/testUpdateHostsFile.py index 3d80691f3..3f09e2308 100644 --- a/testUpdateHostsFile.py +++ b/testUpdateHostsFile.py @@ -1642,6 +1642,20 @@ class GetFileByUrl(BaseStdout): self.assertEqual(expected, actual) + def test_connect_unknown_domain(self): + test_url = "http://doesnotexist.google.com" + return_value = get_file_by_url(test_url) + self.assertIsNone(return_value) + printed_output = sys.stdout.getvalue() + self.assertEqual(printed_output, "Error retrieving data from {}\n".format(test_url)) + + def test_invalid_url(self): + test_url = "http://fe80::5054:ff:fe5a:fc0" + return_value = get_file_by_url(test_url) + self.assertIsNone(return_value) + printed_output = sys.stdout.getvalue() + self.assertEqual(printed_output, "Error retrieving data from {}\n".format(test_url)) + class TestWriteData(Base): def test_write_basic(self): From d85c96576a0ed519944536f32889855bb9707cb6 Mon Sep 17 00:00:00 2001 From: Alexander Cecile <35971201+AlexanderCecile@users.noreply.github.com> Date: Fri, 28 Aug 2020 03:18:41 -0400 Subject: [PATCH 16/17] Remove unused test code --- testUpdateHostsFile.py | 71 ------------------------------------------ 1 file changed, 71 deletions(-) diff --git a/testUpdateHostsFile.py b/testUpdateHostsFile.py index 3f09e2308..7ca920280 100644 --- a/testUpdateHostsFile.py +++ b/testUpdateHostsFile.py @@ -1408,77 +1408,6 @@ class TestRemoveOldHostsFile(BaseMockDir): # End File Logic -# Helper Functions -def mock_url_open(url): - """ - Mock of `urlopen` that returns the url in a `BytesIO` stream. - - Parameters - ---------- - url : str - The URL associated with the file to open. - - Returns - ------- - bytes_stream : BytesIO - The `url` input wrapped in a `BytesIO` stream. - """ - - return BytesIO(url) - - -def mock_url_open_fail(_): - """ - Mock of `urlopen` that fails with an Exception. - """ - - raise Exception() - - -def mock_url_open_read_fail(_): - """ - Mock of `urlopen` that returns an object that fails on `read`. - - Returns - ------- - file_mock : mock.Mock - A mock of a file object that fails when reading. - """ - - def fail_read(): - raise Exception() - - m = mock.Mock() - - m.read = fail_read - return m - - -def mock_url_open_decode_fail(_): - """ - Mock of `urlopen` that returns an object that fails on during decoding - the output of `urlopen`. - - Returns - ------- - file_mock : mock.Mock - A mock of a file object that fails when decoding the output. - """ - - def fail_decode(_): - raise Exception() - - def read(): - s = mock.Mock() - s.decode = fail_decode - - return s - - m = mock.Mock() - m.read = read - return m - - class DomainToIDNA(Base): def __init__(self, *args, **kwargs): super(DomainToIDNA, self).__init__(*args, **kwargs) From b350c685405353685f00b98f62792327ab03f2f2 Mon Sep 17 00:00:00 2001 From: Alexander Cecile <35971201+AlexanderCecile@users.noreply.github.com> Date: Wed, 2 Sep 2020 20:25:01 -0400 Subject: [PATCH 17/17] Improved tests for get_file_by_url --- testUpdateHostsFile.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/testUpdateHostsFile.py b/testUpdateHostsFile.py index 7ca920280..e0199503c 100644 --- a/testUpdateHostsFile.py +++ b/testUpdateHostsFile.py @@ -1572,15 +1572,17 @@ class GetFileByUrl(BaseStdout): self.assertEqual(expected, actual) def test_connect_unknown_domain(self): - test_url = "http://doesnotexist.google.com" - return_value = get_file_by_url(test_url) + test_url = "http://doesnotexist.google.com" # leads to exception: ConnectionError + with mock.patch("requests.get", side_effect=requests.exceptions.ConnectionError): + return_value = get_file_by_url(test_url) self.assertIsNone(return_value) printed_output = sys.stdout.getvalue() self.assertEqual(printed_output, "Error retrieving data from {}\n".format(test_url)) def test_invalid_url(self): - test_url = "http://fe80::5054:ff:fe5a:fc0" - return_value = get_file_by_url(test_url) + test_url = "http://fe80::5054:ff:fe5a:fc0" # leads to exception: InvalidURL + with mock.patch("requests.get", side_effect=requests.exceptions.ConnectionError): + return_value = get_file_by_url(test_url) self.assertIsNone(return_value) printed_output = sys.stdout.getvalue() self.assertEqual(printed_output, "Error retrieving data from {}\n".format(test_url))