diff --git a/certbot-nginx/certbot_nginx/_internal/http_01.py b/certbot-nginx/certbot_nginx/_internal/http_01.py index c7a36b767..5f450782a 100644 --- a/certbot-nginx/certbot_nginx/_internal/http_01.py +++ b/certbot-nginx/certbot_nginx/_internal/http_01.py @@ -77,11 +77,11 @@ class NginxHttp01(common.ChallengePerformer): """ included = False include_directive = ['\n', 'include', ' ', self.challenge_conf] - root = self.configurator.parser.config_root + http_path = self.configurator.parser.http_path bucket_directive = ['\n', 'server_names_hash_bucket_size', ' ', '128'] - main = self.configurator.parser.parsed[root] + main = self.configurator.parser.parsed[http_path] # insert include directive for line in main: if line[0] == ['http']: @@ -126,6 +126,7 @@ class NginxHttp01(common.ChallengePerformer): body.insert(0, bucket_directive) break + root = self.configurator.parser.config_root if not included: raise errors.MisconfigurationError( 'Certbot could not find a block to include ' diff --git a/certbot-nginx/certbot_nginx/_internal/parser.py b/certbot-nginx/certbot_nginx/_internal/parser.py index a43e6c4b8..23dd9575b 100644 --- a/certbot-nginx/certbot_nginx/_internal/parser.py +++ b/certbot-nginx/certbot_nginx/_internal/parser.py @@ -1,4 +1,5 @@ """NginxParser is a member object of the NginxConfigurator class.""" +from __future__ import annotations import copy import functools import glob @@ -41,6 +42,7 @@ class NginxParser: self.parsed: Dict[str, UnspacedList] = {} self.root = os.path.abspath(root) self.config_root = self._find_config_root() + self._http_path: str | None = None # Parse nginx.conf and included files. # TODO: Check sites-available/ as well. For now, the configurator does @@ -54,6 +56,14 @@ class NginxParser: self.parsed = {} self._parse_recursively(self.config_root) + @property + def http_path(self) -> str: + """ Filepath of file containing nginx http block. Set in self._parse_recursively + """ + if self._http_path is None: + raise errors.MisconfigurationError('No nginx http block found') + return self._http_path + def _parse_recursively(self, filepath: str) -> None: """Parses nginx config files recursively by looking at 'include' directives inside 'http' and 'server' blocks. Note that this only @@ -64,13 +74,16 @@ class NginxParser: """ # pylint: disable=too-many-nested-blocks filepath = self.abs_path(filepath) - trees = self._parse_files(filepath) - for tree in trees: + trees: dict[str, UnspacedList] = self._parse_files(filepath) + for filename, tree in trees.items(): for entry in tree: if _is_include_directive(entry): # Parse the top-level included file self._parse_recursively(entry[1]) elif entry[0] == ['http'] or entry[0] == ['server']: + # Note http block location for http_01.py + if entry[0] == ['http']: + self._http_path = filename # Look for includes in the top-level 'http'/'server' context for subentry in entry[1]: if _is_include_directive(subentry): @@ -193,35 +206,35 @@ class NginxParser: pass return result - def _parse_files(self, filepath: str, override: bool = False) -> List[UnspacedList]: + def _parse_files(self, filepath: str, override: bool = False) -> dict[str, UnspacedList]: """Parse files from a glob :param str filepath: Nginx config file path :param bool override: Whether to parse a file that has been parsed - :returns: list of parsed tree structures - :rtype: list + :returns: dict of parsed tree structures indexed by filename + :rtype: dict[str, UnspacedList] """ - files = glob.glob(filepath) # nginx on unix calls glob(3) for this + files: list[str] = glob.glob(filepath) # nginx on unix calls glob(3) for this # XXX Windows nginx uses FindFirstFile, and # should have a narrower call here - trees = [] - for item in files: - if item in self.parsed and not override: + trees: dict[str, UnspacedList] = {} + for filename in files: + if filename in self.parsed and not override: continue try: - with open(item, "r", encoding="utf-8") as _file: + with open(filename, "r", encoding="utf-8") as _file: parsed = nginxparser.load(_file) - self.parsed[item] = parsed - trees.append(parsed) + self.parsed[filename] = parsed + trees[filename] = parsed except OSError: - logger.warning("Could not open file: %s", item) + logger.warning("Could not open file: %s", filename) except UnicodeDecodeError: logger.warning("Could not read file: %s due to invalid " "character. Only UTF-8 encoding is " - "supported.", item) + "supported.", filename) except pyparsing.ParseException as err: - logger.warning("Could not parse file: %s due to %s", item, err) + logger.warning("Could not parse file: %s due to %s", filename, err) return trees def _find_config_root(self) -> str: diff --git a/certbot-nginx/certbot_nginx/_internal/tests/configurator_test.py b/certbot-nginx/certbot_nginx/_internal/tests/configurator_test.py index f10649920..9368fa64f 100644 --- a/certbot-nginx/certbot_nginx/_internal/tests/configurator_test.py +++ b/certbot-nginx/certbot_nginx/_internal/tests/configurator_test.py @@ -134,7 +134,7 @@ class NginxConfiguratorTest(util.NginxTest): ['server_name', 'example.*'], ['listen', '5001', 'ssl'], ['#', parser.COMMENT]]]] == \ - parsed[0] + parsed[filep] def test_choose_vhosts_alias(self): self._test_choose_vhosts_common('alias', 'server_conf') diff --git a/certbot-nginx/certbot_nginx/_internal/tests/parser_test.py b/certbot-nginx/certbot_nginx/_internal/tests/parser_test.py index 7cb1d2730..2b1719052 100644 --- a/certbot-nginx/certbot_nginx/_internal/tests/parser_test.py +++ b/certbot-nginx/certbot_nginx/_internal/tests/parser_test.py @@ -74,6 +74,15 @@ class NginxParserTest(util.NginxTest): nparser.parsed[nparser.abs_path( 'sites-enabled/example.com')] + def test_included_load(self): + # Test for when the root file doesn't have http in it + nparser = parser.NginxParser(self.config_path) + nparser.config_root = os.path.join(self.config_path, "nginx-include.conf") + nparser.load() + assert len(nparser.parsed) > 1 + assert len(nparser.parsed[nparser.config_root]) == 4 + assert os.path.join(self.config_path, "nginx.conf") in nparser.parsed + def test_abs_path(self): nparser = parser.NginxParser(self.config_path) if os.name != 'nt': @@ -99,7 +108,7 @@ class NginxParserTest(util.NginxTest): ['listen', '127.0.0.1'], ['server_name', '.example.com'], ['server_name', 'example.*']]]] == \ - parsed[0] + parsed[nparser.abs_path('sites-enabled/example.com.test')] def test__do_for_subarray(self): # pylint: disable=protected-access @@ -490,8 +499,8 @@ class NginxParserTest(util.NginxTest): nparser = parser.NginxParser(self.config_path) path = nparser.abs_path('valid_unicode_comments.conf') parsed = nparser._parse_files(path) # pylint: disable=protected-access - assert ['server'] == parsed[0][2][0] - assert ['listen', '80'] == parsed[0][2][1][3] + assert ['server'] == parsed[path][2][0] + assert ['listen', '80'] == parsed[path][2][1][3] def test_valid_unicode_roundtrip(self): """This tests the parser's ability to load and save a config containing Unicode""" @@ -507,7 +516,7 @@ class NginxParserTest(util.NginxTest): path = nparser.abs_path('invalid_unicode_comments.conf') parsed = nparser._parse_files(path) # pylint: disable=protected-access - assert [] == parsed + assert {} == parsed assert any( ('invalid character' in output) and ('UTF-8' in output) for output in log.output diff --git a/certbot-nginx/certbot_nginx/_internal/tests/testdata/etc_nginx/nginx-include.conf b/certbot-nginx/certbot_nginx/_internal/tests/testdata/etc_nginx/nginx-include.conf new file mode 100644 index 000000000..e4da3cb5f --- /dev/null +++ b/certbot-nginx/certbot_nginx/_internal/tests/testdata/etc_nginx/nginx-include.conf @@ -0,0 +1,4 @@ +# just a comment on top +# so we're not at the very top +include nginx.conf; +# and another comment for fun diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index f7906e7e7..ba66b6eda 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -30,6 +30,7 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). 3.1.0. * When adding ssl listen directives in nginx server blocks, IP addresses are now preserved. +* Nginx configurations can now have the http block in files other than the root (nginx.conf) More details about these changes can be found on our GitHub repo.