From b48e336554301cf30b134d9b5c38c3173d49ebbd Mon Sep 17 00:00:00 2001 From: ohemorange Date: Thu, 10 Jun 2021 16:21:52 -0700 Subject: [PATCH] Allow nginx parser to handle empty file (#8895) * Allow parsing empty files * add unit test * lint * update parser_test * Update configurator_test * update changelog --- certbot-nginx/certbot_nginx/_internal/nginxparser.py | 3 +-- certbot-nginx/tests/configurator_test.py | 2 +- certbot-nginx/tests/nginxparser_test.py | 4 ++++ certbot-nginx/tests/parser_test.py | 4 ++-- certbot/CHANGELOG.md | 5 ++++- 5 files changed, 12 insertions(+), 6 deletions(-) diff --git a/certbot-nginx/certbot_nginx/_internal/nginxparser.py b/certbot-nginx/certbot_nginx/_internal/nginxparser.py index 787f7c363..2aa677c38 100644 --- a/certbot-nginx/certbot_nginx/_internal/nginxparser.py +++ b/certbot-nginx/certbot_nginx/_internal/nginxparser.py @@ -9,7 +9,6 @@ from pyparsing import Combine from pyparsing import Forward from pyparsing import Group from pyparsing import Literal -from pyparsing import OneOrMore from pyparsing import Optional from pyparsing import QuotedString from pyparsing import Regex @@ -57,7 +56,7 @@ class RawNginxParser: block_innards = Group(ZeroOrMore(contents) + space).leaveWhitespace() block << block_begin + left_bracket + block_innards + right_bracket - script = OneOrMore(contents) + space + stringEnd + script = ZeroOrMore(contents) + space + stringEnd script.parseWithTabs().leaveWhitespace() def __init__(self, source): diff --git a/certbot-nginx/tests/configurator_test.py b/certbot-nginx/tests/configurator_test.py index fae5d41d9..e667d5375 100644 --- a/certbot-nginx/tests/configurator_test.py +++ b/certbot-nginx/tests/configurator_test.py @@ -43,7 +43,7 @@ class NginxConfiguratorTest(util.NginxTest): def test_prepare(self): self.assertEqual((1, 6, 2), self.config.version) - self.assertEqual(13, len(self.config.parser.parsed)) + self.assertEqual(14, len(self.config.parser.parsed)) @mock.patch("certbot_nginx._internal.configurator.util.exe_exists") @mock.patch("certbot_nginx._internal.configurator.subprocess.run") diff --git a/certbot-nginx/tests/nginxparser_test.py b/certbot-nginx/tests/nginxparser_test.py index a5212078f..8f7d5accf 100644 --- a/certbot-nginx/tests/nginxparser_test.py +++ b/certbot-nginx/tests/nginxparser_test.py @@ -350,6 +350,10 @@ class TestRawNginxParser(unittest.TestCase): self.assertEqual(loads("blag${dfgdfg};"), [['blag${dfgdfg}']]) self.assertRaises(ParseException, loads, "blag${dfgdf{g};") + # empty file + parsed = loads("") + self.assertEqual(parsed, []) + class TestUnspacedList(unittest.TestCase): """Test the UnspacedList data structure""" diff --git a/certbot-nginx/tests/parser_test.py b/certbot-nginx/tests/parser_test.py index b062c4196..9047cb446 100644 --- a/certbot-nginx/tests/parser_test.py +++ b/certbot-nginx/tests/parser_test.py @@ -50,7 +50,7 @@ class NginxParserTest(util.NginxTest): nparser = parser.NginxParser(self.config_path) nparser.load() self.assertEqual({nparser.abs_path(x) for x in - ['foo.conf', 'nginx.conf', 'server.conf', + ['foo.conf', 'nginx.conf', 'server.conf', 'mime.types', 'sites-enabled/default', 'sites-enabled/both.com', 'sites-enabled/example.com', @@ -89,7 +89,7 @@ class NginxParserTest(util.NginxTest): # pylint: disable=protected-access parsed = nparser._parse_files(nparser.abs_path( 'sites-enabled/example.com.test')) - self.assertEqual(3, len(glob.glob(nparser.abs_path('*.test')))) + self.assertEqual(4, len(glob.glob(nparser.abs_path('*.test')))) self.assertEqual(10, len( glob.glob(nparser.abs_path('sites-enabled/*.test')))) self.assertEqual([[['server'], [['listen', '69.50.225.155:9000'], diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index 04326ef30..2bd843e31 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -19,7 +19,10 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). ### Fixed -* +* When we increased the logging level on our nginx "Could not parse file" message, + it caused a previously-existing inability to parse empty files to become more + visible. We have now added the ability to correctly parse empty files, so that + message should only show for more significant errors. More details about these changes can be found on our GitHub repo.