From f5a88ade54cc34ae216959a84a658986324ea69c Mon Sep 17 00:00:00 2001 From: alexzorin Date: Sat, 28 Nov 2020 04:15:27 +1100 Subject: [PATCH] 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 --- .../nginx_tests/nginx_config.py | 31 ++++++++++--------- .../certbot_nginx/_internal/http_01.py | 3 +- .../certbot_nginx/_internal/nginxparser.py | 18 ++++++----- .../certbot_nginx/_internal/parser.py | 2 +- certbot-nginx/tests/parser_test.py | 8 +++++ certbot/CHANGELOG.md | 2 +- 6 files changed, 39 insertions(+), 25 deletions(-) diff --git a/certbot-ci/certbot_integration_tests/nginx_tests/nginx_config.py b/certbot-ci/certbot_integration_tests/nginx_tests/nginx_config.py index 18991ae62..bbbe8ea06 100644 --- a/certbot-ci/certbot_integration_tests/nginx_tests/nginx_config.py +++ b/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}; diff --git a/certbot-nginx/certbot_nginx/_internal/http_01.py b/certbot-nginx/certbot_nginx/_internal/http_01.py index 2f6458f87..40f994988 100644 --- a/certbot-nginx/certbot_nginx/_internal/http_01.py +++ b/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): diff --git a/certbot-nginx/certbot_nginx/_internal/nginxparser.py b/certbot-nginx/certbot_nginx/_internal/nginxparser.py index a8ac90427..5f723dcef 100644 --- a/certbot-nginx/certbot_nginx/_internal/nginxparser.py +++ b/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 == '' diff --git a/certbot-nginx/certbot_nginx/_internal/parser.py b/certbot-nginx/certbot_nginx/_internal/parser.py index 641ffb020..72091b03f 100644 --- a/certbot-nginx/certbot_nginx/_internal/parser.py +++ b/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: diff --git a/certbot-nginx/tests/parser_test.py b/certbot-nginx/tests/parser_test.py index 620d1b6de..0083c2448 100644 --- a/certbot-nginx/tests/parser_test.py +++ b/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) diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index a7f48ea3b..b3e2373a0 100644 --- a/certbot/CHANGELOG.md +++ b/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.