Skip to content

Commit

Permalink
Remove ipv6only=on from duplicated vhosts (#5793)
Browse files Browse the repository at this point in the history
* rename delete_default to remove_singleton_listen_params

* update docstring

* add documentation to obj.py

* add test for remove duplicate ipv6only

* Remove ipv6only=on from duplicated vhosts

* add test to make sure ipv6only=on is not erroneously removed
  • Loading branch information
ohemorange authored and bmw committed Mar 27, 2018
1 parent af2cce4 commit 4d082e2
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 6 deletions.
3 changes: 2 additions & 1 deletion certbot-nginx/certbot_nginx/configurator.py
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,8 @@ def ipv6_info(self, port):
def _vhost_from_duplicated_default(self, domain, port=None):
if self.new_vhost is None:
default_vhost = self._get_default_vhost(port)
self.new_vhost = self.parser.duplicate_vhost(default_vhost, delete_default=True)
self.new_vhost = self.parser.duplicate_vhost(default_vhost,
remove_singleton_listen_params=True)
self.new_vhost.names = set()

self._add_server_name_to_vhost(self.new_vhost, domain)
Expand Down
2 changes: 2 additions & 0 deletions certbot-nginx/certbot_nginx/obj.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ class Addr(common.Addr):
:param str port: port number or "\*" or ""
:param bool ssl: Whether the directive includes 'ssl'
:param bool default: Whether the directive includes 'default_server'
:param bool default: Whether this is an IPv6 address
:param bool ipv6only: Whether the directive includes 'ipv6only=on'
"""
UNSPECIFIED_IPV4_ADDRESSES = ('', '*', '0.0.0.0')
Expand Down
12 changes: 8 additions & 4 deletions certbot-nginx/certbot_nginx/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -354,13 +354,14 @@ def _modify_server_directives(self, vhost, block_func):
except errors.MisconfigurationError as err:
raise errors.MisconfigurationError("Problem in %s: %s" % (filename, str(err)))

def duplicate_vhost(self, vhost_template, delete_default=False, only_directives=None):
def duplicate_vhost(self, vhost_template, remove_singleton_listen_params=False,
only_directives=None):
"""Duplicate the vhost in the configuration files.
:param :class:`~certbot_nginx.obj.VirtualHost` vhost_template: The vhost
whose information we copy
:param bool delete_default: If we should remove default_server
from listen directives in the block.
:param bool remove_singleton_listen_params: If we should remove parameters
from listen directives in the block that can only be used once per address
:param list only_directives: If it exists, only duplicate the named directives. Only
looks at first level of depth; does not expand includes.
Expand All @@ -387,15 +388,18 @@ def duplicate_vhost(self, vhost_template, delete_default=False, only_directives=

enclosing_block.append(raw_in_parsed)
new_vhost.path[-1] = len(enclosing_block) - 1
if delete_default:
if remove_singleton_listen_params:
for addr in new_vhost.addrs:
addr.default = False
addr.ipv6only = False
for directive in enclosing_block[new_vhost.path[-1]][1]:
if len(directive) > 0 and directive[0] == 'listen':
if 'default_server' in directive:
del directive[directive.index('default_server')]
if 'default' in directive:
del directive[directive.index('default')]
if 'ipv6only=on' in directive:
del directive[directive.index('ipv6only=on')]
return new_vhost


Expand Down
24 changes: 23 additions & 1 deletion certbot-nginx/certbot_nginx/tests/parser_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ def test_duplicate_vhost(self):

vhosts = nparser.get_vhosts()
default = [x for x in vhosts if 'default' in x.filep][0]
new_vhost = nparser.duplicate_vhost(default, delete_default=True)
new_vhost = nparser.duplicate_vhost(default, remove_singleton_listen_params=True)
nparser.filedump(ext='')

# check properties of new vhost
Expand All @@ -448,6 +448,28 @@ def test_duplicate_vhost(self):
self.assertEqual(len(default.raw), len(new_vhost_parsed.raw))
self.assertTrue(next(iter(default.addrs)).super_eq(next(iter(new_vhost_parsed.addrs))))

def test_duplicate_vhost_remove_ipv6only(self):
nparser = parser.NginxParser(self.config_path)

vhosts = nparser.get_vhosts()
ipv6ssl = [x for x in vhosts if 'ipv6ssl' in x.filep][0]
new_vhost = nparser.duplicate_vhost(ipv6ssl, remove_singleton_listen_params=True)
nparser.filedump(ext='')

for addr in new_vhost.addrs:
self.assertFalse(addr.ipv6only)

identical_vhost = nparser.duplicate_vhost(ipv6ssl, remove_singleton_listen_params=False)
nparser.filedump(ext='')

called = False
for addr in identical_vhost.addrs:
if addr.ipv6:
self.assertTrue(addr.ipv6only)
called = True
self.assertTrue(called)



if __name__ == "__main__":
unittest.main() # pragma: no cover

0 comments on commit 4d082e2

Please sign in to comment.