From 368ca0c1096aa198b2e841dd52778646812e8141 Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Mon, 15 Jan 2018 20:47:03 -0800
Subject: [PATCH] Small cleanup for Apache HTTP-01

* Remove http_doer from self

* Refactor _find_best_vhost
---
 certbot-apache/certbot_apache/configurator.py | 29 +++++++++++--------
 .../certbot_apache/tests/configurator_test.py |  3 +-
 2 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py
index b87189dc0..05c838123 100644
--- a/certbot-apache/certbot_apache/configurator.py
+++ b/certbot-apache/certbot_apache/configurator.py
@@ -164,9 +164,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
                               "ensure-http-header": self._set_http_header,
                               "staple-ocsp": self._enable_ocsp_stapling}
 
-        # This will be set during the perform function
-        self.http_doer = None
-
     @property
     def mod_ssl_conf(self):
         """Full absolute path to SSL configuration file."""
@@ -439,12 +436,16 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
                 return True
         return False
 
-    def _find_best_vhost(self, target_name):
+    def _find_best_vhost(self, target_name, vhosts=None):
         """Finds the best vhost for a target_name.
 
         This does not upgrade a vhost to HTTPS... it only finds the most
         appropriate vhost for the given target_name.
 
+        :param str target_name: domain handled by the desired vhost
+        :param vhosts: vhosts to consider
+        :type vhosts: `collections.Iterable` of :class:`~certbot_apache.obj.VirtualHost`
+
         :returns: VHost or None
 
         """
@@ -456,7 +457,11 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         # Points 1 - Address name with no SSL
         best_candidate = None
         best_points = 0
-        for vhost in self.vhosts:
+
+        if vhosts is None:
+            vhosts = self.vhosts
+
+        for vhost in vhosts:
             if vhost.modmacro is True:
                 continue
             names = vhost.get_names()
@@ -481,7 +486,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         # No winners here... is there only one reasonable vhost?
         if best_candidate is None:
             # reasonable == Not all _default_ addrs
-            vhosts = self._non_default_vhosts()
+            vhosts = self._non_default_vhosts(vhosts)
             # remove mod_macro hosts from reasonable vhosts
             reasonable_vhosts = [vh for vh
                                  in vhosts if vh.modmacro is False]
@@ -490,9 +495,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
 
         return best_candidate
 
-    def _non_default_vhosts(self):
+    def _non_default_vhosts(self, vhosts):
         """Return all non _default_ only vhosts."""
-        return [vh for vh in self.vhosts if not all(
+        return [vh for vh in vhosts if not all(
             addr.get_addr() == "_default_" for addr in vh.addrs
         )]
 
@@ -1911,7 +1916,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         """
         self._chall_out.update(achalls)
         responses = [None] * len(achalls)
-        self.http_doer = http_01.ApacheHttp01(self)
+        http_doer = http_01.ApacheHttp01(self)
         sni_doer = tls_sni_01.ApacheTlsSni01(self)
 
         for i, achall in enumerate(achalls):
@@ -1919,11 +1924,11 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
             # challenge. This helps to put all of the responses back together
             # when they are all complete.
             if isinstance(achall.chall, challenges.HTTP01):
-                self.http_doer.add_chall(achall, i)
+                http_doer.add_chall(achall, i)
             else:  # tls-sni-01
                 sni_doer.add_chall(achall, i)
 
-        http_response = self.http_doer.perform()
+        http_response = http_doer.perform()
         sni_response = sni_doer.perform()
         if http_response or sni_response:
             # Must reload in order to activate the challenges.
@@ -1935,7 +1940,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
             # of identifying when the new configuration is being used.
             time.sleep(3)
 
-            self._update_responses(responses, http_response, self.http_doer)
+            self._update_responses(responses, http_response, http_doer)
             self._update_responses(responses, sni_response, sni_doer)
 
         return responses
diff --git a/certbot-apache/certbot_apache/tests/configurator_test.py b/certbot-apache/certbot_apache/tests/configurator_test.py
index df67104da..c846d2d42 100644
--- a/certbot-apache/certbot_apache/tests/configurator_test.py
+++ b/certbot-apache/certbot_apache/tests/configurator_test.py
@@ -305,7 +305,8 @@ class MultipleVhostsTest(util.ApacheTest):
 
     def test_non_default_vhosts(self):
         # pylint: disable=protected-access
-        self.assertEqual(len(self.config._non_default_vhosts()), 8)
+        vhosts = self.config._non_default_vhosts(self.config.vhosts)
+        self.assertEqual(len(vhosts), 8)
 
     def test_deploy_cert_enable_new_vhost(self):
         # Create