nginx: fix Unicode crash on Python 2 (#8480)

* nginx: fix py2 unicode sandwich

The nginx parser would crash when saving configuraitons containing
Unicode, because py2's `str` type does not support Unicode.

This change fixes that crash by ensuring that a string type supporting
Unicode is used in both Python 2 and Python 3.

* nginx: add unicode to the integration test config

* update CHANGELOG

certbot-ci/certbot_integration_tests/nginx_tests/nginx_config.py

@@ -1,3 +1,4 @@
+# -*- coding: utf-8 -*-
 """General purpose nginx test configuration generator."""
 import getpass
 
@@ -42,6 +43,8 @@ events {{
   worker_connections 1024;
 }}
 
+# “This comment contains valid Unicode”.
+
 http {{
   # Set an array of temp, cache and log file options that will otherwise default to
   # restricted locations accessible only to root.
@@ -51,61 +54,61 @@ http {{
   #scgi_temp_path {nginx_root}/scgi_temp;
   #uwsgi_temp_path {nginx_root}/uwsgi_temp;
   access_log {nginx_root}/error.log;
-  
+
   # This should be turned off in a Virtualbox VM, as it can cause some
   # interesting issues with data corruption in delivered files.
   sendfile off;
-  
+
   tcp_nopush on;
   tcp_nodelay on;
   keepalive_timeout 65;
   types_hash_max_size 2048;
-  
+
   #include /etc/nginx/mime.types;
   index index.html index.htm index.php;
-  
+
   log_format   main '$remote_addr - $remote_user [$time_local] $status '
     '"$request" $body_bytes_sent "$http_referer" '
     '"$http_user_agent" "$http_x_forwarded_for"';
-    
+
   default_type application/octet-stream;
-  
+
   server {{
     # IPv4.
     listen {http_port} {default_server};
     # IPv6.
     listen [::]:{http_port} {default_server};
     server_name nginx.{wtf_prefix}.wtf nginx2.{wtf_prefix}.wtf;
-    
+
     root {nginx_webroot};
-    
+
     location / {{
       # First attempt to serve request as file, then as directory, then fall
       # back to index.html.
       try_files $uri $uri/ /index.html;
     }}
   }}
-  
+
   server {{
     listen {http_port};
     listen [::]:{http_port};
     server_name nginx3.{wtf_prefix}.wtf;
-    
+
     root {nginx_webroot};
-    
+
     location /.well-known/ {{
       return 404;
     }}
-    
+
     return 301 https://$host$request_uri;
   }}
-  
+
   server {{
     listen {other_port};
     listen [::]:{other_port};
     server_name nginx4.{wtf_prefix}.wtf nginx5.{wtf_prefix}.wtf;
   }}
-  
+
   server {{
     listen {http_port};
     listen [::]:{http_port};

certbot-nginx/certbot_nginx/_internal/http_01.py

@@ -1,5 +1,6 @@
 """A class that performs HTTP-01 challenges for Nginx"""
 
+import io
 import logging
 
 from acme import challenges
@@ -102,7 +103,7 @@ class NginxHttp01(common.ChallengePerformer):
         self.configurator.reverter.register_file_creation(
             True, self.challenge_conf)
 
-        with open(self.challenge_conf, "w") as new_conf:
+        with io.open(self.challenge_conf, "w", encoding="utf-8") as new_conf:
             nginxparser.dump(config, new_conf)
 
     def _default_listen_addresses(self):

certbot-nginx/certbot_nginx/_internal/nginxparser.py

@@ -16,6 +16,7 @@ from pyparsing import stringEnd
 from pyparsing import White
 from pyparsing import ZeroOrMore
 import six
+from acme.magic_typing import IO, Any # pylint: disable=unused-import
 
 logger = logging.getLogger(__name__)
 
@@ -130,26 +131,27 @@ def load(_file):
 
 
 def dumps(blocks):
-    """Dump to a string.
+    # type: (UnspacedList) -> six.text_type
+    """Dump to a Unicode string.
 
     :param UnspacedList block: The parsed tree
-    :param int indentation: The number of spaces to indent
-    :rtype: str
+    :rtype: six.text_type
 
     """
-    return str(RawNginxDumper(blocks.spaced))
+    return six.text_type(RawNginxDumper(blocks.spaced))
 
 
 def dump(blocks, _file):
+    # type: (UnspacedList, IO[Any]) -> None
     """Dump to a file.
 
     :param UnspacedList block: The parsed tree
-    :param file _file: The file to dump to
-    :param int indentation: The number of spaces to indent
-    :rtype: NoneType
+    :param IO[Any] _file: The file stream to dump to. It must be opened with
+                          Unicode encoding.
+    :rtype: None
 
     """
-    return _file.write(dumps(blocks))
+    _file.write(dumps(blocks))
 
 
 spacey = lambda x: (isinstance(x, six.string_types) and x.isspace()) or x == ''

certbot-nginx/certbot_nginx/_internal/parser.py

@@ -249,7 +249,7 @@ class NginxParser(object):
                     continue
                 out = nginxparser.dumps(tree)
                 logger.debug('Writing nginx conf tree to %s:\n%s', filename, out)
-                with open(filename, 'w') as _file:
+                with io.open(filename, 'w', encoding='utf-8') as _file:
                     _file.write(out)
 
             except IOError:

certbot-nginx/tests/parser_test.py

@@ -492,6 +492,14 @@ class NginxParserTest(util.NginxTest):
         self.assertEqual(['server'], parsed[0][2][0])
         self.assertEqual(['listen', '80'], parsed[0][2][1][3])
 
+    def test_valid_unicode_roundtrip(self):
+        """This tests the parser's ability to load and save a config containing Unicode"""
+        nparser = parser.NginxParser(self.config_path)
+        nparser._parse_files(
+            nparser.abs_path('valid_unicode_comments.conf')
+        ) # pylint: disable=protected-access
+        nparser.filedump(lazy=False)
+
     def test_invalid_unicode_characters(self):
         with self.assertLogs() as log:
             nparser = parser.NginxParser(self.config_path)

certbot/CHANGELOG.md

@@ -24,7 +24,7 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
 
 ### Fixed
 
-*
+* Fixed a Unicode-related crash in the nginx plugin when running under Python 2.
 
 More details about these changes can be found on our GitHub repo.