From b82ebd9180db60333e87ea55318fa9c48490f200 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 16 Jun 2016 17:17:13 -0700 Subject: [PATCH] Fix desyncronisation with .spaced when modifying sublists - we now actually write directives again! --- certbot-nginx/certbot_nginx/configurator.py | 18 ++++----- certbot-nginx/certbot_nginx/nginxparser.py | 42 +++++++++++++++++---- certbot-nginx/certbot_nginx/parser.py | 8 +++- 3 files changed, 51 insertions(+), 17 deletions(-) diff --git a/certbot-nginx/certbot_nginx/configurator.py b/certbot-nginx/certbot_nginx/configurator.py index 30928e56c..4f46b6a66 100644 --- a/certbot-nginx/certbot_nginx/configurator.py +++ b/certbot-nginx/certbot_nginx/configurator.py @@ -152,17 +152,17 @@ class NginxConfigurator(common.Plugin): "install a cert.") vhost = self.choose_vhost(domain) - cert_directives = [['ssl_certificate', fullchain_path], - ['ssl_certificate_key', key_path]] + cert_directives = [['\n', 'ssl_certificate', ' ', fullchain_path], + ['\n', 'ssl_certificate_key', ' ', key_path]] # OCSP stapling was introduced in Nginx 1.3.7. If we have that version # or greater, add config settings for it. stapling_directives = [] if self.version >= (1, 3, 7): stapling_directives = [ - ['ssl_trusted_certificate', chain_path], - ['ssl_stapling', 'on'], - ['ssl_stapling_verify', 'on']] + ['\n', 'ssl_trusted_certificate', ' ', chain_path], + ['\n', 'ssl_stapling', ' ', 'on'], + ['\n', 'ssl_stapling_verify', ' ', 'on'], ['\n']] if len(stapling_directives) != 0 and not chain_path: raise errors.PluginError( @@ -337,10 +337,10 @@ class NginxConfigurator(common.Plugin): """ snakeoil_cert, snakeoil_key = self._get_snakeoil_paths() - ssl_block = [['listen', '{0} ssl'.format(self.config.tls_sni_01_port)], - ['ssl_certificate', snakeoil_cert], - ['ssl_certificate_key', snakeoil_key], - ['include', self.parser.loc["ssl_options"]]] + ssl_block = [['\n', 'listen', ' ', '{0} ssl'.format(self.config.tls_sni_01_port)], + ['\n', 'ssl_certificate', ' ', snakeoil_cert], + ['\n', 'ssl_certificate_key', ' ', snakeoil_key], + ['\n', 'include', ' ', self.parser.loc["ssl_options"]]] self.parser.add_server_directives( vhost.filep, vhost.names, ssl_block, replace=False) vhost.ssl = True diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py index c8131072c..856113f71 100644 --- a/certbot-nginx/certbot_nginx/nginxparser.py +++ b/certbot-nginx/certbot_nginx/nginxparser.py @@ -76,6 +76,9 @@ class RawNginxDumper(object): indentation = "" if spacey(b[0]): indentation = b.pop(0) + if not b: + yield indentation + continue key = b.pop(0) values = b.pop(0) @@ -154,36 +157,58 @@ def dump(blocks, _file): return _file.write(dumps(blocks)) -spacey = lambda x: isinstance(x, str) and x.isspace() +spacey = lambda x: (isinstance(x, str) and x.isspace()) or x == '' class UnspacedList(list): """Wrap a list [of lists], making any whitespace entries magically invisible""" - def __init__(self, list_source): + def __init__(self, list_source, top=False): self.spaced = copy.deepcopy(list(list_source)) # Turn self into a version of the source list that has spaces removed # and all sub-lists also UnspacedList()ed list.__init__(self, list_source) + self.top = self for i, entry in reversed(list(enumerate(self))): if isinstance(entry, list): - list.__setitem__(self, i, UnspacedList(entry)) + sublist = UnspacedList(entry, top=self.top) + list.__setitem__(self, i, sublist) + assert type(self.spaced) == list, "Type madness %r" % type(self.spaced) + self.spaced[i] = sublist.spaced elif spacey(entry): list.__delitem__(self, i) def insert(self, i, x): - self.spaced.insert(i + self._spaces_before(i), x) + if hasattr(x, "spaced"): + self.spaced.insert(i + self._spaces_before(i), x.spaced) + else: + self.spaced.insert(i + self._spaces_before(i), x) list.insert(self, i, x) def append(self, x): - self.spaced.append(x) + print "Unspaced append", x, self + if hasattr(x, "spaced"): + self.spaced.append(x.spaced) + else: + self.spaced.append(x) list.append(self, x) + print "After: aaaaaaaaaaaaaaaaa" + print self.top + print "Aftertop: bbbbbbbbbbbbbbbbb" + print self.top.spaced + #import ipdb + #ipdb.set_trace() def extend(self, x): - self.spaced.extend(x) + if hasattr(x, "spaced"): + self.spaced.extend(x.spaced) + else: + self.spaced.extend(x) + self.logger.debug("Weird, extending regular list %r to Unspaced %r", x, self) list.extend(self, x) def __add__(self, other): + print "Unspaced add", self, other if hasattr(other, "spaced"): # If the thing added to us is an UnspacedList, use its spaced form self.spaced.__add__(other.spaced) @@ -192,7 +217,10 @@ class UnspacedList(list): list.__add__(self, other) def __setitem__(self, i, value): - self.spaced.__setitem__(i + self._spaces_before(i), value) + if hasattr(value, "spaced"): + self.spaced.__setitem__(i + self._spaces_before(i), value.spaced) + else: + self.spaced.__setitem__(i + self._spaces_before(i), value) list.__setitem__(self, i, value) def __delitem__(self, i): diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py index f2faadd58..61c719816 100644 --- a/certbot-nginx/certbot_nginx/parser.py +++ b/certbot-nginx/certbot_nginx/parser.py @@ -215,7 +215,7 @@ class NginxParser(object): filename = filename + os.path.extsep + ext try: out = nginxparser.dumps(tree) - #logger.debug('Writing nginx conf tree to %s:\n%s', filename, out) + logger.debug('Writing nginx conf tree to %s:\n%s', filename, out) with open(filename, 'w') as _file: _file.write(out) @@ -506,6 +506,12 @@ def _add_directive(block, directive, replace): See _add_directives for more documentation. """ + directive = nginxparser.UnspacedList(directive) + print "Unspacified", directive.spaced, directive + if len(directive) == 0: + # whitespace + block.append(directive) + return location = -1 # Find the index of a config line where the name of the directive matches # the name of the directive we want to add.