Stop caching the results of ipv6_info in http01.py (#6411)

Stop caching the results of ipv6_info in http01.py. A call to choose_vhosts might change the ipv6 results of later calls. Add tests for this and default_listen_addresses more broadly.

CHANGELOG.md

@@ -16,6 +16,7 @@ Certbot adheres to [Semantic Versioning](http://semver.org/).
 
 * Match Nginx parser update in allowing variable names to start with `${`.
 * Correct OVH integration tests on machines without internet access.
+* Stop caching the results of ipv6_info in http01.py
 
 ## 0.27.1 - 2018-09-06
 

certbot-nginx/certbot_nginx/http_01.py

@@ -40,8 +40,6 @@ class NginxHttp01(common.ChallengePerformer):
         super(NginxHttp01, self).__init__(configurator)
         self.challenge_conf = os.path.join(
             configurator.config.config_dir, "le_http_01_cert_challenge.conf")
-        self._ipv6 = None
-        self._ipv6only = None
 
     def perform(self):
         """Perform a challenge on Nginx.
@@ -121,9 +119,7 @@ class NginxHttp01(common.ChallengePerformer):
             self.configurator.config.http01_port)
         port = self.configurator.config.http01_port
 
-        if self._ipv6 is None or self._ipv6only is None:
+        ipv6, ipv6only = self.configurator.ipv6_info(port)
-            self._ipv6, self._ipv6only = self.configurator.ipv6_info(port)
-        ipv6, ipv6only = self._ipv6, self._ipv6only
 
         if ipv6:
             # If IPv6 is active in Nginx configuration

certbot-nginx/certbot_nginx/tests/http_01_test.py

@@ -12,6 +12,7 @@ from certbot import achallenges
 from certbot.plugins import common_test
 from certbot.tests import acme_util
 
+from certbot_nginx.obj import Addr
 from certbot_nginx.tests import util
 
 
@@ -108,6 +109,41 @@ class HttpPerformTest(util.NginxTest):
             #     self.assertEqual(vhost.addrs, set(v_addr2_print))
             # self.assertEqual(vhost.names, set([response.z_domain.decode('ascii')]))
 
+    @mock.patch("certbot_nginx.configurator.NginxConfigurator.ipv6_info")
+    def test_default_listen_addresses_no_memoization(self, ipv6_info):
+        # pylint: disable=protected-access
+        ipv6_info.return_value = (True, True)
+        self.http01._default_listen_addresses()
+        self.assertEqual(ipv6_info.call_count, 1)
+        ipv6_info.return_value = (False, False)
+        self.http01._default_listen_addresses()
+        self.assertEqual(ipv6_info.call_count, 2)
+
+    @mock.patch("certbot_nginx.configurator.NginxConfigurator.ipv6_info")
+    def test_default_listen_addresses_t_t(self, ipv6_info):
+        # pylint: disable=protected-access
+        ipv6_info.return_value = (True, True)
+        addrs = self.http01._default_listen_addresses()
+        http_addr = Addr.fromstring("80")
+        http_ipv6_addr = Addr.fromstring("[::]:80")
+        self.assertEqual(addrs, [http_addr, http_ipv6_addr])
+
+    @mock.patch("certbot_nginx.configurator.NginxConfigurator.ipv6_info")
+    def test_default_listen_addresses_t_f(self, ipv6_info):
+        # pylint: disable=protected-access
+        ipv6_info.return_value = (True, False)
+        addrs = self.http01._default_listen_addresses()
+        http_addr = Addr.fromstring("80")
+        http_ipv6_addr = Addr.fromstring("[::]:80 ipv6only=on")
+        self.assertEqual(addrs, [http_addr, http_ipv6_addr])
+
+    @mock.patch("certbot_nginx.configurator.NginxConfigurator.ipv6_info")
+    def test_default_listen_addresses_f_f(self, ipv6_info):
+        # pylint: disable=protected-access
+        ipv6_info.return_value = (False, False)
+        addrs = self.http01._default_listen_addresses()
+        http_addr = Addr.fromstring("80")
+        self.assertEqual(addrs, [http_addr])
 
 if __name__ == "__main__":
     unittest.main()  # pragma: no cover

certbot-nginx/certbot_nginx/tls_sni_01.py

@@ -51,9 +51,6 @@ class NginxTlsSni01(common.TLSSNI01):
         default_addr = "{0} ssl".format(
             self.configurator.config.tls_sni_01_port)
 
-        ipv6, ipv6only = self.configurator.ipv6_info(
-            self.configurator.config.tls_sni_01_port)
-
         for achall in self.achalls:
             vhosts = self.configurator.choose_vhosts(achall.domain, create_if_no_match=True)
 
@@ -61,6 +58,9 @@ class NginxTlsSni01(common.TLSSNI01):
             if vhosts and vhosts[0].addrs:
                 addresses.append(list(vhosts[0].addrs))
             else:
+                # choose_vhosts might have modified vhosts, so put this after
+                ipv6, ipv6only = self.configurator.ipv6_info(
+                    self.configurator.config.tls_sni_01_port)
                 if ipv6:
                     # If IPv6 is active in Nginx configuration
                     ipv6_addr = "[::]:{0} ssl".format(