From c29e8c3332640611909c5f7c6913525c86d9d8d5 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Tue, 9 Aug 2016 23:43:11 +0300 Subject: [PATCH 1/5] Refactored get_file_path --- certbot-apache/certbot_apache/configurator.py | 30 +++++++------------ 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py index 238255133..4a68461ce 100644 --- a/certbot-apache/certbot_apache/configurator.py +++ b/certbot-apache/certbot_apache/configurator.py @@ -1801,25 +1801,17 @@ def get_file_path(vhost_path): :rtype: str """ - # Strip off /files - avail_fp = vhost_path[6:] - # This can be optimized... - while True: - # Cast all to lowercase to be case insensitive - find_if = avail_fp.lower().find("/ifmodule") - if find_if != -1: - avail_fp = avail_fp[:find_if] - continue - find_vh = avail_fp.lower().find("/virtualhost") - if find_vh != -1: - avail_fp = avail_fp[:find_vh] - continue - find_macro = avail_fp.lower().find("/macro") - if find_macro != -1: - avail_fp = avail_fp[:find_macro] - continue - break - return avail_fp + # Strip off /files/ + avail_fp = vhost_path[7:].split("/") + last_good = "" + # Loop through the path parts and validate after every addition + for p in avail_fp: + cur_path = last_good+"/"+p + if os.path.exists(cur_path): + last_good = cur_path + else: + break + return last_good def install_ssl_options_conf(options_ssl): From f4948855f02b1e1dde4434e7c8e15beb8c06a150 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Wed, 10 Aug 2016 00:25:35 +0300 Subject: [PATCH 2/5] Added None check and according test --- certbot-apache/certbot_apache/configurator.py | 8 +++++++- certbot-apache/certbot_apache/tests/configurator_test.py | 4 ++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py index 4a68461ce..abe3edede 100644 --- a/certbot-apache/certbot_apache/configurator.py +++ b/certbot-apache/certbot_apache/configurator.py @@ -538,6 +538,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): is_ssl = True filename = get_file_path(self.aug.get("/augeas/files%s/path" % get_file_path(path))) + if filename is None: + return None + if self.conf("handle-sites"): is_enabled = self.is_site_enabled(filename) else: @@ -1802,7 +1805,10 @@ def get_file_path(vhost_path): """ # Strip off /files/ - avail_fp = vhost_path[7:].split("/") + try: + avail_fp = vhost_path[7:].split("/") + except TypeError: + return None last_good = "" # Loop through the path parts and validate after every addition for p in avail_fp: diff --git a/certbot-apache/certbot_apache/tests/configurator_test.py b/certbot-apache/certbot_apache/tests/configurator_test.py index ac692ae54..59692302d 100644 --- a/certbot-apache/certbot_apache/tests/configurator_test.py +++ b/certbot-apache/certbot_apache/tests/configurator_test.py @@ -125,6 +125,10 @@ class MultipleVhostsTest(util.ApacheTest): self.assertTrue("google.com" in names) self.assertTrue("certbot.demo" in names) + def test_get_bad_path(self): + from certbot_apache.configurator import get_file_path + self.assertEqual(get_file_path(None), None) + def test_bad_servername_alias(self): ssl_vh1 = obj.VirtualHost( "fp1", "ap1", set([obj.Addr(("*", "443"))]), From 6c3ae10f9b1e94d0f38d73c15f593b348556e5d4 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Wed, 10 Aug 2016 01:39:53 +0300 Subject: [PATCH 3/5] Added test case --- certbot-apache/certbot_apache/tests/configurator_test.py | 1 + 1 file changed, 1 insertion(+) diff --git a/certbot-apache/certbot_apache/tests/configurator_test.py b/certbot-apache/certbot_apache/tests/configurator_test.py index 59692302d..2a8960564 100644 --- a/certbot-apache/certbot_apache/tests/configurator_test.py +++ b/certbot-apache/certbot_apache/tests/configurator_test.py @@ -128,6 +128,7 @@ class MultipleVhostsTest(util.ApacheTest): def test_get_bad_path(self): from certbot_apache.configurator import get_file_path self.assertEqual(get_file_path(None), None) + self.assertEqual(self.config._create_vhost("nonexistent"), None) def test_bad_servername_alias(self): ssl_vh1 = obj.VirtualHost( From 51191c2ea54de8fad135c93b13bb6ab0bc22410c Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Wed, 10 Aug 2016 01:50:40 +0300 Subject: [PATCH 4/5] Added linter exception --- certbot-apache/certbot_apache/tests/configurator_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/certbot-apache/certbot_apache/tests/configurator_test.py b/certbot-apache/certbot_apache/tests/configurator_test.py index 2a8960564..f8bf7676c 100644 --- a/certbot-apache/certbot_apache/tests/configurator_test.py +++ b/certbot-apache/certbot_apache/tests/configurator_test.py @@ -128,7 +128,7 @@ class MultipleVhostsTest(util.ApacheTest): def test_get_bad_path(self): from certbot_apache.configurator import get_file_path self.assertEqual(get_file_path(None), None) - self.assertEqual(self.config._create_vhost("nonexistent"), None) + self.assertEqual(self.config._create_vhost("nonexistent"), None) # pylint: disable=protected-access def test_bad_servername_alias(self): ssl_vh1 = obj.VirtualHost( From 14f371025049bbc5621da943e0ee04cf4e4add8d Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Wed, 10 Aug 2016 10:39:10 +0300 Subject: [PATCH 5/5] Added check for /files/ --- certbot-apache/certbot_apache/configurator.py | 9 +++++++-- certbot-apache/certbot_apache/tests/configurator_test.py | 1 + 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py index abe3edede..02602ace6 100644 --- a/certbot-apache/certbot_apache/configurator.py +++ b/certbot-apache/certbot_apache/configurator.py @@ -1806,9 +1806,14 @@ def get_file_path(vhost_path): """ # Strip off /files/ try: - avail_fp = vhost_path[7:].split("/") - except TypeError: + if vhost_path.startswith("/files/"): + avail_fp = vhost_path[7:].split("/") + else: + return None + except AttributeError: + # If we recieved a None path return None + last_good = "" # Loop through the path parts and validate after every addition for p in avail_fp: diff --git a/certbot-apache/certbot_apache/tests/configurator_test.py b/certbot-apache/certbot_apache/tests/configurator_test.py index f8bf7676c..dc953174e 100644 --- a/certbot-apache/certbot_apache/tests/configurator_test.py +++ b/certbot-apache/certbot_apache/tests/configurator_test.py @@ -128,6 +128,7 @@ class MultipleVhostsTest(util.ApacheTest): def test_get_bad_path(self): from certbot_apache.configurator import get_file_path self.assertEqual(get_file_path(None), None) + self.assertEqual(get_file_path("nonexistent"), None) self.assertEqual(self.config._create_vhost("nonexistent"), None) # pylint: disable=protected-access def test_bad_servername_alias(self):