Fix/10598 nginx comment in multiline directive (#10651)

Fixes #10598.

## Pull Request Checklist

- [x] The Certbot team has recently expressed interest in reviewing a PR
for this. If not, this PR may be closed due our limited resources and
need to prioritize how we spend them.
- [x] If you used AI to create this PR, you have done a self-review of
all AI-generated code and disclosed that your contribution was
AI-generated per [EFF's AI-generated contribution
policy](https://www.eff.org/about/opportunities/volunteer/coding-with-eff).
You assert you have thoroughly understood, reviewed, and tested your
entire submission.
- [x] If the change being made is to a [distributed
component](https://certbot.eff.org/docs/contributing.html#code-components-and-layout),
add a description of your change to the `newsfragments` directory. This
should be a file called `<title>.<type>`, where `<title>` is either a
GitHub issue number or some other unique name starting with `+`, and
`<type>` is either `changed`, `fixed`, or `added`.
* For example, if you fixed a bug for issue number 42, create a file
called `42.fixed` and put a description of your change in that file.
- [x] Add or update any documentation as needed to support the changes
in this PR.
- [x] Include your name in `AUTHORS.md` if you like.

AUTHORS.md

@@ -84,6 +84,7 @@ Authors
 * [Eric Engestrom](https://github.com/1ace)
 * [Eric Rescorla](https://github.com/ekr)
 * [Eric Wustrow](https://github.com/ewust)
+* [Erik Morén](https://github.com/morre95)
 * [Erik Rose](https://github.com/erikrose)
 * [Eugene Kazakov](https://github.com/xgin)
 * [Fabian](https://github.com/faerbit)
@@ -306,3 +307,4 @@ Authors
 * [Zach Shepherd](https://github.com/zjs)
 * [陈三](https://github.com/chenxsan)
 * [Shahar Naveh](https://github.com/ShaharNaveh)
+

certbot/src/certbot/_internal/plugins/nginx/nginxparser.py

@@ -13,6 +13,7 @@ from typing import SupportsIndex
 from typing import Union
 
 from pyparsing import Combine
+from pyparsing import FollowedBy
 from pyparsing import Forward
 from pyparsing import Group
 from pyparsing import Literal
@@ -50,15 +51,29 @@ class RawNginxParser:
 
     token = paren_quote_extend | tokenchars | quoted
 
-    whitespace_token_group = space + token + ZeroOrMore(required_space + token) + space
-    assignment = whitespace_token_group + semicolon
+    # An nginx comment runs from '#' to the end of the line. `restOfLine` does
+    # not consume the trailing newline; the following `required_space` does.
+    comment = Literal('#') + restOfLine
+    # A separator between two tokens is whitespace, optionally interleaved with
+    # one or more comments. The `FollowedBy(token)` lookahead means a comment is
+    # only treated as inline when a real token follows it — otherwise we leave
+    # the `#` alone so existing fallback paths (e.g. comment-as-token in a block
+    # header) still parse. This lets a multi-line directive contain comment
+    # lines between its tokens (valid nginx), which the old grammar mis-parsed:
+    # an unbalanced `"` inside such a comment would be sucked up by the
+    # `multiline=True` quoted strings and run away across the rest of the file
+    # (issue #10598).
+    token_separator = required_space + ZeroOrMore(
+        comment + required_space + FollowedBy(token)
+    )
 
-    comment = space + Literal('#') + restOfLine
+    whitespace_token_group = space + token + ZeroOrMore(token_separator + token) + space
+    assignment = whitespace_token_group + semicolon
 
     block = Forward()
 
     # order matters! see issue 518, and also http { # server { \n}
-    contents = Group(comment) | Group(block) | Group(assignment)
+    contents = Group(space + comment) | Group(block) | Group(assignment)
 
     block_begin = Group(whitespace_token_group)
     block_innards = Group(ZeroOrMore(contents) + space).leave_whitespace()

certbot/src/certbot/_internal/tests/plugins/nginx/nginxparser_test.py

@@ -201,6 +201,20 @@ class TestRawNginxParser(unittest.TestCase):
              [['#', ' server{']]]
         ]
 
+    def test_comment_inside_multiline_directive(self):
+        # See https://github.com/certbot/certbot/issues/10598
+        # An unbalanced double-quote inside a comment that sits between tokens
+        # of a multi-line directive must not be sucked into a multiline quoted
+        # string and run away across the rest of the file.
+        source = ("http {\n"
+                  "    log_format json_test escape=json '{'\n"
+                  "        #  '\"req\": \"$request\"'  # GET \"/test?...\n"
+                  "    '}';\n"
+                  "}\n")
+        parsed = loads(source)
+        # The config parses and round-trips byte-for-byte.
+        assert dumps(parsed) == source
+
     def test_access_log(self):
         # see issue #3798
         parsed = loads('access_log syslog:server=unix:/dev/log,facility=auth,'

newsfragments/10598.fixed

@@ -0,0 +1 @@
+Fixed nginx configuration parsing when comments appear between tokens of a multi-line directive.