From 2e90a22ea0c621013cba366e9fe08cec4c3e1cc6 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Fri, 8 Nov 2019 15:41:06 +0200 Subject: [PATCH] Check augas path inconsistencies in initialization --- certbot-apache/certbot_apache/augeasparser.py | 14 ++++++-- .../certbot_apache/tests/augeasnode_test.py | 35 ++++++++++++++++++- .../certbot_apache/tests/configurator_test.py | 3 +- .../certbot_apache/tests/dualnode_test.py | 21 +++++++---- 4 files changed, 62 insertions(+), 11 deletions(-) diff --git a/certbot-apache/certbot_apache/augeasparser.py b/certbot-apache/certbot_apache/augeasparser.py index cdf430a55..e340519ce 100644 --- a/certbot-apache/certbot_apache/augeasparser.py +++ b/certbot-apache/certbot_apache/augeasparser.py @@ -64,6 +64,9 @@ Translates over to: "/files/etc/apache2/apache2.conf/bLoCk[1]", ] """ +from acme.magic_typing import Set # pylint: disable=unused-import, no-name-in-module +from certbot import errors +from certbot.compat import os from certbot_apache import apache_util from certbot_apache import assertions @@ -71,8 +74,6 @@ from certbot_apache import interfaces from certbot_apache import parser from certbot_apache import parsernode_util as util -from certbot.compat import os -from acme.magic_typing import Set # pylint: disable=unused-import, no-name-in-module class AugeasParserNode(interfaces.ParserNode): @@ -86,6 +87,15 @@ class AugeasParserNode(interfaces.ParserNode): self.dirty = dirty self.metadata = metadata self.parser = self.metadata.get("augeasparser") + try: + if self.metadata["augeaspath"].endswith("/"): + raise errors.PluginError( + "Augeas path: {} has a trailing slash".format( + self.metadata["augeaspath"] + ) + ) + except KeyError: + raise errors.PluginError("Augeas path is required") def save(self, msg): # pragma: no cover pass diff --git a/certbot-apache/certbot_apache/tests/augeasnode_test.py b/certbot-apache/certbot_apache/tests/augeasnode_test.py index d6defc283..8846c3c07 100644 --- a/certbot-apache/certbot_apache/tests/augeasnode_test.py +++ b/certbot-apache/certbot_apache/tests/augeasnode_test.py @@ -2,6 +2,7 @@ import mock from acme.magic_typing import List # pylint: disable=unused-import, no-name-in-module +from certbot import errors from certbot_apache import assertions @@ -25,7 +26,7 @@ class AugeasParserNodeTest(util.ApacheTest): name=assertions.PASS, ancestor=None, filepath=assertions.PASS, - metadata={"augeasparser": mock.Mock()} + metadata={"augeasparser": mock.Mock(), "augeaspath": "/files/anything"} ) testcases = { "/some/path/FirstNode/SecondNode": "SecondNode", @@ -198,3 +199,35 @@ class AugeasParserNodeTest(util.ApacheTest): new_block = parser.aug.match("{}/VirtualHost[2]".format(root_path)) self.assertEqual(len(new_block), 1) self.assertTrue(vh.metadata["augeaspath"].endswith("VirtualHost[2]")) + + def test_node_init_error_bad_augeaspath(self): + from certbot_apache.augeasparser import AugeasBlockNode + parameters = { + "name": assertions.PASS, + "ancestor": None, + "filepath": assertions.PASS, + "metadata": { + "augeasparser": mock.Mock(), + "augeaspath": "/files/path/endswith/slash/" + } + } + self.assertRaises( + errors.PluginError, + AugeasBlockNode, + **parameters + ) + def test_node_init_error_missing_augeaspath(self): + from certbot_apache.augeasparser import AugeasBlockNode + parameters = { + "name": assertions.PASS, + "ancestor": None, + "filepath": assertions.PASS, + "metadata": { + "augeasparser": mock.Mock(), + } + } + self.assertRaises( + errors.PluginError, + AugeasBlockNode, + **parameters + ) diff --git a/certbot-apache/certbot_apache/tests/configurator_test.py b/certbot-apache/certbot_apache/tests/configurator_test.py index 1eafae982..68d9c90fa 100644 --- a/certbot-apache/certbot_apache/tests/configurator_test.py +++ b/certbot-apache/certbot_apache/tests/configurator_test.py @@ -78,7 +78,8 @@ class MultipleVhostsTest(util.ApacheTest): @mock.patch("certbot_apache.parser.ApacheParser") @mock.patch("certbot_apache.configurator.util.exe_exists") - def _test_prepare_locked(self, unused_parser, unused_exe_exists): + @mock.patch("certbot_apache.configurator.ApacheConfigurator.get_parsernode_root") + def _test_prepare_locked(self, _node, _exists, _parser): try: self.config.prepare() except errors.PluginError as err: diff --git a/certbot-apache/certbot_apache/tests/dualnode_test.py b/certbot-apache/certbot_apache/tests/dualnode_test.py index 5d6f827e7..dbce18431 100644 --- a/certbot-apache/certbot_apache/tests/dualnode_test.py +++ b/certbot-apache/certbot_apache/tests/dualnode_test.py @@ -164,10 +164,12 @@ class DualParserNodeTest(unittest.TestCase): # pylint: disable=too-many-public- def test_find_comments(self): pri_comments = [augeasparser.AugeasCommentNode(comment="some comment", ancestor=self.block, - filepath="/path/to/whatever")] + filepath="/path/to/whatever", + metadata=self.metadata)] sec_comments = [augeasparser.AugeasCommentNode(comment=assertions.PASS, ancestor=self.block, - filepath=assertions.PASS)] + filepath=assertions.PASS, + metadata=self.metadata)] find_coms_primary = mock.MagicMock(return_value=pri_comments) find_coms_secondary = mock.MagicMock(return_value=sec_comments) self.block.primary.find_comments = find_coms_primary @@ -303,10 +305,12 @@ class DualParserNodeTest(unittest.TestCase): # pylint: disable=too-many-public- def test_find_coms_second_passing(self): notpassing = [augeasparser.AugeasCommentNode(comment="notpassing", ancestor=self.block, - filepath="/path/to/whatever")] + filepath="/path/to/whatever", + metadata=self.metadata)] passing = [augeasparser.AugeasCommentNode(comment=assertions.PASS, ancestor=self.block, - filepath=assertions.PASS)] + filepath=assertions.PASS, + metadata=self.metadata)] find_coms_primary = mock.MagicMock(return_value=notpassing) find_coms_secondary = mock.MagicMock(return_value=passing) self.block.primary.find_comments = find_coms_primary @@ -398,13 +402,16 @@ class DualParserNodeTest(unittest.TestCase): # pylint: disable=too-many-public- def test_parsernode_notequal(self): ne_block = augeasparser.AugeasBlockNode(name="different", ancestor=self.block, - filepath="/path/to/whatever") + filepath="/path/to/whatever", + metadata=self.metadata) ne_directive = augeasparser.AugeasDirectiveNode(name="different", ancestor=self.block, - filepath="/path/to/whatever") + filepath="/path/to/whatever", + metadata=self.metadata) ne_comment = augeasparser.AugeasCommentNode(comment="different", ancestor=self.block, - filepath="/path/to/whatever") + filepath="/path/to/whatever", + metadata=self.metadata) self.assertFalse(self.block == ne_block) self.assertFalse(self.directive == ne_directive) self.assertFalse(self.comment == ne_comment)