From 70be256c663be8d4163b4abb5587da79dbd2da2a Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Fri, 3 Jan 2020 01:44:11 +0200 Subject: [PATCH] Fix gating to ensure that no parsernode functionality is run unless explicitly requested (#7654) --- certbot-apache/certbot_apache/configurator.py | 33 ++++++++++--------- .../certbot_apache/tests/augeasnode_test.py | 2 +- .../tests/parsernode_configurator_test.py | 3 +- certbot-apache/certbot_apache/tests/util.py | 5 +-- 4 files changed, 24 insertions(+), 19 deletions(-) diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py index d4466cc53..c189552d5 100644 --- a/certbot-apache/certbot_apache/configurator.py +++ b/certbot-apache/certbot_apache/configurator.py @@ -187,6 +187,7 @@ class ApacheConfigurator(common.Installer): """ version = kwargs.pop("version", None) + use_parsernode = kwargs.pop("use_parsernode", False) super(ApacheConfigurator, self).__init__(*args, **kwargs) # Add name_server association dict @@ -203,7 +204,7 @@ class ApacheConfigurator(common.Installer): # Reverter save notes self.save_notes = "" # Should we use ParserNode implementation instead of the old behavior - self.USE_PARSERNODE = False + self.USE_PARSERNODE = use_parsernode # Saves the list of file paths that were parsed initially, and # not added to parser tree by self.conf("vhost-root") for example. self.parsed_paths = [] # type: List[str] @@ -264,8 +265,9 @@ class ApacheConfigurator(common.Installer): pn_meta = {"augeasparser": self.parser, "augeaspath": self.parser.get_root_augpath(), "ac_ast": None} - self.parser_root = self.get_parsernode_root(pn_meta) - self.parsed_paths = self.parser_root.parsed_paths() + if self.USE_PARSERNODE: + self.parser_root = self.get_parsernode_root(pn_meta) + self.parsed_paths = self.parser_root.parsed_paths() # Check for errors in parsing files with Augeas self.parser.check_parsing_errors("httpd.aug") @@ -909,18 +911,18 @@ class ApacheConfigurator(common.Installer): """ v1_vhosts = self.get_virtual_hosts_v1() - v2_vhosts = self.get_virtual_hosts_v2() - - for v1_vh in v1_vhosts: - found = False - for v2_vh in v2_vhosts: - if assertions.isEqualVirtualHost(v1_vh, v2_vh): - found = True - break - if not found: - raise AssertionError("Equivalent for {} was not found".format(v1_vh.path)) - if self.USE_PARSERNODE: + v2_vhosts = self.get_virtual_hosts_v2() + + for v1_vh in v1_vhosts: + found = False + for v2_vh in v2_vhosts: + if assertions.isEqualVirtualHost(v1_vh, v2_vh): + found = True + break + if not found: + raise AssertionError("Equivalent for {} was not found".format(v1_vh.path)) + return v2_vhosts return v1_vhosts @@ -1003,7 +1005,8 @@ class ApacheConfigurator(common.Installer): addrs.add(obj.Addr.fromstring(param)) is_ssl = False - sslengine = node.find_directives("SSLEngine") + # Exclusion to match the behavior in get_virtual_hosts_v2 + sslengine = node.find_directives("SSLEngine", exclude=False) if sslengine: for directive in sslengine: if directive.parameters[0].lower() == "on": diff --git a/certbot-apache/certbot_apache/tests/augeasnode_test.py b/certbot-apache/certbot_apache/tests/augeasnode_test.py index 849a468c9..d090c4aa5 100644 --- a/certbot-apache/certbot_apache/tests/augeasnode_test.py +++ b/certbot-apache/certbot_apache/tests/augeasnode_test.py @@ -16,7 +16,7 @@ class AugeasParserNodeTest(util.ApacheTest): # pylint: disable=too-many-public- super(AugeasParserNodeTest, self).setUp() self.config = util.get_apache_configurator( - self.config_path, self.vhost_path, self.config_dir, self.work_dir) + self.config_path, self.vhost_path, self.config_dir, self.work_dir, use_parsernode=True) self.vh_truth = util.get_vh_truth( self.temp_dir, "debian_apache_2_4/multiple_vhosts") diff --git a/certbot-apache/certbot_apache/tests/parsernode_configurator_test.py b/certbot-apache/certbot_apache/tests/parsernode_configurator_test.py index 97f07d3d2..96bd63fdd 100644 --- a/certbot-apache/certbot_apache/tests/parsernode_configurator_test.py +++ b/certbot-apache/certbot_apache/tests/parsernode_configurator_test.py @@ -13,7 +13,8 @@ class ConfiguratorParserNodeTest(util.ApacheTest): # pylint: disable=too-many-p super(ConfiguratorParserNodeTest, self).setUp() self.config = util.get_apache_configurator( - self.config_path, self.vhost_path, self.config_dir, self.work_dir) + self.config_path, self.vhost_path, self.config_dir, + self.work_dir, use_parsernode=True) self.vh_truth = util.get_vh_truth( self.temp_dir, "debian_apache_2_4/multiple_vhosts") diff --git a/certbot-apache/certbot_apache/tests/util.py b/certbot-apache/certbot_apache/tests/util.py index 4155c93c0..ee941f507 100644 --- a/certbot-apache/certbot_apache/tests/util.py +++ b/certbot-apache/certbot_apache/tests/util.py @@ -85,7 +85,8 @@ def get_apache_configurator( # pylint: disable=too-many-arguments, too-many-loc config_path, vhost_path, config_dir, work_dir, version=(2, 4, 7), os_info="generic", - conf_vhost_path=None): + conf_vhost_path=None, + use_parsernode=False): """Create an Apache Configurator with the specified options. :param conf: Function that returns binary paths. self.conf in Configurator @@ -118,7 +119,7 @@ def get_apache_configurator( # pylint: disable=too-many-arguments, too-many-loc except KeyError: config_class = configurator.ApacheConfigurator config = config_class(config=mock_le_config, name="apache", - version=version) + version=version, use_parsernode=use_parsernode) if not conf_vhost_path: config_class.OS_DEFAULTS["vhost_root"] = vhost_path else: