From a105b587ac042ea2b129bed42cf47586b59818c6 Mon Sep 17 00:00:00 2001 From: alexzorin Date: Fri, 16 Jul 2021 04:12:14 +1000 Subject: [PATCH] apache: fix crash when authenticating empty vhosts (#8941) Fixes #8940. --- certbot-apache/certbot_apache/_internal/parser.py | 6 +++++- certbot-apache/tests/parser_test.py | 5 +++++ .../multiple_vhosts/apache2/sites-available/empty.conf | 0 .../apache2/sites-available/no-directives.conf | 5 +++++ certbot/CHANGELOG.md | 3 ++- 5 files changed, 17 insertions(+), 2 deletions(-) create mode 100644 certbot-apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-available/empty.conf create mode 100644 certbot-apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-available/no-directives.conf diff --git a/certbot-apache/certbot_apache/_internal/parser.py b/certbot-apache/certbot_apache/_internal/parser.py index 141991ccc..3c705e066 100644 --- a/certbot-apache/certbot_apache/_internal/parser.py +++ b/certbot-apache/certbot_apache/_internal/parser.py @@ -440,7 +440,11 @@ class ApacheParser: :type args: list or str """ first_dir = aug_conf_path + "/directive[1]" - self.aug.insert(first_dir, "directive", True) + if self.aug.get(first_dir): + self.aug.insert(first_dir, "directive", True) + else: + self.aug.set(first_dir, "directive") + self.aug.set(first_dir, dirname) if isinstance(args, list): for i, value in enumerate(args, 1): diff --git a/certbot-apache/tests/parser_test.py b/certbot-apache/tests/parser_test.py index 00ca23f7a..5ee64d3fd 100644 --- a/certbot-apache/tests/parser_test.py +++ b/certbot-apache/tests/parser_test.py @@ -105,6 +105,11 @@ class BasicParserTest(util.ParserTest): for i, match in enumerate(matches): self.assertEqual(self.parser.aug.get(match), str(i + 1)) + for name in ("empty.conf", "no-directives.conf"): + conf = "/files" + os.path.join(self.parser.root, "sites-available", name) + self.parser.add_dir_beginning(conf, "AddDirectiveBeginning", "testBegin") + self.assertTrue(self.parser.find_dir("AddDirectiveBeginning", "testBegin", conf)) + def test_empty_arg(self): self.assertEqual(None, self.parser.get_arg("/files/whatever/nonexistent")) diff --git a/certbot-apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-available/empty.conf b/certbot-apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-available/empty.conf new file mode 100644 index 000000000..e69de29bb diff --git a/certbot-apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-available/no-directives.conf b/certbot-apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-available/no-directives.conf new file mode 100644 index 000000000..e7ceab441 --- /dev/null +++ b/certbot-apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-available/no-directives.conf @@ -0,0 +1,5 @@ + + + Require all denied + + diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index 865de631e..8dcc5049f 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -18,7 +18,8 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). ### Fixed -* +* The Apache authenticator no longer crashes with "Unable to insert label" + when encountering a completely empty vhost. This issue affected Certbot 1.17.0. More details about these changes can be found on our GitHub repo.