diff --git a/letsencrypt-nginx/letsencrypt_nginx/nginxparser.py b/letsencrypt-nginx/letsencrypt_nginx/nginxparser.py index f24455d59..7870581b4 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/nginxparser.py +++ b/letsencrypt-nginx/letsencrypt_nginx/nginxparser.py @@ -3,8 +3,9 @@ import string from pyparsing import ( Literal, White, Word, alphanums, CharsNotIn, Forward, Group, - Optional, OneOrMore, Regex, ZeroOrMore, pythonStyleComment) - + Optional, OneOrMore, Regex, ZeroOrMore) +from pyparsing import stringEnd +from pyparsing import restOfLine class RawNginxParser(object): # pylint: disable=expression-not-assigned @@ -24,7 +25,8 @@ class RawNginxParser(object): modifier = Literal("=") | Literal("~*") | Literal("~") | Literal("^~") # rules - assignment = (key + Optional(space + value) + semicolon) + comment = Literal('#') + restOfLine() + assignment = (key + Optional(space + value, default=None) + semicolon) location_statement = Optional(space + modifier) + Optional(space + location) if_statement = Literal("if") + space + Regex(r"\(.+\)") + space block = Forward() @@ -32,10 +34,10 @@ class RawNginxParser(object): block << Group( (Group(key + location_statement) ^ Group(if_statement)) + left_bracket - + Group(ZeroOrMore(Group(assignment) | block)) + + Group(ZeroOrMore(Group(comment | assignment) | block)) + right_bracket) - script = OneOrMore(Group(assignment) | block).ignore(pythonStyleComment) + script = OneOrMore(Group(comment | assignment) | block) + stringEnd def __init__(self, source): self.source = source @@ -60,30 +62,30 @@ class RawNginxDumper(object): """Iterates the dumped nginx content.""" blocks = blocks or self.blocks for key, values in blocks: - if current_indent: - yield spacer indentation = spacer * current_indent if isinstance(key, list): + if current_indent: + yield '' yield indentation + spacer.join(key) + ' {' + for parameter in values: - if isinstance(parameter[0], list): - dumped = self.__iter__( - [parameter], - current_indent + self.indentation) - for line in dumped: - yield line - else: - dumped = spacer.join(parameter) + ';' - yield spacer * ( - current_indent + self.indentation) + dumped + dumped = self.__iter__([parameter], current_indent + self.indentation) + for line in dumped: + yield line yield indentation + '}' else: - yield spacer * current_indent + key + spacer + values + ';' + if key == '#': + yield spacer * current_indent + key + values + else: + if values is None: + yield spacer * current_indent + key + ';' + else: + yield spacer * current_indent + key + spacer + values + ';' def as_string(self): """Return the parsed block as a string.""" - return '\n'.join(self) + return '\n'.join(self) + '\n' # Shortcut functions to respect Python's serialization interface diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py index bd700f144..2b7a4e08e 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py @@ -111,6 +111,10 @@ class NginxConfiguratorTest(util.NginxTest): self.config.parser.load() + parsed_example_conf = util.filter_comments(self.config.parser.parsed[example_conf]) + parsed_server_conf = util.filter_comments(self.config.parser.parsed[server_conf]) + parsed_nginx_conf = util.filter_comments(self.config.parser.parsed[nginx_conf]) + access_log = os.path.join(self.work_dir, "access.log") error_log = os.path.join(self.work_dir, "error.log") self.assertEqual([[['server'], @@ -125,9 +129,9 @@ class NginxConfiguratorTest(util.NginxTest): ['ssl_certificate_key', 'example/key.pem'], ['include', self.config.parser.loc["ssl_options"]]]]], - self.config.parser.parsed[example_conf]) + parsed_example_conf) self.assertEqual([['server_name', 'somename alias another.alias']], - self.config.parser.parsed[server_conf]) + parsed_server_conf) self.assertEqual([['server'], [['listen', '8000'], ['listen', 'somename:8080'], @@ -142,7 +146,7 @@ class NginxConfiguratorTest(util.NginxTest): ['ssl_certificate_key', '/etc/nginx/key.pem'], ['include', self.config.parser.loc["ssl_options"]]]], - self.config.parser.parsed[nginx_conf][-1][-1][-1]) + parsed_nginx_conf[-1][-1][-1]) def test_get_all_certs_keys(self): nginx_conf = self.config.parser.abs_path('nginx.conf') diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/nginxparser_test.py b/letsencrypt-nginx/letsencrypt_nginx/tests/nginxparser_test.py index 73a89534b..0d6e5c453 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/nginxparser_test.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/nginxparser_test.py @@ -2,6 +2,8 @@ import operator import unittest +from pyparsing import ParseException + from letsencrypt_nginx.nginxparser import ( RawNginxParser, load, dumps, dump) from letsencrypt_nginx.tests import util @@ -42,7 +44,7 @@ class TestRawNginxParser(unittest.TestCase): ['server_name', 'foo.com'], ['root', '/home/ubuntu/sites/foo/'], [['location', '/status'], [ - ['check_status'], + ['check_status', None], [['types'], [['image/jpeg', 'jpg']]], ]] ]]]) @@ -52,17 +54,20 @@ class TestRawNginxParser(unittest.TestCase): 'server {\n' ' listen 80;\n' ' server_name foo.com;\n' - ' root /home/ubuntu/sites/foo/;\n \n' + ' root /home/ubuntu/sites/foo/;\n' + '\n' ' location /status {\n' - ' check_status;\n \n' + ' check_status;\n' + '\n' ' types {\n' ' image/jpeg jpg;\n' ' }\n' ' }\n' - '}') + '}\n') def test_parse_from_file(self): - parsed = load(open(util.get_data_filename('foo.conf'))) + with open(util.get_data_filename('foo.conf')) as handle: + parsed = util.filter_comments(load(handle)) self.assertEqual( parsed, [['user', 'www-data'], @@ -85,7 +90,8 @@ class TestRawNginxParser(unittest.TestCase): ) def test_parse_from_file2(self): - parsed = load(open(util.get_data_filename('edge_cases.conf'))) + with open(util.get_data_filename('edge_cases.conf')) as handle: + parsed = util.filter_comments(load(handle)) self.assertEqual( parsed, [[['server'], [['server_name', 'simple']]], @@ -104,8 +110,13 @@ class TestRawNginxParser(unittest.TestCase): ['blah', '"hello;world"'], ['try_files', '$uri @rewrites']]]]]]) + def test_abort_on_parse_failure(self): + with open(util.get_data_filename('broken.conf')) as handle: + self.assertRaises(ParseException, load, handle) + def test_dump_as_file(self): - parsed = load(open(util.get_data_filename('nginx.conf'))) + with open(util.get_data_filename('nginx.conf')) as handle: + parsed = util.filter_comments(load(handle)) parsed[-1][-1].append([['server'], [['listen', '443 ssl'], ['server_name', 'localhost'], @@ -117,12 +128,38 @@ class TestRawNginxParser(unittest.TestCase): [['location', '/'], [['root', 'html'], ['index', 'index.html index.htm']]]]]) - _file = open(util.get_data_filename('nginx.new.conf'), 'w') - dump(parsed, _file) - _file.close() - parsed_new = load(open(util.get_data_filename('nginx.new.conf'))) + + with open(util.get_data_filename('nginx.new.conf'), 'w') as handle: + dump(parsed, handle) + with open(util.get_data_filename('nginx.new.conf')) as handle: + parsed_new = util.filter_comments(load(handle)) self.assertEquals(parsed, parsed_new) + def test_comments(self): + with open(util.get_data_filename('minimalistic_comments.conf')) as handle: + parsed = load(handle) + + with open(util.get_data_filename('minimalistic_comments.new.conf'), 'w') as handle: + dump(parsed, handle) + + with open(util.get_data_filename('minimalistic_comments.new.conf')) as handle: + parsed_new = load(handle) + + self.assertEquals(parsed, parsed_new) + + self.assertEqual(parsed_new, [ + ['#', " Use bar.conf when it's a full moon!"], + ['include', 'foo.conf'], + ['#', ' Kilroy was here'], + ['check_status', None], + [['server'], + [['#', ''], + ['#', " Don't forget to open up your firewall!"], + ['#', ''], + ['listen', '1234'], + ['#', ' listen 80;']]], + ]) + if __name__ == '__main__': unittest.main() # pragma: no cover diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/testdata/etc_nginx/broken.conf b/letsencrypt-nginx/letsencrypt_nginx/tests/testdata/etc_nginx/broken.conf new file mode 100644 index 000000000..98aef55d6 --- /dev/null +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/testdata/etc_nginx/broken.conf @@ -0,0 +1,12 @@ +# A faulty configuration file + +pid logs/nginx.pid; + + +events { + worker_connections 1024; +} + +include foo.conf; + +@@@ diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/testdata/etc_nginx/minimalistic_comments.conf b/letsencrypt-nginx/letsencrypt_nginx/tests/testdata/etc_nginx/minimalistic_comments.conf new file mode 100644 index 000000000..cf4648592 --- /dev/null +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/testdata/etc_nginx/minimalistic_comments.conf @@ -0,0 +1,12 @@ +# Use bar.conf when it's a full moon! +include foo.conf; # Kilroy was here +check_status; + +server { + # + # Don't forget to open up your firewall! + # + listen 1234; + # listen 80; +} + diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/testdata/etc_nginx/minimalistic_comments.new.conf b/letsencrypt-nginx/letsencrypt_nginx/tests/testdata/etc_nginx/minimalistic_comments.new.conf new file mode 100644 index 000000000..d1b7be91e --- /dev/null +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/testdata/etc_nginx/minimalistic_comments.new.conf @@ -0,0 +1,11 @@ +# Use bar.conf when it's a full moon! +include foo.conf; +# Kilroy was here +check_status; +server { + # + # Don't forget to open up your firewall! + # + listen 1234; + # listen 80; +} diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/testdata/etc_nginx/nginx.new.conf b/letsencrypt-nginx/letsencrypt_nginx/tests/testdata/etc_nginx/nginx.new.conf index 0a43b5842..59c1c968f 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/testdata/etc_nginx/nginx.new.conf +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/testdata/etc_nginx/nginx.new.conf @@ -20,52 +20,52 @@ http { tcp_nopush on; keepalive_timeout 0; gzip on; - + server { listen 8080; server_name localhost; server_name ~^(www\.)?(example|bar)\.; charset koi8-r; access_log logs/host.access.log main; - + location / { root html; index index.html index.htm; } error_page 404 /404.html; error_page 500 502 503 504 /50x.html; - + location = /50x.html { root html; } - + location ~ \.php$ { proxy_pass http://127.0.0.1; } - + location ~ \.php$ { root html; fastcgi_pass 127.0.0.1:9000; fastcgi_index index.php; fastcgi_param SCRIPT_FILENAME /scripts$fastcgi_script_name; } - + location ~ /\.ht { deny all; } } - + server { listen 8000; listen somename:8080; include server.conf; - + location / { root html; index index.html index.htm; } } - + server { listen 443 ssl; server_name localhost; @@ -74,10 +74,10 @@ http { ssl_session_cache shared:SSL:1m; ssl_session_timeout 5m; ssl_ciphers HIGH:!aNULL:!MD5; - + location / { root html; index index.html index.htm; } } -} \ No newline at end of file +} diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/util.py b/letsencrypt-nginx/letsencrypt_nginx/tests/util.py index a7db398c6..e7675f34d 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/util.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/util.py @@ -59,3 +59,18 @@ def get_nginx_configurator( version=version) config.prepare() return config + + +def filter_comments(tree): + """Filter comment nodes from parsed configurations.""" + + def traverse(tree): + """Generator dropping comment nodes""" + for key, values in tree: + if isinstance(key, list): + yield [key, filter_comments(values)] + else: + if key != '#': + yield [key, values] + + return list(traverse(tree))