nginx: on encountering lua directives, produce a better warning (#9475)

* nginx: capitalise product names in warning message properly

* nginx: don't crash on encountering lua directives, warn instead

* add tests

* undo excess newline

* fix oldest tests: use old camelCase function name

* add missing newline in new testdata

* add tests for _by_lua, which should parse fine
This commit is contained in:
alexzorin 2022-11-30 12:03:51 +11:00 committed by GitHub
parent c78503f21d
commit c178fa8c0b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 111 additions and 2 deletions

View file

@ -1060,8 +1060,8 @@ class NginxConfigurator(common.Configurator):
product_name, product_version = version_matches[0]
if product_name != 'nginx':
logger.warning("NGINX derivative %s is not officially supported by"
" certbot", product_name)
logger.warning("nginx derivative %s is not officially supported by "
"Certbot.", product_name)
nginx_version = tuple(int(i) for i in product_version.split("."))

View file

@ -33,6 +33,18 @@ if TYPE_CHECKING:
logger = logging.getLogger(__name__)
class UnsupportedDirectiveException(RuntimeError):
"""Exception when encountering an nginx directive which is not supported
by this parser."""
directive_name: str
line_no: int
def __init__(self, directive_name: str, line_no: int) -> None:
self.directive_name = directive_name
self.line_no = line_no
class RawNginxParser:
# pylint: disable=pointless-statement
"""A class that parses nginx configuration with pyparsing."""
@ -74,6 +86,7 @@ class RawNginxParser:
def __init__(self, source: str) -> None:
self.source = source
self.whitespace_token_group.addParseAction(self._check_disallowed_directive)
def parse(self) -> ParseResults:
"""Returns the parsed tree."""
@ -83,6 +96,12 @@ class RawNginxParser:
"""Returns the parsed tree as a list."""
return self.parse().asList()
def _check_disallowed_directive(self, _source: str, line: int, results: ParseResults) -> None:
# *_by_lua_block might be first or second result, due to optional leading whitespace
toks = [t for t in results[0:2] if isinstance(t, str) and t.endswith("_by_lua_block")]
if toks:
raise UnsupportedDirectiveException(toks[0], line)
class RawNginxDumper:
"""A class that dumps nginx configuration from the provided tree."""

View file

@ -223,6 +223,12 @@ class NginxParser:
"supported.", item)
except pyparsing.ParseException as err:
logger.warning("Could not parse file: %s due to %s", item, err)
except nginxparser.UnsupportedDirectiveException as e:
logger.warning(
"%s:%d contained the '%s' directive, which is not supported by Certbot. The "
"file has been ignored, which may prevent Certbot from functioning properly. "
"Consider using the --webroot plugin and manually installing the certificate.",
item, e.line_no, e.directive_name)
return trees
def _find_config_root(self) -> str:

View file

@ -12,6 +12,7 @@ from certbot_nginx._internal.nginxparser import load
from certbot_nginx._internal.nginxparser import loads
from certbot_nginx._internal.nginxparser import RawNginxParser
from certbot_nginx._internal.nginxparser import UnspacedList
from certbot_nginx._internal.nginxparser import UnsupportedDirectiveException
import test_util as util
FIRST = operator.itemgetter(0)
@ -354,6 +355,69 @@ class TestRawNginxParser(unittest.TestCase):
parsed = loads("")
self.assertEqual(parsed, [])
def test_lua(self):
# https://github.com/certbot/certbot/issues/9066
self.assertRaises(UnsupportedDirectiveException, loads, """
location /foo {
content_by_lua_block {
ngx.say('Hello World')
}
}
""")
# Without leading whitespace
self.assertRaises(UnsupportedDirectiveException, loads, """
location /foo {content_by_lua_block {
ngx.say('Hello World')
}
}
""")
# Doesn't trigger if it's commented or not in the right position.
parsed = loads("""
location /foo {server_name content_by_lua_block;
#content_by_lua_block {
# ngx.say('Hello World')
# }
}
""")
self.assertEqual(
parsed,
[
[['location', '/foo'],
[['server_name', 'content_by_lua_block'],
['#', 'content_by_lua_block {'],
['#', " ngx.say('Hello World')"],
['#', ' }']
]]
])
# *_by_lua should parse successfully.
parsed = loads("""
location / {
set $a 32;
set $b 56;
set_by_lua $sum
'return tonumber(ngx.arg[1]) + tonumber(ngx.arg[2])'
$a $b;
content_by_lua '
ngx.say("foo");
';
}
""")
self.assertEqual(
parsed,
[
[['location', '/'],
[['set', '$a', '32'],
['set', '$b', '56'],
['set_by_lua', '$sum',
"'return tonumber(ngx.arg[1]) + tonumber(ngx.arg[2])'", '$a', '$b'
],
['content_by_lua', '\'\n ngx.say("foo");\n \'']
]]
]
)
class TestUnspacedList(unittest.TestCase):
"""Test the UnspacedList data structure"""

View file

@ -4,6 +4,7 @@ import re
import shutil
import unittest
from typing import List
from unittest import mock
from certbot import errors
from certbot.compat import os
@ -532,6 +533,14 @@ class NginxParserTest(util.NginxTest):
for output in log.output
))
@mock.patch('certbot_nginx._internal.parser.logger.warning')
def test_load_unsupported_directive_logged(self, mock_warn):
nparser = parser.NginxParser(self.config_path)
nparser.config_root = nparser.abs_path('unsupported_directives.conf')
nparser.load()
self.assertEqual(mock_warn.call_count, 1)
self.assertIn("which is not supported by Certbot", mock_warn.call_args[0][0])
if __name__ == "__main__":
unittest.main() # pragma: no cover

View file

@ -0,0 +1,11 @@
# This configuration file contains unsupported direcives.
server {
listen 80;
location / {
foobar_by_lua_block {
ngx.say("Hello World")
}
}
}