From ac2ec3457df6b699916864f26a89a1cc6fcadd9d Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 5 Apr 2016 11:33:33 -0700
Subject: [PATCH 001/104] NcursesDisplay.menu: treat ESC as cancel

Currently it will fire a weird traceback like:

File "/home/ubuntu/letsencrypt/letsencrypt/plugins/selection.py", line 113, in choose_plugin
    code, index = disp.menu(question, opts, help_label="More Info")
File "/home/ubuntu/letsencrypt/letsencrypt/display/util.py", line 129, in menu
    return code, int(index) - 1
ValueError: invalid literal for int() with base 10: ''
---
 letsencrypt/display/util.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/letsencrypt/display/util.py b/letsencrypt/display/util.py
index 20c6be156..40dd00f8b 100644
--- a/letsencrypt/display/util.py
+++ b/letsencrypt/display/util.py
@@ -123,7 +123,7 @@ class NcursesDisplay(object):
             # pylint: disable=star-args
             code, index = self.dialog.menu(message, **menu_options)
 
-            if code == CANCEL:
+            if code == CANCEL or index == "":
                 return code, -1
 
             return code, int(index) - 1

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 002/104] 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 003/104] 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 004/104] 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 005/104] 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 006/104] 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 007/104] 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 bc50ad48c6af5ff1b392dd1379a1cda6fe2d7c04 Mon Sep 17 00:00:00 2001
From: Amjad Mashaal <me@amjad.io>
Date: Fri, 10 Jun 2016 06:02:50 +0200
Subject: [PATCH 008/104] Changing default logging level

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

diff --git a/certbot/constants.py b/certbot/constants.py
index 1d4efe80e..fb278161d 100644
--- a/certbot/constants.py
+++ b/certbot/constants.py
@@ -18,7 +18,7 @@ CLI_DEFAULTS = dict(
         os.path.join(os.environ.get("XDG_CONFIG_HOME", "~/.config"),
                      "letsencrypt", "cli.ini"),
     ],
-    verbose_count=-(logging.WARNING / 10),
+    verbose_count=-(logging.INFO / 10),
     server="https://acme-v01.api.letsencrypt.org/directory",
     rsa_key_size=2048,
     rollback_checkpoints=1,

From b51280875081c868698a2a4b8603c1a9f9905926 Mon Sep 17 00:00:00 2001
From: Amjad Mashaal <me@amjad.io>
Date: Fri, 10 Jun 2016 06:15:03 +0200
Subject: [PATCH 009/104] Changing CLI logging format

---
 certbot/main.py | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/certbot/main.py b/certbot/main.py
index fa14bbf99..bb65680af 100644
--- a/certbot/main.py
+++ b/certbot/main.py
@@ -615,11 +615,12 @@ def _cli_log_handler(config, level, fmt):
 
 def setup_logging(config, cli_handler_factory, logfile):
     """Setup logging."""
-    fmt = "%(asctime)s:%(levelname)s:%(name)s:%(message)s"
+    file_fmt = "%(asctime)s:%(levelname)s:%(name)s:%(message)s"
+    cli_fmt = "%(message)s"
     level = -config.verbose_count * 10
     file_handler, log_file_path = setup_log_file_handler(
-        config, logfile=logfile, fmt=fmt)
-    cli_handler = cli_handler_factory(config, level, fmt)
+        config, logfile=logfile, fmt=file_fmt)
+    cli_handler = cli_handler_factory(config, level, cli_fmt)
 
     # TODO: use fileConfig?
 

From 7f9a8f0f089a21769aa80bfd6696cdb19f7f5fa0 Mon Sep 17 00:00:00 2001
From: Amjad Mashaal <me@amjad.io>
Date: Fri, 10 Jun 2016 06:26:33 +0200
Subject: [PATCH 010/104] Adjusting logging level of certain messages

---
 certbot/client.py   | 2 +-
 certbot/main.py     | 2 +-
 certbot/renewal.py  | 6 +++---
 certbot/reporter.py | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/certbot/client.py b/certbot/client.py
index 1dec6a1a9..81b7ccdc6 100644
--- a/certbot/client.py
+++ b/certbot/client.py
@@ -282,7 +282,7 @@ class Client(object):
                 "by your operating system package manager")
 
         if self.config.dry_run:
-            logger.info("Dry run: Skipping creating new lineage for %s",
+            logger.debug("Dry run: Skipping creating new lineage for %s",
                         domains[0])
             return None
         else:
diff --git a/certbot/main.py b/certbot/main.py
index bb65680af..c19375c32 100644
--- a/certbot/main.py
+++ b/certbot/main.py
@@ -525,7 +525,7 @@ def _csr_obtain_cert(config, le_client):
     csr, typ = config.actual_csr
     certr, chain = le_client.obtain_certificate_from_csr(config.domains, csr, typ)
     if config.dry_run:
-        logger.info(
+        logger.debug(
             "Dry run: skipping saving certificate to %s", config.cert_path)
     else:
         cert_path, _, cert_fullchain = le_client.save_certificate(
diff --git a/certbot/renewal.py b/certbot/renewal.py
index d04e2d27c..059fd6cd9 100644
--- a/certbot/renewal.py
+++ b/certbot/renewal.py
@@ -107,7 +107,7 @@ def _restore_webroot_config(config, renewalparams):
         if not cli.set_by_cli("webroot_map"):
             config.namespace.webroot_map = renewalparams["webroot_map"]
     elif "webroot_path" in renewalparams:
-        logger.info("Ancient renewal conf file without webroot-map, restoring webroot-path")
+        logger.debug("Ancient renewal conf file without webroot-map, restoring webroot-path")
         wp = renewalparams["webroot_path"]
         if isinstance(wp, str):  # prior to 0.1.0, webroot_path was a string
             wp = [wp]
@@ -193,7 +193,7 @@ def _restore_required_config_elements(config, renewalparams):
 def should_renew(config, lineage):
     "Return true if any of the circumstances for automatic renewal apply."
     if config.renew_by_default:
-        logger.info("Auto-renewal forced with --force-renewal...")
+        logger.debug("Auto-renewal forced with --force-renewal...")
         return True
     if lineage.should_autorenew(interactive=True):
         logger.info("Cert is due for renewal, auto-renewing...")
@@ -235,7 +235,7 @@ def renew_cert(config, domains, le_client, lineage):
     _avoid_invalidating_lineage(config, lineage, original_server)
     new_certr, new_chain, new_key, _ = le_client.obtain_certificate(domains)
     if config.dry_run:
-        logger.info("Dry run: skipping updating lineage at %s",
+        logger.debug("Dry run: skipping updating lineage at %s",
                     os.path.dirname(lineage.cert))
     else:
         prior_version = lineage.latest_common_version()
diff --git a/certbot/reporter.py b/certbot/reporter.py
index e07011aee..118b13166 100644
--- a/certbot/reporter.py
+++ b/certbot/reporter.py
@@ -58,7 +58,7 @@ class Reporter(object):
         """
         assert self.HIGH_PRIORITY <= priority <= self.LOW_PRIORITY
         self.messages.put(self._msg_type(priority, msg, on_crash))
-        logger.info("Reporting to user: %s", msg)
+        logger.debug("Reporting to user: %s", msg)
 
     def atexit_print_messages(self, pid=None):
         """Function to be registered with atexit to print messages.

From 8c8125c6fd120e7e8d5297ef43ba449b70e924ac Mon Sep 17 00:00:00 2001
From: Geoffroy Doucet <gd@geoffroydoucet.com>
Date: Fri, 10 Jun 2016 20:33:25 -0400
Subject: [PATCH 011/104] Added the argument --quiet and -q so then when used
 with a regular user there is no output to the screen.

---
 letsencrypt-auto-source/letsencrypt-auto          | 13 ++++++++++---
 letsencrypt-auto-source/letsencrypt-auto.template | 13 ++++++++++---
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/letsencrypt-auto-source/letsencrypt-auto b/letsencrypt-auto-source/letsencrypt-auto
index 569f6d6f3..0ef97c8c1 100755
--- a/letsencrypt-auto-source/letsencrypt-auto
+++ b/letsencrypt-auto-source/letsencrypt-auto
@@ -34,6 +34,7 @@ Help for certbot itself cannot be provided until it is installed.
   -n, --non-interactive, --noninteractive   run without asking for user input
   --no-self-upgrade                         do not download updates
   --os-packages-only                        install OS dependencies and exit
+  -q, --quiet                               provide only update/error output
   -v, --verbose                             provide more output
 
 All arguments are accepted and forwarded to the Certbot client when run."
@@ -52,15 +53,19 @@ for arg in "$@" ; do
       HELP=1;;
     --noninteractive|--non-interactive)
       ASSUME_YES=1;;
+    --quiet)
+      QUIET=1;;
     --verbose)
       VERBOSE=1;;
     -[!-]*)
-      while getopts ":hnv" short_arg $arg; do
+      while getopts ":hnvq" short_arg $arg; do
         case "$short_arg" in
           h)
             HELP=1;;
           n)
             ASSUME_YES=1;;
+          q)
+            QUIET=1;;
           v)
             VERBOSE=1;;
         esac
@@ -898,8 +903,10 @@ UNLIKELY_EOF
   fi
   if [ -n "$SUDO" ]; then
     # SUDO is su wrapper or sudo
-    echo "Requesting root privileges to run certbot..."
-    echo "  $VENV_BIN/letsencrypt" "$@"
+    if [  "$QUIET" != 1 ]; then
+      echo "Requesting root privileges to run certbot..."
+      echo "  $VENV_BIN/letsencrypt" "$@"
+    fi
   fi
   if [ -z "$SUDO_ENV" ] ; then
     # SUDO is su wrapper / noop
diff --git a/letsencrypt-auto-source/letsencrypt-auto.template b/letsencrypt-auto-source/letsencrypt-auto.template
index 73d819b4a..b254cd429 100755
--- a/letsencrypt-auto-source/letsencrypt-auto.template
+++ b/letsencrypt-auto-source/letsencrypt-auto.template
@@ -34,6 +34,7 @@ Help for certbot itself cannot be provided until it is installed.
   -n, --non-interactive, --noninteractive   run without asking for user input
   --no-self-upgrade                         do not download updates
   --os-packages-only                        install OS dependencies and exit
+  -q, --quiet                               provide only update/error output
   -v, --verbose                             provide more output
 
 All arguments are accepted and forwarded to the Certbot client when run."
@@ -52,15 +53,19 @@ for arg in "$@" ; do
       HELP=1;;
     --noninteractive|--non-interactive)
       ASSUME_YES=1;;
+    --quiet)
+      QUIET=1;;
     --verbose)
       VERBOSE=1;;
     -[!-]*)
-      while getopts ":hnv" short_arg $arg; do
+      while getopts ":hnvq" short_arg $arg; do
         case "$short_arg" in
           h)
             HELP=1;;
           n)
             ASSUME_YES=1;;
+          q)
+            QUIET=1;;
           v)
             VERBOSE=1;;
         esac
@@ -257,8 +262,10 @@ UNLIKELY_EOF
   fi
   if [ -n "$SUDO" ]; then
     # SUDO is su wrapper or sudo
-    echo "Requesting root privileges to run certbot..."
-    echo "  $VENV_BIN/letsencrypt" "$@"
+    if [  "$QUIET" != 1 ]; then
+      echo "Requesting root privileges to run certbot..."
+      echo "  $VENV_BIN/letsencrypt" "$@"
+    fi
   fi
   if [ -z "$SUDO_ENV" ] ; then
     # SUDO is su wrapper / noop

From 176751f4da939c88741c5438b0ec649e1e9cfbbf Mon Sep 17 00:00:00 2001
From: Jacob Sachs <jacob.s.sachs@gmail.com>
Date: Tue, 14 Jun 2016 23:15:55 -0400
Subject: [PATCH 012/104] Log new cert and cert renewal

---
 certbot/main.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/certbot/main.py b/certbot/main.py
index f68373998..7d6830b18 100644
--- a/certbot/main.py
+++ b/certbot/main.py
@@ -88,9 +88,11 @@ def _auth_from_domains(le_client, config, domains, lineage=None):
     hooks.pre_hook(config)
     try:
         if action == "renew":
+            logger.info("Renewing an existing certificate")
             renewal.renew_cert(config, domains, le_client, lineage)
         elif action == "newcert":
             # TREAT AS NEW REQUEST
+            logger.info("Obtaining a new certificate")
             lineage = le_client.obtain_and_enroll_certificate(domains)
             if lineage is False:
                 raise errors.Error("Certificate could not be obtained")

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 013/104] 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 014/104] 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 015/104] 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 016/104] 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 017/104] 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 018/104] 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 019/104] 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 020/104] 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 021/104] 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 022/104] 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 023/104] 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 024/104] 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 025/104] 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 026/104] 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 027/104] 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 028/104] 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 3c9f4d5fc73204ce9aa840e4eabd8b3dbd01cf74 Mon Sep 17 00:00:00 2001
From: Joona Hoikkala <joona@kuori.org>
Date: Sat, 18 Jun 2016 23:50:30 +0300
Subject: [PATCH 029/104] If port is set for any IP, do not attempt to
 autoconfigure

---
 certbot-apache/certbot_apache/configurator.py | 23 ++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py
index e4c06ba7e..392670985 100644
--- a/certbot-apache/certbot_apache/configurator.py
+++ b/certbot-apache/certbot_apache/configurator.py
@@ -624,11 +624,15 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         # Note: This could be made to also look for ip:443 combo
         listens = [self.parser.get_arg(x).split()[0] for
                    x in self.parser.find_dir("Listen")]
+
         # In case no Listens are set (which really is a broken apache config)
         if not listens:
             listens = ["80"]
-        if port in listens:
+
+        # Listen already in place
+        if self._has_port_already(listens, port):
             return
+
         for listen in listens:
             # For any listen statement, check if the machine also listens on
             # Port 443. If not, add such a listen statement.
@@ -664,6 +668,23 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
                                                    self.parser.loc["listen"])
                     listens.append("%s:%s" % (ip, port))
 
+    def _has_port_already(self, listens, port):
+        """Helper method for prepare_server_https to find out if user
+        already has an active Listen statement for the port we need
+
+        :param list listens: List of listen variables
+        :param string port: Port in question
+        """
+
+        if port in listens:
+            return True
+        # Check if Apache is already listening on a specific IP
+        for listen in listens:
+            if len(listen.split(":")) > 1:
+                # Ugly but takes care of protocol def, eg: 1.1.1.1:443 https
+                if listen.split(":")[-1].split(" ")[0] == port:
+                    return True
+
     def prepare_https_modules(self, temp):
         """Helper method for prepare_server_https, taking care of enabling
         needed modules

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 030/104] 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 031/104] 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 032/104] 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 033/104] 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 5a872b829dfb774dd66d8a769c767534a183aae5 Mon Sep 17 00:00:00 2001
From: Joona Hoikkala <joona@kuori.org>
Date: Mon, 20 Jun 2016 08:57:51 +0300
Subject: [PATCH 034/104] Added tests

---
 .../certbot_apache/tests/configurator_test.py | 31 ++++++++++++++-----
 1 file changed, 24 insertions(+), 7 deletions(-)

diff --git a/certbot-apache/certbot_apache/tests/configurator_test.py b/certbot-apache/certbot_apache/tests/configurator_test.py
index e5c09fd1d..9a034c3e0 100644
--- a/certbot-apache/certbot_apache/tests/configurator_test.py
+++ b/certbot-apache/certbot_apache/tests/configurator_test.py
@@ -497,13 +497,8 @@ class MultipleVhostsTest(util.ApacheTest):
 
         # Test Listen statements with specific ip listeed
         self.config.prepare_server_https("443")
-        # Should only be 2 here, as the third interface
-        # already listens to the correct port
-        self.assertEqual(mock_add_dir.call_count, 2)
-
-        # Check argument to new Listen statements
-        self.assertEqual(mock_add_dir.call_args_list[0][0][2], ["1.2.3.4:443"])
-        self.assertEqual(mock_add_dir.call_args_list[1][0][2], ["[::1]:443"])
+        # Should be 0 as one interface already listens to 443
+        self.assertEqual(mock_add_dir.call_count, 0)
 
         # Reset return lists and inputs
         mock_add_dir.reset_mock()
@@ -519,6 +514,28 @@ class MultipleVhostsTest(util.ApacheTest):
         self.assertEqual(mock_add_dir.call_args_list[2][0][2],
                          ["1.1.1.1:8080", "https"])
 
+        # mock_get.side_effect = ["1.2.3.4:80", "[::1]:80"]
+        # mock_find.return_value = ["test1", "test2", "test3"]
+        # self.config.parser.get_arg = mock_get
+        # self.config.prepare_server_https("8080", temp=True)
+        # self.assertEqual(self.listens, 0)
+
+    def test_prepare_server_https_needed_listen(self):
+        mock_find = mock.Mock()
+        mock_find.return_value = ["test1", "test2"]
+        mock_get = mock.Mock()
+        mock_get.side_effect = ["1.2.3.4:8080", "80"]
+        mock_add_dir = mock.Mock()
+        mock_enable = mock.Mock()
+
+        self.config.parser.find_dir = mock_find
+        self.config.parser.get_arg = mock_get
+        self.config.parser.add_dir_to_ifmodssl = mock_add_dir
+        self.config.enable_mod = mock_enable
+
+        self.config.prepare_server_https("443")
+        self.assertEqual(mock_add_dir.call_count, 1)
+
     def test_prepare_server_https_mixed_listen(self):
 
         mock_find = mock.Mock()

From 418a5d501f8d18fc3b2dbfc48bb2643aab1dd728 Mon Sep 17 00:00:00 2001
From: Joona Hoikkala <joona@kuori.org>
Date: Mon, 20 Jun 2016 08:58:22 +0300
Subject: [PATCH 035/104] Refactored adding of listen statements

---
 certbot-apache/certbot_apache/configurator.py | 72 ++++++++++++-------
 1 file changed, 47 insertions(+), 25 deletions(-)

diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py
index 2a7064190..9e404177a 100644
--- a/certbot-apache/certbot_apache/configurator.py
+++ b/certbot-apache/certbot_apache/configurator.py
@@ -625,6 +625,12 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
 
         """
 
+        # If nonstandard port, add service definition for matching
+        if port != "443":
+            port_service = "%s %s" % (port, "https")
+        else:
+            port_service = port
+
         self.prepare_https_modules(temp)
         # Check for Listen <port>
         # Note: This could be made to also look for ip:443 combo
@@ -639,40 +645,56 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         if self._has_port_already(listens, port):
             return
 
+        listen_dirs = set(listens)
+
         for listen in listens:
             # For any listen statement, check if the machine also listens on
             # Port 443. If not, add such a listen statement.
             if len(listen.split(":")) == 1:
                 # Its listening to all interfaces
-                if port not in listens:
-                    if port == "443":
-                        args = [port]
-                    else:
-                        # Non-standard ports should specify https protocol
-                        args = [port, "https"]
-                    self.parser.add_dir_to_ifmodssl(
-                        parser.get_aug_path(
-                            self.parser.loc["listen"]), "Listen", args)
-                    self.save_notes += "Added Listen %s directive to %s\n" % (
-                        port, self.parser.loc["listen"])
-                    listens.append(port)
+                if port not in listen_dirs and port_service not in listen_dirs:
+                    listen_dirs.add(port_service)
             else:
                 # The Listen statement specifies an ip
                 _, ip = listen[::-1].split(":", 1)
                 ip = ip[::-1]
-                if "%s:%s" % (ip, port) not in listens:
-                    if port == "443":
-                        args = ["%s:%s" % (ip, port)]
-                    else:
-                        # Non-standard ports should specify https protocol
-                        args = ["%s:%s" % (ip, port), "https"]
-                    self.parser.add_dir_to_ifmodssl(
-                        parser.get_aug_path(
-                            self.parser.loc["listen"]), "Listen", args)
-                    self.save_notes += ("Added Listen %s:%s directive to "
-                                        "%s\n") % (ip, port,
-                                                   self.parser.loc["listen"])
-                    listens.append("%s:%s" % (ip, port))
+                if "%s:%s" % (ip, port_service) not in listen_dirs and (
+                   "%s:%s" % (ip, port_service) not in listen_dirs):
+                    listen_dirs.add("%s:%s" % (ip, port_service))
+        self._add_listens(listen_dirs, listens, port)
+
+    def _add_listens(self, listens, listens_orig, port):
+        """Helper method for prepare_server_https to figure out which new
+        listen statements need adding
+
+        :param set listens: Set of all needed Listen statements
+        :param list listens_orig: List of existing listen statements
+        :param string port: Port number we're adding
+        """
+
+        # Add service definition for non-standard ports
+        if port != "443":
+            port_service = "%s %s" % (port, "https")
+        else:
+            port_service = port
+
+        new_listens = listens.difference(listens_orig)
+
+        if port in new_listens or port_service in new_listens:
+            # We have wildcard, skip the rest
+            self.parser.add_dir_to_ifmodssl(
+                parser.get_aug_path(self.parser.loc["listen"]),
+                "Listen", port_service.split(" "))
+            self.save_notes += "Added Listen %s directive to %s\n" % (
+                port_service, self.parser.loc["listen"])
+        else:
+            for listen in new_listens:
+                self.parser.add_dir_to_ifmodssl(
+                    parser.get_aug_path(self.parser.loc["listen"]),
+                    "Listen", listen.split(" "))
+                self.save_notes += ("Added Listen %s directive to "
+                                    "%s\n") % (listen,
+                                               self.parser.loc["listen"])
 
     def _has_port_already(self, listens, port):
         """Helper method for prepare_server_https to find out if user

From 0bfdea86d634109d63b12ac4ad593c76c04f1d80 Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Mon, 20 Jun 2016 14:32:21 -0700
Subject: [PATCH 036/104] Bump cryptography version

---
 letsencrypt-auto-source/letsencrypt-auto      | 45 ++++++++++---------
 .../pieces/letsencrypt-auto-requirements.txt  | 45 ++++++++++---------
 2 files changed, 46 insertions(+), 44 deletions(-)

diff --git a/letsencrypt-auto-source/letsencrypt-auto b/letsencrypt-auto-source/letsencrypt-auto
index 80b81c898..fa3996e9e 100755
--- a/letsencrypt-auto-source/letsencrypt-auto
+++ b/letsencrypt-auto-source/letsencrypt-auto
@@ -598,28 +598,29 @@ ConfigArgParse==0.10.0 \
     --hash=sha256:3b50a83dd58149dfcee98cb6565265d10b53e9c0a2bca7eeef7fb5f5524890a7
 configobj==5.0.6 \
     --hash=sha256:a2f5650770e1c87fb335af19a9b7eb73fc05ccf22144eb68db7d00cd2bcb0902
-cryptography==1.2.3 \
-    --hash=sha256:031938f73a5c5eb3e809e18ff7caeb6865351871417be6050cb8c86a9a202b9a \
-    --hash=sha256:a179a38d50f8d68b491d7a313db78f8cabe290842cecddddc7b34d408e59db0a \
-    --hash=sha256:906c88b2aadcf99cfabb24098263d1bf65ab0c8688acde10dae1f09d865920f1 \
-    --hash=sha256:6e706c5c6088770b1d1b634e959e21963e315b0255f5f4777125ad3d54082977 \
-    --hash=sha256:f5ebf8e31c48f8707921dca0e994de77813a9c9b9bf03c119c5ddf97bdcffe73 \
-    --hash=sha256:c7b89e42288cc7fbee3812e99ef5c744f22452e11d6822f6807afc6d6b3be83e \
-    --hash=sha256:8408d29865947109d8b68f1837a7cde1aa4dc86e0f79ca3ba58c0c44e443d6a5 \
-    --hash=sha256:c7e76cf3c3d925dd31fa238cfb806cffba718c0f08707d77a538768477969956 \
-    --hash=sha256:7d8de35380f31702758b7753bb5c40723832c73006dedb2f9099bf61a37f7287 \
-    --hash=sha256:5edbee71fae5469ee83fe0a37866b9398c8ce3a46325c24fcedfbf097bb48a19 \
-    --hash=sha256:594edafe4801c13bdc1cc305e7704a90c19617e95936f6ab457ee4ffe000ba50 \
-    --hash=sha256:b7fdb16a0a7f481be42da744bfe1ea2163025de21f90f2c688a316f3c354da9c \
-    --hash=sha256:207b8bf0fe0907336df38b733b487521cf9e138189aba9234ad54fe545dd0db8 \
-    --hash=sha256:509a2f05386270cf783993c90d49ffefb3dd62aee45bf1ea8ce3d2cde7271c21 \
-    --hash=sha256:ac69b65dd1af0179ede40c9f15788c88f73e628ea6c0519de3838e279bb388c6 \
-    --hash=sha256:8df6fad6c6ae12fd7004ea29357f0a2b4d3774eaeca7656530d08d2d90cd41aa \
-    --hash=sha256:0b8b96dd81cc1533a04f30382c0fe21c1972e189f794d0c4261a18cec08fd9b5 \
-    --hash=sha256:cae8fca1883f23c50ea78d89de6fe4fefdb4cea83177760f47177559414ded93 \
-    --hash=sha256:1a471ca576a9cdce1b1cd9f3a22b1d09ee44d46862037557de17919c0db44425 \
-    --hash=sha256:8ec4e8e3d453b3a1b63b5f57737a434dcf1ee4a2f26f6ff7c5a37c3f679104d2 \
-    --hash=sha256:8eb11c77dd8e73f48df6b2f7a7e16173fe0fe8fdfe266232832e88477e08454e
+cryptography==1.3.4 \
+    --hash=sha256:bede00edd11a2a62c8c98c271cc103fa3a3d72acf64f6e5e4eaf251128897b17 \
+    --hash=sha256:53b39e687b744bb548a98f40736cc529d9f60959b4e6cc551322cf9505d35eb3 \
+    --hash=sha256:474b73ad1139b4e423e46bbd818efd0d5c0df1c65d9f7c957d64c9215d77afde \
+    --hash=sha256:aaddf9592d5b99e32dd518bb4a25b147c124f9d6b4ad64b94f01b15d1666b8c8 \
+    --hash=sha256:6dcad2f407db8c3cd6ecd78361439c449a4f94786b46c54507e7e68f51e1709d \
+    --hash=sha256:475c153fc622e656f1f10a9c9941d0ac7ab18df7c38d35d563a437c1c0e34f24 \
+    --hash=sha256:86dd61df581cba04e89e45081efbc531faff1c9d99c77b1ce97f87216c356353 \
+    --hash=sha256:75cc697e4ef5fdd0102ca749114c6370dbd11db0c9132a18834858c2566247e3 \
+    --hash=sha256:ea03ad5b9df6d79fc9fc1ab23729e01e1c920d2974c5e3c634ccf45a5c378452 \
+    --hash=sha256:c8872b8fe4f3416d6338ab99612f49ab314f7856cb43bffab2a32d28a6267be8 \
+    --hash=sha256:468fc6e16eaec6ceaa6bc341273e6e9912d01b42b740f8cf896ace7fcd6a321d \
+    --hash=sha256:d6fea3c6502735011c5d61a62aef1c1d770fc6a2def45d9e6c0d94c9651e3317 \
+    --hash=sha256:3cf95f179f4bead3d5649b91860ef4cf60ad4244209190fc405908272576d961 \
+    --hash=sha256:141f77e60a5b9158309b2b60288c7f81d37faa15c22a69b94c190ceefaaa6236 \
+    --hash=sha256:87b7a1fe703c6424451f3372d1879dae91c7fe5e13375441a72833db76fee30e \
+    --hash=sha256:f5ee3cb0cf1a6550bf483ccffa6608db267a377b45f7e3a8201a86d1d8feb19f \
+    --hash=sha256:4e097286651ea318300af3251375d48b71b8228481c56cd617ddd4459a1ff261 \
+    --hash=sha256:1e3d3ae3f22f22d50d340f47f25227511326f3f1396c6d2446a5b45b516c4313 \
+    --hash=sha256:6a057941cb64d79834ea3cf99093fcc4787c2a5d44f686c4f297361ddc419bcd \
+    --hash=sha256:68b3d5390b92559ddd3353c73ab2dfcff758f9c4ec4f5d5226ccede0e5d779f4 \
+    --hash=sha256:545dc003b4b6081f9c3e452da15d819b04b696f49484aff64c0a2aedf766bef8 \
+    --hash=sha256:423ff890c01be7c70dbfeaa967eeef5146f1a43a5f810ffdc07b178e48a105a9
 enum34==1.1.2 \
     --hash=sha256:2475d7fcddf5951e92ff546972758802de5260bf409319a9f1934e6bbc8b1dc7 \
     --hash=sha256:35907defb0f992b75ab7788f65fedc1cf20ffa22688e0e6f6f12afc06b3ea501
diff --git a/letsencrypt-auto-source/pieces/letsencrypt-auto-requirements.txt b/letsencrypt-auto-source/pieces/letsencrypt-auto-requirements.txt
index 1291b2c96..2485491da 100644
--- a/letsencrypt-auto-source/pieces/letsencrypt-auto-requirements.txt
+++ b/letsencrypt-auto-source/pieces/letsencrypt-auto-requirements.txt
@@ -32,28 +32,29 @@ ConfigArgParse==0.10.0 \
     --hash=sha256:3b50a83dd58149dfcee98cb6565265d10b53e9c0a2bca7eeef7fb5f5524890a7
 configobj==5.0.6 \
     --hash=sha256:a2f5650770e1c87fb335af19a9b7eb73fc05ccf22144eb68db7d00cd2bcb0902
-cryptography==1.2.3 \
-    --hash=sha256:031938f73a5c5eb3e809e18ff7caeb6865351871417be6050cb8c86a9a202b9a \
-    --hash=sha256:a179a38d50f8d68b491d7a313db78f8cabe290842cecddddc7b34d408e59db0a \
-    --hash=sha256:906c88b2aadcf99cfabb24098263d1bf65ab0c8688acde10dae1f09d865920f1 \
-    --hash=sha256:6e706c5c6088770b1d1b634e959e21963e315b0255f5f4777125ad3d54082977 \
-    --hash=sha256:f5ebf8e31c48f8707921dca0e994de77813a9c9b9bf03c119c5ddf97bdcffe73 \
-    --hash=sha256:c7b89e42288cc7fbee3812e99ef5c744f22452e11d6822f6807afc6d6b3be83e \
-    --hash=sha256:8408d29865947109d8b68f1837a7cde1aa4dc86e0f79ca3ba58c0c44e443d6a5 \
-    --hash=sha256:c7e76cf3c3d925dd31fa238cfb806cffba718c0f08707d77a538768477969956 \
-    --hash=sha256:7d8de35380f31702758b7753bb5c40723832c73006dedb2f9099bf61a37f7287 \
-    --hash=sha256:5edbee71fae5469ee83fe0a37866b9398c8ce3a46325c24fcedfbf097bb48a19 \
-    --hash=sha256:594edafe4801c13bdc1cc305e7704a90c19617e95936f6ab457ee4ffe000ba50 \
-    --hash=sha256:b7fdb16a0a7f481be42da744bfe1ea2163025de21f90f2c688a316f3c354da9c \
-    --hash=sha256:207b8bf0fe0907336df38b733b487521cf9e138189aba9234ad54fe545dd0db8 \
-    --hash=sha256:509a2f05386270cf783993c90d49ffefb3dd62aee45bf1ea8ce3d2cde7271c21 \
-    --hash=sha256:ac69b65dd1af0179ede40c9f15788c88f73e628ea6c0519de3838e279bb388c6 \
-    --hash=sha256:8df6fad6c6ae12fd7004ea29357f0a2b4d3774eaeca7656530d08d2d90cd41aa \
-    --hash=sha256:0b8b96dd81cc1533a04f30382c0fe21c1972e189f794d0c4261a18cec08fd9b5 \
-    --hash=sha256:cae8fca1883f23c50ea78d89de6fe4fefdb4cea83177760f47177559414ded93 \
-    --hash=sha256:1a471ca576a9cdce1b1cd9f3a22b1d09ee44d46862037557de17919c0db44425 \
-    --hash=sha256:8ec4e8e3d453b3a1b63b5f57737a434dcf1ee4a2f26f6ff7c5a37c3f679104d2 \
-    --hash=sha256:8eb11c77dd8e73f48df6b2f7a7e16173fe0fe8fdfe266232832e88477e08454e
+cryptography==1.3.4 \
+    --hash=sha256:bede00edd11a2a62c8c98c271cc103fa3a3d72acf64f6e5e4eaf251128897b17 \
+    --hash=sha256:53b39e687b744bb548a98f40736cc529d9f60959b4e6cc551322cf9505d35eb3 \
+    --hash=sha256:474b73ad1139b4e423e46bbd818efd0d5c0df1c65d9f7c957d64c9215d77afde \
+    --hash=sha256:aaddf9592d5b99e32dd518bb4a25b147c124f9d6b4ad64b94f01b15d1666b8c8 \
+    --hash=sha256:6dcad2f407db8c3cd6ecd78361439c449a4f94786b46c54507e7e68f51e1709d \
+    --hash=sha256:475c153fc622e656f1f10a9c9941d0ac7ab18df7c38d35d563a437c1c0e34f24 \
+    --hash=sha256:86dd61df581cba04e89e45081efbc531faff1c9d99c77b1ce97f87216c356353 \
+    --hash=sha256:75cc697e4ef5fdd0102ca749114c6370dbd11db0c9132a18834858c2566247e3 \
+    --hash=sha256:ea03ad5b9df6d79fc9fc1ab23729e01e1c920d2974c5e3c634ccf45a5c378452 \
+    --hash=sha256:c8872b8fe4f3416d6338ab99612f49ab314f7856cb43bffab2a32d28a6267be8 \
+    --hash=sha256:468fc6e16eaec6ceaa6bc341273e6e9912d01b42b740f8cf896ace7fcd6a321d \
+    --hash=sha256:d6fea3c6502735011c5d61a62aef1c1d770fc6a2def45d9e6c0d94c9651e3317 \
+    --hash=sha256:3cf95f179f4bead3d5649b91860ef4cf60ad4244209190fc405908272576d961 \
+    --hash=sha256:141f77e60a5b9158309b2b60288c7f81d37faa15c22a69b94c190ceefaaa6236 \
+    --hash=sha256:87b7a1fe703c6424451f3372d1879dae91c7fe5e13375441a72833db76fee30e \
+    --hash=sha256:f5ee3cb0cf1a6550bf483ccffa6608db267a377b45f7e3a8201a86d1d8feb19f \
+    --hash=sha256:4e097286651ea318300af3251375d48b71b8228481c56cd617ddd4459a1ff261 \
+    --hash=sha256:1e3d3ae3f22f22d50d340f47f25227511326f3f1396c6d2446a5b45b516c4313 \
+    --hash=sha256:6a057941cb64d79834ea3cf99093fcc4787c2a5d44f686c4f297361ddc419bcd \
+    --hash=sha256:68b3d5390b92559ddd3353c73ab2dfcff758f9c4ec4f5d5226ccede0e5d779f4 \
+    --hash=sha256:545dc003b4b6081f9c3e452da15d819b04b696f49484aff64c0a2aedf766bef8 \
+    --hash=sha256:423ff890c01be7c70dbfeaa967eeef5146f1a43a5f810ffdc07b178e48a105a9
 enum34==1.1.2 \
     --hash=sha256:2475d7fcddf5951e92ff546972758802de5260bf409319a9f1934e6bbc8b1dc7 \
     --hash=sha256:35907defb0f992b75ab7788f65fedc1cf20ffa22688e0e6f6f12afc06b3ea501

From 8b3528969d4c0939e6f5cd4b4f77d873b2d50283 Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Mon, 20 Jun 2016 14:33:00 -0700
Subject: [PATCH 037/104] Bump pyopenssl version

---
 letsencrypt-auto-source/letsencrypt-auto                    | 6 +++---
 .../pieces/letsencrypt-auto-requirements.txt                | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/letsencrypt-auto-source/letsencrypt-auto b/letsencrypt-auto-source/letsencrypt-auto
index fa3996e9e..a25b3a70e 100755
--- a/letsencrypt-auto-source/letsencrypt-auto
+++ b/letsencrypt-auto-source/letsencrypt-auto
@@ -680,9 +680,9 @@ pyasn1==0.1.9 \
     --hash=sha256:5191ff6b9126d2c039dd87f8ff025bed274baf07fa78afa46f556b1ad7265d6e \
     --hash=sha256:8323e03637b2d072cc7041300bac6ec448c3c28950ab40376036788e9a1af629 \
     --hash=sha256:853cacd96d1f701ddd67aa03ecc05f51890135b7262e922710112f12a2ed2a7f
-pyOpenSSL==0.15.1 \
-    --hash=sha256:88e45e6bb25dfed272a1ef2e728461d44b634c2cd689e989b6e56a349c5a3ae5 \
-    --hash=sha256:f0a26070d6db0881de8bcc7846934b7c3c930d8f9c79d45883ee48984bc0d672
+pyopenssl==16.0.0 \
+    --hash=sha256:5add70cf00273bf957ca31fdb0df9b0ae4639e081897d5f86a0ae1f104901230 \
+    --hash=sha256:363d10ee43d062285facf4e465f4f5163f9f702f9134f0a5896f134cbb92d17d
 pyRFC3339==1.0 \
     --hash=sha256:eea31835c56e2096af4363a5745a784878a61d043e247d3a6d6a0a32a9741f56 \
     --hash=sha256:8dfbc6c458b8daba1c0f3620a8c78008b323a268b27b7359e92a4ae41325f535
diff --git a/letsencrypt-auto-source/pieces/letsencrypt-auto-requirements.txt b/letsencrypt-auto-source/pieces/letsencrypt-auto-requirements.txt
index 2485491da..0c642a33e 100644
--- a/letsencrypt-auto-source/pieces/letsencrypt-auto-requirements.txt
+++ b/letsencrypt-auto-source/pieces/letsencrypt-auto-requirements.txt
@@ -114,9 +114,9 @@ pyasn1==0.1.9 \
     --hash=sha256:5191ff6b9126d2c039dd87f8ff025bed274baf07fa78afa46f556b1ad7265d6e \
     --hash=sha256:8323e03637b2d072cc7041300bac6ec448c3c28950ab40376036788e9a1af629 \
     --hash=sha256:853cacd96d1f701ddd67aa03ecc05f51890135b7262e922710112f12a2ed2a7f
-pyOpenSSL==0.15.1 \
-    --hash=sha256:88e45e6bb25dfed272a1ef2e728461d44b634c2cd689e989b6e56a349c5a3ae5 \
-    --hash=sha256:f0a26070d6db0881de8bcc7846934b7c3c930d8f9c79d45883ee48984bc0d672
+pyopenssl==16.0.0 \
+    --hash=sha256:5add70cf00273bf957ca31fdb0df9b0ae4639e081897d5f86a0ae1f104901230 \
+    --hash=sha256:363d10ee43d062285facf4e465f4f5163f9f702f9134f0a5896f134cbb92d17d
 pyRFC3339==1.0 \
     --hash=sha256:eea31835c56e2096af4363a5745a784878a61d043e247d3a6d6a0a32a9741f56 \
     --hash=sha256:8dfbc6c458b8daba1c0f3620a8c78008b323a268b27b7359e92a4ae41325f535

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 038/104] 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 039/104] 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 040/104] 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 041/104] 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 042/104] 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 56488b189901c2d9f760e241586cab7517f75d07 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 20 Jun 2016 18:18:25 -0700
Subject: [PATCH 043/104] Explain the most likely cause of a missing replay
 nonce error

---
 acme/acme/errors.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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))
 
 

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 044/104] 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 045/104] 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 7ea0ce5a1774bf3fadf1e5e2313bd3190a25a80f Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 21 Jun 2016 15:34:26 -0700
Subject: [PATCH 046/104] Remove warning about nginx options file

 - This would be a security issue if we ran as setuid root at the request of
   non-privileged users, but we don't
---
 certbot-nginx/certbot_nginx/configurator.py | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/certbot-nginx/certbot_nginx/configurator.py b/certbot-nginx/certbot_nginx/configurator.py
index 30928e56c..26640a527 100644
--- a/certbot-nginx/certbot_nginx/configurator.py
+++ b/certbot-nginx/certbot_nginx/configurator.py
@@ -689,11 +689,6 @@ def nginx_restart(nginx_ctl, nginx_conf="/etc/nginx.conf"):
 
 def temp_install(options_ssl):
     """Temporary install for convenience."""
-    # WARNING: THIS IS A POTENTIAL SECURITY VULNERABILITY
-    # THIS SHOULD BE HANDLED BY THE PACKAGE MANAGER
-    # AND TAKEN OUT BEFORE RELEASE, INSTEAD
-    # SHOWING A NICE ERROR MESSAGE ABOUT THE PROBLEM.
-
     # Check to make sure options-ssl.conf is installed
     if not os.path.isfile(options_ssl):
         shutil.copyfile(constants.MOD_SSL_CONF_SRC, options_ssl)

From 1caf3e993517554d2cde2fdc4ab071bccf923c4b Mon Sep 17 00:00:00 2001
From: Dominic Cleal <dominic@cleal.org>
Date: Wed, 22 Jun 2016 09:29:37 +0100
Subject: [PATCH 047/104] Merge Augeas fix for comment line continuations

From https://github.com/hercules-team/augeas/commit/64189250e2c49442a0875fa46dfb1d76a4371bde

Fixes #2050
---
 .../certbot_apache/augeas_lens/httpd.aug      |   5 +-
 .../passing/comment-continuations-2050.conf   | 428 ++++++++++++++++++
 2 files changed, 432 insertions(+), 1 deletion(-)
 create mode 100644 certbot-apache/certbot_apache/tests/apache-conf-files/passing/comment-continuations-2050.conf

diff --git a/certbot-apache/certbot_apache/augeas_lens/httpd.aug b/certbot-apache/certbot_apache/augeas_lens/httpd.aug
index 7a5129b56..2729f4b60 100644
--- a/certbot-apache/certbot_apache/augeas_lens/httpd.aug
+++ b/certbot-apache/certbot_apache/augeas_lens/httpd.aug
@@ -52,11 +52,14 @@ let sep_eq              = del /[ \t]*=[ \t]*/ "="
 let nmtoken             = /[a-zA-Z:_][a-zA-Z0-9:_.-]*/
 let word                = /[a-z][a-z0-9._-]*/i
 
-let comment             = Util.comment
 let eol                 = Util.doseol
 let empty               = Util.empty_dos
 let indent              = Util.indent
 
+let comment_val_re      = /([^ \t\r\n](.|\\\\\r?\n)*[^ \\\t\r\n]|[^ \t\r\n])/
+let comment             = [ label "#comment" . del /[ \t]*#[ \t]*/ "# "
+                          . store comment_val_re . eol ]
+
 (* borrowed from shellvars.aug *)
 let char_arg_dir  = /([^\\ '"{\t\r\n]|[^ '"{\t\r\n]+[^\\ \t\r\n])|\\\\"|\\\\'|\\\\ /
 let char_arg_sec  = /([^\\ '"\t\r\n>]|[^ '"\t\r\n>]+[^\\ \t\r\n>])|\\\\"|\\\\'|\\\\ /
diff --git a/certbot-apache/certbot_apache/tests/apache-conf-files/passing/comment-continuations-2050.conf b/certbot-apache/certbot_apache/tests/apache-conf-files/passing/comment-continuations-2050.conf
new file mode 100644
index 000000000..48b344d8a
--- /dev/null
+++ b/certbot-apache/certbot_apache/tests/apache-conf-files/passing/comment-continuations-2050.conf
@@ -0,0 +1,428 @@
+# ---------------------------------------------------------------
+# Core ModSecurity Rule Set ver.2.2.6
+# Copyright (C) 2006-2012 Trustwave All rights reserved.
+#
+# The OWASP ModSecurity Core Rule Set is distributed under 
+# Apache Software License (ASL) version 2
+# Please see the enclosed LICENCE file for full details.
+# ---------------------------------------------------------------
+
+
+#
+# -- [[ Recommended Base Configuration ]] ------------------------------------------------- 
+#
+# The configuration directives/settings in this file are used to control
+# the OWASP ModSecurity CRS. These settings do **NOT** configure the main
+# ModSecurity settings such as:
+# 
+# - SecRuleEngine
+# - SecRequestBodyAccess
+# - SecAuditEngine
+# - SecDebugLog
+#
+# You should use the modsecurity.conf-recommended file that comes with the
+# ModSecurity source code archive.
+#
+# Ref: http://mod-security.svn.sourceforge.net/viewvc/mod-security/m2/trunk/modsecurity.conf-recommended
+#
+
+
+#
+# -- [[ Rule Version ]] -------------------------------------------------------------------
+#
+# Rule version data is added to the "Producer" line of Section H of the Audit log:
+#
+# - Producer: ModSecurity for Apache/2.7.0-rc1 (http://www.modsecurity.org/); OWASP_CRS/2.2.4.
+#
+# Ref: https://sourceforge.net/apps/mediawiki/mod-security/index.php?title=Reference_Manual#SecComponentSignature
+#
+#SecComponentSignature "OWASP_CRS/2.2.6"
+
+
+#
+# -- [[ Modes of Operation: Self-Contained vs. Collaborative Detection ]] -----------------
+#
+# Each detection rule uses the "block" action which will inherit the SecDefaultAction
+# specified below.  Your settings here will determine which mode of operation you use.
+#
+# -- [[ Self-Contained Mode ]] --
+# Rules inherit the "deny" disruptive action.  The first rule that matches will block.
+#
+# -- [[ Collaborative Detection Mode ]] --
+# This is a "delayed blocking" mode of operation where each matching rule will inherit
+# the "pass" action and will only contribute to anomaly scores.  Transactional blocking
+# can be applied 
+#
+# -- [[ Alert Logging Control ]] --
+# You have three options -
+#
+# - To log to both the Apache error_log and ModSecurity audit_log file use: "log"
+# - To log *only* to the ModSecurity audit_log file use: "nolog,auditlog"
+# - To log *only* to the Apache error_log file use: "log,noauditlog"
+#
+# Ref: http://blog.spiderlabs.com/2010/11/advanced-topic-of-the-week-traditional-vs-anomaly-scoring-detection-modes.html
+# Ref: https://sourceforge.net/apps/mediawiki/mod-security/index.php?title=Reference_Manual#SecDefaultAction
+#
+#SecDefaultAction "phase:1,deny,log"
+
+
+#
+# -- [[ Collaborative Detection Severity Levels ]] ----------------------------------------
+#
+# These are the default scoring points for each severity level.  You may
+# adjust these to you liking.  These settings will be used in macro expansion
+# in the rules to increment the anomaly scores when rules match.
+#
+# These are the default Severity ratings (with anomaly scores) of the individual rules -
+#
+#    - 2: Critical - Anomaly Score of 5.
+#         Is the highest severity level possible without correlation.  It is
+#         normally generated by the web attack rules (40 level files).
+#    - 3: Error - Anomaly Score of 4.
+#         Is generated mostly from outbound leakage rules (50 level files).
+#    - 4: Warning - Anomaly Score of 3.
+#         Is generated by malicious client rules (35 level files).
+#    - 5: Notice - Anomaly Score of 2.
+#         Is generated by the Protocol policy and anomaly files.
+#
+#SecAction \
+  "id:'900001', \
+  phase:1, \
+  t:none, \
+  setvar:tx.critical_anomaly_score=5, \
+  setvar:tx.error_anomaly_score=4, \
+  setvar:tx.warning_anomaly_score=3, \
+  setvar:tx.notice_anomaly_score=2, \
+  nolog, \
+  pass"
+
+
+#
+# -- [[ Collaborative Detection Scoring Threshold Levels ]] ------------------------------
+#
+# These variables are used in macro expansion in the 49 inbound blocking and 59
+# outbound blocking files.
+#
+# **MUST HAVE** ModSecurity v2.5.12 or higher to use macro expansion in numeric
+# operators.  If you have an earlier version, edit the 49/59 files directly to
+# set the appropriate anomaly score levels.
+#
+# You should set the score to the proper threshold you would prefer. If set to "5"
+# it will work similarly to previous Mod CRS rules and will create an event in the error_log
+# file if there are any rules that match.  If you would like to lessen the number of events
+# generated in the error_log file, you should increase the anomaly score threshold to
+# something like "20".  This would only generate an event in the error_log file if
+# there are multiple lower severity rule matches or if any 1 higher severity item matches.
+#
+#SecAction \
+  "id:'900002', \
+  phase:1, \
+  t:none, \
+  setvar:tx.inbound_anomaly_score_level=5, \
+  nolog, \
+  pass"
+
+
+#SecAction \
+  "id:'900003', \
+  phase:1, \
+  t:none, \
+  setvar:tx.outbound_anomaly_score_level=4, \
+  nolog, \
+  pass"
+
+
+# 
+# -- [[ Collaborative Detection Blocking ]] -----------------------------------------------
+#
+# This is a collaborative detection mode where each rule will increment an overall
+# anomaly score for the transaction. The scores are then evaluated in the following files:
+#
+# Inbound anomaly score - checked in the modsecurity_crs_49_inbound_blocking.conf file
+# Outbound anomaly score - checked in the modsecurity_crs_59_outbound_blocking.conf file
+#
+# If you want to use anomaly scoring mode, then uncomment this line.
+#
+#SecAction \
+  "id:'900004', \
+  phase:1, \
+  t:none, \
+  setvar:tx.anomaly_score_blocking=on, \
+  nolog, \
+  pass"
+
+
+#
+# -- [[ GeoIP Database ]] -----------------------------------------------------------------
+#
+# There are some rulesets that need to inspect the GEO data of the REMOTE_ADDR data.
+# 
+# You must first download the MaxMind GeoIP Lite City DB -
+#
+#       http://geolite.maxmind.com/download/geoip/database/GeoLiteCity.dat.gz
+#
+# You then need to define the proper path for the SecGeoLookupDb directive
+#
+# Ref: http://blog.spiderlabs.com/2010/10/detecting-malice-with-modsecurity-geolocation-data.html
+# Ref: http://blog.spiderlabs.com/2010/11/detecting-malice-with-modsecurity-ip-forensics.html
+#
+#SecGeoLookupDb /opt/modsecurity/lib/GeoLiteCity.dat
+
+#
+# -- [[ Regression Testing Mode ]] --------------------------------------------------------
+#
+# If you are going to run the regression testing mode, you should uncomment the
+# following rule. It will enable DetectionOnly mode for the SecRuleEngine and
+# will enable Response Header tagging so that the client testing script can see
+# which rule IDs have matched.
+#
+# You must specify the your source IP address where you will be running the tests
+# from.
+#
+#SecRule REMOTE_ADDR "@ipMatch 192.168.1.100" \
+  "id:'900005', \
+  phase:1, \
+  t:none, \
+  ctl:ruleEngine=DetectionOnly, \
+  setvar:tx.regression_testing=1, \
+  nolog, \
+  pass"
+
+
+#
+# -- [[ HTTP Policy Settings ]] ----------------------------------------------------------
+#
+# Set the following policy settings here and they will be propagated to the 23 rules
+# file (modsecurity_common_23_request_limits.conf) by using macro expansion.  
+# If you run into false positives, you can adjust the settings here.
+#
+# Only the max number of args is uncommented by default as there are a high rate
+# of false positives.  Uncomment the items you wish to set.
+# 
+#
+# -- Maximum number of arguments in request limited
+#SecAction \
+  "id:'900006', \
+  phase:1, \
+  t:none, \
+  setvar:tx.max_num_args=255, \
+  nolog, \
+  pass"
+
+#
+# -- Limit argument name length
+#SecAction \
+  "id:'900007', \
+  phase:1, \
+  t:none, \
+  setvar:tx.arg_name_length=100, \
+  nolog, \
+  pass"
+
+#
+# -- Limit value name length
+#SecAction \
+  "id:'900008', \
+  phase:1, \
+  t:none, \
+  setvar:tx.arg_length=400, \
+  nolog, \
+  pass"
+
+#
+# -- Limit arguments total length
+#SecAction \
+  "id:'900009', \
+  phase:1, \
+  t:none, \
+  setvar:tx.total_arg_length=64000, \
+  nolog, \
+  pass"
+
+#
+# -- Individual file size is limited
+#SecAction \
+  "id:'900010', \
+  phase:1, \
+  t:none, \
+  setvar:tx.max_file_size=1048576, \
+  nolog, \
+  pass"
+
+#
+# -- Combined file size is limited
+#SecAction \
+  "id:'900011', \
+  phase:1, \
+  t:none, \
+  setvar:tx.combined_file_sizes=1048576, \
+  nolog, \
+  pass"
+
+
+#
+# Set the following policy settings here and they will be propagated to the 30 rules
+# file (modsecurity_crs_30_http_policy.conf) by using macro expansion.  
+# If you run into false positves, you can adjust the settings here.
+#
+#SecAction \
+  "id:'900012', \
+  phase:1, \
+  t:none, \
+  setvar:'tx.allowed_methods=GET HEAD POST OPTIONS', \
+  setvar:'tx.allowed_request_content_type=application/x-www-form-urlencoded|multipart/form-data|text/xml|application/xml|application/x-amf', \
+  setvar:'tx.allowed_http_versions=HTTP/0.9 HTTP/1.0 HTTP/1.1', \
+  setvar:'tx.restricted_extensions=.asa/ .asax/ .ascx/ .axd/ .backup/ .bak/ .bat/ .cdx/ .cer/ .cfg/ .cmd/ .com/ .config/ .conf/ .cs/ .csproj/ .csr/ .dat/ .db/ .dbf/ .dll/ .dos/ .htr/ .htw/ .ida/ .idc/ .idq/ .inc/ .ini/ .key/ .licx/ .lnk/ .log/ .mdb/ .old/ .pass/ .pdb/ .pol/ .printer/ .pwd/ .resources/ .resx/ .sql/ .sys/ .vb/ .vbs/ .vbproj/ .vsdisco/ .webinfo/ .xsd/ .xsx/', \
+  setvar:'tx.restricted_headers=/Proxy-Connection/ /Lock-Token/ /Content-Range/ /Translate/ /via/ /if/', \
+  nolog, \
+  pass"
+
+
+#
+# -- [[ Content Security Policy (CSP) Settings ]] -----------------------------------------
+#
+# The purpose of these settings is to send CSP response headers to
+# Mozilla FireFox users so that you can enforce how dynamic content
+# is used. CSP usage helps to prevent XSS attacks against your users.
+#  
+# Reference Link:
+#
+#	https://developer.mozilla.org/en/Security/CSP
+#
+# Uncomment this SecAction line if you want use CSP enforcement.
+# You need to set the appropriate directives and settings for your site/domain and
+# and activate the CSP file in the experimental_rules directory.
+# 
+# Ref: http://blog.spiderlabs.com/2011/04/modsecurity-advanced-topic-of-the-week-integrating-content-security-policy-csp.html
+#
+#SecAction \
+  "id:'900013', \
+  phase:1, \
+  t:none, \
+  setvar:tx.csp_report_only=1, \
+  setvar:tx.csp_report_uri=/csp_violation_report, \
+  setenv:'csp_policy=allow \'self\'; img-src *.yoursite.com; media-src *.yoursite.com; style-src *.yoursite.com; frame-ancestors *.yoursite.com; script-src *.yoursite.com; report-uri %{tx.csp_report_uri}', \
+  nolog, \
+  pass"
+
+
+#
+# -- [[ Brute Force Protection ]] ---------------------------------------------------------
+#
+# If you are using the Brute Force Protection rule set, then uncomment the following
+# lines and set the following variables:
+# - Protected URLs: resources to protect (e.g. login pages) - set to your login page
+# - Burst Time Slice Interval: time interval window to monitor for bursts
+# - Request Threshold: request # threshold to trigger a burst
+# - Block Period: temporary block timeout
+#
+#SecAction \
+  "id:'900014', \
+  phase:1, \
+  t:none, \
+  setvar:'tx.brute_force_protected_urls=/login.jsp /partner_login.php', \
+  setvar:'tx.brute_force_burst_time_slice=60', \
+  setvar:'tx.brute_force_counter_threshold=10', \
+  setvar:'tx.brute_force_block_timeout=300', \
+  nolog, \
+  pass"
+
+
+#
+# -- [[ DoS Protection ]] ----------------------------------------------------------------
+#
+# If you are using the DoS Protection rule set, then uncomment the following
+# lines and set the following variables:
+# - Burst Time Slice Interval: time interval window to monitor for bursts
+# - Request Threshold: request # threshold to trigger a burst
+# - Block Period: temporary block timeout
+#
+#SecAction \
+  "id:'900015', \
+  phase:1, \
+  t:none, \
+  setvar:'tx.dos_burst_time_slice=60', \
+  setvar:'tx.dos_counter_threshold=100', \
+  setvar:'tx.dos_block_timeout=600', \
+  nolog, \
+  pass"
+
+
+#
+# -- [[ Check UTF enconding ]] -----------------------------------------------------------
+#
+# We only want to apply this check if UTF-8 encoding is actually used by the site, otherwise
+# it will result in false positives.
+#
+# Uncomment this line if your site uses UTF8 encoding
+#SecAction \
+  "id:'900016', \
+  phase:1, \
+  t:none, \
+  setvar:tx.crs_validate_utf8_encoding=1, \
+  nolog, \
+  pass"
+
+
+#
+# -- [[ Enable XML Body Parsing ]] -------------------------------------------------------
+#
+# The rules in this file will trigger the XML parser upon an XML request
+#
+# Initiate XML Processor in case of xml content-type
+#
+#SecRule REQUEST_HEADERS:Content-Type "text/xml" \
+  "id:'900017', \
+  phase:1, \
+  t:none,t:lowercase, \
+  nolog, \
+  pass, \
+  chain"
+	#SecRule REQBODY_PROCESSOR "!@streq XML" \
+	  "ctl:requestBodyProcessor=XML"
+
+
+#
+# -- [[ Global and IP Collections ]] -----------------------------------------------------
+#
+# Create both Global and IP collections for rules to use
+# There are some CRS rules that assume that these two collections
+# have already been initiated.
+#
+#SecRule REQUEST_HEADERS:User-Agent "^(.*)$" \
+  "id:'900018', \
+  phase:1, \
+  t:none,t:sha1,t:hexEncode, \
+  setvar:tx.ua_hash=%{matched_var}, \
+  nolog, \
+  pass"
+
+
+#SecRule REQUEST_HEADERS:x-forwarded-for "^\b(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})\b" \
+  "id:'900019', \
+  phase:1, \
+  t:none, \
+  capture, \
+  setvar:tx.real_ip=%{tx.1}, \
+  nolog, \
+  pass"
+
+
+#SecRule &TX:REAL_IP "!@eq 0" \
+  "id:'900020', \
+  phase:1, \
+  t:none, \
+  initcol:global=global, \
+  initcol:ip=%{tx.real_ip}_%{tx.ua_hash}, \
+  nolog, \
+  pass"
+
+
+#SecRule &TX:REAL_IP "@eq 0" \
+  "id:'900021', \
+  phase:1, \
+  t:none, \
+  initcol:global=global, \
+  initcol:ip=%{remote_addr}_%{tx.ua_hash}, \
+  nolog, \
+  pass"

From 24cc6b208ae7fb869d3b93ef9d06c68f07a3e848 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Wed, 22 Jun 2016 15:24:33 -0700
Subject: [PATCH 048/104] Avoid newline

---
 acme/acme/errors.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/acme/acme/errors.py b/acme/acme/errors.py
index ca2ab1874..70894a808 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}\n(This may be a service outage)'.format(
+                'nonce, headers: {1} (This may be a service outage)'.format(
                     self.response.request.method, self.response.headers))
 
 

From 8a28cb7352eb5d534e79cdd213ede8fd8283dedb Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Wed, 22 Jun 2016 15:50:21 -0700
Subject: [PATCH 049/104] Implement Brad's more systematic solution for this

---
 certbot/display/util.py            | 33 ++++++++++++++++++++++--------
 certbot/tests/display/util_test.py |  1 +
 2 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/certbot/display/util.py b/certbot/display/util.py
index 683dbc037..c998f78f3 100644
--- a/certbot/display/util.py
+++ b/certbot/display/util.py
@@ -29,7 +29,6 @@ CANCEL = "cancel"
 HELP = "help"
 """Display exit code when for when the user requests more help."""
 
-
 def _wrap_lines(msg):
     """Format lines nicely to 80 chars.
 
@@ -51,6 +50,21 @@ def _wrap_lines(msg):
 
     return os.linesep.join(fixed_l)
 
+
+def _clean(dialog_result):
+    """Work around inconsistent return codes from python-dialog.
+
+    :param tuple dialog_result: (code, result)
+    :returns: the argument but with unknown codes set to -1 (Error)
+    :rtype: tuple
+    """
+    code, result = dialog_result
+    if code in (OK, HELP):
+        return dialog_result
+    else:
+        return (CANCEL, result)
+
+
 @zope.interface.implementer(interfaces.IDisplay)
 class NcursesDisplay(object):
     """Ncurses-based display."""
@@ -92,7 +106,7 @@ class NcursesDisplay(object):
         :param dict unused_kwargs: absorbs default / cli_args
 
         :returns: tuple of the form (`code`, `index`) where
-            `code` - int display exit code
+            `code` - display exit code
             `int` - index of the selected item
         :rtype: tuple
 
@@ -111,7 +125,7 @@ class NcursesDisplay(object):
         # Can accept either tuples or just the actual choices
         if choices and isinstance(choices[0], tuple):
             # pylint: disable=star-args
-            code, selection = self.dialog.menu(message, **menu_options)
+            code, selection = _clean(self.dialog.menu(message, **menu_options))
 
             # Return the selection index
             for i, choice in enumerate(choices):
@@ -126,7 +140,7 @@ class NcursesDisplay(object):
                 (str(i), choice) for i, choice in enumerate(choices, 1)
             ]
             # pylint: disable=star-args
-            code, index = self.dialog.menu(message, **menu_options)
+            code, index = _clean(self.dialog.menu(message, **menu_options))
 
             if code == CANCEL or index == "":
                 return code, -1
@@ -140,7 +154,7 @@ class NcursesDisplay(object):
         :param dict _kwargs: absorbs default / cli_args
 
         :returns: tuple of the form (`code`, `string`) where
-            `code` - int display exit code
+            `code` - display exit code
             `string` - input entered by the user
 
         """
@@ -148,7 +162,7 @@ class NcursesDisplay(object):
         # each section takes at least one line, plus extras if it's longer than self.width
         wordlines = [1 + (len(section) / self.width) for section in sections]
         height = 6 + sum(wordlines) + len(sections)
-        return self.dialog.inputbox(message, width=self.width, height=height)
+        return _clean(self.dialog.inputbox(message, width=self.width, height=height))
 
     def yesno(self, message, yes_label="Yes", no_label="No", **unused_kwargs):
         """Display a Yes/No dialog box.
@@ -164,6 +178,7 @@ class NcursesDisplay(object):
         :rtype: bool
 
         """
+        assert OK == self.dialog.DIALOG_OK, "What kind of absurdity is this?"
         return self.dialog.DIALOG_OK == self.dialog.yesno(
             message, self.height, self.width,
             yes_label=yes_label, no_label=no_label)
@@ -179,7 +194,7 @@ class NcursesDisplay(object):
 
 
         :returns: tuple of the form (`code`, `list_tags`) where
-            `code` - int display exit code
+            `code` - display exit code
             `list_tags` - list of str tags selected by the user
 
         """
@@ -193,7 +208,7 @@ class NcursesDisplay(object):
         :param str message: prompt to give the user
 
         :returns: tuple of the form (`code`, `string`) where
-            `code` - int display exit code
+            `code` - display exit code
             `string` - input entered by the user
 
         """
@@ -355,7 +370,7 @@ class FileDisplay(object):
         :param str message: prompt to give the user
 
         :returns: tuple of the form (`code`, `string`) where
-            `code` - int display exit code
+            `code` - display exit code
             `string` - input entered by the user
 
         """
diff --git a/certbot/tests/display/util_test.py b/certbot/tests/display/util_test.py
index 4a38803d1..94338118d 100644
--- a/certbot/tests/display/util_test.py
+++ b/certbot/tests/display/util_test.py
@@ -96,6 +96,7 @@ class NcursesDisplayTest(unittest.TestCase):
     @mock.patch("certbot.display.util."
                 "dialog.Dialog.inputbox")
     def test_input(self, mock_input):
+        mock_input.return_value = (mock.MagicMock(), mock.MagicMock())
         self.displayer.input("message")
         self.assertEqual(mock_input.call_count, 1)
 

From 975599e9ca359586990c21a23f999eb93c243d51 Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Wed, 22 Jun 2016 18:25:09 -0700
Subject: [PATCH 050/104] Remove bad save in nginx perform

---
 certbot-nginx/certbot_nginx/tests/tls_sni_01_test.py | 2 +-
 certbot-nginx/certbot_nginx/tls_sni_01.py            | 2 --
 2 files changed, 1 insertion(+), 3 deletions(-)

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..a92caf788 100644
--- a/certbot-nginx/certbot_nginx/tests/tls_sni_01_test.py
+++ b/certbot-nginx/certbot_nginx/tests/tls_sni_01_test.py
@@ -80,7 +80,7 @@ class TlsSniPerformTest(util.NginxTest):
 
         mock_setup_cert.assert_called_once_with(self.achalls[0])
         self.assertEqual([response], responses)
-        self.assertEqual(mock_save.call_count, 2)
+        self.assertEqual(mock_save.call_count, 1)
 
         # Make sure challenge config is included in main config
         http = self.sni.configurator.parser.parsed[
diff --git a/certbot-nginx/certbot_nginx/tls_sni_01.py b/certbot-nginx/certbot_nginx/tls_sni_01.py
index e4c5d31a6..478810475 100644
--- a/certbot-nginx/certbot_nginx/tls_sni_01.py
+++ b/certbot-nginx/certbot_nginx/tls_sni_01.py
@@ -46,8 +46,6 @@ class NginxTlsSni01(common.TLSSNI01):
         if not self.achalls:
             return []
 
-        self.configurator.save()
-
         addresses = []
         default_addr = "{0} default_server ssl".format(
             self.configurator.config.tls_sni_01_port)

From 230b09bb15f5ba264b894b55634140e3c7002f4f Mon Sep 17 00:00:00 2001
From: Jacob Sachs <jacob.s.sachs@gmail.com>
Date: Thu, 23 Jun 2016 08:23:27 -0400
Subject: [PATCH 051/104] add logging for keeping existing cert

---
 certbot/main.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/certbot/main.py b/certbot/main.py
index 7d6830b18..90fa62fad 100644
--- a/certbot/main.py
+++ b/certbot/main.py
@@ -83,6 +83,7 @@ def _auth_from_domains(le_client, config, domains, lineage=None):
     if action == "reinstall":
         # The lineage already exists; allow the caller to try installing
         # it without getting a new certificate at all.
+        logger.info("Keeping the existing certificate")
         return lineage, "reinstall"
 
     hooks.pre_hook(config)

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 052/104] 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 053/104] 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 054/104] 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 3178a9f0f3365ef4665474a50ccab20b02ec8d91 Mon Sep 17 00:00:00 2001
From: Viktor Szakats <vszakats@users.noreply.github.com>
Date: Fri, 24 Jun 2016 00:24:53 +0200
Subject: [PATCH 055/104] cli-help: fix spelling

---
 docs/cli-help.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/cli-help.txt b/docs/cli-help.txt
index e5f1fdcb4..749983d0e 100644
--- a/docs/cli-help.txt
+++ b/docs/cli-help.txt
@@ -147,7 +147,7 @@ security:
                         HTTPS for the newly authenticated vhost. (default:
                         None)
   --hsts                Add the Strict-Transport-Security header to every HTTP
-                        response. Forcing browser to use always use SSL for
+                        response. Forcing browser to always use SSL for
                         the domain. Defends against SSL Stripping. (default:
                         False)
   --no-hsts             Do not automatically add the Strict-Transport-Security

From cb441606d52fe3fd67fe8b3f2fd1672cd645ba49 Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Thu, 23 Jun 2016 16:14:34 -0700
Subject: [PATCH 056/104] Explictly state assumptions made by certbot

---
 certbot/interfaces.py | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/certbot/interfaces.py b/certbot/interfaces.py
index 37835462e..a6f0b9735 100644
--- a/certbot/interfaces.py
+++ b/certbot/interfaces.py
@@ -180,6 +180,9 @@ class IAuthenticator(IPlugin):
     def cleanup(achalls):
         """Revert changes and shutdown after challenges complete.
 
+        This method should be able to revert all changes made by
+        perform, even if perform exited abnormally.
+
         :param list achalls: Non-empty (guaranteed) list of
             :class:`~certbot.achallenges.AnnotatedChallenge`
             instances, a subset of those previously passed to :func:`perform`.
@@ -304,8 +307,11 @@ class IInstaller(IPlugin):
 
         Both title and temporary are needed because a save may be
         intended to be permanent, but the save is not ready to be a full
-        checkpoint. If an exception is raised, it is assumed a new
-        checkpoint was not created.
+        checkpoint.
+
+        It is assumed that at most one checkpoint is finalized by this
+        method. Additionally, if an exception is raised, it is assumed a
+        new checkpoint was not finalized.
 
         :param str title: The title of the save. If a title is given, the
             configuration will be saved as a new checkpoint and put in a

From 38feb37d3ee0f9b0db689b0db2a2a9df1facd207 Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Thu, 23 Jun 2016 17:03:01 -0700
Subject: [PATCH 057/104] Further document reverter

---
 certbot/reverter.py | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/certbot/reverter.py b/certbot/reverter.py
index f8140d60d..c7eab1690 100644
--- a/certbot/reverter.py
+++ b/certbot/reverter.py
@@ -24,6 +24,39 @@ logger = logging.getLogger(__name__)
 class Reverter(object):
     """Reverter Class - save and revert configuration checkpoints.
 
+    This class can be used by the plugins, especially installers, to
+    undo changes made to the user's system. Modifications to files and
+    commands to do undo actions taken by the plugin should be registered
+    with this class before the action is taken.
+
+    Once a change has been registered with this class, there are three
+    states the change can be in. First, the change can be a temporary
+    change. This should be used for changes that will soon be reverted,
+    such as config changes for the purpose of solving a challenge.
+    Changes are added to this state through calls to
+    :func:`~add_to_temp_checkpoint` and reverted when
+    :func:`~revert_temporary_config` or :func:`~recovery_routine` is
+    called.
+
+    The second state a change can be in is in progress. These changes
+    are not temporary, however, they also have not been finalized in a
+    checkpoint. A change must become in progress before it can be
+    finalized. Changes are added to this state through calls to
+    :func:`~add_to_checkpoint` and reverted when
+    :func:`~recovery_routine` is called.
+
+    The last state a change can be in is finalized in a checkpoint. A
+    change is put into this state by first becoming an in progress
+    change and then calling :func:`~finalize_checkpoint`. Changes
+    in this state can be reverted through calls to
+    :func:`~rollback_checkpoints`.
+
+    As a final note, creating new files and registering undo commands
+    are handled specially and use the methods
+    :func:`~register_file_creation` and :func:`~register_undo_command`
+    respectively. Both of these methods can be used to create either
+    temporary or in progress changes.
+
     .. note:: Consider moving everything over to CSV format.
 
     :param config: Configuration.

From 7e5e016982a0852f4151c434890102d733eedeba Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Thu, 23 Jun 2016 17:10:03 -0700
Subject: [PATCH 058/104] Document that only save should make checkpoints

---
 certbot/interfaces.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/certbot/interfaces.py b/certbot/interfaces.py
index a6f0b9735..2bc25723c 100644
--- a/certbot/interfaces.py
+++ b/certbot/interfaces.py
@@ -241,6 +241,11 @@ class IInstaller(IPlugin):
 
     Represents any server that an X509 certificate can be placed.
 
+    It is assumed that :func:`save` is the only method that finalizes a
+    checkpoint. This is important to ensure that checkpoints are
+    restored in a consistent manner if requested by the user or in case
+    of an error.
+
     """
 
     def get_all_names():

From 7aa6becc5bffa19e85108b0286b08aa29aaf50aa Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Thu, 23 Jun 2016 17:16:58 -0700
Subject: [PATCH 059/104] Suggest reverter for installers

---
 certbot/interfaces.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/certbot/interfaces.py b/certbot/interfaces.py
index 2bc25723c..e4e62e0a2 100644
--- a/certbot/interfaces.py
+++ b/certbot/interfaces.py
@@ -246,6 +246,9 @@ class IInstaller(IPlugin):
     restored in a consistent manner if requested by the user or in case
     of an error.
 
+    Using :class:`certbot.reverter.Reverter` to implement checkpoints,
+    rollback, and recovery can dramatically simplify plugin development.
+
     """
 
     def get_all_names():

From 02c61cffb713ef3609b2dffd8e2465a95cd4fcfe Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Thu, 23 Jun 2016 17:17:17 -0700
Subject: [PATCH 060/104] Consistent capitialization with installer

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

diff --git a/certbot/reverter.py b/certbot/reverter.py
index c7eab1690..6dde05077 100644
--- a/certbot/reverter.py
+++ b/certbot/reverter.py
@@ -24,7 +24,7 @@ logger = logging.getLogger(__name__)
 class Reverter(object):
     """Reverter Class - save and revert configuration checkpoints.
 
-    This class can be used by the plugins, especially installers, to
+    This class can be used by the plugins, especially Installers, to
     undo changes made to the user's system. Modifications to files and
     commands to do undo actions taken by the plugin should be registered
     with this class before the action is taken.

From bac988a4042537a2f6429e43b18553b1d0d9f710 Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Thu, 23 Jun 2016 17:24:05 -0700
Subject: [PATCH 061/104] Fix typo

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

diff --git a/certbot/cli.py b/certbot/cli.py
index ba1f23708..35b3b74ae 100644
--- a/certbot/cli.py
+++ b/certbot/cli.py
@@ -773,7 +773,7 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
     helpful.add(
         "security", "--hsts", action="store_true",
         help="Add the Strict-Transport-Security header to every HTTP response."
-             " Forcing browser to use always use SSL for the domain."
+             " Forcing browser to always use SSL for the domain."
              " Defends against SSL Stripping.", dest="hsts", default=False)
     helpful.add(
         "security", "--no-hsts", action="store_false",

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 062/104] 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 063/104] 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 064/104] 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 065/104] 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 066/104] 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 067/104] 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 068/104] 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 069/104] 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 070/104] 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 071/104] 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 f6069c2297364dbed088265745f9fd7ede7c4402 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Fri, 24 Jun 2016 18:23:09 -0700
Subject: [PATCH 072/104] Explain what is happening when the user cancels vhost
 selection

---
 certbot-apache/certbot_apache/obj.py        | 4 ++++
 certbot-apache/certbot_apache/tls_sni_01.py | 1 +
 2 files changed, 5 insertions(+)

diff --git a/certbot-apache/certbot_apache/obj.py b/certbot-apache/certbot_apache/obj.py
index b88b22428..c1af352cb 100644
--- a/certbot-apache/certbot_apache/obj.py
+++ b/certbot-apache/certbot_apache/obj.py
@@ -6,6 +6,7 @@ from certbot.plugins import common
 
 class Addr(common.Addr):
     """Represents an Apache address."""
+
     def __eq__(self, other):
         """This is defined as equalivalent within Apache.
 
@@ -18,6 +19,9 @@ class Addr(common.Addr):
                      self.is_wildcard() and other.is_wildcard()))
         return False
 
+    def __repr__(self):
+        return "certbot_apache.obj.Addr(" + repr(self.tup) + ")"
+
     def __ne__(self, other):
         return not self.__eq__(other)
 
diff --git a/certbot-apache/certbot_apache/tls_sni_01.py b/certbot-apache/certbot_apache/tls_sni_01.py
index f14f7be0f..187ecd279 100644
--- a/certbot-apache/certbot_apache/tls_sni_01.py
+++ b/certbot-apache/certbot_apache/tls_sni_01.py
@@ -129,6 +129,7 @@ class ApacheTlsSni01(common.TLSSNI01):
             # because it's a new vhost that's not configured yet (GH #677),
             # or perhaps because there were multiple <VirtualHost> sections
             # in the config file (GH #1042).  See also GH #2600.
+            logger.warn("Attempting to fall back to default vhost %s...", default_addr)
             addrs.add(default_addr)
             return addrs
 

From e0691ede2c25ed361bee9cd7ae4e76515e65f564 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Fri, 24 Jun 2016 18:43:29 -0700
Subject: [PATCH 073/104] Provide clear log messages when Apache tries a
 default vhost

---
 certbot-apache/certbot_apache/configurator.py | 9 ++++++---
 certbot-apache/certbot_apache/display_ops.py  | 9 +++++----
 certbot-apache/certbot_apache/tls_sni_01.py   | 4 ++--
 3 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py
index 9e404177a..c51778ce5 100644
--- a/certbot-apache/certbot_apache/configurator.py
+++ b/certbot-apache/certbot_apache/configurator.py
@@ -327,9 +327,12 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         vhost = display_ops.select_vhost(target_name, self.vhosts)
         if vhost is None:
             logger.error(
-                "No vhost exists with servername or alias of: %s. "
-                "No vhost was selected. Please specify servernames "
-                "in the Apache config", target_name)
+                "No vhost exists with servername or alias of: %s "
+                "(or it's in a file with multiple files, which Certbot "
+                "can't parse yet). "
+                "No vhost was selected. Please specify ServerName or ServerAlias "
+                "in the Apache config, or split vhosts into separate files.",
+                target_name)
             raise errors.PluginError("No vhost selected")
         elif temp:
             return vhost
diff --git a/certbot-apache/certbot_apache/display_ops.py b/certbot-apache/certbot_apache/display_ops.py
index c9359e7d3..6b2964426 100644
--- a/certbot-apache/certbot_apache/display_ops.py
+++ b/certbot-apache/certbot_apache/display_ops.py
@@ -87,10 +87,11 @@ def _vhost_menu(domain, vhosts):
             "vhosts are not yet supported)".format(domain, os.linesep),
             choices, help_label="More Info", ok_label="Select")
     except errors.MissingCommandlineFlag as e:
-        msg = ("Failed to run Apache plugin non-interactively{1}{0}{1}"
-               "(The best solution is to add ServerName or ServerAlias "
-               "entries to the VirtualHost directives of your apache "
-               "configuration files.)".format(e, os.linesep))
+        msg = ("Encountered vhost ambiguity but unable to ask for user guidance in "
+               "non-interactive mode. Currently Certbot needs each vhost to be "
+               "in its own conf file, and may need vhosts to be explicitly "
+               "labelled with ServerName or ServerAlias directories.")
+        logger.warn(msg)
         raise errors.MissingCommandlineFlag(msg)
 
     return code, tag
diff --git a/certbot-apache/certbot_apache/tls_sni_01.py b/certbot-apache/certbot_apache/tls_sni_01.py
index 187ecd279..32529485b 100644
--- a/certbot-apache/certbot_apache/tls_sni_01.py
+++ b/certbot-apache/certbot_apache/tls_sni_01.py
@@ -124,12 +124,12 @@ class ApacheTlsSni01(common.TLSSNI01):
 
         try:
             vhost = self.configurator.choose_vhost(achall.domain, temp=True)
-        except (PluginError, MissingCommandlineFlag):
+        except (PluginError, MissingCommandlineFlag), e:
             # We couldn't find the virtualhost for this domain, possibly
             # because it's a new vhost that's not configured yet (GH #677),
             # or perhaps because there were multiple <VirtualHost> sections
             # in the config file (GH #1042).  See also GH #2600.
-            logger.warn("Attempting to fall back to default vhost %s...", default_addr)
+            logger.warn("Falling back to default vhost %s...", default_addr)
             addrs.add(default_addr)
             return addrs
 

From e93ace79cc58063f34cc665c96cbcaa951478ebd Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Sat, 25 Jun 2016 10:51:14 -0700
Subject: [PATCH 074/104] Test coverage & fix

---
 certbot-apache/certbot_apache/display_ops.py            | 2 +-
 certbot-apache/certbot_apache/tests/display_ops_test.py | 2 +-
 certbot-apache/certbot_apache/tests/obj_test.py         | 3 +++
 certbot-apache/certbot_apache/tls_sni_01.py             | 2 +-
 4 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/certbot-apache/certbot_apache/display_ops.py b/certbot-apache/certbot_apache/display_ops.py
index 6b2964426..24aabaed4 100644
--- a/certbot-apache/certbot_apache/display_ops.py
+++ b/certbot-apache/certbot_apache/display_ops.py
@@ -86,7 +86,7 @@ def _vhost_menu(domain, vhosts):
             "like to choose?\n(note: conf files with multiple "
             "vhosts are not yet supported)".format(domain, os.linesep),
             choices, help_label="More Info", ok_label="Select")
-    except errors.MissingCommandlineFlag as e:
+    except errors.MissingCommandlineFlag:
         msg = ("Encountered vhost ambiguity but unable to ask for user guidance in "
                "non-interactive mode. Currently Certbot needs each vhost to be "
                "in its own conf file, and may need vhosts to be explicitly "
diff --git a/certbot-apache/certbot_apache/tests/display_ops_test.py b/certbot-apache/certbot_apache/tests/display_ops_test.py
index fd1e52fde..585661c7f 100644
--- a/certbot-apache/certbot_apache/tests/display_ops_test.py
+++ b/certbot-apache/certbot_apache/tests/display_ops_test.py
@@ -38,7 +38,7 @@ class SelectVhostTest(unittest.TestCase):
         try:
             self._call(self.vhosts)
         except errors.MissingCommandlineFlag as e:
-            self.assertTrue("VirtualHost directives" in e.message)
+            self.assertTrue("vhost ambiguity" in e.message)
 
     @mock.patch("certbot_apache.display_ops.zope.component.getUtility")
     def test_more_info_cancel(self, mock_util):
diff --git a/certbot-apache/certbot_apache/tests/obj_test.py b/certbot-apache/certbot_apache/tests/obj_test.py
index 4c3d331be..10dba18bc 100644
--- a/certbot-apache/certbot_apache/tests/obj_test.py
+++ b/certbot-apache/certbot_apache/tests/obj_test.py
@@ -22,6 +22,9 @@ class VirtualHostTest(unittest.TestCase):
         self.vhost2 = VirtualHost(
             "fp", "vhp", set([self.addr2]), False, False, "localhost")
 
+    def test_repr(self):
+        self.assertEqual(repr(self.addr2), "certbot_apache.obj.Addr(('127.0.0.1', '443'))")
+
     def test_eq(self):
         self.assertTrue(self.vhost1b == self.vhost1)
         self.assertFalse(self.vhost1 == self.vhost2)
diff --git a/certbot-apache/certbot_apache/tls_sni_01.py b/certbot-apache/certbot_apache/tls_sni_01.py
index 32529485b..06d9e9ebd 100644
--- a/certbot-apache/certbot_apache/tls_sni_01.py
+++ b/certbot-apache/certbot_apache/tls_sni_01.py
@@ -124,7 +124,7 @@ class ApacheTlsSni01(common.TLSSNI01):
 
         try:
             vhost = self.configurator.choose_vhost(achall.domain, temp=True)
-        except (PluginError, MissingCommandlineFlag), e:
+        except (PluginError, MissingCommandlineFlag):
             # We couldn't find the virtualhost for this domain, possibly
             # because it's a new vhost that's not configured yet (GH #677),
             # or perhaps because there were multiple <VirtualHost> sections

From 23f0ccbc8ee7f03b094c96fd2f776919b4f72d7c Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Sat, 25 Jun 2016 12:22:45 -0700
Subject: [PATCH 075/104] Address review issues

---
 certbot/display/util.py            | 21 ++++++++++++++++-----
 certbot/tests/display/util_test.py |  2 ++
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/certbot/display/util.py b/certbot/display/util.py
index c998f78f3..4e7d45741 100644
--- a/certbot/display/util.py
+++ b/certbot/display/util.py
@@ -1,4 +1,5 @@
 """Certbot display."""
+import logging
 import os
 import textwrap
 
@@ -9,6 +10,9 @@ from certbot import interfaces
 from certbot import errors
 from certbot.display import completer
 
+
+logger = logging.getLogger(__name__)
+
 WIDTH = 72
 HEIGHT = 20
 
@@ -29,6 +33,10 @@ CANCEL = "cancel"
 HELP = "help"
 """Display exit code when for when the user requests more help."""
 
+ESC = "esc"
+"""Display exit code when the user hits Escape"""
+
+
 def _wrap_lines(msg):
     """Format lines nicely to 80 chars.
 
@@ -52,7 +60,7 @@ def _wrap_lines(msg):
 
 
 def _clean(dialog_result):
-    """Work around inconsistent return codes from python-dialog.
+    """Treat sundy python-dialog return codes as CANCEL
 
     :param tuple dialog_result: (code, result)
     :returns: the argument but with unknown codes set to -1 (Error)
@@ -61,7 +69,10 @@ def _clean(dialog_result):
     code, result = dialog_result
     if code in (OK, HELP):
         return dialog_result
+    elif code in (CANCEL, ESC):
+        return (CANCEL, result)
     else:
+        logger.info("Surprising dialog return code %s", code)
         return (CANCEL, result)
 
 
@@ -199,8 +210,8 @@ class NcursesDisplay(object):
 
         """
         choices = [(tag, "", default_status) for tag in tags]
-        return self.dialog.checklist(
-            message, width=self.width, height=self.height, choices=choices)
+        return _clean(self.dialog.checklist(
+            message, width=self.width, height=self.height, choices=choices))
 
     def directory_select(self, message, **unused_kwargs):
         """Display a directory selection screen.
@@ -213,9 +224,9 @@ class NcursesDisplay(object):
 
         """
         root_directory = os.path.abspath(os.sep)
-        return self.dialog.dselect(
+        return _clean(self.dialog.dselect(
             filepath=root_directory, width=self.width,
-            height=self.height, help_button=True, title=message)
+            height=self.height, help_button=True, title=message))
 
 
 @zope.interface.implementer(interfaces.IDisplay)
diff --git a/certbot/tests/display/util_test.py b/certbot/tests/display/util_test.py
index 94338118d..a6ced90ab 100644
--- a/certbot/tests/display/util_test.py
+++ b/certbot/tests/display/util_test.py
@@ -113,6 +113,7 @@ class NcursesDisplayTest(unittest.TestCase):
     @mock.patch("certbot.display.util."
                 "dialog.Dialog.checklist")
     def test_checklist(self, mock_checklist):
+        mock_checklist.return_value = (mock.MagicMock(), mock.MagicMock())
         self.displayer.checklist("message", TAGS)
 
         choices = [
@@ -126,6 +127,7 @@ class NcursesDisplayTest(unittest.TestCase):
 
     @mock.patch("certbot.display.util.dialog.Dialog.dselect")
     def test_directory_select(self, mock_dselect):
+        mock_dselect.return_value = (mock.MagicMock(), mock.MagicMock())
         self.displayer.directory_select("message")
         self.assertEqual(mock_dselect.call_count, 1)
 

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 076/104] 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 077/104] 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")

From 6017a6cb6daca4a0eddd7a85b874f66e26d9d78c Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Fri, 24 Jun 2016 17:14:14 -0700
Subject: [PATCH 078/104] Only write nginx config files if we've modified them

---
 certbot-nginx/certbot_nginx/nginxparser.py      | 17 +++++++++++++++--
 certbot-nginx/certbot_nginx/parser.py           |  5 ++++-
 .../certbot_nginx/tests/parser_test.py          |  2 +-
 3 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py
index ccf680dfc..816507738 100644
--- a/certbot-nginx/certbot_nginx/nginxparser.py
+++ b/certbot-nginx/certbot_nginx/nginxparser.py
@@ -163,6 +163,7 @@ class UnspacedList(list):
     def __init__(self, list_source):
         # ensure our argument is not a generator, and duplicate any sublists
         self.spaced = copy.deepcopy(list(list_source))
+        self.dirty = False
 
         # Turn self into a version of the source list that has spaces removed
         # and all sub-lists also UnspacedList()ed
@@ -198,20 +199,24 @@ class UnspacedList(list):
         item, spaced_item = self._coerce(x)
         self.spaced.insert(self._spaced_position(i), spaced_item)
         list.insert(self, i, item)
+        self.dirty = True
 
     def append(self, x):
         item, spaced_item = self._coerce(x)
         self.spaced.append(spaced_item)
         list.append(self, item)
+        self.dirty = True
 
     def extend(self, x):
         item, spaced_item = self._coerce(x)
         self.spaced.extend(spaced_item)
         list.extend(self, item)
+        self.dirty = True
 
     def __add__(self, other):
         l = copy.deepcopy(self)
         l.extend(other)
+        l.dirty = True
         return l
 
     def pop(self, _i=None):
@@ -231,16 +236,24 @@ class UnspacedList(list):
         item, spaced_item = self._coerce(value)
         self.spaced.__setitem__(self._spaced_position(i), spaced_item)
         list.__setitem__(self, i, item)
+        self.dirty = True
 
     def __delitem__(self, i):
         self.spaced.__delitem__(self._spaced_position(i))
         list.__delitem__(self, i)
+        self.dirty = True
 
-    def __deepcopy__(self, unused_memo):
+    def __deepcopy__(self, memo):
         l = UnspacedList(self[:])
-        l.spaced = copy.deepcopy(self.spaced)
+        l.spaced = copy.deepcopy(self.spaced, memo=memo)
+        l.dirty = self.dirty
         return l
 
+    def is_dirty(self):
+        """Recurse through the parse tree to figure out if any sublists are dirty"""
+        if self.dirty:
+            return True
+        return any((isinstance(x, list) and x.is_dirty() for x in self))
 
     def _spaced_position(self, idx):
         "Convert from indexes in the unspaced list to positions in the spaced one"
diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py
index f2c10ab70..e7992810a 100644
--- a/certbot-nginx/certbot_nginx/parser.py
+++ b/certbot-nginx/certbot_nginx/parser.py
@@ -205,11 +205,12 @@ class NginxParser(object):
         raise errors.NoInstallationError(
             "Could not find configuration root")
 
-    def filedump(self, ext='tmp'):
+    def filedump(self, ext='tmp', lazy=True):
         """Dumps parsed configurations into files.
 
         :param str ext: The file extension to use for the dumped files. If
             empty, this overrides the existing conf files.
+        :param bool lazy: Only write files that have been modified
 
         """
         # Best-effort atomicity is enforced above us by reverter.py
@@ -218,6 +219,8 @@ class NginxParser(object):
             if ext:
                 filename = filename + os.path.extsep + ext
             try:
+                if lazy and not tree.is_dirty():
+                    continue
                 out = nginxparser.dumps(tree)
                 logger.debug('Writing nginx conf tree to %s:\n%s', filename, out)
                 with open(filename, 'w') as _file:
diff --git a/certbot-nginx/certbot_nginx/tests/parser_test.py b/certbot-nginx/certbot_nginx/tests/parser_test.py
index 6d046178a..464d717ed 100644
--- a/certbot-nginx/certbot_nginx/tests/parser_test.py
+++ b/certbot-nginx/certbot_nginx/tests/parser_test.py
@@ -66,7 +66,7 @@ class NginxParserTest(util.NginxTest):
 
     def test_filedump(self):
         nparser = parser.NginxParser(self.config_path, self.ssl_options)
-        nparser.filedump('test')
+        nparser.filedump('test', lazy=False)
         # pylint: disable=protected-access
         parsed = nparser._parse_files(nparser.abs_path(
             'sites-enabled/example.com.test'))

From 3e9bf2f35f6fc91e7b56109d3ee81685970b2271 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Thu, 23 Jun 2016 14:03:28 -0700
Subject: [PATCH 079/104] Tests for UnspacedList.is_dirty()

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

diff --git a/certbot-nginx/certbot_nginx/tests/nginxparser_test.py b/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
index a0ef82cd5..a0a0736a5 100644
--- a/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
+++ b/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
@@ -231,6 +231,17 @@ class TestUnspacedList(unittest.TestCase):
         del ul3[2]
         self.assertEqual(ul3, ["some", "things", "why", "did", "whether"])
 
+    def test_is_dirty(self):
+        self.assertEqual(False, self.ul2.is_dirty())
+        ul3 = UnspacedList([])
+        ul3.append(self.ul)
+        self.assertEqual(False, self.ul.is_dirty())
+        self.assertEqual(True, ul3.is_dirty())
+        ul4 = UnspacedList([[1], [2, 3, 4]])
+        self.assertEqual(False, ul4.is_dirty())
+        ul4[1][2] = 5
+        self.assertEqual(True, ul4.is_dirty())
+
 
 if __name__ == '__main__':
     unittest.main()  # pragma: no cover

From bdbdd826de3ec8cecf3834e7b44848faa93b3721 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 27 Jun 2016 12:44:53 -0700
Subject: [PATCH 080/104] Extra comment & concision

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

diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py
index 816507738..d6c352296 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."""
+# Forked from https://github.com/fatiherikli/nginxparser (MIT Licensed)
 import copy
 import logging
 import string
@@ -81,8 +82,7 @@ class RawNginxDumper(object):
                 yield b.pop(0) # indentation
                 if not b:
                     continue
-            key = b.pop(0)
-            values = b.pop(0)
+            key, values = b.pop(0), b.pop(0)
 
             if isinstance(key, list):
                 yield "".join(key) + '{'
@@ -91,9 +91,9 @@ class RawNginxDumper(object):
                         yield line
                 yield '}'
             else:
-                if isinstance(key, str) and key.strip() == '#':
+                if isinstance(key, str) and key.strip() == '#':  # comment
                     yield key + values
-                else:
+                else:                                            # assignment
                     gap = ""
                     # Sometimes the parser has stuck some gap whitespace in here;
                     # if so rotate it into gap

From 7b50960ac0559e4f2e9fea20a72470127b264515 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Wed, 29 Jun 2016 13:57:58 -0700
Subject: [PATCH 081/104] Address review comments

---
 certbot-apache/certbot_apache/configurator.py | 2 +-
 certbot-apache/certbot_apache/obj.py          | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py
index c51778ce5..89d602f5f 100644
--- a/certbot-apache/certbot_apache/configurator.py
+++ b/certbot-apache/certbot_apache/configurator.py
@@ -328,7 +328,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         if vhost is None:
             logger.error(
                 "No vhost exists with servername or alias of: %s "
-                "(or it's in a file with multiple files, which Certbot "
+                "(or it's in a file with multiple vhosts, which Certbot "
                 "can't parse yet). "
                 "No vhost was selected. Please specify ServerName or ServerAlias "
                 "in the Apache config, or split vhosts into separate files.",
diff --git a/certbot-apache/certbot_apache/obj.py b/certbot-apache/certbot_apache/obj.py
index c1af352cb..c71443e92 100644
--- a/certbot-apache/certbot_apache/obj.py
+++ b/certbot-apache/certbot_apache/obj.py
@@ -19,12 +19,12 @@ class Addr(common.Addr):
                      self.is_wildcard() and other.is_wildcard()))
         return False
 
-    def __repr__(self):
-        return "certbot_apache.obj.Addr(" + repr(self.tup) + ")"
-
     def __ne__(self, other):
         return not self.__eq__(other)
 
+    def __repr__(self):
+        return "certbot_apache.obj.Addr(" + repr(self.tup) + ")"
+
     def _addr_less_specific(self, addr):
         """Returns if addr.get_addr() is more specific than self.get_addr()."""
         # pylint: disable=protected-access

From a70cf6fcf80c3c3322033d0f69f0802df91a4c9f Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Wed, 29 Jun 2016 15:22:19 -0700
Subject: [PATCH 082/104] Use correct port name

---
 docs/using.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/using.rst b/docs/using.rst
index 4e49c3eb5..fb96bb853 100644
--- a/docs/using.rst
+++ b/docs/using.rst
@@ -428,7 +428,7 @@ Operating System Packages
 
 **FreeBSD**
 
-  * Port: ``cd /usr/ports/security/py-letsencrypt && make install clean``
+  * Port: ``cd /usr/ports/security/py-certbot && make install clean``
   * Package: ``pkg install py27-letsencrypt``
 
 **OpenBSD**

From a9abc7b39e89ee26c116244e528d49931f922252 Mon Sep 17 00:00:00 2001
From: sagi <sagi@users.noreply.github.com>
Date: Fri, 1 Jul 2016 15:17:37 +0000
Subject: [PATCH 083/104] typo

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

diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py
index 89d602f5f..fdc0f37d8 100644
--- a/certbot-apache/certbot_apache/configurator.py
+++ b/certbot-apache/certbot_apache/configurator.py
@@ -874,7 +874,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
                         if self._sift_line(line):
                             if not sift:
                                 new_file.write(
-                                    "# Some rewrite rules in this file were "
+                                    "# Some rewrite rules in this file "
                                     "were disabled on your HTTPS site,\n"
                                     "# because they have the potential to "
                                     "create redirection loops.\n")

From 15ba12ed4647990d8e72f244a682c00327493443 Mon Sep 17 00:00:00 2001
From: sagi <sagi@users.noreply.github.com>
Date: Fri, 1 Jul 2016 21:06:16 +0000
Subject: [PATCH 084/104] Parsing State Machine + some tests

---
 certbot-apache/certbot_apache/configurator.py | 64 ++++++++++++++++---
 .../certbot_apache/tests/configurator_test.py | 11 ++--
 2 files changed, 61 insertions(+), 14 deletions(-)

diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py
index fdc0f37d8..23c7a0c29 100644
--- a/certbot-apache/certbot_apache/configurator.py
+++ b/certbot-apache/certbot_apache/configurator.py
@@ -819,7 +819,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         else:
             return non_ssl_vh_fp + self.conf("le_vhost_ext")
 
-    def _sift_line(self, line):
+    def _sift_rewrite_rule(self, line):
         """Decides whether a line should be copied to a SSL vhost.
 
         A canonical example of when sifting a line is required:
@@ -861,7 +861,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         A new file is created on the filesystem.
 
         """
-        # First register the creation so that it is properly removed if
+        # First register the creation so thatu it is properly removed if
         # configuration is rolled back
         self.reverter.register_file_creation(False, ssl_fp)
         sift = False
@@ -870,18 +870,62 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
             with open(avail_fp, "r") as orig_file:
                 with open(ssl_fp, "w") as new_file:
                     new_file.write("<IfModule mod_ssl.c>\n")
+
+                    comment = ("# Some rewrite rules in this file were "
+                              "disabled on your HTTPS site,\n"
+                              "# because they have the potential to create "
+                              "redirection loops.\n")
+
                     for line in orig_file:
-                        if self._sift_line(line):
+                        A = line.lstrip().startswith("RewriteCond")
+                        B = line.lstrip().startswith("RewriteRule")
+
+                        if not (A or B):
+                            new_file.write(line)
+                            continue
+
+                        # A RewriteRule that doesn't need filtering
+                        if B and not self._sift_rewrite_rule(line):
+                            new_file.write(line)
+                            continue
+
+                        # A RewriteRule that does need filtering
+                        if B and self._sift_rewrite_rule(line):
                             if not sift:
-                                new_file.write(
-                                    "# Some rewrite rules in this file "
-                                    "were disabled on your HTTPS site,\n"
-                                    "# because they have the potential to "
-                                    "create redirection loops.\n")
+                                new_file.write(comment)
                                 sift = True
                             new_file.write("# " + line)
-                        else:
-                            new_file.write(line)
+                            continue
+
+                        # We save RewriteCond(s) and their corresponding
+                        # RewriteRule in 'chunk'.
+                        # We then decide whether we comment out the entire
+                        # chunk based on its RewriteRule.
+                        chunk = []
+                        if A:
+                            chunk.append(line)
+                            line = next(orig_file)
+
+                            # RewriteCond(s) must be followed by one RewriteRule
+                            while not line.lstrip().startswith("RewriteRule"):
+                                chunk.append(line)
+                                line = next(orig_file)
+
+                            # Now, current line must start with a RewriteRule
+                            chunk.append(line)
+
+                            if self._sift_rewrite_rule(line):
+                                if not sift:
+                                    new_file.write(comment)
+                                    sift = True
+
+                                new_file.write(''.join(
+                                    ['# ' + l for l in chunk]))
+                                continue
+                            else:
+                                new_file.write(''.join(chunk))
+                                continue
+
                     new_file.write("</IfModule>\n")
         except IOError:
             logger.fatal("Error writing/reading to file in make_vhost_ssl")
diff --git a/certbot-apache/certbot_apache/tests/configurator_test.py b/certbot-apache/certbot_apache/tests/configurator_test.py
index 9a034c3e0..5a8684c9a 100644
--- a/certbot-apache/certbot_apache/tests/configurator_test.py
+++ b/certbot-apache/certbot_apache/tests/configurator_test.py
@@ -1110,16 +1110,19 @@ class MultipleVhostsTest(util.ApacheTest):
         self.config._enable_redirect(self.vh_truth[1], "")
         self.assertEqual(len(self.config.vhosts), 9)
 
-    def test_sift_line(self):
+    def test_sift_rewrite_rule(self):
         # pylint: disable=protected-access
         small_quoted_target = "RewriteRule ^ \"http://\""
-        self.assertFalse(self.config._sift_line(small_quoted_target))
+        self.assertFalse(self.config._sift_rewrite_rule(small_quoted_target))
 
         https_target = "RewriteRule ^ https://satoshi"
-        self.assertTrue(self.config._sift_line(https_target))
+        self.assertTrue(self.config._sift_rewrite_rule(https_target))
 
         normal_target = "RewriteRule ^/(.*) http://www.a.com:1234/$1 [L,R]"
-        self.assertFalse(self.config._sift_line(normal_target))
+        self.assertFalse(self.config._sift_rewrite_rule(normal_target))
+
+        not_rewriterule = "NotRewriteRule ^ ..."
+        self.assertFalse(self.config._sift_rewrite_rule(not_rewriterule))
 
     @mock.patch("certbot_apache.configurator.zope.component.getUtility")
     def test_make_vhost_ssl_with_existing_rewrite_rule(self, mock_get_utility):

From 74593607803e67818ab23b0e3f7a772ee99bc417 Mon Sep 17 00:00:00 2001
From: sagi <sagi@users.noreply.github.com>
Date: Fri, 1 Jul 2016 22:08:37 +0000
Subject: [PATCH 085/104] Add more test cases

---
 .../certbot_apache/tests/configurator_test.py | 54 +++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/certbot-apache/certbot_apache/tests/configurator_test.py b/certbot-apache/certbot_apache/tests/configurator_test.py
index 5a8684c9a..57c6a8009 100644
--- a/certbot-apache/certbot_apache/tests/configurator_test.py
+++ b/certbot-apache/certbot_apache/tests/configurator_test.py
@@ -1151,7 +1151,61 @@ class MultipleVhostsTest(util.ApacheTest):
                                   "[L,QSA,R=permanent]")
         self.assertTrue(commented_rewrite_rule in conf_text)
         mock_get_utility().add_message.assert_called_once_with(mock.ANY,
+
                                                                mock.ANY)
+    @mock.patch("certbot_apache.configurator.zope.component.getUtility")
+    def test_make_vhost_ssl_with_existing_rewrite_conds(self, mock_get_utility):
+        self.config.parser.modules.add("rewrite_module")
+
+        http_vhost = self.vh_truth[0]
+
+        self.config.parser.add_dir(
+            http_vhost.path, "RewriteEngine", "on")
+
+        # Add a chunk that should not be commented out.
+        self.config.parser.add_dir(http_vhost.path,
+                "RewriteCond", ["%{DOCUMENT_ROOT}/%{REQUEST_FILENAME}", "!-f"])
+        self.config.parser.add_dir(
+            http_vhost.path, "RewriteRule",
+            ["^(.*)$", "b://u%{REQUEST_URI}", "[P,QSA,L]"])
+
+        # Add a chunk that should be commented out.
+        self.config.parser.add_dir(http_vhost.path,
+                "RewriteCond", ["%{HTTPS}", "!=on"])
+        self.config.parser.add_dir(http_vhost.path,
+                "RewriteCond", ["%{HTTPS}", "!^$"])
+        self.config.parser.add_dir(
+            http_vhost.path, "RewriteRule",
+            ["^",
+             "https://%{SERVER_NAME}%{REQUEST_URI}",
+             "[L,QSA,R=permanent]"])
+
+        self.config.save()
+
+        ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[0])
+
+        conf_line_set = set(open(ssl_vhost.filep).read().splitlines())
+
+        not_commented_cond1 = ("RewriteCond "
+                "%{DOCUMENT_ROOT}/%{REQUEST_FILENAME} !-f")
+        not_commented_rewrite_rule = ("RewriteRule "
+            "^(.*)$ b://u%{REQUEST_URI} [P,QSA,L]")
+
+        commented_cond1 = "# RewriteCond %{HTTPS} !=on"
+        commented_cond2 = "# RewriteCond %{HTTPS} !^$"
+        commented_rewrite_rule = ("# RewriteRule ^ "
+                                  "https://%{SERVER_NAME}%{REQUEST_URI} "
+                                  "[L,QSA,R=permanent]")
+
+        self.assertTrue(not_commented_cond1 in conf_line_set)
+        self.assertTrue(not_commented_rewrite_rule in conf_line_set)
+
+        self.assertTrue(commented_cond1 in conf_line_set)
+        self.assertTrue(commented_cond2 in conf_line_set)
+        self.assertTrue(commented_rewrite_rule in conf_line_set)
+        mock_get_utility().add_message.assert_called_once_with(mock.ANY,
+                                                               mock.ANY)
+
 
     def get_achalls(self):
         """Return testing achallenges."""

From 0e9622322a89f8efbeb149d4ccf8cb33ddc19660 Mon Sep 17 00:00:00 2001
From: sagi <sagi@users.noreply.github.com>
Date: Fri, 1 Jul 2016 22:17:41 +0000
Subject: [PATCH 086/104] typo

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

diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py
index 23c7a0c29..0a24759dc 100644
--- a/certbot-apache/certbot_apache/configurator.py
+++ b/certbot-apache/certbot_apache/configurator.py
@@ -861,7 +861,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         A new file is created on the filesystem.
 
         """
-        # First register the creation so thatu it is properly removed if
+        # First register the creation so that it is properly removed if
         # configuration is rolled back
         self.reverter.register_file_creation(False, ssl_fp)
         sift = False

From 2cd4f6f008a9762db08f053084cbabd2d53c7384 Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Tue, 5 Jul 2016 14:14:31 -0700
Subject: [PATCH 087/104] update FreeBSD package name

---
 docs/using.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/using.rst b/docs/using.rst
index fb96bb853..806dfb340 100644
--- a/docs/using.rst
+++ b/docs/using.rst
@@ -429,7 +429,7 @@ Operating System Packages
 **FreeBSD**
 
   * Port: ``cd /usr/ports/security/py-certbot && make install clean``
-  * Package: ``pkg install py27-letsencrypt``
+  * Package: ``pkg install py27-certbot``
 
 **OpenBSD**
 

From fd35a1c724ae2d1501fc64ac6be945fb5c7b8786 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Wed, 6 Jul 2016 12:40:24 -0700
Subject: [PATCH 088/104] Explain why Apache [appears] not to be installed

Would help debug #3244
---
 certbot-apache/certbot_apache/configurator.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py
index 89d602f5f..d1c2b7165 100644
--- a/certbot-apache/certbot_apache/configurator.py
+++ b/certbot-apache/certbot_apache/configurator.py
@@ -157,8 +157,10 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
             raise errors.NoInstallationError("Problem in Augeas installation")
 
         # Verify Apache is installed
-        if not util.exe_exists(constants.os_constant("restart_cmd")[0]):
-            raise errors.NoInstallationError
+        restart_cmd = constants.os_constant("restart_cmd")[0]
+        if not util.exe_exists(restart_cmd):
+            raise errors.NoInstallationError(
+                'Cannot find Apache install ({0} not in PATH)'.format(restart_cmd))
 
         # Make sure configuration is valid
         self.config_test()

From 4b84538c8c9c0bb852eb2874cf38cc144dbd3f0d Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Wed, 6 Jul 2016 12:57:16 -0700
Subject: [PATCH 089/104] Address review comments

---
 certbot/display/util.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/certbot/display/util.py b/certbot/display/util.py
index 4e7d45741..39486b2bd 100644
--- a/certbot/display/util.py
+++ b/certbot/display/util.py
@@ -72,7 +72,7 @@ def _clean(dialog_result):
     elif code in (CANCEL, ESC):
         return (CANCEL, result)
     else:
-        logger.info("Surprising dialog return code %s", code)
+        logger.debug("Surprising dialog return code %s", code)
         return (CANCEL, result)
 
 
@@ -83,6 +83,7 @@ class NcursesDisplay(object):
     def __init__(self, width=WIDTH, height=HEIGHT):
         super(NcursesDisplay, self).__init__()
         self.dialog = dialog.Dialog()
+        assert OK == self.dialog.DIALOG_OK, "What kind of absurdity is this?"
         self.width = width
         self.height = height
 
@@ -189,7 +190,6 @@ class NcursesDisplay(object):
         :rtype: bool
 
         """
-        assert OK == self.dialog.DIALOG_OK, "What kind of absurdity is this?"
         return self.dialog.DIALOG_OK == self.dialog.yesno(
             message, self.height, self.width,
             yes_label=yes_label, no_label=no_label)

From 83857baf30bd6a25ce29612885666a3fdd30abf6 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Wed, 6 Jul 2016 13:25:52 -0700
Subject: [PATCH 090/104] Update / cleanup installer error message

Closes: #1756
Updating since we landed #788 and have shipped Apache support almost
everywhere
---
 certbot/plugins/selection.py | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/certbot/plugins/selection.py b/certbot/plugins/selection.py
index ac509d779..1d7fa323f 100644
--- a/certbot/plugins/selection.py
+++ b/certbot/plugins/selection.py
@@ -260,14 +260,9 @@ def diagnose_configurator_problem(cfg_type, requested, plugins):
                    "your existing configuration.\nThe error was: {1!r}"
                    .format(requested, plugins[requested].problem))
     elif cfg_type == "installer":
-        if os.path.exists("/etc/debian_version"):
-            # Debian... installers are at least possible
-            msg = ('No installers seem to be present and working on your system; '
-                   'fix that or try running certbot with the "certonly" command')
-        else:
-            # XXX update this logic as we make progress on #788 and nginx support
-            msg = ('No installers are available on your OS yet; try running '
-                   '"letsencrypt-auto certonly" to get a cert you can install manually')
+        msg = ('No installer plugins seem to be present and working on your system; '
+               'fix that or try running certbot with the "certonly" command to obtain'
+               ' a certificate you can install manually')
     else:
         msg = "{0} could not be determined or is not installed".format(cfg_type)
     raise errors.PluginSelectionError(msg)

From fd35e407ca221e145805cdb88d76dc8b094d4e2d Mon Sep 17 00:00:00 2001
From: Robert Buchholz <rbu@goodpoint.de>
Date: Thu, 7 Jul 2016 11:20:52 +0200
Subject: [PATCH 091/104] Reference certbot-auto in CLI help

---
 certbot/cli.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/certbot/cli.py b/certbot/cli.py
index 35b3b74ae..470267029 100644
--- a/certbot/cli.py
+++ b/certbot/cli.py
@@ -721,10 +721,10 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
              "(both can be renewed in parallel)")
     helpful.add(
         "automation", "--os-packages-only", action="store_true",
-        help="(letsencrypt-auto only) install OS package dependencies and then stop")
+        help="(certbot-auto only) install OS package dependencies and then stop")
     helpful.add(
         "automation", "--no-self-upgrade", action="store_true",
-        help="(letsencrypt-auto only) prevent the letsencrypt-auto script from"
+        help="(certbot-auto only) prevent the certbot-auto script from"
              " upgrading itself to newer released versions")
     helpful.add(
         "automation", "-q", "--quiet", dest="quiet", action="store_true",
@@ -737,7 +737,7 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
         "really know what you're doing!")
     helpful.add(
         "testing", "--debug", action="store_true",
-        help="Show tracebacks in case of errors, and allow letsencrypt-auto "
+        help="Show tracebacks in case of errors, and allow certbot-auto "
              "execution on experimental platforms")
     helpful.add(
         "testing", "--no-verify-ssl", action="store_true",

From 40449ed2747634ae77928cae45424e5031a66c5b Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Fri, 8 Jul 2016 12:49:41 -0700
Subject: [PATCH 092/104] Add single _PERM_ERR_FMT string

---
 certbot/main.py | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/certbot/main.py b/certbot/main.py
index be68d694e..8e47e736a 100644
--- a/certbot/main.py
+++ b/certbot/main.py
@@ -36,6 +36,12 @@ from certbot.display import util as display_util, ops as display_ops
 from certbot.plugins import disco as plugins_disco
 from certbot.plugins import selection as plug_sel
 
+
+_PERM_ERR_FMT = ("An error occurred while trying to create or modify {0}. To "
+                 "run as non-root, set --config-dir, --logs-dir, and "
+                 "--work-dir to writeable paths.")
+
+
 logger = logging.getLogger(__name__)
 
 

From 9ae755ef4cf3f8f4978d7c5594d6d16ed835a1f5 Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Fri, 8 Jul 2016 12:53:09 -0700
Subject: [PATCH 093/104] simplify log file error handling

---
 certbot/main.py | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/certbot/main.py b/certbot/main.py
index 8e47e736a..d3f90926a 100644
--- a/certbot/main.py
+++ b/certbot/main.py
@@ -2,7 +2,6 @@
 from __future__ import print_function
 import atexit
 import dialog
-import errno
 import functools
 import logging.handlers
 import os
@@ -602,13 +601,8 @@ def setup_log_file_handler(config, logfile, fmt):
     try:
         handler = logging.handlers.RotatingFileHandler(
             log_file_path, maxBytes=2 ** 20, backupCount=10)
-    except IOError as e:
-        if e.errno == errno.EACCES:
-            msg = ("Access denied writing to {0}. To run as non-root, set " +
-                "--logs-dir, --config-dir, --work-dir to writable paths.")
-            raise errors.Error(msg.format(log_file_path))
-        else:
-            raise
+    except IOError:
+        raise errors.Error(_PERM_ERR_FMT.format(log_file_path))
     # rotate on each invocation, rollover only possible when maxBytes
     # is nonzero and backupCount is nonzero, so we set maxBytes as big
     # as possible not to overrun in single CLI invocation (1MB).

From f3c6bac31065a200e1a2b79579c907279d48af1b Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Fri, 8 Jul 2016 13:02:28 -0700
Subject: [PATCH 094/104] stop spacing out

---
 certbot/tests/main_test.py | 2 --
 1 file changed, 2 deletions(-)

diff --git a/certbot/tests/main_test.py b/certbot/tests/main_test.py
index 66cba64a3..d044a50b7 100644
--- a/certbot/tests/main_test.py
+++ b/certbot/tests/main_test.py
@@ -1,10 +1,8 @@
 """Tests for certbot.main."""
 import unittest
 
-
 import mock
 
-
 from certbot import cli
 from certbot import configuration
 from certbot.plugins import disco as plugins_disco

From 4f35f3fdf7e8631282d11d3c7a854770dd02dccf Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Fri, 8 Jul 2016 13:07:49 -0700
Subject: [PATCH 095/104] Add SetupLogFileHandlerTest

---
 certbot/tests/main_test.py | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/certbot/tests/main_test.py b/certbot/tests/main_test.py
index d044a50b7..5f6723bd7 100644
--- a/certbot/tests/main_test.py
+++ b/certbot/tests/main_test.py
@@ -1,10 +1,13 @@
 """Tests for certbot.main."""
+import shutil
+import tempfile
 import unittest
 
 import mock
 
 from certbot import cli
 from certbot import configuration
+from certbot import errors
 from certbot.plugins import disco as plugins_disco
 
 
@@ -42,5 +45,26 @@ class ObtainCertTest(unittest.TestCase):
         self.assertFalse(pause)
 
 
+class SetupLogFileHandlerTest(unittest.TestCase):
+    """Tests for certbot.main.setup_log_file_handler."""
+
+    def setUp(self):
+        self.config = mock.Mock(spec_set=['logs_dir'],
+                                logs_dir=tempfile.mkdtemp())
+
+    def tearDown(self):
+        shutil.rmtree(self.config.logs_dir)
+
+    def _call(self, *args, **kwargs):
+        from certbot.main import setup_log_file_handler
+        return setup_log_file_handler(*args, **kwargs)
+
+    @mock.patch('certbot.main.logging.handlers.RotatingFileHandler')
+    def test_ioerror(self, mock_handler):
+        mock_handler.side_effect = IOError
+        self.assertRaises(errors.Error, self._call,
+                          self.config, "test.log", "%s")
+
+
 if __name__ == '__main__':
     unittest.main()  # pragma: no cover

From 754b7956b3003cc75b2b1a11e40d86a6bf6828f3 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Wed, 6 Jul 2016 15:49:22 -0700
Subject: [PATCH 096/104] Make the error even more informative

---
 certbot-apache/certbot_apache/configurator.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py
index d1c2b7165..0c95fe18e 100644
--- a/certbot-apache/certbot_apache/configurator.py
+++ b/certbot-apache/certbot_apache/configurator.py
@@ -159,8 +159,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         # Verify Apache is installed
         restart_cmd = constants.os_constant("restart_cmd")[0]
         if not util.exe_exists(restart_cmd):
+            logger.warn("Failed to find %s in PATH: %s", restart_cmd, os.environ["PATH"])
             raise errors.NoInstallationError(
-                'Cannot find Apache install ({0} not in PATH)'.format(restart_cmd))
+                'Cannot find Apache control command {0}'.format(restart_cmd))
 
         # Make sure configuration is valid
         self.config_test()

From a322f44f2b7c0ef0302de956ed068671cf4ef32f Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Thu, 7 Jul 2016 17:54:39 -0700
Subject: [PATCH 097/104] Implement PATH fallback for apachectl search

---
 certbot-apache/certbot_apache/configurator.py | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py
index 0c95fe18e..c9a00a64e 100644
--- a/certbot-apache/certbot_apache/configurator.py
+++ b/certbot-apache/certbot_apache/configurator.py
@@ -159,9 +159,16 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         # Verify Apache is installed
         restart_cmd = constants.os_constant("restart_cmd")[0]
         if not util.exe_exists(restart_cmd):
-            logger.warn("Failed to find %s in PATH: %s", restart_cmd, os.environ["PATH"])
-            raise errors.NoInstallationError(
-                'Cannot find Apache control command {0}'.format(restart_cmd))
+            # mitigate https://github.com/certbot/certbot/issues/1833
+            logger.debug("Can't find %s, attempting PATH mitigation by adding "
+                         "/usr/sbin/ and /usr/local/bin/", restart_cmd)
+            os.environ["PATH"] = os.pathsep.join((os.environ["PATH"], "/usr/sbin/", 
+                                                  "/usr/local/bin/"))
+            if not util.exe_exists(restart_cmd):
+                logger.warn("Failed to find %s in expanded PATH: %s",
+                            restart_cmd, os.environ["PATH"])
+                raise errors.NoInstallationError(
+                    'Cannot find Apache control command {0}'.format(restart_cmd))
 
         # Make sure configuration is valid
         self.config_test()

From cecac803a09c8c934f6fbe14bbe9204467d7174b Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Thu, 7 Jul 2016 18:17:45 -0700
Subject: [PATCH 098/104] Do this more cleanly

---
 certbot-apache/certbot_apache/configurator.py | 20 ++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py
index c9a00a64e..329e62135 100644
--- a/certbot-apache/certbot_apache/configurator.py
+++ b/certbot-apache/certbot_apache/configurator.py
@@ -141,6 +141,20 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         return os.path.join(self.config.config_dir,
                             constants.MOD_SSL_CONF_DEST)
 
+    def _path_surgery(self):
+        """Mitigate https://github.com/certbot/certbot/issues/1833"""
+        dirs = ("/usr/sbin/", "/usr/local/bin/", "/usr/local/sbin/")
+        path = os.environ["PATH"]
+        added = []
+        for d in dirs:
+            if d not in path:
+                path += os.pathsep + d
+                added.append(d)
+        if any(added):
+            logger.debug("Can't find %s, attempting PATH mitigation by adding %s"
+                         restart_cmd, os.pathsep.join(added))
+            os.environ["PATH"] = path
+
     def prepare(self):
         """Prepare the authenticator/installer.
 
@@ -159,11 +173,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         # Verify Apache is installed
         restart_cmd = constants.os_constant("restart_cmd")[0]
         if not util.exe_exists(restart_cmd):
-            # mitigate https://github.com/certbot/certbot/issues/1833
-            logger.debug("Can't find %s, attempting PATH mitigation by adding "
-                         "/usr/sbin/ and /usr/local/bin/", restart_cmd)
-            os.environ["PATH"] = os.pathsep.join((os.environ["PATH"], "/usr/sbin/", 
-                                                  "/usr/local/bin/"))
+            self._path_surgery()
             if not util.exe_exists(restart_cmd):
                 logger.warn("Failed to find %s in expanded PATH: %s",
                             restart_cmd, os.environ["PATH"])

From 757a8ddae7c5ac1a8acd500cda9b1f7505fc4963 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Fri, 8 Jul 2016 00:37:52 -0700
Subject: [PATCH 099/104] Fixes & tests

---
 certbot-apache/certbot_apache/configurator.py | 20 +++++++++-----
 .../certbot_apache/tests/configurator_test.py | 26 +++++++++++++++++++
 2 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py
index 329e62135..12cba34f1 100644
--- a/certbot-apache/certbot_apache/configurator.py
+++ b/certbot-apache/certbot_apache/configurator.py
@@ -141,9 +141,13 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         return os.path.join(self.config.config_dir,
                             constants.MOD_SSL_CONF_DEST)
 
-    def _path_surgery(self):
-        """Mitigate https://github.com/certbot/certbot/issues/1833"""
-        dirs = ("/usr/sbin/", "/usr/local/bin/", "/usr/local/sbin/")
+    def _path_surgery(self, restart_cmd):
+        """Mitigate https://github.com/certbot/certbot/issues/1833
+
+        :returns: " expanded" if an expansion of the PATH occurred;
+                  "" otherwise
+        """
+        dirs = ("/usr/sbin", "/usr/local/bin", "/usr/local/sbin")
         path = os.environ["PATH"]
         added = []
         for d in dirs:
@@ -151,9 +155,11 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
                 path += os.pathsep + d
                 added.append(d)
         if any(added):
-            logger.debug("Can't find %s, attempting PATH mitigation by adding %s"
+            logger.debug("Can't find %s, attempting PATH mitigation by adding %s",
                          restart_cmd, os.pathsep.join(added))
             os.environ["PATH"] = path
+            return " expanded"
+        return ""
 
     def prepare(self):
         """Prepare the authenticator/installer.
@@ -173,10 +179,10 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         # Verify Apache is installed
         restart_cmd = constants.os_constant("restart_cmd")[0]
         if not util.exe_exists(restart_cmd):
-            self._path_surgery()
+            expanded = self._path_surgery(restart_cmd)
             if not util.exe_exists(restart_cmd):
-                logger.warn("Failed to find %s in expanded PATH: %s",
-                            restart_cmd, os.environ["PATH"])
+                logger.warn("Failed to find %s in %s PATH: %s",
+                            restart_cmd, expanded, os.environ["PATH"])
                 raise errors.NoInstallationError(
                     'Cannot find Apache control command {0}'.format(restart_cmd))
 
diff --git a/certbot-apache/certbot_apache/tests/configurator_test.py b/certbot-apache/certbot_apache/tests/configurator_test.py
index 9a034c3e0..d5139912e 100644
--- a/certbot-apache/certbot_apache/tests/configurator_test.py
+++ b/certbot-apache/certbot_apache/tests/configurator_test.py
@@ -86,6 +86,32 @@ class MultipleVhostsTest(util.ApacheTest):
         self.assertRaises(
             errors.NotSupportedError, self.config.prepare)
 
+    @mock.patch("certbot_apache.configurator.logger.debug")
+    def test_path_surgery(self, mock_debug):
+        # pylint: disable=protected-access
+        all_path = {"PATH": "/usr/local/bin:/bin/:/usr/sbin/:/usr/local/sbin/"}
+        with mock.patch.dict('os.environ', all_path):
+            self.config._path_surgery("thingy")
+            self.assertEquals(mock_debug.call_count, 0)
+            self.assertEquals(os.environ["PATH"], all_path["PATH"])
+        no_path = {"PATH": "/tmp/"}
+        with mock.patch.dict('os.environ', no_path):
+            self.config._path_surgery("thingy")
+            self.assertEquals(mock_debug.call_count, 1)
+            self.assertTrue("/usr/local/bin" in os.environ["PATH"])
+            self.assertTrue("/tmp" in os.environ["PATH"])
+
+    @mock.patch("certbot_apache.configurator.ApacheConfigurator.init_augeas")
+    @mock.patch("certbot_apache.configurator.ApacheConfigurator._path_surgery")
+    @mock.patch("certbot_apache.configurator.logger.warn")
+    def test_no_install(self, mock_warn, mock_surgery, _init_augeas):
+        silly_path = {"PATH": "/tmp/nothingness2342"}
+        with mock.patch.dict('os.environ', silly_path):
+            self.assertRaises(errors.NoInstallationError, self.config.prepare)
+            self.assertEquals(mock_warn.call_count, 1)
+            self.assertEquals(mock_surgery.call_count, 1)
+            self.assertTrue("Failed to find" in mock_warn.call_args[0][0])
+
     def test_add_parser_arguments(self):  # pylint: disable=no-self-use
         from certbot_apache.configurator import ApacheConfigurator
         # Weak test..

From 0bedeb449a239e79ccba3989c12ba07b3c93e363 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Fri, 8 Jul 2016 13:58:39 -0700
Subject: [PATCH 100/104] Refactor path_surgery into plugins.util so that nginx
 can call it

---
 certbot-apache/certbot_apache/configurator.py | 25 +-----------
 .../certbot_apache/tests/configurator_test.py | 38 ++++---------------
 certbot/plugins/util.py                       | 30 +++++++++++++++
 certbot/plugins/util_test.py                  | 24 ++++++++++++
 4 files changed, 64 insertions(+), 53 deletions(-)

diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py
index 12cba34f1..74aab242e 100644
--- a/certbot-apache/certbot_apache/configurator.py
+++ b/certbot-apache/certbot_apache/configurator.py
@@ -18,6 +18,7 @@ from certbot import interfaces
 from certbot import util
 
 from certbot.plugins import common
+from certbot.plugins.util import path_surgery
 
 from certbot_apache import augeas_configurator
 from certbot_apache import constants
@@ -141,25 +142,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         return os.path.join(self.config.config_dir,
                             constants.MOD_SSL_CONF_DEST)
 
-    def _path_surgery(self, restart_cmd):
-        """Mitigate https://github.com/certbot/certbot/issues/1833
-
-        :returns: " expanded" if an expansion of the PATH occurred;
-                  "" otherwise
-        """
-        dirs = ("/usr/sbin", "/usr/local/bin", "/usr/local/sbin")
-        path = os.environ["PATH"]
-        added = []
-        for d in dirs:
-            if d not in path:
-                path += os.pathsep + d
-                added.append(d)
-        if any(added):
-            logger.debug("Can't find %s, attempting PATH mitigation by adding %s",
-                         restart_cmd, os.pathsep.join(added))
-            os.environ["PATH"] = path
-            return " expanded"
-        return ""
 
     def prepare(self):
         """Prepare the authenticator/installer.
@@ -179,10 +161,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         # Verify Apache is installed
         restart_cmd = constants.os_constant("restart_cmd")[0]
         if not util.exe_exists(restart_cmd):
-            expanded = self._path_surgery(restart_cmd)
-            if not util.exe_exists(restart_cmd):
-                logger.warn("Failed to find %s in %s PATH: %s",
-                            restart_cmd, expanded, os.environ["PATH"])
+            if not path_surgery(restart_cmd):
                 raise errors.NoInstallationError(
                     'Cannot find Apache control command {0}'.format(restart_cmd))
 
diff --git a/certbot-apache/certbot_apache/tests/configurator_test.py b/certbot-apache/certbot_apache/tests/configurator_test.py
index d5139912e..eac16c7fe 100644
--- a/certbot-apache/certbot_apache/tests/configurator_test.py
+++ b/certbot-apache/certbot_apache/tests/configurator_test.py
@@ -49,11 +49,14 @@ class MultipleVhostsTest(util.ApacheTest):
         shutil.rmtree(self.config_dir)
         shutil.rmtree(self.work_dir)
 
-    @mock.patch("certbot_apache.configurator.util.exe_exists")
-    def test_prepare_no_install(self, mock_exe_exists):
-        mock_exe_exists.return_value = False
-        self.assertRaises(
-            errors.NoInstallationError, self.config.prepare)
+    @mock.patch("certbot_apache.configurator.ApacheConfigurator.init_augeas")
+    @mock.patch("certbot_apache.configurator.path_surgery")
+    def test_prepare_no_install(self, mock_surgery, _init_augeas):
+        silly_path = {"PATH": "/tmp/nothingness2342"}
+        mock_surgery.return_value = False
+        with mock.patch.dict('os.environ', silly_path):
+            self.assertRaises(errors.NoInstallationError, self.config.prepare)
+            self.assertEquals(mock_surgery.call_count, 1)
 
     @mock.patch("certbot_apache.augeas_configurator.AugeasConfigurator.init_augeas")
     def test_prepare_no_augeas(self, mock_init_augeas):
@@ -86,31 +89,6 @@ class MultipleVhostsTest(util.ApacheTest):
         self.assertRaises(
             errors.NotSupportedError, self.config.prepare)
 
-    @mock.patch("certbot_apache.configurator.logger.debug")
-    def test_path_surgery(self, mock_debug):
-        # pylint: disable=protected-access
-        all_path = {"PATH": "/usr/local/bin:/bin/:/usr/sbin/:/usr/local/sbin/"}
-        with mock.patch.dict('os.environ', all_path):
-            self.config._path_surgery("thingy")
-            self.assertEquals(mock_debug.call_count, 0)
-            self.assertEquals(os.environ["PATH"], all_path["PATH"])
-        no_path = {"PATH": "/tmp/"}
-        with mock.patch.dict('os.environ', no_path):
-            self.config._path_surgery("thingy")
-            self.assertEquals(mock_debug.call_count, 1)
-            self.assertTrue("/usr/local/bin" in os.environ["PATH"])
-            self.assertTrue("/tmp" in os.environ["PATH"])
-
-    @mock.patch("certbot_apache.configurator.ApacheConfigurator.init_augeas")
-    @mock.patch("certbot_apache.configurator.ApacheConfigurator._path_surgery")
-    @mock.patch("certbot_apache.configurator.logger.warn")
-    def test_no_install(self, mock_warn, mock_surgery, _init_augeas):
-        silly_path = {"PATH": "/tmp/nothingness2342"}
-        with mock.patch.dict('os.environ', silly_path):
-            self.assertRaises(errors.NoInstallationError, self.config.prepare)
-            self.assertEquals(mock_warn.call_count, 1)
-            self.assertEquals(mock_surgery.call_count, 1)
-            self.assertTrue("Failed to find" in mock_warn.call_args[0][0])
 
     def test_add_parser_arguments(self):  # pylint: disable=no-self-use
         from certbot_apache.configurator import ApacheConfigurator
diff --git a/certbot/plugins/util.py b/certbot/plugins/util.py
index 5fc98dff6..cdba88a87 100644
--- a/certbot/plugins/util.py
+++ b/certbot/plugins/util.py
@@ -1,15 +1,45 @@
 """Plugin utilities."""
 import logging
+import os
 import socket
 
 import psutil
 import zope.component
 
 from certbot import interfaces
+from certbot import util
 
 
 logger = logging.getLogger(__name__)
 
+def path_surgery(restart_cmd):
+    """Attempt to perform PATH surgery to find restart_cmd
+
+    Mitigates https://github.com/certbot/certbot/issues/1833
+
+    :param str restart_cmd: the command that is being searched for in the PATH
+
+    :returns: True if the operation succeeded, False otherwise
+    """
+    dirs = ("/usr/sbin", "/usr/local/bin", "/usr/local/sbin")
+    path = os.environ["PATH"]
+    added = []
+    for d in dirs:
+        if d not in path:
+            path += os.pathsep + d
+            added.append(d)
+
+    if any(added):
+        logger.debug("Can't find %s, attempting PATH mitigation by adding %s",
+                     restart_cmd, os.pathsep.join(added))
+        os.environ["PATH"] = path
+
+    if util.exe_exists(restart_cmd):
+        return True
+    else:
+        expanded = " expanded" if any(added) else ""
+        logger.warn("Failed to find %s in%s PATH: %s", restart_cmd, expanded, path)
+        return False
 
 def already_listening(port, renewer=False):
     """Check if a process is already listening on the port.
diff --git a/certbot/plugins/util_test.py b/certbot/plugins/util_test.py
index 9bc8793c7..fa8b364d9 100644
--- a/certbot/plugins/util_test.py
+++ b/certbot/plugins/util_test.py
@@ -1,9 +1,33 @@
 """Tests for certbot.plugins.util."""
+import os
 import unittest
 
 import mock
 import psutil
 
+class PathSurgeryTest(unittest.TestCase):
+    """Tests for certbot.plugins.path_surgery."""
+
+    @mock.patch("certbot.plugins.util.logger.warn")
+    @mock.patch("certbot.plugins.util.logger.debug")
+    def test_path_surgery(self, mock_debug, mock_warn):
+        from certbot.plugins.util import path_surgery
+        all_path = {"PATH": "/usr/local/bin:/bin/:/usr/sbin/:/usr/local/sbin/"}
+        with mock.patch.dict('os.environ', all_path):
+            with mock.patch('certbot.util.exe_exists') as mock_exists:
+                mock_exists.return_value = True
+                self.assertEquals(path_surgery("eg"), True)
+                self.assertEquals(mock_debug.call_count, 0)
+                self.assertEquals(mock_warn.call_count, 0)
+                self.assertEquals(os.environ["PATH"], all_path["PATH"])
+        no_path = {"PATH": "/tmp/"}
+        with mock.patch.dict('os.environ', no_path):
+            path_surgery("thingy")
+            self.assertEquals(mock_debug.call_count, 1)
+            self.assertEquals(mock_warn.call_count, 1)
+            self.assertTrue("Failed to find" in mock_warn.call_args[0][0])
+            self.assertTrue("/usr/local/bin" in os.environ["PATH"])
+            self.assertTrue("/tmp" in os.environ["PATH"])
 
 class AlreadyListeningTest(unittest.TestCase):
     """Tests for certbot.plugins.already_listening."""

From 48b7c01a5925e476f6b4197fa34370cc07d97607 Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Fri, 8 Jul 2016 14:06:15 -0700
Subject: [PATCH 101/104] bring make_or_verify_dir docstring up to date

---
 certbot/util.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/certbot/util.py b/certbot/util.py
index 301fc669b..2b40a0f2c 100644
--- a/certbot/util.py
+++ b/certbot/util.py
@@ -95,6 +95,7 @@ def make_or_verify_dir(directory, mode=0o755, uid=0, strict=False):
     :param str directory: Path to a directory.
     :param int mode: Directory mode.
     :param int uid: Directory owner.
+    :param bool strict: require directory to be owned by current user
 
     :raises .errors.Error: if a directory already exists,
         but has wrong permissions or owner

From d7772217032235337405f8e4b62a9970f057a8fc Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Fri, 8 Jul 2016 14:17:19 -0700
Subject: [PATCH 102/104] write make_or_verify_core_dir

---
 certbot/main.py | 33 +++++++++++++++++++++++++--------
 1 file changed, 25 insertions(+), 8 deletions(-)

diff --git a/certbot/main.py b/certbot/main.py
index d3f90926a..2a18aa528 100644
--- a/certbot/main.py
+++ b/certbot/main.py
@@ -702,6 +702,23 @@ def _handle_exception(exc_type, exc_value, trace, config):
             traceback.format_exception(exc_type, exc_value, trace)))
 
 
+def make_or_verify_core_dir(directory, mode, uid, strict):
+    """Make sure directory exists with proper permissions.
+
+    :param str directory: Path to a directory.
+    :param int mode: Directory mode.
+    :param int uid: Directory owner.
+    :param bool strict: require directory to be owned by current user
+
+    :raises .errors.Error: if the directory cannot be made or verified
+
+    """
+    try:
+        util.make_or_verify_dir(directory, mode, uid, strict)
+    except OSError:
+        raise errors.Error(_PERM_ERR_FMT.format(directory))
+
+
 def main(cli_args=sys.argv[1:]):
     """Command line argument parsing and main script execution."""
     sys.excepthook = functools.partial(_handle_exception, config=None)
@@ -712,16 +729,16 @@ def main(cli_args=sys.argv[1:]):
     config = configuration.NamespaceConfig(args)
     zope.component.provideUtility(config)
 
-    # Setup logging ASAP, otherwise "No handlers could be found for
-    # logger ..." TODO: this should be done before plugins discovery
-    for directory in config.config_dir, config.work_dir:
-        util.make_or_verify_dir(
-            directory, constants.CONFIG_DIRS_MODE, os.geteuid(),
-            "--strict-permissions" in cli_args)
+    make_or_verify_core_dir(config.config_dir, constants.CONFIG_DIRS_MODE,
+                            os.geteuid(), config.strict_permissions)
+    make_or_verify_core_dir(config.work_dir, constants.CONFIG_DIRS_MODE,
+                            os.geteuid(), config.strict_permissions)
     # TODO: logs might contain sensitive data such as contents of the
     # private key! #525
-    util.make_or_verify_dir(
-        config.logs_dir, 0o700, os.geteuid(), "--strict-permissions" in cli_args)
+    make_or_verify_core_dir(config.logs_dir, 0o700,
+                            os.geteuid(), config.strict_permissions)
+    # Setup logging ASAP, otherwise "No handlers could be found for
+    # logger ..." TODO: this should be done before plugins discovery
     setup_logging(config, _cli_log_handler, logfile='letsencrypt.log')
     cli.possible_deprecation_warning(config)
 

From e598e907bdbfb69418e6909a5e0f8bc3eb1994e4 Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Fri, 8 Jul 2016 14:54:51 -0700
Subject: [PATCH 103/104] create MakeOrVerifyCoreDirTest

---
 certbot/tests/main_test.py | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/certbot/tests/main_test.py b/certbot/tests/main_test.py
index 5f6723bd7..32df525f0 100644
--- a/certbot/tests/main_test.py
+++ b/certbot/tests/main_test.py
@@ -1,4 +1,5 @@
 """Tests for certbot.main."""
+import os
 import shutil
 import tempfile
 import unittest
@@ -66,5 +67,30 @@ class SetupLogFileHandlerTest(unittest.TestCase):
                           self.config, "test.log", "%s")
 
 
+class MakeOrVerifyCoreDirTest(unittest.TestCase):
+    """Tests for certbot.main.make_or_verify_core_dir."""
+
+    def setUp(self):
+        self.dir = tempfile.mkdtemp()
+
+    def tearDown(self):
+        shutil.rmtree(self.dir)
+
+    def _call(self, *args, **kwargs):
+        from certbot.main import make_or_verify_core_dir
+        return make_or_verify_core_dir(*args, **kwargs)
+
+    def test_success(self):
+        new_dir = os.path.join(self.dir, 'new')
+        self._call(new_dir, 0o700, os.geteuid(), False)
+        self.assertTrue(os.path.exists(new_dir))
+
+    @mock.patch('certbot.main.util.make_or_verify_dir')
+    def test_failure(self, mock_make_or_verify):
+        mock_make_or_verify.side_effect = OSError
+        self.assertRaises(errors.Error, self._call,
+                          self.dir, 0o700, os.geteuid(), False)
+
+
 if __name__ == '__main__':
     unittest.main()  # pragma: no cover

From 9372914c67e189c00a4f6c4143011811b4a617d9 Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Fri, 8 Jul 2016 15:51:31 -0700
Subject: [PATCH 104/104] Improve error message

---
 certbot/main.py | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/certbot/main.py b/certbot/main.py
index 2a18aa528..8bccc524d 100644
--- a/certbot/main.py
+++ b/certbot/main.py
@@ -36,9 +36,10 @@ from certbot.plugins import disco as plugins_disco
 from certbot.plugins import selection as plug_sel
 
 
-_PERM_ERR_FMT = ("An error occurred while trying to create or modify {0}. To "
-                 "run as non-root, set --config-dir, --logs-dir, and "
-                 "--work-dir to writeable paths.")
+_PERM_ERR_FMT = os.linesep.join((
+    "The following error was encountered:", "{0}",
+    "If running as non-root, set --config-dir, "
+    "--logs-dir, and --work-dir to writeable paths."))
 
 
 logger = logging.getLogger(__name__)
@@ -601,8 +602,8 @@ def setup_log_file_handler(config, logfile, fmt):
     try:
         handler = logging.handlers.RotatingFileHandler(
             log_file_path, maxBytes=2 ** 20, backupCount=10)
-    except IOError:
-        raise errors.Error(_PERM_ERR_FMT.format(log_file_path))
+    except IOError as error:
+        raise errors.Error(_PERM_ERR_FMT.format(error))
     # rotate on each invocation, rollover only possible when maxBytes
     # is nonzero and backupCount is nonzero, so we set maxBytes as big
     # as possible not to overrun in single CLI invocation (1MB).
@@ -715,8 +716,8 @@ def make_or_verify_core_dir(directory, mode, uid, strict):
     """
     try:
         util.make_or_verify_dir(directory, mode, uid, strict)
-    except OSError:
-        raise errors.Error(_PERM_ERR_FMT.format(directory))
+    except OSError as error:
+        raise errors.Error(_PERM_ERR_FMT.format(error))
 
 
 def main(cli_args=sys.argv[1:]):