Remove internal comments from server_names directive (#10147)

Fixes https://github.com/certbot/certbot/issues/7090

certbot-nginx/certbot_nginx/_internal/parser.py

@@ -816,7 +816,11 @@ def _parse_server_raw(server: UnspacedList) -> Dict[str, Any]:
             if addr.ssl:
                 ssl = True
         elif directive[0] == 'server_name':
-            names.update(x.strip('"\'') for x in directive[1:])
+            params = directive[1:]
+            while '#' in params:
+                end_index = [i for i, param in enumerate(params) if param.startswith('\n')][0]
+                params = params[:params.index('#')] + params[end_index+1:]
+            names.update(x.strip('"\'') for x in params)
         elif _is_ssl_on_directive(directive):
             ssl = True
             apply_ssl_to_all_addrs = True

certbot-nginx/certbot_nginx/_internal/tests/parser_test.py

@@ -438,6 +438,60 @@ class NginxParserTest(util.NginxTest):
         ])
         assert server['ssl']
 
+    def test_parse_server_raw_comment(self):
+        testdata = """
+        server_name *.goo.far
+            # commented
+            baz.com;
+        """
+        loaded = nginxparser.loads(testdata)
+        server = parser._parse_server_raw(loaded) #pylint: disable=protected-access
+        assert server['names'] == {'*.goo.far', 'baz.com'}
+
+        testdata = """
+        server_name *.goo.far # commented
+            baz.com;
+        """
+        loaded = nginxparser.loads(testdata)
+        server = parser._parse_server_raw(loaded) #pylint: disable=protected-access
+        assert server['names'] == {'*.goo.far', 'baz.com'}
+
+        testdata = """
+        server_name *.goo.far # commented
+            ;
+        """
+        loaded = nginxparser.loads(testdata)
+        server = parser._parse_server_raw(loaded) #pylint: disable=protected-access
+        assert server['names'] == {'*.goo.far'}
+
+        # known bug; see https://github.com/certbot/certbot/issues/9942
+        testdata = """
+        server_name *.goo.far
+            #commented
+            ;
+        """
+        loaded = nginxparser.loads(testdata)
+        server = parser._parse_server_raw(loaded) #pylint: disable=protected-access
+        assert server['names'] == {'*.goo.far', '#commented'}
+
+        # same bug; # isn't actually allowed in domains
+        testdata = """
+        server_name *.go#o.far
+            ;
+        """
+        loaded = nginxparser.loads(testdata)
+        server = parser._parse_server_raw(loaded) #pylint: disable=protected-access
+        assert server['names'] == {'*.go#o.far'}
+
+        testdata = """
+        listen 443
+            # commented
+            ssl;
+        """
+        loaded = nginxparser.loads(testdata)
+        server = parser._parse_server_raw(loaded) #pylint: disable=protected-access
+        assert server['addrs'] == {obj.Addr.fromstring('443 ssl')}
+
     def test_parse_server_raw_unix(self):
         server = parser._parse_server_raw([ #pylint: disable=protected-access
             ['listen', 'unix:/var/run/nginx.sock']

certbot/CHANGELOG.md

@@ -53,6 +53,7 @@ More details about these changes can be found on our GitHub repo.
 * 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)
+* Nginx `server_name` directives with internal comments now ignore commented names
 
 More details about these changes can be found on our GitHub repo.