From ec49afb7c05b5523b0eb8fc87f749478086d6a36 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Fri, 3 Jun 2016 16:45:24 -0700
Subject: [PATCH 01/48] UnspacedList: infrastructure for parsing but hiding
 nginx whitespace

---
 certbot-nginx/certbot_nginx/nginxparser.py | 68 +++++++++++++++++++++-
 1 file changed, 66 insertions(+), 2 deletions(-)

diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py
index 1577c7b1e..f9348398a 100644
--- a/certbot-nginx/certbot_nginx/nginxparser.py
+++ b/certbot-nginx/certbot_nginx/nginxparser.py
@@ -1,4 +1,5 @@
 """Very low-level nginx config parser based on pyparsing."""
+import copy
 import string
 
 from pyparsing import (
@@ -26,8 +27,8 @@ class RawNginxParser(object):
     modifier = Literal("=") | Literal("~*") | Literal("~") | Literal("^~")
 
     # rules
-    comment = Literal('#') + restOfLine()
-    assignment = (key + Optional(space + value, default=None) + semicolon)
+    comment = White() + Literal('#') + restOfLine()
+    assignment = White() + key + Optional(space + value, default=None) + semicolon
     location_statement = Optional(space + modifier) + Optional(space + location)
     if_statement = Literal("if") + space + Regex(r"\(.+\)") + space
     map_statement = Literal("map") + space + Regex(r"\S+") + space + Regex(r"\$\S+") + space
@@ -136,3 +137,66 @@ def dump(blocks, _file, indentation=4):
 
     """
     return _file.write(dumps(blocks, indentation))
+
+
+
+spacey = lambda x: isinstance(x, str) and x.isspace()
+
+class UnspacedList(list):
+    """Wrap a list [of lists], making any whitespace entries magically invisible"""
+
+    def __init__(self, list_source):
+        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 UnspaceList()ed
+        list.__init__(self, list_source)
+        for i, entry in reversed(list(enumerate(self))):
+            if isinstance(entry, list):
+                list.__setitem__(self, i, UnspacedList(entry))
+            elif spacey(entry):
+                list.__delitem__(self, i)
+    
+    def insert(self, i, x):
+        self.spaced.insert(i + self._spaces_before(i), x)
+        list.insert(self, i, x)
+
+    def append(self, x):
+        self.spaced.append(x)
+        list.append(self, x)
+
+    def extend(self, x):
+        self.spaced.extend(x)
+        list.extend(self, x)
+
+    def __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)
+        else:
+            self.spaced.__add__(other)
+        list.__add__(self, other)
+    
+    def __setitem__(self, i, value):
+        self.spaced.__setitem__(i + self._spaces_before(i), value)
+        list.__setitem__(self, i, value)
+
+    def __delitem__(self, i):
+        self.spaced.__delitem__(i + self._spaces_before(i))
+        list.__delitem__(self, i)
+
+    def _spaces_before(self, idx):
+        "Count the number of spaces in the spaced list before pos idx in the spaceless one"
+        spaces = 0
+        pos = 0
+        while idx != -1:
+            if spacey(self.spaced[pos]):
+                spaces += 1
+            else:
+                idx -= 1
+            pos += 1
+        return spaces
+
+    def with_spaces(self):
+        return self.spaced
+

From ac220976f1032a26d1801da2528ffd554e0e400d Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 7 Jun 2016 14:46:49 -0700
Subject: [PATCH 02/48] work in progress

---
 acme/acme/errors.py                        |  2 +-
 certbot-nginx/certbot_nginx/nginxparser.py | 52 ++++++++++++++++++----
 2 files changed, 45 insertions(+), 9 deletions(-)

diff --git a/acme/acme/errors.py b/acme/acme/errors.py
index 77d47c522..ca2ab1874 100644
--- a/acme/acme/errors.py
+++ b/acme/acme/errors.py
@@ -49,7 +49,7 @@ class MissingNonce(NonceError):
 
     def __str__(self):
         return ('Server {0} response did not include a replay '
-                'nonce, headers: {1}'.format(
+                'nonce, headers: {1}\n(This may be a service outage)'.format(
                     self.response.request.method, self.response.headers))
 
 
diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py
index f9348398a..7b39234dc 100644
--- a/certbot-nginx/certbot_nginx/nginxparser.py
+++ b/certbot-nginx/certbot_nginx/nginxparser.py
@@ -53,8 +53,44 @@ class RawNginxParser(object):
         """Returns the parsed tree as a list."""
         return self.parse().asList()
 
-
 class RawNginxDumper(object):
+    # pylint: disable=too-few-public-methods
+    """A class that dumps nginx configuration from the provided tree."""
+    def __init__(self, blocks, indentation=0):
+        self.blocks = blocks
+        self.indentation = indentation
+
+    def __iter__(self, blocks=None, current_indent=0, spacer=''):
+        """Iterates the dumped nginx content."""
+        blocks = blocks or self.blocks
+        for key, values in blocks.spaced:
+            #indentation = spacer * current_indent
+            if isinstance(key, list):
+                yield "".join(key) + ' {'
+
+                for parameter in values:
+                    dumped = self.__iter__([parameter], current_indent + self.indentation)
+                    for line in dumped:
+                        yield line
+
+                yield '}'
+            else:
+                if key == '#':
+                    yield key + values
+                else:
+                    if values is None:
+                        yield key + ';'
+                    else:
+                        yield key + values + ';'
+
+    def __str__(self):
+        """Return the parsed block as a string."""
+        return '\n'.join(self) + '\n'
+
+
+
+
+class OldRawNginxDumper(object):
     # pylint: disable=too-few-public-methods
     """A class that dumps nginx configuration from the provided tree."""
     def __init__(self, blocks, indentation=4):
@@ -102,7 +138,7 @@ def loads(source):
     :rtype: list
 
     """
-    return RawNginxParser(source).as_list()
+    return UnspacedList(RawNginxParser(source).as_list())
 
 
 def load(_file):
@@ -113,13 +149,13 @@ def load(_file):
     :rtype: list
 
     """
-    return loads(_file.read())
+    return UnspacedList(loads(_file.read()))
 
 
 def dumps(blocks, indentation=4):
     """Dump to a string.
 
-    :param list block: The parsed tree
+    :param UnspacedList block: The parsed tree
     :param int indentation: The number of spaces to indent
     :rtype: str
 
@@ -130,7 +166,7 @@ def dumps(blocks, indentation=4):
 def dump(blocks, _file, indentation=4):
     """Dump to a file.
 
-    :param list block: The parsed tree
+    :param UnspacedList block: The parsed tree
     :param file _file: The file to dump to
     :param int indentation: The number of spaces to indent
     :rtype: NoneType
@@ -149,14 +185,14 @@ class UnspacedList(list):
         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 UnspaceList()ed
+        # and all sub-lists also UnspacedList()ed
         list.__init__(self, list_source)
         for i, entry in reversed(list(enumerate(self))):
             if isinstance(entry, list):
                 list.__setitem__(self, i, UnspacedList(entry))
             elif spacey(entry):
                 list.__delitem__(self, i)
-    
+
     def insert(self, i, x):
         self.spaced.insert(i + self._spaces_before(i), x)
         list.insert(self, i, x)
@@ -176,7 +212,7 @@ class UnspacedList(list):
         else:
             self.spaced.__add__(other)
         list.__add__(self, other)
-    
+
     def __setitem__(self, i, value):
         self.spaced.__setitem__(i + self._spaces_before(i), value)
         list.__setitem__(self, i, value)

From 9be5f7d7d961cf1def5872acec9cb9bc41e11016 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 7 Jun 2016 17:17:17 -0700
Subject: [PATCH 03/48] Further WIP

---
 certbot-nginx/certbot_nginx/nginxparser.py | 99 +++++++++++++++++++---
 certbot-nginx/certbot_nginx/parser.py      |  2 +-
 2 files changed, 90 insertions(+), 11 deletions(-)

diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py
index 7b39234dc..8c2ba197e 100644
--- a/certbot-nginx/certbot_nginx/nginxparser.py
+++ b/certbot-nginx/certbot_nginx/nginxparser.py
@@ -1,6 +1,7 @@
 """Very low-level nginx config parser based on pyparsing."""
 import copy
 import string
+import sys
 
 from pyparsing import (
     Literal, White, Word, alphanums, CharsNotIn, Forward, Group,
@@ -18,6 +19,7 @@ class RawNginxParser(object):
     right_bracket = Literal("}").suppress()
     semicolon = Literal(";").suppress()
     space = White().suppress()
+    keepSpace = Optional(White())
     key = Word(alphanums + "_/+-.")
     # Matches anything that is not a special character AND any chars in single
     # or double quotes
@@ -27,8 +29,9 @@ class RawNginxParser(object):
     modifier = Literal("=") | Literal("~*") | Literal("~") | Literal("^~")
 
     # rules
-    comment = White() + Literal('#') + restOfLine()
-    assignment = White() + key + Optional(space + value, default=None) + semicolon
+    comment = Literal('#') + restOfLine()
+
+    assignment = keepSpace + key + Optional(space + value, default=None) + semicolon
     location_statement = Optional(space + modifier) + Optional(space + location)
     if_statement = Literal("if") + space + Regex(r"\(.+\)") + space
     map_statement = Literal("map") + space + Regex(r"\S+") + space + Regex(r"\$\S+") + space
@@ -53,6 +56,51 @@ class RawNginxParser(object):
         """Returns the parsed tree as a list."""
         return self.parse().asList()
 
+class OldRawNginxParser(object):
+    # pylint: disable=expression-not-assigned
+    """A class that parses nginx configuration with pyparsing."""
+
+    # constants
+    left_bracket = Literal("{").suppress()
+    right_bracket = Literal("}").suppress()
+    semicolon = Literal(";").suppress()
+    space = White().suppress()
+    key = Word(alphanums + "_/+-.")
+    # Matches anything that is not a special character AND any chars in single
+    # or double quotes
+    value = Regex(r"((\".*\")?(\'.*\')?[^\{\};,]?)+")
+    location = CharsNotIn("{};," + string.whitespace)
+    # modifier for location uri [ = | ~ | ~* | ^~ ]
+    modifier = Literal("=") | Literal("~*") | Literal("~") | Literal("^~")
+
+    # rules
+    comment = Literal("#") + restOfLine()
+    assignment = (key + Optional(space + value, default=None) + semicolon)
+    location_statement = Optional(space + modifier) + Optional(space + location)
+    if_statement = Literal("if") + space + Regex(r"\(.+\)") + space
+    map_statement = Literal("map") + space + Regex(r"\S+") + space + Regex(r"\$\S+") + space
+    block = Forward()
+
+    block << Group(
+        (Group(key + location_statement) ^ Group(if_statement) ^ Group(map_statement)) +
+        left_bracket +
+        Group(ZeroOrMore(Group(comment | assignment) | block)) +
+        right_bracket)
+
+    script = OneOrMore(Group(comment | assignment) ^ block) + stringEnd
+
+    def __init__(self, source):
+        self.source = source
+
+    def parse(self):
+        """Returns the parsed tree."""
+        return self.script.parseString(self.source)
+
+    def as_list(self):
+        """Returns the parsed tree as a list."""
+        return self.parse().asList()
+
+
 class RawNginxDumper(object):
     # pylint: disable=too-few-public-methods
     """A class that dumps nginx configuration from the provided tree."""
@@ -60,13 +108,24 @@ class RawNginxDumper(object):
         self.blocks = blocks
         self.indentation = indentation
 
-    def __iter__(self, blocks=None, current_indent=0, spacer=''):
+    def __iter__(self, blocks=None, current_indent=0, spacer=' '):
         """Iterates the dumped nginx content."""
         blocks = blocks or self.blocks
-        for key, values in blocks.spaced:
+        print "iterating", blocks
+        for b in blocks:
+            if len(b) == 2:
+                key, values = b
+                indentation = ""
+            elif len(b) == 3:
+                indentation, key, values = b
+                assert indentation.isspace(), indentation + " is not space"
+                yield indentation
+            else:
+                print "Cannot process", b
+                sys.exit(1)
             #indentation = spacer * current_indent
             if isinstance(key, list):
-                yield "".join(key) + ' {'
+                yield spacer.join(key) + ' {'
 
                 for parameter in values:
                     dumped = self.__iter__([parameter], current_indent + self.indentation)
@@ -75,13 +134,13 @@ class RawNginxDumper(object):
 
                 yield '}'
             else:
-                if key == '#':
+                if isinstance(key, str) and key.strip() == '#':
                     yield key + values
                 else:
                     if values is None:
                         yield key + ';'
                     else:
-                        yield key + values + ';'
+                        yield key + spacer + values + ';'
 
     def __str__(self):
         """Return the parsed block as a string."""
@@ -138,7 +197,28 @@ def loads(source):
     :rtype: list
 
     """
-    return UnspacedList(RawNginxParser(source).as_list())
+    old = OldRawNginxParser(source).as_list()
+    print "Old:"
+    for entry in old:
+        print len(entry), " ",
+    new = UnspacedList(RawNginxParser(source).as_list())
+    print "\nNew:"
+    print new
+    for entry in new:
+        print len(entry), " ",
+    print "\nNewspaced:"
+    for entry in new.spaced:
+        print str(len(entry))+ " ",
+    print "\ngo"
+    if old != new:
+        print "NON-MATCH"
+        for a, b in zip(old, new):
+            if a != b:
+                print "Entry", a, "!=", b
+        import sys
+    else:
+        print "Parallel"
+    return new
 
 
 def load(_file):
@@ -149,7 +229,7 @@ def load(_file):
     :rtype: list
 
     """
-    return UnspacedList(loads(_file.read()))
+    return loads(_file.read())
 
 
 def dumps(blocks, indentation=4):
@@ -175,7 +255,6 @@ def dump(blocks, _file, indentation=4):
     return _file.write(dumps(blocks, indentation))
 
 
-
 spacey = lambda x: isinstance(x, str) and x.isspace()
 
 class UnspacedList(list):
diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py
index 2f08c15d3..2872654b5 100644
--- a/certbot-nginx/certbot_nginx/parser.py
+++ b/certbot-nginx/certbot_nginx/parser.py
@@ -209,7 +209,7 @@ class NginxParser(object):
 
         """
         for filename in self.parsed:
-            tree = self.parsed[filename]
+            tree = self.parsed[filename].spaced
             if ext:
                 filename = filename + os.path.extsep + ext
             try:

From 80a52d8f018c99f186d6d7642edc86294852702d Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 7 Jun 2016 17:55:53 -0700
Subject: [PATCH 04/48] Vaguely close to working?

---
 certbot-nginx/certbot_nginx/nginxparser.py | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py
index 8c2ba197e..9e7136ced 100644
--- a/certbot-nginx/certbot_nginx/nginxparser.py
+++ b/certbot-nginx/certbot_nginx/nginxparser.py
@@ -31,7 +31,7 @@ class RawNginxParser(object):
     # rules
     comment = Literal('#') + restOfLine()
 
-    assignment = keepSpace + key + Optional(space + value, default=None) + semicolon
+    assignment = keepSpace + key + Optional(White() + value, default=None) + semicolon
     location_statement = Optional(space + modifier) + Optional(space + location)
     if_statement = Literal("if") + space + Regex(r"\(.+\)") + space
     map_statement = Literal("map") + space + Regex(r"\S+") + space + Regex(r"\$\S+") + space
@@ -119,14 +119,13 @@ class RawNginxDumper(object):
             elif len(b) == 3:
                 indentation, key, values = b
                 assert indentation.isspace(), indentation + " is not space"
-                yield indentation
+                indentation = indentation.replace("\n", "", 1)
             else:
                 print "Cannot process", b
                 sys.exit(1)
             #indentation = spacer * current_indent
             if isinstance(key, list):
-                yield spacer.join(key) + ' {'
-
+                yield indentation + spacer.join(key) + ' {'
                 for parameter in values:
                     dumped = self.__iter__([parameter], current_indent + self.indentation)
                     for line in dumped:
@@ -135,15 +134,16 @@ class RawNginxDumper(object):
                 yield '}'
             else:
                 if isinstance(key, str) and key.strip() == '#':
-                    yield key + values
+                    yield indentation + key + values
                 else:
                     if values is None:
-                        yield key + ';'
+                        yield indentation + key + ';'
                     else:
-                        yield key + spacer + values + ';'
+                        yield indentation + key + spacer + values + ';'
 
     def __str__(self):
         """Return the parsed block as a string."""
+        print "Merging:\n", '\n'.join(self)
         return '\n'.join(self) + '\n'
 
 

From 443f2ebb580faf6a582efe359405c4180f7ee0a6 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 7 Jun 2016 18:19:58 -0700
Subject: [PATCH 05/48] WIP

---
 certbot-nginx/certbot_nginx/nginxparser.py | 23 ++++++++++------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py
index 9e7136ced..2a68de77b 100644
--- a/certbot-nginx/certbot_nginx/nginxparser.py
+++ b/certbot-nginx/certbot_nginx/nginxparser.py
@@ -31,7 +31,7 @@ class RawNginxParser(object):
     # rules
     comment = Literal('#') + restOfLine()
 
-    assignment = keepSpace + key + Optional(White() + value, default=None) + semicolon
+    assignment = keepSpace + key + Optional(space + value, default=None) + semicolon
     location_statement = Optional(space + modifier) + Optional(space + location)
     if_statement = Literal("if") + space + Regex(r"\(.+\)") + space
     map_statement = Literal("map") + space + Regex(r"\S+") + space + Regex(r"\$\S+") + space
@@ -108,31 +108,28 @@ class RawNginxDumper(object):
         self.blocks = blocks
         self.indentation = indentation
 
-    def __iter__(self, blocks=None, current_indent=0, spacer=' '):
+    def __iter__(self, blocks=None, spacer=' '):
         """Iterates the dumped nginx content."""
         blocks = blocks or self.blocks
         print "iterating", blocks
         for b in blocks:
-            if len(b) == 2:
-                key, values = b
-                indentation = ""
-            elif len(b) == 3:
-                indentation, key, values = b
-                assert indentation.isspace(), indentation + " is not space"
+            indentation = ""
+            if spacey(b[0]):
+                indentation = b.pop(0)
                 indentation = indentation.replace("\n", "", 1)
-            else:
-                print "Cannot process", b
-                sys.exit(1)
-            #indentation = spacer * current_indent
+            key = b.pop(0)
+
+            values = b
             if isinstance(key, list):
                 yield indentation + spacer.join(key) + ' {'
                 for parameter in values:
-                    dumped = self.__iter__([parameter], current_indent + self.indentation)
+                    dumped = self.__iter__([parameter])
                     for line in dumped:
                         yield line
 
                 yield '}'
             else:
+                #yield b
                 if isinstance(key, str) and key.strip() == '#':
                     yield indentation + key + values
                 else:

From 8147c671e42c95cd7aa63f15f2d8bb3cf280c0bd Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Wed, 8 Jun 2016 17:52:35 -0700
Subject: [PATCH 06/48] Now handles some conf files in whitespace-preserving
 mode

(but not all of them)
---
 certbot-nginx/certbot_nginx/nginxparser.py | 136 ++++-----------------
 certbot-nginx/certbot_nginx/parser.py      |  22 +++-
 certbot-nginx/certbot_nginx/tls_sni_01.py  |   9 +-
 3 files changed, 53 insertions(+), 114 deletions(-)

diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py
index 2a68de77b..13498bee2 100644
--- a/certbot-nginx/certbot_nginx/nginxparser.py
+++ b/certbot-nginx/certbot_nginx/nginxparser.py
@@ -18,8 +18,7 @@ class RawNginxParser(object):
     left_bracket = Literal("{").suppress()
     right_bracket = Literal("}").suppress()
     semicolon = Literal(";").suppress()
-    space = White().suppress()
-    keepSpace = Optional(White())
+    space = Optional(White())
     key = Word(alphanums + "_/+-.")
     # Matches anything that is not a special character AND any chars in single
     # or double quotes
@@ -29,21 +28,23 @@ class RawNginxParser(object):
     modifier = Literal("=") | Literal("~*") | Literal("~") | Literal("^~")
 
     # rules
-    comment = Literal('#') + restOfLine()
+    comment = space + Literal('#') + restOfLine()
 
-    assignment = keepSpace + key + Optional(space + value, default=None) + semicolon
-    location_statement = Optional(space + modifier) + Optional(space + location)
-    if_statement = Literal("if") + space + Regex(r"\(.+\)") + space
-    map_statement = Literal("map") + space + Regex(r"\S+") + space + Regex(r"\$\S+") + space
+    assignment = space + key + Optional(space + value, default=None) + semicolon
+    location_statement = space + Optional(modifier) + Optional(space + location)
+    if_statement = space + Literal("if") + space + Regex(r"\(.+\)") + space
+    map_statement = space + Literal("map") + space + Regex(r"\S+") + space + Regex(r"\$\S+") + space
     block = Forward()
 
     block << Group(
+        # XXX could this "key" be Literal("location")?
         (Group(key + location_statement) ^ Group(if_statement) ^ Group(map_statement)) +
         left_bracket +
         Group(ZeroOrMore(Group(comment | assignment) | block)) +
         right_bracket)
 
     script = OneOrMore(Group(comment | assignment) ^ block) + stringEnd
+    script.parseWithTabs()
 
     def __init__(self, source):
         self.source = source
@@ -56,51 +57,6 @@ class RawNginxParser(object):
         """Returns the parsed tree as a list."""
         return self.parse().asList()
 
-class OldRawNginxParser(object):
-    # pylint: disable=expression-not-assigned
-    """A class that parses nginx configuration with pyparsing."""
-
-    # constants
-    left_bracket = Literal("{").suppress()
-    right_bracket = Literal("}").suppress()
-    semicolon = Literal(";").suppress()
-    space = White().suppress()
-    key = Word(alphanums + "_/+-.")
-    # Matches anything that is not a special character AND any chars in single
-    # or double quotes
-    value = Regex(r"((\".*\")?(\'.*\')?[^\{\};,]?)+")
-    location = CharsNotIn("{};," + string.whitespace)
-    # modifier for location uri [ = | ~ | ~* | ^~ ]
-    modifier = Literal("=") | Literal("~*") | Literal("~") | Literal("^~")
-
-    # rules
-    comment = Literal("#") + restOfLine()
-    assignment = (key + Optional(space + value, default=None) + semicolon)
-    location_statement = Optional(space + modifier) + Optional(space + location)
-    if_statement = Literal("if") + space + Regex(r"\(.+\)") + space
-    map_statement = Literal("map") + space + Regex(r"\S+") + space + Regex(r"\$\S+") + space
-    block = Forward()
-
-    block << Group(
-        (Group(key + location_statement) ^ Group(if_statement) ^ Group(map_statement)) +
-        left_bracket +
-        Group(ZeroOrMore(Group(comment | assignment) | block)) +
-        right_bracket)
-
-    script = OneOrMore(Group(comment | assignment) ^ block) + stringEnd
-
-    def __init__(self, source):
-        self.source = source
-
-    def parse(self):
-        """Returns the parsed tree."""
-        return self.script.parseString(self.source)
-
-    def as_list(self):
-        """Returns the parsed tree as a list."""
-        return self.parse().asList()
-
-
 class RawNginxDumper(object):
     # pylint: disable=too-few-public-methods
     """A class that dumps nginx configuration from the provided tree."""
@@ -108,35 +64,41 @@ class RawNginxDumper(object):
         self.blocks = blocks
         self.indentation = indentation
 
-    def __iter__(self, blocks=None, spacer=' '):
+    def __iter__(self, blocks=None):
         """Iterates the dumped nginx content."""
         blocks = blocks or self.blocks
         print "iterating", blocks
         for b in blocks:
+            b0 = b
+            b = copy.deepcopy(b)
             indentation = ""
             if spacey(b[0]):
                 indentation = b.pop(0)
                 indentation = indentation.replace("\n", "", 1)
             key = b.pop(0)
+            values = b.pop(0)
 
-            values = b
             if isinstance(key, list):
-                yield indentation + spacer.join(key) + ' {'
+                yield indentation + "".join(key) + '{'
                 for parameter in values:
                     dumped = self.__iter__([parameter])
                     for line in dumped:
                         yield line
-
                 yield '}'
             else:
-                #yield b
                 if isinstance(key, str) and key.strip() == '#':
                     yield indentation + key + values
                 else:
+                    gap = ""
+                    # Sometimes the parser has stuck some gap whitespace in here;
+                    # if so rotate it into gap
+                    if spacey(values):
+                        gap = values
+                        values = b.pop(0)
                     if values is None:
-                        yield indentation + key + ';'
+                        yield indentation + key + gap + ';'
                     else:
-                        yield indentation + key + spacer + values + ';'
+                        yield indentation + key + gap + values + ';'
 
     def __str__(self):
         """Return the parsed block as a string."""
@@ -146,42 +108,6 @@ class RawNginxDumper(object):
 
 
 
-class OldRawNginxDumper(object):
-    # pylint: disable=too-few-public-methods
-    """A class that dumps nginx configuration from the provided tree."""
-    def __init__(self, blocks, indentation=4):
-        self.blocks = blocks
-        self.indentation = indentation
-
-    def __iter__(self, blocks=None, current_indent=0, spacer=' '):
-        """Iterates the dumped nginx content."""
-        blocks = blocks or self.blocks
-        for key, values in blocks:
-            indentation = spacer * current_indent
-            if isinstance(key, list):
-                if current_indent:
-                    yield ''
-                yield indentation + spacer.join(key) + ' {'
-
-                for parameter in values:
-                    dumped = self.__iter__([parameter], current_indent + self.indentation)
-                    for line in dumped:
-                        yield line
-
-                yield indentation + '}'
-            else:
-                if key == '#':
-                    yield spacer * current_indent + key + values
-                else:
-                    if values is None:
-                        yield spacer * current_indent + key + ';'
-                    else:
-                        yield spacer * current_indent + key + spacer + values + ';'
-
-    def __str__(self):
-        """Return the parsed block as a string."""
-        return '\n'.join(self) + '\n'
-
 
 # Shortcut functions to respect Python's serialization interface
 # (like pyyaml, picker or json)
@@ -194,10 +120,6 @@ def loads(source):
     :rtype: list
 
     """
-    old = OldRawNginxParser(source).as_list()
-    print "Old:"
-    for entry in old:
-        print len(entry), " ",
     new = UnspacedList(RawNginxParser(source).as_list())
     print "\nNew:"
     print new
@@ -207,14 +129,6 @@ def loads(source):
     for entry in new.spaced:
         print str(len(entry))+ " ",
     print "\ngo"
-    if old != new:
-        print "NON-MATCH"
-        for a, b in zip(old, new):
-            if a != b:
-                print "Entry", a, "!=", b
-        import sys
-    else:
-        print "Parallel"
     return new
 
 
@@ -229,7 +143,7 @@ def load(_file):
     return loads(_file.read())
 
 
-def dumps(blocks, indentation=4):
+def dumps(blocks):
     """Dump to a string.
 
     :param UnspacedList block: The parsed tree
@@ -237,10 +151,10 @@ def dumps(blocks, indentation=4):
     :rtype: str
 
     """
-    return str(RawNginxDumper(blocks, indentation))
+    return str(RawNginxDumper(blocks))
 
 
-def dump(blocks, _file, indentation=4):
+def dump(blocks, _file):
     """Dump to a file.
 
     :param UnspacedList block: The parsed tree
@@ -249,7 +163,7 @@ def dump(blocks, _file, indentation=4):
     :rtype: NoneType
 
     """
-    return _file.write(dumps(blocks, indentation))
+    return _file.write(dumps(blocks))
 
 
 spacey = lambda x: isinstance(x, str) and x.isspace()
diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py
index 2872654b5..642702c9f 100644
--- a/certbot-nginx/certbot_nginx/parser.py
+++ b/certbot-nginx/certbot_nginx/parser.py
@@ -4,6 +4,7 @@ import logging
 import os
 import pyparsing
 import re
+import sys
 
 from certbot import errors
 
@@ -213,9 +214,26 @@ class NginxParser(object):
             if ext:
                 filename = filename + os.path.extsep + ext
             try:
-                logger.debug('Dumping to %s:\n%s', filename, nginxparser.dumps(tree))
+                out = nginxparser.dumps(tree)
+                logger.debug('Dumping to %s:\n%s', filename, out)
                 with open(filename, 'w') as _file:
-                    nginxparser.dump(tree, _file)
+                    _file.write(out)
+
+                if "owncloud" in filename:
+                    print "Outputting", filename
+                    print out
+                    a = open("/tmp/nginx/sites-enabled/owncloud.conf").read()
+                    b = open(filename).read()
+                    for linea, lineb in zip(a.split('\n'), b.split('\n')):
+                        if linea != lineb:
+                            print "a", repr(linea)
+                            print "b", repr(lineb)
+                    if a != b:
+                        print "Mismatch!"
+                        if a != out:
+                            print "Double mismatch", len(a), len(out)
+                    else:
+                        print "Match!"
             except IOError:
                 logger.error("Could not open file for writing: %s", filename)
 
diff --git a/certbot-nginx/certbot_nginx/tls_sni_01.py b/certbot-nginx/certbot_nginx/tls_sni_01.py
index e4c5d31a6..efb5d53e6 100644
--- a/certbot-nginx/certbot_nginx/tls_sni_01.py
+++ b/certbot-nginx/certbot_nginx/tls_sni_01.py
@@ -3,6 +3,7 @@
 import itertools
 import logging
 import os
+import sys
 
 from certbot import errors
 from certbot.plugins import common
@@ -123,7 +124,13 @@ class NginxTlsSni01(common.TLSSNI01):
             True, self.challenge_conf)
 
         with open(self.challenge_conf, "w") as new_conf:
-            nginxparser.dump(config, new_conf)
+            if "mime" in self.challenge_conf:
+                print "Weird"
+                out = nginxparser.dumps(config)
+                print out
+                #sys.exit(1)
+            #nginxparser.dump(config, new_conf)
+            new_conf.write(out)
 
     def _make_server_block(self, achall, addrs):
         """Creates a server block for a challenge.

From 98a2e0c6d6a519b251f23735d83a40244eab3349 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Wed, 15 Jun 2016 16:02:33 -0700
Subject: [PATCH 07/48] Another waypoint

---
 certbot-nginx/certbot_nginx/nginxparser.py | 20 ++++++++++++--------
 certbot-nginx/certbot_nginx/tls_sni_01.py  |  2 +-
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py
index 13498bee2..b30a71bea 100644
--- a/certbot-nginx/certbot_nginx/nginxparser.py
+++ b/certbot-nginx/certbot_nginx/nginxparser.py
@@ -18,8 +18,10 @@ class RawNginxParser(object):
     left_bracket = Literal("{").suppress()
     right_bracket = Literal("}").suppress()
     semicolon = Literal(";").suppress()
-    space = Optional(White())
     key = Word(alphanums + "_/+-.")
+    space = Optional(White())
+    nspace = Optional(Regex(r"[ \t]+"))
+    blankLine = ZeroOrMore(Regex(r"^\S$"))
     # Matches anything that is not a special character AND any chars in single
     # or double quotes
     value = Regex(r"((\".*\")?(\'.*\')?[^\{\};,]?)+")
@@ -31,23 +33,25 @@ class RawNginxParser(object):
     comment = space + Literal('#') + restOfLine()
 
     assignment = space + key + Optional(space + value, default=None) + semicolon
-    location_statement = space + Optional(modifier) + Optional(space + location)
+    location_statement = space + Optional(modifier) + Optional(space + location + space)
     if_statement = space + Literal("if") + space + Regex(r"\(.+\)") + space
     map_statement = space + Literal("map") + space + Regex(r"\S+") + space + Regex(r"\$\S+") + space
     block = Forward()
 
     block << Group(
         # XXX could this "key" be Literal("location")?
-        (Group(key + location_statement) ^ Group(if_statement) ^ Group(map_statement)) +
+        # WIP: in order to allow this leaveWhitespace(), we're going to need an explicit
+        # "whitespaceline" construction...
+        (Group(space + key + location_statement) ^ Group(if_statement) ^
+        Group(map_statement)).leaveWhitespace() +
         left_bracket +
         Group(ZeroOrMore(Group(comment | assignment) | block)) +
-        right_bracket)
+        space + right_bracket)
 
     script = OneOrMore(Group(comment | assignment) ^ block) + stringEnd
     script.parseWithTabs()
 
-    def __init__(self, source):
-        self.source = source
+    def __init__(self, source): self.source = source
 
     def parse(self):
         """Returns the parsed tree."""
@@ -74,7 +78,7 @@ class RawNginxDumper(object):
             indentation = ""
             if spacey(b[0]):
                 indentation = b.pop(0)
-                indentation = indentation.replace("\n", "", 1)
+                #indentation = indentation.replace("\n", "", 1)
             key = b.pop(0)
             values = b.pop(0)
 
@@ -103,7 +107,7 @@ class RawNginxDumper(object):
     def __str__(self):
         """Return the parsed block as a string."""
         print "Merging:\n", '\n'.join(self)
-        return '\n'.join(self) + '\n'
+        return ''.join(self) + '\n'
 
 
 
diff --git a/certbot-nginx/certbot_nginx/tls_sni_01.py b/certbot-nginx/certbot_nginx/tls_sni_01.py
index efb5d53e6..d24cb830d 100644
--- a/certbot-nginx/certbot_nginx/tls_sni_01.py
+++ b/certbot-nginx/certbot_nginx/tls_sni_01.py
@@ -124,9 +124,9 @@ class NginxTlsSni01(common.TLSSNI01):
             True, self.challenge_conf)
 
         with open(self.challenge_conf, "w") as new_conf:
+            out = nginxparser.dumps(config)
             if "mime" in self.challenge_conf:
                 print "Weird"
-                out = nginxparser.dumps(config)
                 print out
                 #sys.exit(1)
             #nginxparser.dump(config, new_conf)

From 5e59b8ad46872657f9a6fd0a12ae1f1dd8eceaf7 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Wed, 15 Jun 2016 16:42:47 -0700
Subject: [PATCH 08/48] Woohoo! it works

---
 certbot-nginx/certbot_nginx/nginxparser.py | 28 +++++++++++++++-------
 certbot-nginx/certbot_nginx/parser.py      |  2 +-
 2 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py
index b30a71bea..c8893f80a 100644
--- a/certbot-nginx/certbot_nginx/nginxparser.py
+++ b/certbot-nginx/certbot_nginx/nginxparser.py
@@ -15,12 +15,12 @@ class RawNginxParser(object):
     """A class that parses nginx configuration with pyparsing."""
 
     # constants
-    left_bracket = Literal("{").suppress()
-    right_bracket = Literal("}").suppress()
-    semicolon = Literal(";").suppress()
-    key = Word(alphanums + "_/+-.")
     space = Optional(White())
     nspace = Optional(Regex(r"[ \t]+"))
+    left_bracket = Literal("{").suppress()
+    right_bracket = space.leaveWhitespace() + Literal("}").suppress()
+    semicolon = Literal(";").suppress()
+    key = Word(alphanums + "_/+-.")
     blankLine = ZeroOrMore(Regex(r"^\S$"))
     # Matches anything that is not a special character AND any chars in single
     # or double quotes
@@ -45,8 +45,8 @@ class RawNginxParser(object):
         (Group(space + key + location_statement) ^ Group(if_statement) ^
         Group(map_statement)).leaveWhitespace() +
         left_bracket +
-        Group(ZeroOrMore(Group(comment | assignment) | block)) +
-        space + right_bracket)
+        Group(ZeroOrMore(Group(comment | assignment) | block) + space).leaveWhitespace() +
+        right_bracket)
 
     script = OneOrMore(Group(comment | assignment) ^ block) + stringEnd
     script.parseWithTabs()
@@ -76,11 +76,22 @@ class RawNginxDumper(object):
             b0 = b
             b = copy.deepcopy(b)
             indentation = ""
+            if isinstance(b, str):
+                yield b
+                continue
             if spacey(b[0]):
-                indentation = b.pop(0)
+                try:
+                    indentation = b.pop(0)
+                except:
+                    import ipdb
+                    ipdb.set_trace()
+
                 #indentation = indentation.replace("\n", "", 1)
             key = b.pop(0)
             values = b.pop(0)
+            if "worker_processes" in str(b0) and False:
+                import ipdb
+                ipdb.set_trace()
 
             if isinstance(key, list):
                 yield indentation + "".join(key) + '{'
@@ -106,7 +117,8 @@ class RawNginxDumper(object):
 
     def __str__(self):
         """Return the parsed block as a string."""
-        print "Merging:\n", '\n'.join(self)
+        x = ''.join(self)
+        print "Merging:\n", repr(x)
         return ''.join(self) + '\n'
 
 
diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py
index 642702c9f..61f728d2f 100644
--- a/certbot-nginx/certbot_nginx/parser.py
+++ b/certbot-nginx/certbot_nginx/parser.py
@@ -219,7 +219,7 @@ class NginxParser(object):
                 with open(filename, 'w') as _file:
                     _file.write(out)
 
-                if "owncloud" in filename:
+                if True or "owncloud" in filename:
                     print "Outputting", filename
                     print out
                     a = open("/tmp/nginx/sites-enabled/owncloud.conf").read()

From 4f46289c1b9b582f6ad90db0a5651e39d1b2f929 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Wed, 15 Jun 2016 17:26:38 -0700
Subject: [PATCH 09/48] Start cleanup

---
 certbot-nginx/certbot_nginx/nginxparser.py | 44 +++-------------------
 certbot-nginx/certbot_nginx/parser.py      | 17 +--------
 certbot-nginx/certbot_nginx/tls_sni_01.py  |  5 ---
 3 files changed, 7 insertions(+), 59 deletions(-)

diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py
index c8893f80a..d1d17493a 100644
--- a/certbot-nginx/certbot_nginx/nginxparser.py
+++ b/certbot-nginx/certbot_nginx/nginxparser.py
@@ -16,12 +16,10 @@ class RawNginxParser(object):
 
     # constants
     space = Optional(White())
-    nspace = Optional(Regex(r"[ \t]+"))
     left_bracket = Literal("{").suppress()
     right_bracket = space.leaveWhitespace() + Literal("}").suppress()
     semicolon = Literal(";").suppress()
     key = Word(alphanums + "_/+-.")
-    blankLine = ZeroOrMore(Regex(r"^\S$"))
     # Matches anything that is not a special character AND any chars in single
     # or double quotes
     value = Regex(r"((\".*\")?(\'.*\')?[^\{\};,]?)+")
@@ -40,8 +38,6 @@ class RawNginxParser(object):
 
     block << Group(
         # XXX could this "key" be Literal("location")?
-        # WIP: in order to allow this leaveWhitespace(), we're going to need an explicit
-        # "whitespaceline" construction...
         (Group(space + key + location_statement) ^ Group(if_statement) ^
         Group(map_statement)).leaveWhitespace() +
         left_bracket +
@@ -51,7 +47,8 @@ class RawNginxParser(object):
     script = OneOrMore(Group(comment | assignment) ^ block) + stringEnd
     script.parseWithTabs()
 
-    def __init__(self, source): self.source = source
+    def __init__(self, source):
+        self.source = source
 
     def parse(self):
         """Returns the parsed tree."""
@@ -71,27 +68,16 @@ class RawNginxDumper(object):
     def __iter__(self, blocks=None):
         """Iterates the dumped nginx content."""
         blocks = blocks or self.blocks
-        print "iterating", blocks
         for b in blocks:
-            b0 = b
-            b = copy.deepcopy(b)
-            indentation = ""
             if isinstance(b, str):
                 yield b
                 continue
+            b = copy.deepcopy(b)
+            indentation = ""
             if spacey(b[0]):
-                try:
-                    indentation = b.pop(0)
-                except:
-                    import ipdb
-                    ipdb.set_trace()
-
-                #indentation = indentation.replace("\n", "", 1)
+                indentation = b.pop(0)
             key = b.pop(0)
             values = b.pop(0)
-            if "worker_processes" in str(b0) and False:
-                import ipdb
-                ipdb.set_trace()
 
             if isinstance(key, list):
                 yield indentation + "".join(key) + '{'
@@ -117,14 +103,9 @@ class RawNginxDumper(object):
 
     def __str__(self):
         """Return the parsed block as a string."""
-        x = ''.join(self)
-        print "Merging:\n", repr(x)
         return ''.join(self) + '\n'
 
 
-
-
-
 # Shortcut functions to respect Python's serialization interface
 # (like pyyaml, picker or json)
 
@@ -136,16 +117,7 @@ def loads(source):
     :rtype: list
 
     """
-    new = UnspacedList(RawNginxParser(source).as_list())
-    print "\nNew:"
-    print new
-    for entry in new:
-        print len(entry), " ",
-    print "\nNewspaced:"
-    for entry in new.spaced:
-        print str(len(entry))+ " ",
-    print "\ngo"
-    return new
+    return UnspacedList(RawNginxParser(source).as_list())
 
 
 def load(_file):
@@ -238,7 +210,3 @@ class UnspacedList(list):
                 idx -= 1
             pos += 1
         return spaces
-
-    def with_spaces(self):
-        return self.spaced
-
diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py
index 61f728d2f..18301de0e 100644
--- a/certbot-nginx/certbot_nginx/parser.py
+++ b/certbot-nginx/certbot_nginx/parser.py
@@ -215,25 +215,10 @@ class NginxParser(object):
                 filename = filename + os.path.extsep + ext
             try:
                 out = nginxparser.dumps(tree)
-                logger.debug('Dumping 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)
 
-                if True or "owncloud" in filename:
-                    print "Outputting", filename
-                    print out
-                    a = open("/tmp/nginx/sites-enabled/owncloud.conf").read()
-                    b = open(filename).read()
-                    for linea, lineb in zip(a.split('\n'), b.split('\n')):
-                        if linea != lineb:
-                            print "a", repr(linea)
-                            print "b", repr(lineb)
-                    if a != b:
-                        print "Mismatch!"
-                        if a != out:
-                            print "Double mismatch", len(a), len(out)
-                    else:
-                        print "Match!"
             except IOError:
                 logger.error("Could not open file for writing: %s", filename)
 
diff --git a/certbot-nginx/certbot_nginx/tls_sni_01.py b/certbot-nginx/certbot_nginx/tls_sni_01.py
index d24cb830d..13ae2c358 100644
--- a/certbot-nginx/certbot_nginx/tls_sni_01.py
+++ b/certbot-nginx/certbot_nginx/tls_sni_01.py
@@ -125,11 +125,6 @@ class NginxTlsSni01(common.TLSSNI01):
 
         with open(self.challenge_conf, "w") as new_conf:
             out = nginxparser.dumps(config)
-            if "mime" in self.challenge_conf:
-                print "Weird"
-                print out
-                #sys.exit(1)
-            #nginxparser.dump(config, new_conf)
             new_conf.write(out)
 
     def _make_server_block(self, achall, addrs):

From 2cbd680bd543c91f846c98d3bd9972c9d9105b4c Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Wed, 15 Jun 2016 17:36:53 -0700
Subject: [PATCH 10/48] Hide .spaced from users outside nginxparser.py

---
 certbot-nginx/certbot_nginx/nginxparser.py | 2 +-
 certbot-nginx/certbot_nginx/parser.py      | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py
index d1d17493a..272e39bbc 100644
--- a/certbot-nginx/certbot_nginx/nginxparser.py
+++ b/certbot-nginx/certbot_nginx/nginxparser.py
@@ -139,7 +139,7 @@ def dumps(blocks):
     :rtype: str
 
     """
-    return str(RawNginxDumper(blocks))
+    return str(RawNginxDumper(blocks.spaced))
 
 
 def dump(blocks, _file):
diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py
index 18301de0e..f2faadd58 100644
--- a/certbot-nginx/certbot_nginx/parser.py
+++ b/certbot-nginx/certbot_nginx/parser.py
@@ -210,7 +210,7 @@ class NginxParser(object):
 
         """
         for filename in self.parsed:
-            tree = self.parsed[filename].spaced
+            tree = self.parsed[filename]
             if ext:
                 filename = filename + os.path.extsep + ext
             try:

From ff7addefb399ff5ef451a2590755c3976942f101 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Wed, 15 Jun 2016 17:57:24 -0700
Subject: [PATCH 11/48] Start fixing tests

---
 certbot-nginx/certbot_nginx/nginxparser.py           |  4 ++--
 .../certbot_nginx/tests/nginxparser_test.py          | 12 ++++++------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py
index 272e39bbc..b2f785d0e 100644
--- a/certbot-nginx/certbot_nginx/nginxparser.py
+++ b/certbot-nginx/certbot_nginx/nginxparser.py
@@ -22,7 +22,7 @@ class RawNginxParser(object):
     key = Word(alphanums + "_/+-.")
     # Matches anything that is not a special character AND any chars in single
     # or double quotes
-    value = Regex(r"((\".*\")?(\'.*\')?[^\{\};,]?)+")
+    value = Regex(r"((\".*\")?(\'.*\')?[^\{\};, ]?)+")
     location = CharsNotIn("{};," + string.whitespace)
     # modifier for location uri [ = | ~ | ~* | ^~ ]
     modifier = Literal("=") | Literal("~*") | Literal("~") | Literal("^~")
@@ -30,7 +30,7 @@ class RawNginxParser(object):
     # rules
     comment = space + Literal('#') + restOfLine()
 
-    assignment = space + key + Optional(space + value, default=None) + semicolon
+    assignment = space + key + Optional(space + value, default=None) + space + semicolon
     location_statement = space + Optional(modifier) + Optional(space + location + space)
     if_statement = space + Literal("if") + space + Regex(r"\(.+\)") + space
     map_statement = space + Literal("map") + space + Regex(r"\S+") + space + Regex(r"\$\S+") + space
diff --git a/certbot-nginx/certbot_nginx/tests/nginxparser_test.py b/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
index 80e82c903..0a09b4a64 100644
--- a/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
+++ b/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
@@ -17,23 +17,23 @@ class TestRawNginxParser(unittest.TestCase):
 
     def test_assignments(self):
         parsed = RawNginxParser.assignment.parseString('root /test;').asList()
-        self.assertEqual(parsed, ['root', '/test'])
+        self.assertEqual(parsed, ['root', ' ', '/test'])
         parsed = RawNginxParser.assignment.parseString('root /test;'
                                                        'foo bar;').asList()
         self.assertEqual(parsed, ['root', '/test'], ['foo', 'bar'])
 
     def test_blocks(self):
         parsed = RawNginxParser.block.parseString('foo {}').asList()
-        self.assertEqual(parsed, [[['foo'], []]])
+        self.assertEqual(parsed, [[['foo', ' '], []]])
         parsed = RawNginxParser.block.parseString('location /foo{}').asList()
-        self.assertEqual(parsed, [[['location', '/foo'], []]])
-        parsed = RawNginxParser.block.parseString('foo { bar foo; }').asList()
-        self.assertEqual(parsed, [[['foo'], [['bar', 'foo']]]])
+        self.assertEqual(parsed, [[['location', ' ', '/foo'], []]])
+        parsed = RawNginxParser.block.parseString('foo { bar foo ; }').asList()
+        self.assertEqual(parsed, [[['foo', ' '], [[' ', 'bar', ' ', 'foo', ' '], ' ']]])
 
     def test_nested_blocks(self):
         parsed = RawNginxParser.block.parseString('foo { bar {} }').asList()
         block, content = FIRST(parsed)
-        self.assertEqual(FIRST(content), [['bar'], []])
+        self.assertEqual(FIRST(content), [[' ', 'bar', ' '], []])
         self.assertEqual(FIRST(block), 'foo')
 
     def test_dump_as_string(self):

From e5ce03b312c48b43136628b3fd4a756dc04d754c Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Thu, 16 Jun 2016 12:24:36 -0700
Subject: [PATCH 12/48] More test wrangling

---
 certbot-nginx/certbot_nginx/nginxparser.py    |  4 ++--
 .../certbot_nginx/tests/nginxparser_test.py   | 24 ++++++++++++-------
 2 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py
index b2f785d0e..272e39bbc 100644
--- a/certbot-nginx/certbot_nginx/nginxparser.py
+++ b/certbot-nginx/certbot_nginx/nginxparser.py
@@ -22,7 +22,7 @@ class RawNginxParser(object):
     key = Word(alphanums + "_/+-.")
     # Matches anything that is not a special character AND any chars in single
     # or double quotes
-    value = Regex(r"((\".*\")?(\'.*\')?[^\{\};, ]?)+")
+    value = Regex(r"((\".*\")?(\'.*\')?[^\{\};,]?)+")
     location = CharsNotIn("{};," + string.whitespace)
     # modifier for location uri [ = | ~ | ~* | ^~ ]
     modifier = Literal("=") | Literal("~*") | Literal("~") | Literal("^~")
@@ -30,7 +30,7 @@ class RawNginxParser(object):
     # rules
     comment = space + Literal('#') + restOfLine()
 
-    assignment = space + key + Optional(space + value, default=None) + space + semicolon
+    assignment = space + key + Optional(space + value, default=None) + semicolon
     location_statement = space + Optional(modifier) + Optional(space + location + space)
     if_statement = space + Literal("if") + space + Regex(r"\(.+\)") + space
     map_statement = space + Literal("map") + space + Regex(r"\S+") + space + Regex(r"\$\S+") + space
diff --git a/certbot-nginx/certbot_nginx/tests/nginxparser_test.py b/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
index 0a09b4a64..2a8558bfb 100644
--- a/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
+++ b/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
@@ -5,7 +5,7 @@ import unittest
 from pyparsing import ParseException
 
 from certbot_nginx.nginxparser import (
-    RawNginxParser, loads, load, dumps, dump)
+    RawNginxParser, loads, load, dumps, dump, UnspacedList)
 from certbot_nginx.tests import util
 
 
@@ -18,9 +18,8 @@ class TestRawNginxParser(unittest.TestCase):
     def test_assignments(self):
         parsed = RawNginxParser.assignment.parseString('root /test;').asList()
         self.assertEqual(parsed, ['root', ' ', '/test'])
-        parsed = RawNginxParser.assignment.parseString('root /test;'
-                                                       'foo bar;').asList()
-        self.assertEqual(parsed, ['root', '/test'], ['foo', 'bar'])
+        parsed = RawNginxParser.assignment.parseString('root /test;foo bar;').asList()
+        self.assertEqual(parsed, ['root', ' ', '/test'], ['foo', ' ', 'bar'])
 
     def test_blocks(self):
         parsed = RawNginxParser.block.parseString('foo {}').asList()
@@ -28,7 +27,7 @@ class TestRawNginxParser(unittest.TestCase):
         parsed = RawNginxParser.block.parseString('location /foo{}').asList()
         self.assertEqual(parsed, [[['location', ' ', '/foo'], []]])
         parsed = RawNginxParser.block.parseString('foo { bar foo ; }').asList()
-        self.assertEqual(parsed, [[['foo', ' '], [[' ', 'bar', ' ', 'foo', ' '], ' ']]])
+        self.assertEqual(parsed, [[['foo', ' '], [[' ', 'bar', ' ', 'foo '], ' ']]])
 
     def test_nested_blocks(self):
         parsed = RawNginxParser.block.parseString('foo { bar {} }').asList()
@@ -116,7 +115,13 @@ class TestRawNginxParser(unittest.TestCase):
 
     def test_dump_as_file(self):
         with open(util.get_data_filename('nginx.conf')) as handle:
-            parsed = util.filter_comments(load(handle))
+            try:
+                parsed = load(handle)
+            except:
+                handle.seek(0)
+                print "Failed on", handle.read()
+                raise
+            #parsed = util.filter_comments(parsed)
         parsed[-1][-1].append([['server'],
                                [['listen', '443 ssl'],
                                 ['server_name', 'localhost'],
@@ -128,12 +133,15 @@ class TestRawNginxParser(unittest.TestCase):
                                 [['location', '/'],
                                  [['root', 'html'],
                                   ['index', 'index.html index.htm']]]]])
+        
 
         with open(util.get_data_filename('nginx.new.conf'), 'w') as handle:
             dump(parsed, handle)
         with open(util.get_data_filename('nginx.new.conf')) as handle:
-            parsed_new = util.filter_comments(load(handle))
-        self.assertEquals(parsed, parsed_new)
+            parsed_new = load(handle)
+        self.maxDiff = None
+        self.assertEquals(parsed[0], parsed_new[0])
+        self.assertEquals(parsed[1:], parsed_new[1:])
 
     def test_comments(self):
         with open(util.get_data_filename('minimalistic_comments.conf')) as handle:

From 72ba5b72cc2d11deb85385f9ca3813a4f0aecd2e Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Thu, 16 Jun 2016 12:25:11 -0700
Subject: [PATCH 13/48] Try to preserve the exact form of end-of-file
 whitespace

---
 certbot-nginx/certbot_nginx/nginxparser.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py
index 272e39bbc..c8131072c 100644
--- a/certbot-nginx/certbot_nginx/nginxparser.py
+++ b/certbot-nginx/certbot_nginx/nginxparser.py
@@ -44,7 +44,7 @@ class RawNginxParser(object):
         Group(ZeroOrMore(Group(comment | assignment) | block) + space).leaveWhitespace() +
         right_bracket)
 
-    script = OneOrMore(Group(comment | assignment) ^ block) + stringEnd
+    script = OneOrMore(Group(comment | assignment) ^ block) + space + stringEnd
     script.parseWithTabs()
 
     def __init__(self, source):
@@ -103,7 +103,7 @@ class RawNginxDumper(object):
 
     def __str__(self):
         """Return the parsed block as a string."""
-        return ''.join(self) + '\n'
+        return ''.join(self)
 
 
 # Shortcut functions to respect Python's serialization interface

From b82ebd9180db60333e87ea55318fa9c48490f200 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Thu, 16 Jun 2016 17:17:13 -0700
Subject: [PATCH 14/48] 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.

From e76e3a953a81769ebdc1e9890dab6254cc48da96 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Thu, 16 Jun 2016 17:58:05 -0700
Subject: [PATCH 15/48] Fix test cases

  - but we're still mangling files in place...
---
 certbot-nginx/certbot_nginx/nginxparser.py    | 37 ++++++------
 .../certbot_nginx/tests/nginxparser_test.py   | 57 +++++++++----------
 2 files changed, 43 insertions(+), 51 deletions(-)

diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py
index 856113f71..d7248ad49 100644
--- a/certbot-nginx/certbot_nginx/nginxparser.py
+++ b/certbot-nginx/certbot_nginx/nginxparser.py
@@ -1,7 +1,6 @@
 """Very low-level nginx config parser based on pyparsing."""
 import copy
 import string
-import sys
 
 from pyparsing import (
     Literal, White, Word, alphanums, CharsNotIn, Forward, Group,
@@ -68,11 +67,11 @@ class RawNginxDumper(object):
     def __iter__(self, blocks=None):
         """Iterates the dumped nginx content."""
         blocks = blocks or self.blocks
-        for b in blocks:
-            if isinstance(b, str):
-                yield b
+        for b0 in blocks:
+            if isinstance(b0, str):
+                yield b0
                 continue
-            b = copy.deepcopy(b)
+            b = copy.deepcopy(b0)
             indentation = ""
             if spacey(b[0]):
                 indentation = b.pop(0)
@@ -96,13 +95,18 @@ class RawNginxDumper(object):
                     gap = ""
                     # Sometimes the parser has stuck some gap whitespace in here;
                     # if so rotate it into gap
-                    if spacey(values):
+                    if values and spacey(values):
                         gap = values
-                        values = b.pop(0)
-                    if values is None:
-                        yield indentation + key + gap + ';'
-                    else:
-                        yield indentation + key + gap + values + ';'
+                        try:
+                            values = b.pop(0)
+                        except:
+                            import ipdb
+                            ipdb.set_trace()
+                    #if values is None:
+                    #    yield indentation + key + gap + ';'
+                    #else:
+                    #    yield indentation + key + gap + values + ';'
+                    yield indentation + key + gap + values + ';'
 
     def __str__(self):
         """Return the parsed block as a string."""
@@ -168,12 +172,11 @@ class UnspacedList(list):
         # 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
+        self.top = top if top else self
         for i, entry in reversed(list(enumerate(self))):
             if isinstance(entry, list):
                 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)
@@ -186,18 +189,11 @@ class UnspacedList(list):
         list.insert(self, i, x)
 
     def append(self, 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):
         if hasattr(x, "spaced"):
@@ -208,7 +204,6 @@ class UnspacedList(list):
         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)
diff --git a/certbot-nginx/certbot_nginx/tests/nginxparser_test.py b/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
index 2a8558bfb..2efacd940 100644
--- a/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
+++ b/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
@@ -36,19 +36,20 @@ class TestRawNginxParser(unittest.TestCase):
         self.assertEqual(FIRST(block), 'foo')
 
     def test_dump_as_string(self):
-        dumped = dumps([
-            ['user', 'www-data'],
-            [['server'], [
-                ['listen', '80'],
-                ['server_name', 'foo.com'],
-                ['root', '/home/ubuntu/sites/foo/'],
-                [['location', '/status'], [
-                    ['check_status', None],
-                    [['types'], [['image/jpeg', 'jpg']]],
+        dumped = dumps(UnspacedList([
+            ['user', ' ', 'www-data'],
+            [['\n', 'server', ' '], [
+                ['\n    ', 'listen', ' ', '80'],
+                ['\n    ', 'server_name', ' ', 'foo.com'],
+                ['\n    ', 'root', ' ', '/home/ubuntu/sites/foo/'],
+                [['\n\n    ', 'location', ' ', '/status', ' '], [
+                    ['\n        ', 'check_status', ''],
+                    [['\n\n        ', 'types', ' '],
+                    [['\n            ', 'image/jpeg', ' ', 'jpg']]],
                 ]]
-            ]]])
+            ]]]))
 
-        self.assertEqual(dumped,
+        self.assertEqual(dumped.split('\n'),
                          'user www-data;\n'
                          'server {\n'
                          '    listen 80;\n'
@@ -59,10 +60,7 @@ class TestRawNginxParser(unittest.TestCase):
                          '        check_status;\n'
                          '\n'
                          '        types {\n'
-                         '            image/jpeg jpg;\n'
-                         '        }\n'
-                         '    }\n'
-                         '}\n')
+                         '            image/jpeg jpg;}}}'.split('\n'))
 
     def test_parse_from_file(self):
         with open(util.get_data_filename('foo.conf')) as handle:
@@ -122,18 +120,17 @@ class TestRawNginxParser(unittest.TestCase):
                 print "Failed on", handle.read()
                 raise
             #parsed = util.filter_comments(parsed)
-        parsed[-1][-1].append([['server'],
-                               [['listen', '443 ssl'],
-                                ['server_name', 'localhost'],
-                                ['ssl_certificate', 'cert.pem'],
-                                ['ssl_certificate_key', 'cert.key'],
-                                ['ssl_session_cache', 'shared:SSL:1m'],
-                                ['ssl_session_timeout', '5m'],
-                                ['ssl_ciphers', 'HIGH:!aNULL:!MD5'],
-                                [['location', '/'],
-                                 [['root', 'html'],
-                                  ['index', 'index.html index.htm']]]]])
-        
+        parsed[-1][-1].append(UnspacedList([['server'],
+                               [['listen', ' ', '443 ssl'],
+                                ['server_name', ' ', 'localhost'],
+                                ['ssl_certificate', ' ', 'cert.pem'],
+                                ['ssl_certificate_key', ' ', 'cert.key'],
+                                ['ssl_session_cache', ' ', 'shared:SSL:1m'],
+                                ['ssl_session_timeout', ' ', '5m'],
+                                ['ssl_ciphers', ' ', 'HIGH:!aNULL:!MD5'],
+                                [['location', ' ', '/'],
+                                 [['root', ' ', 'html'],
+                                  ['index', ' ', 'index.html index.htm']]]]]))
 
         with open(util.get_data_filename('nginx.new.conf'), 'w') as handle:
             dump(parsed, handle)
@@ -159,11 +156,11 @@ class TestRawNginxParser(unittest.TestCase):
             ['#', " Use bar.conf when it's a full moon!"],
             ['include', 'foo.conf'],
             ['#', ' Kilroy was here'],
-            ['check_status', None],
+            ['check_status'],
             [['server'],
-             [['#', ''],
+             [['#'],
               ['#', " Don't forget to open up your firewall!"],
-              ['#', ''],
+              ['#'],
               ['listen', '1234'],
               ['#', ' listen 80;']]],
         ])

From da250a4f4bfb5e8feb6efda25745a03cbcd23041 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Thu, 16 Jun 2016 17:59:49 -0700
Subject: [PATCH 16/48] Experimentally delete these output files

---
 .../etc_nginx/minimalistic_comments.new.conf  | 11 ---
 .../tests/testdata/etc_nginx/nginx.new.conf   | 83 -------------------
 2 files changed, 94 deletions(-)
 delete mode 100644 certbot-nginx/certbot_nginx/tests/testdata/etc_nginx/minimalistic_comments.new.conf
 delete mode 100644 certbot-nginx/certbot_nginx/tests/testdata/etc_nginx/nginx.new.conf

diff --git a/certbot-nginx/certbot_nginx/tests/testdata/etc_nginx/minimalistic_comments.new.conf b/certbot-nginx/certbot_nginx/tests/testdata/etc_nginx/minimalistic_comments.new.conf
deleted file mode 100644
index d1b7be91e..000000000
--- a/certbot-nginx/certbot_nginx/tests/testdata/etc_nginx/minimalistic_comments.new.conf
+++ /dev/null
@@ -1,11 +0,0 @@
-# Use bar.conf when it's a full moon!
-include foo.conf;
-# Kilroy was here
-check_status;
-server {
-    #
-    # Don't forget to open up your firewall!
-    #
-    listen 1234;
-    # listen 80;
-}
diff --git a/certbot-nginx/certbot_nginx/tests/testdata/etc_nginx/nginx.new.conf b/certbot-nginx/certbot_nginx/tests/testdata/etc_nginx/nginx.new.conf
deleted file mode 100644
index 59c1c968f..000000000
--- a/certbot-nginx/certbot_nginx/tests/testdata/etc_nginx/nginx.new.conf
+++ /dev/null
@@ -1,83 +0,0 @@
-user nobody;
-worker_processes 1;
-error_log logs/error.log;
-error_log logs/error.log  notice;
-error_log logs/error.log  info;
-pid logs/nginx.pid;
-events {
-    worker_connections 1024;
-}
-include foo.conf;
-http {
-    include mime.types;
-    include sites-enabled/*;
-    default_type application/octet-stream;
-    log_format main  '$remote_addr - $remote_user [$time_local] "$request" '
-                      '$status $body_bytes_sent "$http_referer" '
-                      '"$http_user_agent" "$http_x_forwarded_for"';
-    access_log logs/access.log  main;
-    sendfile on;
-    tcp_nopush on;
-    keepalive_timeout 0;
-    gzip on;
-
-    server {
-        listen 8080;
-        server_name localhost;
-        server_name ~^(www\.)?(example|bar)\.;
-        charset koi8-r;
-        access_log logs/host.access.log  main;
-
-        location / {
-            root html;
-            index index.html index.htm;
-        }
-        error_page 404              /404.html;
-        error_page 500 502 503 504  /50x.html;
-
-        location = /50x.html {
-            root html;
-        }
-
-        location ~ \.php$ {
-            proxy_pass http://127.0.0.1;
-        }
-
-        location ~ \.php$ {
-            root html;
-            fastcgi_pass 127.0.0.1:9000;
-            fastcgi_index index.php;
-            fastcgi_param SCRIPT_FILENAME  /scripts$fastcgi_script_name;
-        }
-
-        location ~ /\.ht {
-            deny all;
-        }
-    }
-
-    server {
-        listen 8000;
-        listen somename:8080;
-        include server.conf;
-
-        location / {
-            root html;
-            index index.html index.htm;
-        }
-    }
-
-    server {
-        listen 443 ssl;
-        server_name localhost;
-        ssl_certificate cert.pem;
-        ssl_certificate_key cert.key;
-        ssl_session_cache shared:SSL:1m;
-        ssl_session_timeout 5m;
-        ssl_ciphers HIGH:!aNULL:!MD5;
-
-        location / {
-            root html;
-            index index.html index.htm;
-        }
-    }
-}

From 9459daa6d03940f6c8fdfe169807a32df2a36b14 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Thu, 16 Jun 2016 18:04:13 -0700
Subject: [PATCH 17/48] Delete new.conf files after tests

Not least, to prevent git conflicts with old branches
---
 .../certbot_nginx/tests/nginxparser_test.py   | 38 +++++++++++--------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/certbot-nginx/certbot_nginx/tests/nginxparser_test.py b/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
index 2efacd940..87c7a0430 100644
--- a/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
+++ b/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
@@ -136,9 +136,12 @@ class TestRawNginxParser(unittest.TestCase):
             dump(parsed, handle)
         with open(util.get_data_filename('nginx.new.conf')) as handle:
             parsed_new = load(handle)
-        self.maxDiff = None
-        self.assertEquals(parsed[0], parsed_new[0])
-        self.assertEquals(parsed[1:], parsed_new[1:])
+        try:
+            self.maxDiff = None
+            self.assertEquals(parsed[0], parsed_new[0])
+            self.assertEquals(parsed[1:], parsed_new[1:])
+        finally:
+            os.unlink(util.get_data_filename('nginx.new.conf'))
 
     def test_comments(self):
         with open(util.get_data_filename('minimalistic_comments.conf')) as handle:
@@ -150,20 +153,23 @@ class TestRawNginxParser(unittest.TestCase):
         with open(util.get_data_filename('minimalistic_comments.new.conf')) as handle:
             parsed_new = load(handle)
 
-        self.assertEquals(parsed, parsed_new)
+        try:
+            self.assertEquals(parsed, parsed_new)
 
-        self.assertEqual(parsed_new, [
-            ['#', " Use bar.conf when it's a full moon!"],
-            ['include', 'foo.conf'],
-            ['#', ' Kilroy was here'],
-            ['check_status'],
-            [['server'],
-             [['#'],
-              ['#', " Don't forget to open up your firewall!"],
-              ['#'],
-              ['listen', '1234'],
-              ['#', ' listen 80;']]],
-        ])
+            self.assertEqual(parsed_new, [
+                ['#', " Use bar.conf when it's a full moon!"],
+                ['include', 'foo.conf'],
+                ['#', ' Kilroy was here'],
+                ['check_status'],
+                [['server'],
+                 [['#'],
+                  ['#', " Don't forget to open up your firewall!"],
+                  ['#'],
+                  ['listen', '1234'],
+                  ['#', ' listen 80;']]],
+            ])
+        finally:
+            os.unlink(util.get_data_filename('minimalistic_comments.new.conf'))
 
     def test_issue_518(self):
         parsed = loads('if ($http_accept ~* "webp") { set $webp "true"; }')

From efd1ff46c696294341f9f8537e25023c8d01f196 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Thu, 16 Jun 2016 18:18:33 -0700
Subject: [PATCH 18/48] Lint

---
 certbot-nginx/certbot_nginx/nginxparser.py            | 10 ++++------
 certbot-nginx/certbot_nginx/parser.py                 |  1 -
 certbot-nginx/certbot_nginx/tests/nginxparser_test.py |  1 +
 certbot-nginx/certbot_nginx/tls_sni_01.py             |  4 +---
 4 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py
index d7248ad49..1664f94cf 100644
--- a/certbot-nginx/certbot_nginx/nginxparser.py
+++ b/certbot-nginx/certbot_nginx/nginxparser.py
@@ -1,5 +1,6 @@
 """Very low-level nginx config parser based on pyparsing."""
 import copy
+import logging
 import string
 
 from pyparsing import (
@@ -8,6 +9,7 @@ from pyparsing import (
 from pyparsing import stringEnd
 from pyparsing import restOfLine
 
+logger = logging.getLogger(__name__)
 
 class RawNginxParser(object):
     # pylint: disable=expression-not-assigned
@@ -97,11 +99,7 @@ class RawNginxDumper(object):
                     # if so rotate it into gap
                     if values and spacey(values):
                         gap = values
-                        try:
-                            values = b.pop(0)
-                        except:
-                            import ipdb
-                            ipdb.set_trace()
+                        values = b.pop(0)
                     #if values is None:
                     #    yield indentation + key + gap + ';'
                     #else:
@@ -200,7 +198,7 @@ class UnspacedList(list):
             self.spaced.extend(x.spaced)
         else:
             self.spaced.extend(x)
-            self.logger.debug("Weird, extending regular list %r to Unspaced %r", x, self)
+            logger.debug("Weird, extending regular list %r to Unspaced %r", x, self)
         list.extend(self, x)
 
     def __add__(self, other):
diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py
index 61c719816..21127bc08 100644
--- a/certbot-nginx/certbot_nginx/parser.py
+++ b/certbot-nginx/certbot_nginx/parser.py
@@ -4,7 +4,6 @@ import logging
 import os
 import pyparsing
 import re
-import sys
 
 from certbot import errors
 
diff --git a/certbot-nginx/certbot_nginx/tests/nginxparser_test.py b/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
index 87c7a0430..7354c118c 100644
--- a/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
+++ b/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
@@ -1,5 +1,6 @@
 """Test for certbot_nginx.nginxparser."""
 import operator
+import os
 import unittest
 
 from pyparsing import ParseException
diff --git a/certbot-nginx/certbot_nginx/tls_sni_01.py b/certbot-nginx/certbot_nginx/tls_sni_01.py
index 13ae2c358..e4c5d31a6 100644
--- a/certbot-nginx/certbot_nginx/tls_sni_01.py
+++ b/certbot-nginx/certbot_nginx/tls_sni_01.py
@@ -3,7 +3,6 @@
 import itertools
 import logging
 import os
-import sys
 
 from certbot import errors
 from certbot.plugins import common
@@ -124,8 +123,7 @@ class NginxTlsSni01(common.TLSSNI01):
             True, self.challenge_conf)
 
         with open(self.challenge_conf, "w") as new_conf:
-            out = nginxparser.dumps(config)
-            new_conf.write(out)
+            nginxparser.dump(config, new_conf)
 
     def _make_server_block(self, achall, addrs):
         """Creates a server block for a challenge.

From ba0a0e9c2664ffade3bc916cf5fc10d8c5d57a53 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Fri, 17 Jun 2016 14:32:24 -0700
Subject: [PATCH 19/48] Tests for UnspacedList

---
 certbot-nginx/certbot_nginx/nginxparser.py    | 20 +++----
 .../certbot_nginx/tests/nginxparser_test.py   | 54 +++++++++++++++++++
 2 files changed, 65 insertions(+), 9 deletions(-)

diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py
index 1664f94cf..caf6fe725 100644
--- a/certbot-nginx/certbot_nginx/nginxparser.py
+++ b/certbot-nginx/certbot_nginx/nginxparser.py
@@ -164,16 +164,15 @@ 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, top=False):
+    def __init__(self, list_source):
         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 = top if top else self
         for i, entry in reversed(list(enumerate(self))):
             if isinstance(entry, list):
-                sublist = UnspacedList(entry, top=self.top)
+                sublist = UnspacedList(entry)
                 list.__setitem__(self, i, sublist)
                 self.spaced[i] = sublist.spaced
             elif spacey(entry):
@@ -202,12 +201,9 @@ class UnspacedList(list):
         list.extend(self, x)
 
     def __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)
-        else:
-            self.spaced.__add__(other)
-        list.__add__(self, other)
+        l = copy.deepcopy(self)
+        l.extend(other)
+        return l
 
     def __setitem__(self, i, value):
         if hasattr(value, "spaced"):
@@ -220,6 +216,12 @@ class UnspacedList(list):
         self.spaced.__delitem__(i + self._spaces_before(i))
         list.__delitem__(self, i)
 
+    def __deepcopy__(self, memo):
+        l = UnspacedList(self[:])
+        l.spaced = copy.deepcopy(self.spaced)
+        return l
+
+
     def _spaces_before(self, idx):
         "Count the number of spaces in the spaced list before pos idx in the spaceless one"
         spaces = 0
diff --git a/certbot-nginx/certbot_nginx/tests/nginxparser_test.py b/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
index 7354c118c..e78adfac1 100644
--- a/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
+++ b/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
@@ -1,4 +1,5 @@
 """Test for certbot_nginx.nginxparser."""
+import copy
 import operator
 import os
 import unittest
@@ -180,5 +181,58 @@ class TestRawNginxParser(unittest.TestCase):
              [['set', '$webp "true"']]]
         ])
 
+class TestUnspacedList(unittest.TestCase):
+    """Test the raw low-level Nginx config parser."""
+    def setUp(self):
+        self.a = ["\n    ", "things", " ", "quirk"]
+        self.b = ["y", " "]
+        self.l = self.a[:]
+        self.l2 = self.b[:]
+        self.ul = UnspacedList(self.l)
+        self.ul2 = UnspacedList(self.l2)
+
+    def test_construction(self):
+        self.assertEqual(self.ul, ["things", "quirk"])
+        self.assertEqual(self.ul2, ["y"])
+
+    def test_append(self):
+        ul3 = copy.deepcopy(self.ul)
+        ul3.append("wise")
+        self.assertEqual(ul3, ["things", "quirk", "wise"])
+        self.assertEqual(ul3.spaced, self.a + ["wise"])
+
+    def test_add(self):
+        ul3 = self.ul + self.ul2
+        self.assertEqual(ul3, ["things", "quirk", "y"])
+        self.assertEqual(ul3.spaced, self.a + self.b)
+        self.assertEqual(self.ul.spaced, self.a)
+        ul3 = self.ul + self.l2
+        self.assertEqual(ul3, ["things", "quirk", "y", " "])
+        self.assertEqual(ul3.spaced, self.a + self.b)
+
+    def test_extend(self):
+        ul3 = copy.deepcopy(self.ul)
+        ul3.extend(self.ul2)
+        self.assertEqual(ul3, ["things", "quirk", "y"])
+        self.assertEqual(ul3.spaced, self.a + self.b)
+        self.assertEqual(self.ul.spaced, self.a)
+
+    def test_set(self):
+        ul3 = copy.deepcopy(self.ul)
+        ul3[0] = "zither"
+        l = ["\n ", "zather", "zest"]
+        ul3[1] = UnspacedList(l)
+        self.assertEqual(ul3, ["zither", ["zather", "zest"]])
+        self.assertEqual(ul3.spaced, [self.a[0], "zither", " ", l])
+
+    def test_rawlists(self):
+        ul3 = copy.deepcopy(self.ul)
+        ul3.insert(0, "some")
+        ul3.append("why")
+        ul3.extend(["did", "whether"])
+        del ul3[2]
+        self.assertEqual(ul3, ["some", "things", "why", "did", "whether"])
+
+
 if __name__ == '__main__':
     unittest.main()  # pragma: no cover

From b2c36f85274176a31ed16bfe7b210793165d3fac Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Fri, 17 Jun 2016 14:39:55 -0700
Subject: [PATCH 20/48] Lint & test fix

---
 certbot-nginx/certbot_nginx/nginxparser.py       | 2 +-
 certbot-nginx/certbot_nginx/tests/parser_test.py | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py
index caf6fe725..765194806 100644
--- a/certbot-nginx/certbot_nginx/nginxparser.py
+++ b/certbot-nginx/certbot_nginx/nginxparser.py
@@ -216,7 +216,7 @@ class UnspacedList(list):
         self.spaced.__delitem__(i + self._spaces_before(i))
         list.__delitem__(self, i)
 
-    def __deepcopy__(self, memo):
+    def __deepcopy__(self, unused_memo):
         l = UnspacedList(self[:])
         l.spaced = copy.deepcopy(self.spaced)
         return l
diff --git a/certbot-nginx/certbot_nginx/tests/parser_test.py b/certbot-nginx/certbot_nginx/tests/parser_test.py
index 8ac995dfc..6d046178a 100644
--- a/certbot-nginx/certbot_nginx/tests/parser_test.py
+++ b/certbot-nginx/certbot_nginx/tests/parser_test.py
@@ -126,7 +126,7 @@ class NginxParserTest(util.NginxTest):
         nparser.add_server_directives(nparser.abs_path('nginx.conf'),
                                       set(['localhost',
                                            r'~^(www\.)?(example|bar)\.']),
-                                      [['foo', 'bar'], ['ssl_certificate',
+                                      [['foo', 'bar'], ['\n ', 'ssl_certificate', ' ',
                                                         '/etc/ssl/cert.pem']],
                                       replace=False)
         ssl_re = re.compile(r'\n\s+ssl_certificate /etc/ssl/cert.pem')

From 679101cfb0659eba484a625bc244a595d86edf93 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Fri, 17 Jun 2016 18:29:45 -0700
Subject: [PATCH 21/48] Object printing improvements

---
 certbot-nginx/certbot_nginx/obj.py | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/certbot-nginx/certbot_nginx/obj.py b/certbot-nginx/certbot_nginx/obj.py
index 0d1151f39..20f9f0a6f 100644
--- a/certbot-nginx/certbot_nginx/obj.py
+++ b/certbot-nginx/certbot_nginx/obj.py
@@ -85,6 +85,9 @@ class Addr(common.Addr):
 
         return parts
 
+    def __repr__(self):
+        return "Addr(" + self.__str__() + ")"
+
     def __eq__(self, other):
         if isinstance(other, self.__class__):
             return (self.tup == other.tup and
@@ -126,6 +129,9 @@ class VirtualHost(object):  # pylint: disable=too-few-public-methods
                 "enabled: %s" % (self.filep, addr_str,
                                  self.names, self.ssl, self.enabled))
 
+    def __repr__(self):
+        return "VirtualHost(" + self.__str__().replace("\n",",") + ")\n"
+
     def __eq__(self, other):
         if isinstance(other, self.__class__):
             return (self.filep == other.filep and

From 7bcc23d9f54fea226538dc3d6d4e7f2d37232977 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Fri, 17 Jun 2016 18:30:12 -0700
Subject: [PATCH 22/48] Debugging

---
 certbot-nginx/certbot_nginx/tests/tls_sni_01_test.py | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/certbot-nginx/certbot_nginx/tests/tls_sni_01_test.py b/certbot-nginx/certbot_nginx/tests/tls_sni_01_test.py
index 3264d6ed3..19d22365d 100644
--- a/certbot-nginx/certbot_nginx/tests/tls_sni_01_test.py
+++ b/certbot-nginx/certbot_nginx/tests/tls_sni_01_test.py
@@ -12,6 +12,7 @@ from certbot import errors
 from certbot.plugins import common_test
 from certbot.tests import acme_util
 
+from certbot_nginx import nginxparser
 from certbot_nginx import obj
 from certbot_nginx.tests import util
 
@@ -132,16 +133,21 @@ class TlsSniPerformTest(util.NginxTest):
 
         http = self.sni.configurator.parser.parsed[
             self.sni.configurator.parser.loc["root"]][-1]
+        print "http", http
+        #print "SPACED\n", http.spaced
         self.assertTrue(['include', self.sni.challenge_conf] in http[1])
 
         vhosts = self.sni.configurator.parser.get_vhosts()
+        print "Got", vhosts
         vhs = [vh for vh in vhosts if vh.filep == self.sni.challenge_conf]
+        print "And now", vhs
 
         for vhost in vhs:
             if vhost.addrs == set(v_addr1):
                 response = self.achalls[0].response(self.account_key)
             else:
                 response = self.achalls[2].response(self.account_key)
+                print vhost.addrs, set(v_addr2)
                 self.assertEqual(vhost.addrs, set(v_addr2))
             self.assertEqual(vhost.names, set([response.z_domain]))
 

From e4f88506cc84b852b6bc339cede9ed8abfebc2db Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Sat, 18 Jun 2016 14:52:07 -0700
Subject: [PATCH 23/48] Fix TLS_SNI & associated tests

---
 certbot-nginx/certbot_nginx/nginxparser.py    | 20 +++++++++++++-----
 certbot-nginx/certbot_nginx/obj.py            |  2 +-
 certbot-nginx/certbot_nginx/parser.py         |  7 +++++--
 .../certbot_nginx/tests/configurator_test.py  |  2 +-
 .../certbot_nginx/tests/nginxparser_test.py   |  4 ++--
 .../certbot_nginx/tests/tls_sni_01_test.py    |  5 -----
 certbot-nginx/certbot_nginx/tests/util.py     | 11 +++++++---
 certbot-nginx/certbot_nginx/tls_sni_01.py     | 21 ++++++++++---------
 8 files changed, 43 insertions(+), 29 deletions(-)

diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py
index 765194806..6b3896fd9 100644
--- a/certbot-nginx/certbot_nginx/nginxparser.py
+++ b/certbot-nginx/certbot_nginx/nginxparser.py
@@ -165,6 +165,7 @@ class UnspacedList(list):
     """Wrap a list [of lists], making any whitespace entries magically invisible"""
 
     def __init__(self, list_source):
+        # ensure our argument is not a generator, and duplicate any sublists
         self.spaced = copy.deepcopy(list(list_source))
 
         # Turn self into a version of the source list that has spaces removed
@@ -173,16 +174,25 @@ class UnspacedList(list):
         for i, entry in reversed(list(enumerate(self))):
             if isinstance(entry, list):
                 sublist = UnspacedList(entry)
-                list.__setitem__(self, i, sublist)
+                if sublist != [] or sublist.spaced == []:
+                    list.__setitem__(self, i, sublist)
+                else:
+                    # if a sublist is exclusively spacey entries, it might
+                    # choke the high level parser, so make it disappear
+                    list.__delitem__(self, i)
                 self.spaced[i] = sublist.spaced
             elif spacey(entry):
-                list.__delitem__(self, i)
+                # don't delete comments
+                if "#" not in self[:i]:
+                    list.__delitem__(self, i)
 
     def insert(self, i, x):
-        if hasattr(x, "spaced"):
-            self.spaced.insert(i + self._spaces_before(i), x.spaced)
-        else:
+        if not isinstance(x, list):                      # str or None
             self.spaced.insert(i + self._spaces_before(i), x)
+        else:
+            if not hasattr(x, "spaced"):
+                x = UnspacedList(x)
+            self.spaced.insert(i + self._spaces_before(i), x.spaced)
         list.insert(self, i, x)
 
     def append(self, x):
diff --git a/certbot-nginx/certbot_nginx/obj.py b/certbot-nginx/certbot_nginx/obj.py
index 20f9f0a6f..a559b5e02 100644
--- a/certbot-nginx/certbot_nginx/obj.py
+++ b/certbot-nginx/certbot_nginx/obj.py
@@ -130,7 +130,7 @@ class VirtualHost(object):  # pylint: disable=too-few-public-methods
                                  self.names, self.ssl, self.enabled))
 
     def __repr__(self):
-        return "VirtualHost(" + self.__str__().replace("\n",",") + ")\n"
+        return "VirtualHost(" + self.__str__().replace("\n",", ") + ")\n"
 
     def __eq__(self, other):
         if isinstance(other, self.__class__):
diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py
index 21127bc08..31595d56d 100644
--- a/certbot-nginx/certbot_nginx/parser.py
+++ b/certbot-nginx/certbot_nginx/parser.py
@@ -1,4 +1,5 @@
 """NginxParser is a member object of the NginxConfigurator class."""
+import copy
 import glob
 import logging
 import os
@@ -113,6 +114,7 @@ class NginxParser(object):
         for filename in servers:
             for server in servers[filename]:
                 # Parse the server block into a VirtualHost object
+
                 parsed_server = parse_server(server)
                 vhost = obj.VirtualHost(filename,
                                         parsed_server['addrs'],
@@ -132,7 +134,7 @@ class NginxParser(object):
         :rtype: list
 
         """
-        result = list(block)  # Copy the list to keep self.parsed idempotent
+        result = copy.deepcopy(block)  # Copy the list to keep self.parsed idempotent
         for directive in block:
             if _is_include_directive(directive):
                 included_files = glob.glob(
@@ -465,6 +467,8 @@ def parse_server(server):
                      'names': set()}
 
     for directive in server:
+        if not directive:
+            continue
         if directive[0] == 'listen':
             addr = obj.Addr.fromstring(directive[1])
             parsed_server['addrs'].add(addr)
@@ -506,7 +510,6 @@ def _add_directive(block, directive, replace):
 
     """
     directive = nginxparser.UnspacedList(directive)
-    print "Unspacified", directive.spaced, directive
     if len(directive) == 0:
         # whitespace
         block.append(directive)
diff --git a/certbot-nginx/certbot_nginx/tests/configurator_test.py b/certbot-nginx/certbot_nginx/tests/configurator_test.py
index 30f287249..a1d0e75cc 100644
--- a/certbot-nginx/certbot_nginx/tests/configurator_test.py
+++ b/certbot-nginx/certbot_nginx/tests/configurator_test.py
@@ -83,7 +83,7 @@ class NginxConfiguratorTest(util.NginxTest):
         filep = self.config.parser.abs_path('sites-enabled/example.com')
         self.config.parser.add_server_directives(
             filep, set(['.example.com', 'example.*']),
-            [['listen', '5001 ssl']],
+            [['listen', ' ', '5001 ssl']],
             replace=False)
         self.config.save()
 
diff --git a/certbot-nginx/certbot_nginx/tests/nginxparser_test.py b/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
index e78adfac1..2e8011443 100644
--- a/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
+++ b/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
@@ -164,9 +164,9 @@ class TestRawNginxParser(unittest.TestCase):
                 ['#', ' Kilroy was here'],
                 ['check_status'],
                 [['server'],
-                 [['#'],
+                 [['#', ''],
                   ['#', " Don't forget to open up your firewall!"],
-                  ['#'],
+                  ['#', ''],
                   ['listen', '1234'],
                   ['#', ' listen 80;']]],
             ])
diff --git a/certbot-nginx/certbot_nginx/tests/tls_sni_01_test.py b/certbot-nginx/certbot_nginx/tests/tls_sni_01_test.py
index 19d22365d..51301145c 100644
--- a/certbot-nginx/certbot_nginx/tests/tls_sni_01_test.py
+++ b/certbot-nginx/certbot_nginx/tests/tls_sni_01_test.py
@@ -133,21 +133,16 @@ class TlsSniPerformTest(util.NginxTest):
 
         http = self.sni.configurator.parser.parsed[
             self.sni.configurator.parser.loc["root"]][-1]
-        print "http", http
-        #print "SPACED\n", http.spaced
         self.assertTrue(['include', self.sni.challenge_conf] in http[1])
 
         vhosts = self.sni.configurator.parser.get_vhosts()
-        print "Got", vhosts
         vhs = [vh for vh in vhosts if vh.filep == self.sni.challenge_conf]
-        print "And now", vhs
 
         for vhost in vhs:
             if vhost.addrs == set(v_addr1):
                 response = self.achalls[0].response(self.account_key)
             else:
                 response = self.achalls[2].response(self.account_key)
-                print vhost.addrs, set(v_addr2)
                 self.assertEqual(vhost.addrs, set(v_addr2))
             self.assertEqual(vhost.names, set([response.z_domain]))
 
diff --git a/certbot-nginx/certbot_nginx/tests/util.py b/certbot-nginx/certbot_nginx/tests/util.py
index ddacd041b..4ea4a03a7 100644
--- a/certbot-nginx/certbot_nginx/tests/util.py
+++ b/certbot-nginx/certbot_nginx/tests/util.py
@@ -1,4 +1,5 @@
 """Common utilities for certbot_nginx."""
+import copy
 import os
 import pkg_resources
 import unittest
@@ -16,6 +17,7 @@ from certbot.plugins import common
 
 from certbot_nginx import constants
 from certbot_nginx import configurator
+from certbot_nginx import nginxparser
 
 
 class NginxTest(unittest.TestCase):  # pylint: disable=too-few-public-methods
@@ -82,12 +84,15 @@ def filter_comments(tree):
 
     def traverse(tree):
         """Generator dropping comment nodes"""
-        for key, values in tree:
+        for entry in tree:
+            key, values = entry
             if isinstance(key, list):
-                yield [key, filter_comments(values)]
+                new = copy.deepcopy(entry)
+                new[1] = filter_comments(values)
+                yield new
             else:
                 if key != '#':
-                    yield [key, values]
+                    yield entry
 
     return list(traverse(tree))
 
diff --git a/certbot-nginx/certbot_nginx/tls_sni_01.py b/certbot-nginx/certbot_nginx/tls_sni_01.py
index e4c5d31a6..a9f31b84a 100644
--- a/certbot-nginx/certbot_nginx/tls_sni_01.py
+++ b/certbot-nginx/certbot_nginx/tls_sni_01.py
@@ -93,10 +93,10 @@ class NginxTlsSni01(common.TLSSNI01):
         # Add the 'include' statement for the challenges if it doesn't exist
         # already in the main config
         included = False
-        include_directive = ['include', self.challenge_conf]
+        include_directive = ['include', ' ', self.challenge_conf]
         root = self.configurator.parser.loc["root"]
 
-        bucket_directive = ['server_names_hash_bucket_size', '128']
+        bucket_directive = ['server_names_hash_bucket_size', ' ', '128']
 
         main = self.configurator.parser.parsed[root]
         for key, body in main:
@@ -118,6 +118,7 @@ class NginxTlsSni01(common.TLSSNI01):
 
         config = [self._make_server_block(pair[0], pair[1])
                   for pair in itertools.izip(self.achalls, ll_addrs)]
+        config = nginxparser.UnspacedList(config)
 
         self.configurator.reverter.register_file_creation(
             True, self.challenge_conf)
@@ -142,19 +143,19 @@ class NginxTlsSni01(common.TLSSNI01):
         document_root = os.path.join(
             self.configurator.config.work_dir, "tls_sni_01_page")
 
-        block = [['listen', str(addr)] for addr in addrs]
+        block = [['listen', ' ', str(addr)] for addr in addrs]
 
-        block.extend([['server_name',
+        block.extend([['server_name', ' ',
                        achall.response(achall.account_key).z_domain],
-                      ['include', self.configurator.parser.loc["ssl_options"]],
+                      ['include', ' ', self.configurator.parser.loc["ssl_options"]],
                       # access and error logs necessary for
                       # integration testing (non-root)
-                      ['access_log', os.path.join(
+                      ['access_log', ' ', os.path.join(
                           self.configurator.config.work_dir, 'access.log')],
-                      ['error_log', os.path.join(
+                      ['error_log', ' ', os.path.join(
                           self.configurator.config.work_dir, 'error.log')],
-                      ['ssl_certificate', self.get_cert_path(achall)],
-                      ['ssl_certificate_key', self.get_key_path(achall)],
-                      [['location', '/'], [['root', document_root]]]])
+                      ['ssl_certificate', ' ', self.get_cert_path(achall)],
+                      ['ssl_certificate_key', ' ', self.get_key_path(achall)],
+                      [['location', ' ', '/'], [['root', ' ', document_root]]]])
 
         return [['server'], block]

From 1e59bf81123df94210f5a4627e1af7e444b1eff2 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Sat, 18 Jun 2016 14:53:17 -0700
Subject: [PATCH 24/48] fixup

---
 certbot-nginx/certbot_nginx/tests/tls_sni_01_test.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/certbot-nginx/certbot_nginx/tests/tls_sni_01_test.py b/certbot-nginx/certbot_nginx/tests/tls_sni_01_test.py
index 51301145c..3264d6ed3 100644
--- a/certbot-nginx/certbot_nginx/tests/tls_sni_01_test.py
+++ b/certbot-nginx/certbot_nginx/tests/tls_sni_01_test.py
@@ -12,7 +12,6 @@ from certbot import errors
 from certbot.plugins import common_test
 from certbot.tests import acme_util
 
-from certbot_nginx import nginxparser
 from certbot_nginx import obj
 from certbot_nginx.tests import util
 

From b663ec039ed41012f89d2dcac950be323a7d3a19 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Sat, 18 Jun 2016 15:05:48 -0700
Subject: [PATCH 25/48] lint

---
 certbot-nginx/certbot_nginx/obj.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/certbot-nginx/certbot_nginx/obj.py b/certbot-nginx/certbot_nginx/obj.py
index a559b5e02..f5ac88f6c 100644
--- a/certbot-nginx/certbot_nginx/obj.py
+++ b/certbot-nginx/certbot_nginx/obj.py
@@ -130,7 +130,7 @@ class VirtualHost(object):  # pylint: disable=too-few-public-methods
                                  self.names, self.ssl, self.enabled))
 
     def __repr__(self):
-        return "VirtualHost(" + self.__str__().replace("\n",", ") + ")\n"
+        return "VirtualHost(" + self.__str__().replace("\n", ", ") + ")\n"
 
     def __eq__(self, other):
         if isinstance(other, self.__class__):

From d2b4ae57404486f5d5b34aeaea6fc3637c263a75 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Sat, 18 Jun 2016 15:17:59 -0700
Subject: [PATCH 26/48] Consistently coerce inbound data to Unspacines

---
 certbot-nginx/certbot_nginx/nginxparser.py | 48 ++++++++++++----------
 1 file changed, 27 insertions(+), 21 deletions(-)

diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py
index 6b3896fd9..d0f53e1a8 100644
--- a/certbot-nginx/certbot_nginx/nginxparser.py
+++ b/certbot-nginx/certbot_nginx/nginxparser.py
@@ -186,29 +186,37 @@ class UnspacedList(list):
                 if "#" not in self[:i]:
                     list.__delitem__(self, i)
 
-    def insert(self, i, x):
-        if not isinstance(x, list):                      # str or None
-            self.spaced.insert(i + self._spaces_before(i), x)
+    def _coerce(self, inbound):
+        """
+        Coerce some inbound object to be appropriately usable in this object
+
+        :param inbound: string or None or list or UnspacedList
+        :returns: (coerced UnspacedList or string or None, spaced equivalent)
+        :rtype: tuple
+
+        """
+        if not isinstance(inbound, list):                      # str or None
+            return (inbound, inbound)
         else:
             if not hasattr(x, "spaced"):
-                x = UnspacedList(x)
-            self.spaced.insert(i + self._spaces_before(i), x.spaced)
-        list.insert(self, i, x)
+                inbound = UnspacedList(inbound)
+            return (inbound, inbound.spaced)
+
+
+    def insert(self, i, x):
+        item, spaced_item = self._coerce(x)
+        self.spaced.insert(i + self._spaces_before(i), spaced_item)
+        list.insert(self, i, item)
 
     def append(self, x):
-        if hasattr(x, "spaced"):
-            self.spaced.append(x.spaced)
-        else:
-            self.spaced.append(x)
-        list.append(self, x)
+        item, spaced_item = self._coerce(x)
+        self.spaced.append(spaced_item)
+        list.append(self, item)
 
     def extend(self, x):
-        if hasattr(x, "spaced"):
-            self.spaced.extend(x.spaced)
-        else:
-            self.spaced.extend(x)
-            logger.debug("Weird, extending regular list %r to Unspaced %r", x, self)
-        list.extend(self, x)
+        item, spaced_item = self._coerce(x)
+        self.spaced.extend(spaced_item)
+        list.extend(self, item)
 
     def __add__(self, other):
         l = copy.deepcopy(self)
@@ -216,10 +224,8 @@ class UnspacedList(list):
         return l
 
     def __setitem__(self, 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)
+        item, spaced_item = self._coerce(x)
+        self.spaced.__setitem__(i + self._spaces_before(i), value)
         list.__setitem__(self, i, value)
 
     def __delitem__(self, i):

From ef31a837f7242691e8c6ae83f6a92a3cf6dc5792 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 20 Jun 2016 15:56:19 -0700
Subject: [PATCH 27/48] Lint

---
 certbot-nginx/certbot_nginx/nginxparser.py | 8 ++++----
 certbot-nginx/certbot_nginx/tests/util.py  | 1 -
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py
index d0f53e1a8..f74f3066e 100644
--- a/certbot-nginx/certbot_nginx/nginxparser.py
+++ b/certbot-nginx/certbot_nginx/nginxparser.py
@@ -198,7 +198,7 @@ class UnspacedList(list):
         if not isinstance(inbound, list):                      # str or None
             return (inbound, inbound)
         else:
-            if not hasattr(x, "spaced"):
+            if not hasattr(inbound, "spaced"):
                 inbound = UnspacedList(inbound)
             return (inbound, inbound.spaced)
 
@@ -224,9 +224,9 @@ class UnspacedList(list):
         return l
 
     def __setitem__(self, i, value):
-        item, spaced_item = self._coerce(x)
-        self.spaced.__setitem__(i + self._spaces_before(i), value)
-        list.__setitem__(self, i, value)
+        item, spaced_item = self._coerce(value)
+        self.spaced.__setitem__(i + self._spaces_before(i), spaced_item)
+        list.__setitem__(self, i, item)
 
     def __delitem__(self, i):
         self.spaced.__delitem__(i + self._spaces_before(i))
diff --git a/certbot-nginx/certbot_nginx/tests/util.py b/certbot-nginx/certbot_nginx/tests/util.py
index 4ea4a03a7..866e5a9c7 100644
--- a/certbot-nginx/certbot_nginx/tests/util.py
+++ b/certbot-nginx/certbot_nginx/tests/util.py
@@ -17,7 +17,6 @@ from certbot.plugins import common
 
 from certbot_nginx import constants
 from certbot_nginx import configurator
-from certbot_nginx import nginxparser
 
 
 class NginxTest(unittest.TestCase):  # pylint: disable=too-few-public-methods

From e2fd1369f33dc8e8adbbf40562f5e90e3811bcb2 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 20 Jun 2016 16:08:40 -0700
Subject: [PATCH 28/48] Update test for fancier semantics

---
 certbot-nginx/certbot_nginx/tests/nginxparser_test.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/certbot-nginx/certbot_nginx/tests/nginxparser_test.py b/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
index 2e8011443..1b1073de0 100644
--- a/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
+++ b/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
@@ -207,7 +207,7 @@ class TestUnspacedList(unittest.TestCase):
         self.assertEqual(ul3.spaced, self.a + self.b)
         self.assertEqual(self.ul.spaced, self.a)
         ul3 = self.ul + self.l2
-        self.assertEqual(ul3, ["things", "quirk", "y", " "])
+        self.assertEqual(ul3, ["things", "quirk", "y"])
         self.assertEqual(ul3.spaced, self.a + self.b)
 
     def test_extend(self):

From 02844904f0bd4a11f2d5f24e69033bbb580b0a9f Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 20 Jun 2016 16:11:32 -0700
Subject: [PATCH 29/48] Improve coverage

---
 certbot-nginx/certbot_nginx/tests/nginxparser_test.py | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/certbot-nginx/certbot_nginx/tests/nginxparser_test.py b/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
index 1b1073de0..4c239ac10 100644
--- a/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
+++ b/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
@@ -115,13 +115,7 @@ class TestRawNginxParser(unittest.TestCase):
 
     def test_dump_as_file(self):
         with open(util.get_data_filename('nginx.conf')) as handle:
-            try:
-                parsed = load(handle)
-            except:
-                handle.seek(0)
-                print "Failed on", handle.read()
-                raise
-            #parsed = util.filter_comments(parsed)
+            parsed = load(handle)
         parsed[-1][-1].append(UnspacedList([['server'],
                                [['listen', ' ', '443 ssl'],
                                 ['server_name', ' ', 'localhost'],

From 14ea8d8cdf8a63870b741886af3da21b946d9775 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 20 Jun 2016 17:45:02 -0700
Subject: [PATCH 30/48] Fix server_name spaciness

---
 certbot-nginx/certbot_nginx/configurator.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/certbot-nginx/certbot_nginx/configurator.py b/certbot-nginx/certbot_nginx/configurator.py
index 4f46b6a66..dd9a44aad 100644
--- a/certbot-nginx/certbot_nginx/configurator.py
+++ b/certbot-nginx/certbot_nginx/configurator.py
@@ -225,7 +225,7 @@ class NginxConfigurator(common.Plugin):
         if not matches:
             # No matches. Create a new vhost with this name in nginx.conf.
             filep = self.parser.loc["root"]
-            new_block = [['server'], [['server_name', target_name]]]
+            new_block = [['server'], [['\n', 'server_name', ' ', target_name]]]
             self.parser.add_http_directives(filep, new_block)
             vhost = obj.VirtualHost(filep, set([]), False, True,
                                     set([target_name]), list(new_block[1]))

From 5960376d36f4ccec38814c87bf972c01a23f10b0 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 20 Jun 2016 18:17:47 -0700
Subject: [PATCH 31/48] Cleanups

---
 certbot-nginx/certbot_nginx/nginxparser.py | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py
index f74f3066e..5c2bec577 100644
--- a/certbot-nginx/certbot_nginx/nginxparser.py
+++ b/certbot-nginx/certbot_nginx/nginxparser.py
@@ -62,9 +62,8 @@ class RawNginxParser(object):
 class RawNginxDumper(object):
     # pylint: disable=too-few-public-methods
     """A class that dumps nginx configuration from the provided tree."""
-    def __init__(self, blocks, indentation=0):
+    def __init__(self, blocks):
         self.blocks = blocks
-        self.indentation = indentation
 
     def __iter__(self, blocks=None):
         """Iterates the dumped nginx content."""
@@ -100,10 +99,6 @@ class RawNginxDumper(object):
                     if values and spacey(values):
                         gap = values
                         values = b.pop(0)
-                    #if values is None:
-                    #    yield indentation + key + gap + ';'
-                    #else:
-                    #    yield indentation + key + gap + values + ';'
                     yield indentation + key + gap + values + ';'
 
     def __str__(self):

From 884b21ffbe3b577eb82f2da58793b31e7d5dd6b8 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 21 Jun 2016 15:11:32 -0700
Subject: [PATCH 32/48] fix docstring typo

---
 certbot-nginx/certbot_nginx/parser.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py
index 31595d56d..3c3942d22 100644
--- a/certbot-nginx/certbot_nginx/parser.py
+++ b/certbot-nginx/certbot_nginx/parser.py
@@ -18,7 +18,7 @@ logger = logging.getLogger(__name__)
 class NginxParser(object):
     """Class handles the fine details of parsing the Nginx Configuration.
 
-    :ivar str root: Normalized abosulte path to the server root
+    :ivar str root: Normalized absolute path to the server root
         directory. Without trailing slash.
     :ivar dict parsed: Mapping of file paths to parsed trees
 

From 098d23ac984d9c242dcaf3436ac1d13dac579769 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 21 Jun 2016 15:33:57 -0700
Subject: [PATCH 33/48] Comment a couple of things

---
 certbot-nginx/certbot_nginx/parser.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py
index 3c3942d22..a0c29fc59 100644
--- a/certbot-nginx/certbot_nginx/parser.py
+++ b/certbot-nginx/certbot_nginx/parser.py
@@ -155,7 +155,9 @@ class NginxParser(object):
         :rtype: list
 
         """
-        files = glob.glob(filepath)
+        files = 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:
@@ -210,6 +212,8 @@ class NginxParser(object):
             empty, this overrides the existing conf files.
 
         """
+        # XXX probably this should be modified to perform atomic write
+        # operations and/or to defer control-c until completed
         for filename in self.parsed:
             tree = self.parsed[filename]
             if ext:

From 4ef7131a4b6868aa32370a27dd8f722a5ae2e429 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Thu, 23 Jun 2016 11:05:49 -0700
Subject: [PATCH 34/48] Name all of nginxparser's magic regexps

---
 certbot-nginx/certbot_nginx/nginxparser.py | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py
index 5c2bec577..ab57089b9 100644
--- a/certbot-nginx/certbot_nginx/nginxparser.py
+++ b/certbot-nginx/certbot_nginx/nginxparser.py
@@ -4,7 +4,7 @@ import logging
 import string
 
 from pyparsing import (
-    Literal, White, Word, alphanums, CharsNotIn, Forward, Group,
+    Literal, White, Word, alphanums, CharsNotIn, Combine, Forward, Group,
     Optional, OneOrMore, Regex, ZeroOrMore)
 from pyparsing import stringEnd
 from pyparsing import restOfLine
@@ -17,10 +17,13 @@ class RawNginxParser(object):
 
     # constants
     space = Optional(White())
+    nonspace = Regex(r"\S+")
     left_bracket = Literal("{").suppress()
     right_bracket = space.leaveWhitespace() + Literal("}").suppress()
     semicolon = Literal(";").suppress()
     key = Word(alphanums + "_/+-.")
+    dollar_var = Combine(Literal('$') + nonspace)
+    condition = Regex(r"\(.+\)")
     # Matches anything that is not a special character AND any chars in single
     # or double quotes
     value = Regex(r"((\".*\")?(\'.*\')?[^\{\};,]?)+")
@@ -33,8 +36,8 @@ class RawNginxParser(object):
 
     assignment = space + key + Optional(space + value, default=None) + semicolon
     location_statement = space + Optional(modifier) + Optional(space + location + space)
-    if_statement = space + Literal("if") + space + Regex(r"\(.+\)") + space
-    map_statement = space + Literal("map") + space + Regex(r"\S+") + space + Regex(r"\$\S+") + space
+    if_statement = space + Literal("if") + space + condition + space
+    map_statement = space + Literal("map") + space + nonspace + space + dollar_var + space
     block = Forward()
 
     block << Group(

From db66050a7ae3048f4daf0a06ba6e87231abddcfb Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Thu, 23 Jun 2016 11:39:51 -0700
Subject: [PATCH 35/48] Make atomicity comment more accurate

---
 certbot-nginx/certbot_nginx/parser.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py
index a0c29fc59..f2c10ab70 100644
--- a/certbot-nginx/certbot_nginx/parser.py
+++ b/certbot-nginx/certbot_nginx/parser.py
@@ -212,8 +212,7 @@ class NginxParser(object):
             empty, this overrides the existing conf files.
 
         """
-        # XXX probably this should be modified to perform atomic write
-        # operations and/or to defer control-c until completed
+        # Best-effort atomicity is enforced above us by reverter.py
         for filename in self.parsed:
             tree = self.parsed[filename]
             if ext:

From a29c6e3102ca1d093b7ffbaed93e88f635907e8d Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Thu, 23 Jun 2016 14:06:32 -0700
Subject: [PATCH 36/48] Try simplifying handling of spacey sublists

 - this was causing test failures but they may all have been fixed by other
   changes...
---
 certbot-nginx/certbot_nginx/nginxparser.py | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py
index ab57089b9..efcdf94ef 100644
--- a/certbot-nginx/certbot_nginx/nginxparser.py
+++ b/certbot-nginx/certbot_nginx/nginxparser.py
@@ -172,12 +172,7 @@ class UnspacedList(list):
         for i, entry in reversed(list(enumerate(self))):
             if isinstance(entry, list):
                 sublist = UnspacedList(entry)
-                if sublist != [] or sublist.spaced == []:
-                    list.__setitem__(self, i, sublist)
-                else:
-                    # if a sublist is exclusively spacey entries, it might
-                    # choke the high level parser, so make it disappear
-                    list.__delitem__(self, i)
+                list.__setitem__(self, i, sublist)
                 self.spaced[i] = sublist.spaced
             elif spacey(entry):
                 # don't delete comments

From 6930523c1469be5b9254405d045e897dfb85ee56 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Thu, 23 Jun 2016 17:59:26 -0700
Subject: [PATCH 37/48] Refactor to simplify indenation handling

---
 certbot-nginx/certbot_nginx/nginxparser.py | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py
index efcdf94ef..0ec1aeefb 100644
--- a/certbot-nginx/certbot_nginx/nginxparser.py
+++ b/certbot-nginx/certbot_nginx/nginxparser.py
@@ -41,7 +41,7 @@ class RawNginxParser(object):
     block = Forward()
 
     block << Group(
-        # XXX could this "key" be Literal("location")?
+        # key could for instance be "server" or "http"
         (Group(space + key + location_statement) ^ Group(if_statement) ^
         Group(map_statement)).leaveWhitespace() +
         left_bracket +
@@ -78,15 +78,14 @@ class RawNginxDumper(object):
             b = copy.deepcopy(b0)
             indentation = ""
             if spacey(b[0]):
-                indentation = b.pop(0)
+                yield b.pop(0) # indentation
                 if not b:
-                    yield indentation
                     continue
             key = b.pop(0)
             values = b.pop(0)
 
             if isinstance(key, list):
-                yield indentation + "".join(key) + '{'
+                yield "".join(key) + '{'
                 for parameter in values:
                     dumped = self.__iter__([parameter])
                     for line in dumped:
@@ -94,7 +93,7 @@ class RawNginxDumper(object):
                 yield '}'
             else:
                 if isinstance(key, str) and key.strip() == '#':
-                    yield indentation + key + values
+                    yield key + values
                 else:
                     gap = ""
                     # Sometimes the parser has stuck some gap whitespace in here;
@@ -102,7 +101,7 @@ class RawNginxDumper(object):
                     if values and spacey(values):
                         gap = values
                         values = b.pop(0)
-                    yield indentation + key + gap + values + ';'
+                    yield key + gap + values + ';'
 
     def __str__(self):
         """Return the parsed block as a string."""

From ad13b525b28542378a8f506732b8ac1146d69b3e Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Fri, 24 Jun 2016 14:35:42 -0700
Subject: [PATCH 38/48] For reference, a buggy attempt to implement slicing

but slice assignment seems to mangle the .spaced list in some cases
---
 certbot-nginx/certbot_nginx/nginxparser.py | 31 +++++++++++++++++-----
 1 file changed, 24 insertions(+), 7 deletions(-)

diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py
index 0ec1aeefb..3db6ff501 100644
--- a/certbot-nginx/certbot_nginx/nginxparser.py
+++ b/certbot-nginx/certbot_nginx/nginxparser.py
@@ -76,7 +76,6 @@ class RawNginxDumper(object):
                 yield b0
                 continue
             b = copy.deepcopy(b0)
-            indentation = ""
             if spacey(b[0]):
                 yield b.pop(0) # indentation
                 if not b:
@@ -197,7 +196,7 @@ class UnspacedList(list):
 
     def insert(self, i, x):
         item, spaced_item = self._coerce(x)
-        self.spaced.insert(i + self._spaces_before(i), spaced_item)
+        self.spaced.insert(self._spaced_position(i), spaced_item)
         list.insert(self, i, item)
 
     def append(self, x):
@@ -215,13 +214,28 @@ class UnspacedList(list):
         l.extend(other)
         return l
 
+    def __setslice__(self, i, j, newslice):
+        for idx in reversed(range(self._spaced_position(i), self._spaced_position(j))):
+            print "delspace", idx
+            del self.spaced[idx]
+        for idx in reversed(range(i,j)):
+            print "del", idx
+            list.__delitem__(self, idx)
+        for idx, item in enumerate(newslice):
+            self.insert(i + idx, item)
+
     def __setitem__(self, i, value):
+        if isinstance(i, slice):
+            #raise NotImplementedError("Slice operations on UnspacedLists not yet implemented")
+            for pos in xrange(i.start or 0, i.stop or len(self), i.step or 1):
+                self.__setitem__(pos, value)
+            return
         item, spaced_item = self._coerce(value)
-        self.spaced.__setitem__(i + self._spaces_before(i), spaced_item)
+        self.spaced.__setitem__(self._spaced_position(i), spaced_item)
         list.__setitem__(self, i, item)
 
     def __delitem__(self, i):
-        self.spaced.__delitem__(i + self._spaces_before(i))
+        self.spaced.__delitem__(self._spaced_position(i))
         list.__delitem__(self, i)
 
     def __deepcopy__(self, unused_memo):
@@ -230,14 +244,17 @@ class UnspacedList(list):
         return l
 
 
-    def _spaces_before(self, idx):
-        "Count the number of spaces in the spaced list before pos idx in the spaceless one"
+    def _spaced_position(self, idx):
+        "Convert from indexes in the unspaced list to positions in the spaced one"
+        print "lookup", idx
         spaces = 0
         pos = 0
+        if idx < 0:
+            idx = len(self) + idx
         while idx != -1:
             if spacey(self.spaced[pos]):
                 spaces += 1
             else:
                 idx -= 1
             pos += 1
-        return spaces
+        return idx + spaces

From e415a4d402d73af1f71171bae844ed435363cacd Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Fri, 24 Jun 2016 14:36:23 -0700
Subject: [PATCH 39/48] For now, instead, consider this NotImplemented

---
 certbot-nginx/certbot_nginx/nginxparser.py | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py
index 3db6ff501..8a423848c 100644
--- a/certbot-nginx/certbot_nginx/nginxparser.py
+++ b/certbot-nginx/certbot_nginx/nginxparser.py
@@ -215,21 +215,11 @@ class UnspacedList(list):
         return l
 
     def __setslice__(self, i, j, newslice):
-        for idx in reversed(range(self._spaced_position(i), self._spaced_position(j))):
-            print "delspace", idx
-            del self.spaced[idx]
-        for idx in reversed(range(i,j)):
-            print "del", idx
-            list.__delitem__(self, idx)
-        for idx, item in enumerate(newslice):
-            self.insert(i + idx, item)
+        raise NotImplementedError("Slice operations on UnspacedLists not yet implemented")
 
     def __setitem__(self, i, value):
         if isinstance(i, slice):
-            #raise NotImplementedError("Slice operations on UnspacedLists not yet implemented")
-            for pos in xrange(i.start or 0, i.stop or len(self), i.step or 1):
-                self.__setitem__(pos, value)
-            return
+            raise NotImplementedError("Slice operations on UnspacedLists not yet implemented")
         item, spaced_item = self._coerce(value)
         self.spaced.__setitem__(self._spaced_position(i), spaced_item)
         list.__setitem__(self, i, item)

From 9da533d92e432d6ad365de9dbed7e2810491f577 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Fri, 24 Jun 2016 16:13:12 -0700
Subject: [PATCH 40/48] Be more succinct

---
 certbot-nginx/certbot_nginx/nginxparser.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py
index 8a423848c..f65864df4 100644
--- a/certbot-nginx/certbot_nginx/nginxparser.py
+++ b/certbot-nginx/certbot_nginx/nginxparser.py
@@ -86,8 +86,7 @@ class RawNginxDumper(object):
             if isinstance(key, list):
                 yield "".join(key) + '{'
                 for parameter in values:
-                    dumped = self.__iter__([parameter])
-                    for line in dumped:
+                    for line in self.__iter__([parameter]): # negate "for b0 in blocks"
                         yield line
                 yield '}'
             else:

From 20b92a3c1fba3a922ea3063954b3691d4ed462d7 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Fri, 24 Jun 2016 16:48:22 -0700
Subject: [PATCH 41/48] Add some test coverage

---
 certbot-nginx/certbot_nginx/tests/configurator_test.py | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/certbot-nginx/certbot_nginx/tests/configurator_test.py b/certbot-nginx/certbot_nginx/tests/configurator_test.py
index a1d0e75cc..7529bc10b 100644
--- a/certbot-nginx/certbot_nginx/tests/configurator_test.py
+++ b/certbot-nginx/certbot_nginx/tests/configurator_test.py
@@ -265,7 +265,8 @@ class NginxConfiguratorTest(util.NginxTest):
 
     @mock.patch("certbot_nginx.configurator.tls_sni_01.NginxTlsSni01.perform")
     @mock.patch("certbot_nginx.configurator.NginxConfigurator.restart")
-    def test_perform(self, mock_restart, mock_perform):
+    @mock.patch("certbot_nginx.configurator.NginxConfigurator.revert_challenge_config")
+    def test_perform_and_cleanup(self, mock_revert, mock_restart, mock_perform):
         # Only tests functionality specific to configurator.perform
         # Note: As more challenges are offered this will have to be expanded
         achall1 = achallenges.KeyAuthorizationAnnotatedChallenge(
@@ -291,7 +292,11 @@ class NginxConfiguratorTest(util.NginxTest):
 
         self.assertEqual(mock_perform.call_count, 1)
         self.assertEqual(responses, expected)
-        self.assertEqual(mock_restart.call_count, 1)
+
+        self.config.cleanup([achall1, achall2])
+        self.assertEqual(0, self.config._chall_out) # pylint: disable=protected-access
+        self.assertEqual(mock_revert.call_count, 1)
+        self.assertEqual(mock_restart.call_count, 2)
 
     @mock.patch("certbot_nginx.configurator.subprocess.Popen")
     def test_get_version(self, mock_popen):

From 8f0a5fdc66f6b72d210a2edff32c27636bc3ecd7 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Fri, 24 Jun 2016 16:48:30 -0700
Subject: [PATCH 42/48] Fix a bug in the new index calculations

---
 certbot-nginx/certbot_nginx/nginxparser.py | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py
index f65864df4..412268d22 100644
--- a/certbot-nginx/certbot_nginx/nginxparser.py
+++ b/certbot-nginx/certbot_nginx/nginxparser.py
@@ -235,15 +235,16 @@ class UnspacedList(list):
 
     def _spaced_position(self, idx):
         "Convert from indexes in the unspaced list to positions in the spaced one"
-        print "lookup", idx
-        spaces = 0
-        pos = 0
+        pos = spaces = 0
+        # Normalize indexes like list[-1] etc, and save the result
         if idx < 0:
             idx = len(self) + idx
+        idx0 = idx
+        # Count the number of spaces in the spaced list before idx in the unspaced one
         while idx != -1:
             if spacey(self.spaced[pos]):
                 spaces += 1
             else:
                 idx -= 1
             pos += 1
-        return idx + spaces
+        return idx0 + spaces

From 6a938f2ee7ba48e891a783b0dd8880e0faa55883 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Fri, 24 Jun 2016 16:52:02 -0700
Subject: [PATCH 43/48] Fix docstring nit

---
 certbot-nginx/certbot_nginx/tests/nginxparser_test.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/certbot-nginx/certbot_nginx/tests/nginxparser_test.py b/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
index 4c239ac10..1707bc6d6 100644
--- a/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
+++ b/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
@@ -176,7 +176,7 @@ class TestRawNginxParser(unittest.TestCase):
         ])
 
 class TestUnspacedList(unittest.TestCase):
-    """Test the raw low-level Nginx config parser."""
+    """Test the UnspacedList data structure"""
     def setUp(self):
         self.a = ["\n    ", "things", " ", "quirk"]
         self.b = ["y", " "]

From 880cb0319164d63ee6fed7a1e6cabcd4e46fd18c Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Fri, 24 Jun 2016 18:50:18 -0700
Subject: [PATCH 44/48] Make assertions about index policing

---
 certbot-nginx/certbot_nginx/tests/nginxparser_test.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/certbot-nginx/certbot_nginx/tests/nginxparser_test.py b/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
index 1707bc6d6..a0ef82cd5 100644
--- a/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
+++ b/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
@@ -219,6 +219,10 @@ class TestUnspacedList(unittest.TestCase):
         self.assertEqual(ul3, ["zither", ["zather", "zest"]])
         self.assertEqual(ul3.spaced, [self.a[0], "zither", " ", l])
 
+    def test_get(self):
+        self.assertRaises(IndexError, self.ul2.__getitem__, 2)
+        self.assertRaises(IndexError, self.ul2.__getitem__, -3)
+
     def test_rawlists(self):
         ul3 = copy.deepcopy(self.ul)
         ul3.insert(0, "some")

From 0dc4639cbffe023520c18af375d2e909cdab0137 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Fri, 24 Jun 2016 18:56:30 -0700
Subject: [PATCH 45/48] Be more explicit about range policing

(rather than doing it in some roundabout crazy way)
---
 certbot-nginx/certbot_nginx/nginxparser.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py
index 412268d22..1de079cab 100644
--- a/certbot-nginx/certbot_nginx/nginxparser.py
+++ b/certbot-nginx/certbot_nginx/nginxparser.py
@@ -239,6 +239,8 @@ class UnspacedList(list):
         # Normalize indexes like list[-1] etc, and save the result
         if idx < 0:
             idx = len(self) + idx
+        if not 0 <= idx < len(self):
+            raise IndexError("list index out of range")
         idx0 = idx
         # Count the number of spaces in the spaced list before idx in the unspaced one
         while idx != -1:

From 98d261596c554e31346f4aec69fd8356167e851c Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Fri, 24 Jun 2016 19:03:46 -0700
Subject: [PATCH 46/48] Raise NotImplemented for all problematic list methods

---
 certbot-nginx/certbot_nginx/nginxparser.py | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py
index 1de079cab..065858797 100644
--- a/certbot-nginx/certbot_nginx/nginxparser.py
+++ b/certbot-nginx/certbot_nginx/nginxparser.py
@@ -213,6 +213,16 @@ class UnspacedList(list):
         l.extend(other)
         return l
 
+    def index(self, _, _j=0, _k=0):
+        raise NotImplementedError("UnspacedList.index() not yet implemented")
+    def pop(self, _):
+        raise NotImplementedError("UnspacedList.pop() not yet implemented")
+    def remove(self, _):
+        raise NotImplementedError("UnspacedList.remove() not yet implemented")
+    def reverse(self, _):
+        raise NotImplementedError("UnspacedList.reverse() not yet implemented")
+    def sort(self, _cmp=None, _key=None, _Rev=None):
+        raise NotImplementedError("UnspacedList.sort() not yet implemented")
     def __setslice__(self, i, j, newslice):
         raise NotImplementedError("Slice operations on UnspacedLists not yet implemented")
 

From fdbb69930b53e8032dcb390b7c8c94e7f35408ec Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 27 Jun 2016 12:00:41 -0700
Subject: [PATCH 47/48] Tweak the NotImplementedError cases

---
 certbot-nginx/certbot_nginx/nginxparser.py | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py
index 065858797..1f0569651 100644
--- a/certbot-nginx/certbot_nginx/nginxparser.py
+++ b/certbot-nginx/certbot_nginx/nginxparser.py
@@ -213,17 +213,15 @@ class UnspacedList(list):
         l.extend(other)
         return l
 
-    def index(self, _, _j=0, _k=0):
-        raise NotImplementedError("UnspacedList.index() not yet implemented")
-    def pop(self, _):
+    def pop(self, _i=0):
         raise NotImplementedError("UnspacedList.pop() not yet implemented")
     def remove(self, _):
         raise NotImplementedError("UnspacedList.remove() not yet implemented")
-    def reverse(self, _):
+    def reverse(self):
         raise NotImplementedError("UnspacedList.reverse() not yet implemented")
     def sort(self, _cmp=None, _key=None, _Rev=None):
         raise NotImplementedError("UnspacedList.sort() not yet implemented")
-    def __setslice__(self, i, j, newslice):
+    def __setslice__(self, _i, _j, _newslice):
         raise NotImplementedError("Slice operations on UnspacedLists not yet implemented")
 
     def __setitem__(self, i, value):

From 184f54cbc799039276ad2c7eb17a307b363ca9ac Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 27 Jun 2016 12:36:28 -0700
Subject: [PATCH 48/48] cosmetic improvements

---
 certbot-nginx/certbot_nginx/nginxparser.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py
index 1f0569651..ccf680dfc 100644
--- a/certbot-nginx/certbot_nginx/nginxparser.py
+++ b/certbot-nginx/certbot_nginx/nginxparser.py
@@ -41,7 +41,8 @@ class RawNginxParser(object):
     block = Forward()
 
     block << Group(
-        # key could for instance be "server" or "http"
+        # key could for instance be "server" or "http", or "location" (in which case
+        # location_statement needs to have a non-empty location)
         (Group(space + key + location_statement) ^ Group(if_statement) ^
         Group(map_statement)).leaveWhitespace() +
         left_bracket +
@@ -213,7 +214,7 @@ class UnspacedList(list):
         l.extend(other)
         return l
 
-    def pop(self, _i=0):
+    def pop(self, _i=None):
         raise NotImplementedError("UnspacedList.pop() not yet implemented")
     def remove(self, _):
         raise NotImplementedError("UnspacedList.remove() not yet implemented")