Repository: certbot/certbot (language: python)

Issue:
nginx parser cannot handle empty file
We have some simple tooling that empties a vhosts config file to disable it. However certbot throws errors in the debug log about it.

latest python-certbot-nginx on ubuntu 18.04 PPA

```
2019-05-25 16:18:38,333:DEBUG:certbot_nginx.parser:Could not parse file: /etc/nginx/sites-generated/xdomain due to Expected {Group:({[<SPC><TAB><CR><LF>] "#" rest of line}) | Group:(Forward: ...) | Group:({[<SPC><TAB><CR><LF>] {Combine:({{quoted string, starting with " ending with " | quoted string, starting with ' ending with '} ")" [Re:('(\\$\\{)|[^{;\\s]')]...}) | Combine:({Re:('(\\$\\{)|[^{};\\s\'\\"]') [Re:('(\\$\\{)|[^{;\\s]')]...}) | quoted string, starting with " ending with " | quoted string, starting with ' ending with '} [{<SPC><TAB><CR><LF> {Combine:({{quoted string, starting with " ending with " | quoted string, starting with ' ending with '} ")" [Re:('(\\$\\{)|[^{;\\s]')]...}) | Combine:({Re:('(\\$\\{)|[^{};\\s\'\\"]') [Re:('(\\$\\{)|[^{;\\s]')]...}) | quoted string, starting with " ending with " | quoted string, starting with ' ending with '}}]... [<SPC><TAB><CR><LF>] Suppress:(";")})} (at char 1), (line:2, col:1)
```

In this case `/etc/nginx/sites-generated/xdomain` would be pointing to an empty file, nginx is ok with that, the certbot parser isn't.


Discussion / hints:
I can reproduce the problem. However, this is not a critical error, as indicates the `DEBUG` level.

The case is handled by:
https://github.com/certbot/certbot/blob/bf818036eb4f23164f92800b9ea83591149c0814/certbot-nginx/certbot_nginx/parser.py#L212-L213

So the exception is not propagated, just logged. It is true however that any parsing issue will be silently ignored in this case, and not processed by certbot. I pass to @ohemorange to see if this is a concerning problem or not.

Fix patch:
diff --git a/certbot-nginx/certbot_nginx/_internal/nginxparser.py b/certbot-nginx/certbot_nginx/_internal/nginxparser.py
index 787f7c363f2..2aa677c3801 100644
--- a/certbot-nginx/certbot_nginx/_internal/nginxparser.py
+++ b/certbot-nginx/certbot_nginx/_internal/nginxparser.py
@@ -9,7 +9,6 @@
 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/CHANGELOG.md b/certbot/CHANGELOG.md
index 04326ef305e..2bd843e31fa 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.
 


Test patch:
diff --git a/certbot-nginx/tests/configurator_test.py b/certbot-nginx/tests/configurator_test.py
index fae5d41d9fa..e667d5375d7 100644
--- a/certbot-nginx/tests/configurator_test.py
+++ b/certbot-nginx/tests/configurator_test.py
@@ -43,7 +43,7 @@ def test_prepare_no_install(self, mock_exe_exists):
 
     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 a5212078f35..8f7d5accfe9 100644
--- a/certbot-nginx/tests/nginxparser_test.py
+++ b/certbot-nginx/tests/nginxparser_test.py
@@ -350,6 +350,10 @@ def test_edge_cases(self):
         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 b062c4196c9..9047cb44615 100644
--- a/certbot-nginx/tests/parser_test.py
+++ b/certbot-nginx/tests/parser_test.py
@@ -50,7 +50,7 @@ def test_load(self):
         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 @@ def test_filedump(self):
         # 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'],