From 19f348b4166771b2ce439217d4506f94f7c9e1e6 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Thu, 19 Nov 2015 03:53:20 -0800
Subject: [PATCH 01/20] First implementation of -w for multi-webroot
 specification

* Will need tests and cleanup
---
 letsencrypt/cli.py             | 43 +++++++++++++++-------------------
 letsencrypt/plugins/webroot.py | 19 ++++++++++++++-
 2 files changed, 37 insertions(+), 25 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index bd947e191..5015e5651 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -683,31 +683,8 @@ class HelpfulArgumentParser(object):
         parsed_args = self.parser.parse_args(self.args)
         parsed_args.func = self.VERBS[self.verb]
 
-        parsed_args.domains = self._parse_domains(parsed_args.domains)
         return parsed_args
 
-    def _parse_domains(self, domains):
-        """Helper function for parse_args() that parses domains from a
-        (possibly) comma separated list and returns list of unique domains.
-
-        :param domains: List of domain flags
-        :type domains: `list` of `string`
-
-        :returns: List of unique domains
-        :rtype: `list` of `string`
-
-        """
-
-        uniqd = None
-
-        if domains:
-            dlist = []
-            for domain in domains:
-                dlist.extend([d.strip() for d in domain.split(",")])
-            # Make sure we don't have duplicates
-            uniqd = [d for i, d in enumerate(dlist) if d not in dlist[:i]]
-
-        return uniqd
 
     def determine_verb(self):
         """Determines the verb/subcommand provided by the user.
@@ -824,6 +801,7 @@ class HelpfulArgumentParser(object):
             return dict([(t, t == chosen_topic) for t in self.help_topics])
 
 
+
 def prepare_and_parse_args(plugins, args):
     """Returns parsed command line arguments.
 
@@ -861,7 +839,7 @@ def prepare_and_parse_args(plugins, args):
     #for subparser in parser_run, parser_auth, parser_install:
     #    subparser.add_argument("domains", nargs="*", metavar="domain")
     helpful.add(None, "-d", "--domains", dest="domains",
-                metavar="DOMAIN", action="append",
+                metavar="DOMAIN", action=DomainFlagProcessor,
                 help="Domain names to apply. For multiple domains you can use "
                 "multiple -d flags or enter a comma separated list of domains "
                 "as a parameter.")
@@ -1044,6 +1022,8 @@ def _plugins_parsing(helpful, plugins):
                 help='Provide laborious manual instructions for obtaining a cert')
     helpful.add("plugins", "--webroot", action="store_true",
                 help='Obtain certs by placing files in a webroot directory.')
+    #helpful.add("plugins", "-w", action=WebrootAction,
+    #            help='Obtain certs by placing files in a webroot directory.')
 
     # things should not be reorder past/pre this comment:
     # plugins_group should be displayed in --help before plugin
@@ -1051,6 +1031,21 @@ def _plugins_parsing(helpful, plugins):
 
     helpful.add_plugin_args(plugins)
 
+class DomainFlagProcessor(argparse.Action):
+    def __call__(self, parser, config, domain_arg, option_string=None):
+        """
+        Process a new -d flag, helping the webroot plugin construct a map of
+        {domain : webrootpath} if -w / --webroot-path is in use
+        """
+        if not config.domains: config.domains = []
+        new_domains = [d.strip() for d in domain_arg.split(",")
+                                  if d not in config.domains]
+        config.domains.extend(new_domains)
+
+        if config.webroot_path:
+            # Each domain has a webroot_path of the most recent -w flag
+            for d in new_domains:
+                config.webroot_map[d] = config.webroot_path[-1]
 
 def setup_log_file_handler(args, logfile, fmt):
     """Setup file debug logging."""
diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py
index 42bfe312b..421429ff0 100644
--- a/letsencrypt/plugins/webroot.py
+++ b/letsencrypt/plugins/webroot.py
@@ -38,6 +38,7 @@ Use the following snippet in your ``server{...}`` stanza::
 and reload your daemon.
 
 """
+import argparse
 import errno
 import logging
 import os
@@ -53,6 +54,22 @@ from letsencrypt.plugins import common
 
 logger = logging.getLogger(__name__)
 
+class WebrootPathProcessor(argparse.Action):
+    def __call__(self, parser, config, webroot, option_string=None):
+        """
+        Keep a record of --webroot-path / -w flags during processing, so that
+        we know which apply to which -d flags
+        """
+        if not config.webroot_path:
+            config.webroot_path = []
+            # if any --domain flags preceded the first --webroot-path flag,
+            # apply that webroot path to those; subsequent entries in
+            # config.webroot_map are filled in by cli.DomainFlagProcessor
+            if config.domains:
+                config.webroot_map = dict([(d, webroot) for d in config.domains])
+            else:
+                config.webroot_map = {}
+        config.webroot_path.append(webroot)
 
 class Authenticator(common.Plugin):
     """Webroot Authenticator."""
@@ -72,7 +89,7 @@ to serve all files under specified web root ({0})."""
 
     @classmethod
     def add_parser_arguments(cls, add):
-        add("path", help="public_html / webroot path")
+        add("path", "-w", help="public_html / webroot path", action=WebrootPathAccumulator)
 
     def get_chall_pref(self, domain):  # pragma: no cover
         # pylint: disable=missing-docstring,no-self-use,unused-argument

From e1f0fcca8f19c60a51b445cae0a880c7bb2c4daf Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Thu, 19 Nov 2015 09:57:31 -0800
Subject: [PATCH 02/20] Move --webroot-path processing into cli.py

Since it is now interdependent with --domains

(This is much more elegant than trying to APIify the interaction)
---
 letsencrypt/cli.py             | 26 ++++++++++++++++++++++++++
 letsencrypt/plugins/webroot.py | 19 ++-----------------
 2 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 5015e5651..914df7fb5 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -1022,6 +1022,12 @@ def _plugins_parsing(helpful, plugins):
                 help='Provide laborious manual instructions for obtaining a cert')
     helpful.add("plugins", "--webroot", action="store_true",
                 help='Obtain certs by placing files in a webroot directory.')
+
+    # This would normally be a flag within the webroot plugin,  the webroot
+    # plugin, but because it is parsed in conjunction with --domains, it lives
+    # here
+    helpful.add("webroot", "-w", "--webroot-path", action=WebrootPathProcessor,
+                help="public_html / webroot path")
     #helpful.add("plugins", "-w", action=WebrootAction,
     #            help='Obtain certs by placing files in a webroot directory.')
 
@@ -1031,6 +1037,25 @@ def _plugins_parsing(helpful, plugins):
 
     helpful.add_plugin_args(plugins)
 
+
+class WebrootPathProcessor(argparse.Action):
+    def __call__(self, parser, config, webroot, option_string=None):
+        """
+        Keep a record of --webroot-path / -w flags during processing, so that
+        we know which apply to which -d flags
+        """
+        if not config.webroot_path:
+            config.webroot_path = []
+            # if any --domain flags preceded the first --webroot-path flag,
+            # apply that webroot path to those; subsequent entries in
+            # config.webroot_map are filled in by cli.DomainFlagProcessor
+            if config.domains:
+                config.webroot_map = dict([(d, webroot) for d in config.domains])
+            else:
+                config.webroot_map = {}
+        config.webroot_path.append(webroot)
+
+
 class DomainFlagProcessor(argparse.Action):
     def __call__(self, parser, config, domain_arg, option_string=None):
         """
@@ -1047,6 +1072,7 @@ class DomainFlagProcessor(argparse.Action):
             for d in new_domains:
                 config.webroot_map[d] = config.webroot_path[-1]
 
+
 def setup_log_file_handler(args, logfile, fmt):
     """Setup file debug logging."""
     log_file_path = os.path.join(args.logs_dir, logfile)
diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py
index 421429ff0..33e8699a2 100644
--- a/letsencrypt/plugins/webroot.py
+++ b/letsencrypt/plugins/webroot.py
@@ -54,22 +54,7 @@ from letsencrypt.plugins import common
 
 logger = logging.getLogger(__name__)
 
-class WebrootPathProcessor(argparse.Action):
-    def __call__(self, parser, config, webroot, option_string=None):
-        """
-        Keep a record of --webroot-path / -w flags during processing, so that
-        we know which apply to which -d flags
-        """
-        if not config.webroot_path:
-            config.webroot_path = []
-            # if any --domain flags preceded the first --webroot-path flag,
-            # apply that webroot path to those; subsequent entries in
-            # config.webroot_map are filled in by cli.DomainFlagProcessor
-            if config.domains:
-                config.webroot_map = dict([(d, webroot) for d in config.domains])
-            else:
-                config.webroot_map = {}
-        config.webroot_path.append(webroot)
+
 
 class Authenticator(common.Plugin):
     """Webroot Authenticator."""
@@ -89,7 +74,7 @@ to serve all files under specified web root ({0})."""
 
     @classmethod
     def add_parser_arguments(cls, add):
-        add("path", "-w", help="public_html / webroot path", action=WebrootPathAccumulator)
+        pass
 
     def get_chall_pref(self, domain):  # pragma: no cover
         # pylint: disable=missing-docstring,no-self-use,unused-argument

From ffe6226edc17cf9657136cc178f569f5eb97b6d5 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Thu, 19 Nov 2015 09:58:35 -0800
Subject: [PATCH 03/20] Switch webroot.prepare() to use config.webroot_map

---
 letsencrypt/plugins/webroot.py | 33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py
index 33e8699a2..5ad438a5f 100644
--- a/letsencrypt/plugins/webroot.py
+++ b/letsencrypt/plugins/webroot.py
@@ -85,24 +85,27 @@ to serve all files under specified web root ({0})."""
         self.full_root = None
 
     def prepare(self):  # pylint: disable=missing-docstring
-        path = self.conf("path")
-        if path is None:
+        path_map = self.conf("map")
+        self.full_roots = {}
+
+        if not path_map:
             raise errors.PluginError("--{0} must be set".format(
                 self.option_name("path")))
-        if not os.path.isdir(path):
-            raise errors.PluginError(
-                path + " does not exist or is not a directory")
-        self.full_root = os.path.join(path, challenges.HTTP01.URI_ROOT_PATH)
-
-        logger.debug("Creating root challenges validation dir at %s",
-                     self.full_root)
-        try:
-            os.makedirs(self.full_root)
-        except OSError as exception:
-            if exception.errno != errno.EEXIST:
+        for name, path in path_map.items():
+            if not os.path.isdir(path):
                 raise errors.PluginError(
-                    "Couldn't create root for http-01 "
-                    "challenge responses: {0}", exception)
+                    path + " does not exist or is not a directory")
+            self.full_roots[name] = os.path.join(path, challenges.HTTP01.URI_ROOT_PATH)
+
+            logger.debug("Creating root challenges validation dir at %s",
+                         self.full_roots[name])
+            try:
+                os.makedirs(self.full_roots[name])
+            except OSError as exception:
+                if exception.errno != errno.EEXIST:
+                    raise errors.PluginError(
+                        "Couldn't create root for {0} http-01 "
+                        "challenge responses: {1}", name, exception)
 
     def perform(self, achalls):  # pylint: disable=missing-docstring
         assert self.full_root is not None

From f2f9d33e035c8e0b1c42412cac1be63746f1c9f5 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Thu, 19 Nov 2015 10:01:08 -0800
Subject: [PATCH 04/20] Update _path_for_achall

Borrowing from @grubberr's changes at:
https://github.com/letsencrypt/letsencrypt/pull/1284/files#diff-522ab130649a0ce14df40114d4ccd0b5L111
---
 letsencrypt/plugins/webroot.py | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py
index 5ad438a5f..1c5093b03 100644
--- a/letsencrypt/plugins/webroot.py
+++ b/letsencrypt/plugins/webroot.py
@@ -82,11 +82,10 @@ to serve all files under specified web root ({0})."""
 
     def __init__(self, *args, **kwargs):
         super(Authenticator, self).__init__(*args, **kwargs)
-        self.full_root = None
+        self.full_roots = {}
 
     def prepare(self):  # pylint: disable=missing-docstring
         path_map = self.conf("map")
-        self.full_roots = {}
 
         if not path_map:
             raise errors.PluginError("--{0} must be set".format(
@@ -108,11 +107,15 @@ to serve all files under specified web root ({0})."""
                         "challenge responses: {1}", name, exception)
 
     def perform(self, achalls):  # pylint: disable=missing-docstring
-        assert self.full_root is not None
+        assert self.full_root, "Webroot plugin appears to be missing webroot map"
         return [self._perform_single(achall) for achall in achalls]
 
     def _path_for_achall(self, achall):
-        return os.path.join(self.full_root, achall.chall.encode("token"))
+        path = self.full_roots[achall.domain]
+        if not path:
+            raise errors.PluginError("Cannot find path {0} for domain: {1}"
+                        .format(path, achall.domain))
+        return os.path.join(path, achall.chall.encode("token"))
 
     def _perform_single(self, achall):
         response, validation = achall.response_and_validation()

From f48ef6ded9af20deb6db296c12498902052b4493 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Thu, 19 Nov 2015 10:09:10 -0800
Subject: [PATCH 05/20] lint

---
 letsencrypt/cli.py             | 7 ++++---
 letsencrypt/plugins/webroot.py | 6 ++----
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 914df7fb5..3dd53f011 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -1038,7 +1038,7 @@ def _plugins_parsing(helpful, plugins):
     helpful.add_plugin_args(plugins)
 
 
-class WebrootPathProcessor(argparse.Action):
+class WebrootPathProcessor(argparse.Action): # pylint: disable=missing-docstring
     def __call__(self, parser, config, webroot, option_string=None):
         """
         Keep a record of --webroot-path / -w flags during processing, so that
@@ -1056,13 +1056,14 @@ class WebrootPathProcessor(argparse.Action):
         config.webroot_path.append(webroot)
 
 
-class DomainFlagProcessor(argparse.Action):
+class DomainFlagProcessor(argparse.Action): # pylint: disable=missing-docstring
     def __call__(self, parser, config, domain_arg, option_string=None):
         """
         Process a new -d flag, helping the webroot plugin construct a map of
         {domain : webrootpath} if -w / --webroot-path is in use
         """
-        if not config.domains: config.domains = []
+        if not config.domains:
+            config.domains = []
         new_domains = [d.strip() for d in domain_arg.split(",")
                                   if d not in config.domains]
         config.domains.extend(new_domains)
diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py
index 1c5093b03..4e18f5ca2 100644
--- a/letsencrypt/plugins/webroot.py
+++ b/letsencrypt/plugins/webroot.py
@@ -38,7 +38,6 @@ Use the following snippet in your ``server{...}`` stanza::
 and reload your daemon.
 
 """
-import argparse
 import errno
 import logging
 import os
@@ -92,8 +91,7 @@ to serve all files under specified web root ({0})."""
                 self.option_name("path")))
         for name, path in path_map.items():
             if not os.path.isdir(path):
-                raise errors.PluginError(
-                    path + " does not exist or is not a directory")
+                raise errors.PluginError(path + " does not exist or is not a directory")
             self.full_roots[name] = os.path.join(path, challenges.HTTP01.URI_ROOT_PATH)
 
             logger.debug("Creating root challenges validation dir at %s",
@@ -107,7 +105,7 @@ to serve all files under specified web root ({0})."""
                         "challenge responses: {1}", name, exception)
 
     def perform(self, achalls):  # pylint: disable=missing-docstring
-        assert self.full_root, "Webroot plugin appears to be missing webroot map"
+        assert self.full_roots, "Webroot plugin appears to be missing webroot map"
         return [self._perform_single(achall) for achall in achalls]
 
     def _path_for_achall(self, achall):

From d5e92289fc4deab45de9c175da7a4f8a25d4989b Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Sat, 21 Nov 2015 01:42:39 -0800
Subject: [PATCH 06/20] Test case for webroot_map construction

---
 letsencrypt/tests/cli_test.py | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index d53b4700a..f2827ec09 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -339,6 +339,18 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         namespace = cli.prepare_and_parse_args(plugins, long_args)
         self.assertEqual(namespace.domains, ['example.com', 'another.net'])
 
+        webroot_args = ['--webroot', '-d', 'stray.example.com', '-w',
+            '/var/www/example', '-d', 'example.com,www.example.com', '-w',
+            '/var/www/superfluous', '-d', 'superfluo.us', '-d', 'www.superfluo.us']
+        namespace = cli.prepare_and_parse_args(plugins, webroot_args)
+        open("/tmp/frogs", "w").write("%r" % namespace)
+        self.assertEqual(namespace.webroot_map, {
+            'example.com' : '/var/www/example',
+            'stray.example.com' : '/var/www/example',
+            'www.example.com' : '/var/www/example',
+            'www.superfluo.us' : '/var/www/superfluous',
+            'superfluo.us' : '/var/www/superfluous'})
+
     @mock.patch('letsencrypt.crypto_util.notAfter')
     @mock.patch('letsencrypt.cli.zope.component.getUtility')
     def test_certonly_new_request_success(self, mock_get_utility, mock_notAfter):

From 6a50a98ebe197ab5cef39e3f2134d0df9c29705e Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Fri, 20 Nov 2015 12:38:47 -0800
Subject: [PATCH 07/20] unoneline

---
 letsencrypt/cli.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 3dd53f011..092b79577 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -1062,8 +1062,7 @@ class DomainFlagProcessor(argparse.Action): # pylint: disable=missing-docstring
         Process a new -d flag, helping the webroot plugin construct a map of
         {domain : webrootpath} if -w / --webroot-path is in use
         """
-        if not config.domains:
-            config.domains = []
+        if not config.domains: config.domains = []
         new_domains = [d.strip() for d in domain_arg.split(",")
                                   if d not in config.domains]
         config.domains.extend(new_domains)

From 3cc8e7e0198ff783eaf725d2b1c227745d58ca27 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Sat, 21 Nov 2015 10:59:05 -0800
Subject: [PATCH 08/20] Webroot cli now passes some tests!

---
 letsencrypt/cli.py            | 37 +++++++++++++++++++----------------
 letsencrypt/tests/cli_test.py | 10 ++++++----
 2 files changed, 26 insertions(+), 21 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 092b79577..0bab6d034 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -3,10 +3,12 @@
 import argparse
 import atexit
 import functools
+import json
 import logging
 import logging.handlers
 import os
 import pkg_resources
+import string
 import sys
 import time
 import traceback
@@ -670,7 +672,6 @@ class HelpfulArgumentParser(object):
             print usage
             sys.exit(0)
         self.visible_topics = self.determine_help_topics(self.help_arg)
-        #print self.visible_topics
         self.groups = {}  # elements are added by .add_group()
 
     def parse_args(self):
@@ -1023,20 +1024,22 @@ def _plugins_parsing(helpful, plugins):
     helpful.add("plugins", "--webroot", action="store_true",
                 help='Obtain certs by placing files in a webroot directory.')
 
-    # This would normally be a flag within the webroot plugin,  the webroot
-    # plugin, but because it is parsed in conjunction with --domains, it lives
-    # here
-    helpful.add("webroot", "-w", "--webroot-path", action=WebrootPathProcessor,
-                help="public_html / webroot path")
-    #helpful.add("plugins", "-w", action=WebrootAction,
-    #            help='Obtain certs by placing files in a webroot directory.')
-
     # things should not be reorder past/pre this comment:
     # plugins_group should be displayed in --help before plugin
     # specific groups (so that plugins_group.description makes sense)
 
     helpful.add_plugin_args(plugins)
 
+    # These would normally be a flag within the webroot plugin, but because
+    # they are parsed in conjunction with --domains, they live here for
+    # legibiility. helpful.add_plugin_ags must be called first to add the
+    # "webroot" topic
+    helpful.add("webroot", "-w", "--webroot-path", action=WebrootPathProcessor,
+                help="public_html / webroot path")
+    parse_dict = lambda s : dict(json.loads(s))
+    helpful.add("webroot", "--webroot-map", default={}, type=parse_dict,
+                help="Mapping from domains to webroot paths")
+
 
 class WebrootPathProcessor(argparse.Action): # pylint: disable=missing-docstring
     def __call__(self, parser, config, webroot, option_string=None):
@@ -1062,15 +1065,15 @@ class DomainFlagProcessor(argparse.Action): # pylint: disable=missing-docstring
         Process a new -d flag, helping the webroot plugin construct a map of
         {domain : webrootpath} if -w / --webroot-path is in use
         """
-        if not config.domains: config.domains = []
-        new_domains = [d.strip() for d in domain_arg.split(",")
-                                  if d not in config.domains]
-        config.domains.extend(new_domains)
+        if not config.domains:
+            config.domains = []
 
-        if config.webroot_path:
-            # Each domain has a webroot_path of the most recent -w flag
-            for d in new_domains:
-                config.webroot_map[d] = config.webroot_path[-1]
+        for d in map(string.strip, domain_arg.split(",")):
+            if d not in config.domains:
+                config.domains.append(d)
+                # Each domain has a webroot_path of the most recent -w flag
+                if config.webroot_path:
+                    config.webroot_map[d] = config.webroot_path[-1]
 
 
 def setup_log_file_handler(args, logfile, fmt):
diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index f2827ec09..e5780b8d2 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -236,7 +236,8 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
             self._call(['plugins'] + list(args))
 
     @mock.patch('letsencrypt.cli.plugins_disco')
-    def test_plugins_no_args(self, mock_disco):
+    @mock.patch('letsencrypt.cli.HelpfulArgumentParser.determine_help_topics')
+    def test_plugins_no_args(self, _det, mock_disco):
         ifaces = []
         plugins = mock_disco.PluginsRegistry.find_all()
 
@@ -247,7 +248,8 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         stdout.write.called_once_with(str(filtered))
 
     @mock.patch('letsencrypt.cli.plugins_disco')
-    def test_plugins_init(self, mock_disco):
+    @mock.patch('letsencrypt.cli.HelpfulArgumentParser.determine_help_topics')
+    def test_plugins_init(self, _det, mock_disco):
         ifaces = []
         plugins = mock_disco.PluginsRegistry.find_all()
 
@@ -261,10 +263,10 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         stdout.write.called_once_with(str(verified))
 
     @mock.patch('letsencrypt.cli.plugins_disco')
-    def test_plugins_prepare(self, mock_disco):
+    @mock.patch('letsencrypt.cli.HelpfulArgumentParser.determine_help_topics')
+    def test_plugins_prepare(self, _det, mock_disco):
         ifaces = []
         plugins = mock_disco.PluginsRegistry.find_all()
-
         _, stdout, _, _ = self._call(['plugins', '--init', '--prepare'])
         plugins.visible.assert_called_once_with()
         plugins.visible().ifaces.assert_called_once_with(ifaces)

From 5beccc080d176a0f6e58316422d6bcb0eac60d6b Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Sat, 21 Nov 2015 11:09:07 -0800
Subject: [PATCH 09/20] Test for CLI --webroot-map parsing

---
 letsencrypt/tests/cli_test.py | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index e5780b8d2..991446447 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -341,11 +341,12 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         namespace = cli.prepare_and_parse_args(plugins, long_args)
         self.assertEqual(namespace.domains, ['example.com', 'another.net'])
 
+    def test_parse_webroot(self):
+        plugins = disco.PluginsRegistry.find_all()
         webroot_args = ['--webroot', '-d', 'stray.example.com', '-w',
             '/var/www/example', '-d', 'example.com,www.example.com', '-w',
             '/var/www/superfluous', '-d', 'superfluo.us', '-d', 'www.superfluo.us']
         namespace = cli.prepare_and_parse_args(plugins, webroot_args)
-        open("/tmp/frogs", "w").write("%r" % namespace)
         self.assertEqual(namespace.webroot_map, {
             'example.com' : '/var/www/example',
             'stray.example.com' : '/var/www/example',
@@ -353,6 +354,10 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
             'www.superfluo.us' : '/var/www/superfluous',
             'superfluo.us' : '/var/www/superfluous'})
 
+        webroot_map_args = ['--webroot-map', '{"eg.com" : "/tmp"}']
+        namespace = cli.prepare_and_parse_args(plugins, webroot_map_args)
+        self.assertEqual(namespace.webroot_map, {u"eg.com" : u"/tmp"})
+
     @mock.patch('letsencrypt.crypto_util.notAfter')
     @mock.patch('letsencrypt.cli.zope.component.getUtility')
     def test_certonly_new_request_success(self, mock_get_utility, mock_notAfter):

From 544fe8d7086a8c9a093a350a1f814943dbe05240 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Sat, 21 Nov 2015 11:19:52 -0800
Subject: [PATCH 10/20] Fix webroot tests

---
 letsencrypt/plugins/webroot_test.py | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/letsencrypt/plugins/webroot_test.py b/letsencrypt/plugins/webroot_test.py
index aa8f16e38..261293005 100644
--- a/letsencrypt/plugins/webroot_test.py
+++ b/letsencrypt/plugins/webroot_test.py
@@ -23,7 +23,7 @@ class AuthenticatorTest(unittest.TestCase):
     """Tests for letsencrypt.plugins.webroot.Authenticator."""
 
     achall = achallenges.KeyAuthorizationAnnotatedChallenge(
-        challb=acme_util.HTTP01_P, domain=None, account_key=KEY)
+        challb=acme_util.HTTP01_P, domain="thing.com", account_key=KEY)
 
     def setUp(self):
         from letsencrypt.plugins.webroot import Authenticator
@@ -31,7 +31,8 @@ class AuthenticatorTest(unittest.TestCase):
         self.validation_path = os.path.join(
             self.path, ".well-known", "acme-challenge",
             "ZXZhR3hmQURzNnBTUmIyTEF2OUlaZjE3RHQzanV4R0orUEN0OTJ3citvQQ")
-        self.config = mock.MagicMock(webroot_path=self.path)
+        self.config = mock.MagicMock(webroot_path=self.path,
+                                     webroot_map={"thing.com":self.path})
         self.auth = Authenticator(self.config, "webroot")
         self.auth.prepare()
 
@@ -46,14 +47,16 @@ class AuthenticatorTest(unittest.TestCase):
     def test_add_parser_arguments(self):
         add = mock.MagicMock()
         self.auth.add_parser_arguments(add)
-        self.assertEqual(1, add.call_count)
+        self.assertEqual(0, add.call_count) # became 0 when we moved the args to cli.py!
 
     def test_prepare_bad_root(self):
         self.config.webroot_path = os.path.join(self.path, "null")
+        self.config.webroot_map["thing.com"] = self.config.webroot_path
         self.assertRaises(errors.PluginError, self.auth.prepare)
 
     def test_prepare_missing_root(self):
         self.config.webroot_path = None
+        self.config.webroot_map = {}
         self.assertRaises(errors.PluginError, self.auth.prepare)
 
     def test_prepare_full_root_exists(self):

From a3b0588cea57d063a6e70b57cb4c1ad0efa1a614 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Sat, 21 Nov 2015 11:50:13 -0800
Subject: [PATCH 11/20] lintmonster

---
 letsencrypt/cli.py                  |  4 ++--
 letsencrypt/plugins/webroot_test.py |  2 +-
 letsencrypt/tests/cli_test.py       | 12 ++++++------
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 0bab6d034..9c4c4a5f5 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -1036,7 +1036,7 @@ def _plugins_parsing(helpful, plugins):
     # "webroot" topic
     helpful.add("webroot", "-w", "--webroot-path", action=WebrootPathProcessor,
                 help="public_html / webroot path")
-    parse_dict = lambda s : dict(json.loads(s))
+    parse_dict = lambda s: dict(json.loads(s))
     helpful.add("webroot", "--webroot-map", default={}, type=parse_dict,
                 help="Mapping from domains to webroot paths")
 
@@ -1068,7 +1068,7 @@ class DomainFlagProcessor(argparse.Action): # pylint: disable=missing-docstring
         if not config.domains:
             config.domains = []
 
-        for d in map(string.strip, domain_arg.split(",")):
+        for d in map(string.strip, domain_arg.split(",")): # pylint: disable=bad-builtin
             if d not in config.domains:
                 config.domains.append(d)
                 # Each domain has a webroot_path of the most recent -w flag
diff --git a/letsencrypt/plugins/webroot_test.py b/letsencrypt/plugins/webroot_test.py
index 261293005..902f74e9f 100644
--- a/letsencrypt/plugins/webroot_test.py
+++ b/letsencrypt/plugins/webroot_test.py
@@ -32,7 +32,7 @@ class AuthenticatorTest(unittest.TestCase):
             self.path, ".well-known", "acme-challenge",
             "ZXZhR3hmQURzNnBTUmIyTEF2OUlaZjE3RHQzanV4R0orUEN0OTJ3citvQQ")
         self.config = mock.MagicMock(webroot_path=self.path,
-                                     webroot_map={"thing.com":self.path})
+                                     webroot_map={"thing.com": self.path})
         self.auth = Authenticator(self.config, "webroot")
         self.auth.prepare()
 
diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index 991446447..853109636 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -348,15 +348,15 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
             '/var/www/superfluous', '-d', 'superfluo.us', '-d', 'www.superfluo.us']
         namespace = cli.prepare_and_parse_args(plugins, webroot_args)
         self.assertEqual(namespace.webroot_map, {
-            'example.com' : '/var/www/example',
-            'stray.example.com' : '/var/www/example',
-            'www.example.com' : '/var/www/example',
-            'www.superfluo.us' : '/var/www/superfluous',
-            'superfluo.us' : '/var/www/superfluous'})
+            'example.com': '/var/www/example',
+            'stray.example.com': '/var/www/example',
+            'www.example.com': '/var/www/example',
+            'www.superfluo.us': '/var/www/superfluous',
+            'superfluo.us': '/var/www/superfluous'})
 
         webroot_map_args = ['--webroot-map', '{"eg.com" : "/tmp"}']
         namespace = cli.prepare_and_parse_args(plugins, webroot_map_args)
-        self.assertEqual(namespace.webroot_map, {u"eg.com" : u"/tmp"})
+        self.assertEqual(namespace.webroot_map, {u"eg.com": u"/tmp"})
 
     @mock.patch('letsencrypt.crypto_util.notAfter')
     @mock.patch('letsencrypt.cli.zope.component.getUtility')

From b3851edb73f4b56f516c8bff3dd6f2d37f331967 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 24 Nov 2015 13:58:38 -0800
Subject: [PATCH 12/20] Since --webroot-map is not elegant, do not document it

---
 letsencrypt/cli.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 9c4c4a5f5..ffccd56b4 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -1037,8 +1037,9 @@ def _plugins_parsing(helpful, plugins):
     helpful.add("webroot", "-w", "--webroot-path", action=WebrootPathProcessor,
                 help="public_html / webroot path")
     parse_dict = lambda s: dict(json.loads(s))
+    # --webroot-map still has some awkward properties, so it is undocumented
     helpful.add("webroot", "--webroot-map", default={}, type=parse_dict,
-                help="Mapping from domains to webroot paths")
+                help=argparse.SUPPRESS)
 
 
 class WebrootPathProcessor(argparse.Action): # pylint: disable=missing-docstring

From 2b87d6f700d8134cbdec10adb69c3ef97fe6ec54 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 30 Nov 2015 18:15:07 -0800
Subject: [PATCH 13/20] Do not accept -d first in the presence of multiple -w
 flags

 * informal testing suggested that many people found this behaviour confusing
---
 letsencrypt/cli.py            |  8 ++++++++
 letsencrypt/tests/cli_test.py | 11 +++++++----
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index ffccd56b4..cc458d7fe 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -1043,6 +1043,10 @@ def _plugins_parsing(helpful, plugins):
 
 
 class WebrootPathProcessor(argparse.Action): # pylint: disable=missing-docstring
+    def __init__(self, *args, **kwargs):
+        self.domain_before_webroot = False
+        argparse.Action.__init__(self, *args, **kwargs)
+
     def __call__(self, parser, config, webroot, option_string=None):
         """
         Keep a record of --webroot-path / -w flags during processing, so that
@@ -1055,8 +1059,12 @@ class WebrootPathProcessor(argparse.Action): # pylint: disable=missing-docstring
             # config.webroot_map are filled in by cli.DomainFlagProcessor
             if config.domains:
                 config.webroot_map = dict([(d, webroot) for d in config.domains])
+                self.domain_before_webroot = True
             else:
                 config.webroot_map = {}
+        elif self.domain_before_webroot:
+            raise errors.Error("If you specify multiple webroot paths, one of "
+                               "them must precede all domain flags")
         config.webroot_path.append(webroot)
 
 
diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index 853109636..9f6538eb8 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -343,17 +343,20 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
 
     def test_parse_webroot(self):
         plugins = disco.PluginsRegistry.find_all()
-        webroot_args = ['--webroot', '-d', 'stray.example.com', '-w',
-            '/var/www/example', '-d', 'example.com,www.example.com', '-w',
-            '/var/www/superfluous', '-d', 'superfluo.us', '-d', 'www.superfluo.us']
+        webroot_args = ['--webroot',  '-w', '/var/www/example',
+            '-d', 'example.com,www.example.com', '-w', '/var/www/superfluous',
+            '-d', 'superfluo.us', '-d', 'www.superfluo.us']
         namespace = cli.prepare_and_parse_args(plugins, webroot_args)
         self.assertEqual(namespace.webroot_map, {
             'example.com': '/var/www/example',
-            'stray.example.com': '/var/www/example',
             'www.example.com': '/var/www/example',
             'www.superfluo.us': '/var/www/superfluous',
             'superfluo.us': '/var/www/superfluous'})
 
+        webroot_args = ['-d', 'stray.example.com'] + webroot_args
+        with self.assertRaises(errors.Error):
+            cli.prepare_and_parse_args(plugins, webroot_args)
+
         webroot_map_args = ['--webroot-map', '{"eg.com" : "/tmp"}']
         namespace = cli.prepare_and_parse_args(plugins, webroot_map_args)
         self.assertEqual(namespace.webroot_map, {u"eg.com": u"/tmp"})

From 328f8cdc5b65f4fe43082faee8e34fe4cba2d98d Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 30 Nov 2015 18:24:40 -0800
Subject: [PATCH 14/20] Document --webroot-path

---
 letsencrypt/cli.py             | 5 ++++-
 letsencrypt/plugins/webroot.py | 2 ++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index cc458d7fe..cbdd5465a 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -1035,7 +1035,10 @@ def _plugins_parsing(helpful, plugins):
     # legibiility. helpful.add_plugin_ags must be called first to add the
     # "webroot" topic
     helpful.add("webroot", "-w", "--webroot-path", action=WebrootPathProcessor,
-                help="public_html / webroot path")
+                help="public_html / webroot path. This can be specified multiple times to "
+                     "handle different domains; each domain will have the webroot path that"
+                     " precededed it.  For instance: `-w /var/www/example -d example.com -d "
+                     "www.example.com -w /var/www/thing -d thing.net -d m.thing.net`")
     parse_dict = lambda s: dict(json.loads(s))
     # --webroot-map still has some awkward properties, so it is undocumented
     helpful.add("webroot", "--webroot-map", default={}, type=parse_dict,
diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py
index 4e18f5ca2..8be5d80cd 100644
--- a/letsencrypt/plugins/webroot.py
+++ b/letsencrypt/plugins/webroot.py
@@ -73,6 +73,8 @@ to serve all files under specified web root ({0})."""
 
     @classmethod
     def add_parser_arguments(cls, add):
+        # --webroot-path and --webroot-map are added in cli.py because they
+        # are parsed in conjunction with --domains
         pass
 
     def get_chall_pref(self, domain):  # pragma: no cover

From 77778e85cce1ffab4beef22386316b11738d09af Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 30 Nov 2015 18:24:52 -0800
Subject: [PATCH 15/20] Restore --domain compatibility

It probably should never have lapsed...
---
 letsencrypt/cli.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index cbdd5465a..530cba638 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -839,7 +839,7 @@ def prepare_and_parse_args(plugins, args):
     # --domains is useful, because it can be stored in config
     #for subparser in parser_run, parser_auth, parser_install:
     #    subparser.add_argument("domains", nargs="*", metavar="domain")
-    helpful.add(None, "-d", "--domains", dest="domains",
+    helpful.add(None, "-d", "--domains", "--domain", dest="domains",
                 metavar="DOMAIN", action=DomainFlagProcessor,
                 help="Domain names to apply. For multiple domains you can use "
                 "multiple -d flags or enter a comma separated list of domains "

From ffd30d8c1edb9c1e6426e619d86c5a1ef588aa3f Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 30 Nov 2015 20:57:07 -0800
Subject: [PATCH 16/20] lint

---
 letsencrypt/tests/cli_test.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index 9f6538eb8..b3b55a981 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -343,7 +343,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
 
     def test_parse_webroot(self):
         plugins = disco.PluginsRegistry.find_all()
-        webroot_args = ['--webroot',  '-w', '/var/www/example',
+        webroot_args = ['--webroot', '-w', '/var/www/example',
             '-d', 'example.com,www.example.com', '-w', '/var/www/superfluous',
             '-d', 'superfluo.us', '-d', 'www.superfluo.us']
         namespace = cli.prepare_and_parse_args(plugins, webroot_args)

From 6b122a044adc758847f92fb564f822f5bc14bd91 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 30 Nov 2015 21:08:23 -0800
Subject: [PATCH 17/20] Too many lines?

(That's probably true)
---
 letsencrypt/cli.py | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index b06b288eb..bd95cd372 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -1,5 +1,7 @@
 """Let's Encrypt CLI."""
 # TODO: Sanity check all input.  Be sure to avoid shell code etc...
+# pylint: disable=too-many-lines
+# (TODO: split this file into main.py and cli.py)
 import argparse
 import atexit
 import functools
@@ -384,7 +386,6 @@ def diagnose_configurator_problem(cfg_type, requested, plugins):
 def choose_configurator_plugins(args, config, plugins, verb):  # pylint: disable=too-many-branches
     """
     Figure out which configurator we're going to use
-
     :raises error.PluginSelectionError if there was a problem
     """
 
@@ -802,7 +803,6 @@ class HelpfulArgumentParser(object):
             return dict([(t, t == chosen_topic) for t in self.help_topics])
 
 
-
 def prepare_and_parse_args(plugins, args):
     """Returns parsed command line arguments.
 
@@ -946,8 +946,7 @@ def _create_subparsers(helpful):
         help="Set a custom user agent string for the client. User agent strings allow "
              "the CA to collect high level statistics about success rates by OS and "
              "plugin. If you wish to hide your server OS version from the Let's "
-             'Encrypt server, set this to "".'
-    )
+             'Encrypt server, set this to "".')
     helpful.add("certonly",
                 "--csr", type=read_file,
                 help="Path to a Certificate Signing Request (CSR) in DER"

From 2befb2d5c174b5bfd8c53174ceb12a0ac594ea62 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 30 Nov 2015 21:27:41 -0800
Subject: [PATCH 18/20] remove python2.7ism

---
 letsencrypt/tests/cli_test.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index c90c1b836..e07f5e83c 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -354,8 +354,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
             'superfluo.us': '/var/www/superfluous'})
 
         webroot_args = ['-d', 'stray.example.com'] + webroot_args
-        with self.assertRaises(errors.Error):
-            cli.prepare_and_parse_args(plugins, webroot_args)
+        self.assertRaises(errors.Error, cli.prepare_and_parse_args, plugins, webroot_args)
 
         webroot_map_args = ['--webroot-map', '{"eg.com" : "/tmp"}']
         namespace = cli.prepare_and_parse_args(plugins, webroot_map_args)

From 06e273413b609883cbd4587f002460c089a77fa4 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 1 Dec 2015 16:33:35 -0800
Subject: [PATCH 19/20] Fix nits and address review comments

---
 letsencrypt/cli.py             | 27 ++++++++++++---------------
 letsencrypt/plugins/webroot.py |  8 ++++----
 2 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index bd95cd372..85478132e 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -10,7 +10,6 @@ import logging
 import logging.handlers
 import os
 import pkg_resources
-import string
 import sys
 import time
 import traceback
@@ -103,7 +102,7 @@ def usage_strings(plugins):
 
 
 def _find_domains(args, installer):
-    if args.domains is None:
+    if not args.domains:
         domains = display_ops.choose_names(installer)
     else:
         domains = args.domains
@@ -477,7 +476,7 @@ def run(args, config, plugins):  # pylint: disable=too-many-branches,too-many-lo
 def obtain_cert(args, config, plugins):
     """Authenticate & obtain cert, but do not install it."""
 
-    if args.domains is not None and args.csr is not None:
+    if args.domains and args.csr is not None:
         # TODO: --csr could have a priority, when --domains is
         # supplied, check if CSR matches given domains?
         return "--domains and --csr are mutually exclusive"
@@ -840,7 +839,7 @@ def prepare_and_parse_args(plugins, args):
     #for subparser in parser_run, parser_auth, parser_install:
     #    subparser.add_argument("domains", nargs="*", metavar="domain")
     helpful.add(None, "-d", "--domains", "--domain", dest="domains",
-                metavar="DOMAIN", action=DomainFlagProcessor,
+                metavar="DOMAIN", action=DomainFlagProcessor, default=[],
                 help="Domain names to apply. For multiple domains you can use "
                 "multiple -d flags or enter a comma separated list of domains "
                 "as a parameter.")
@@ -1073,17 +1072,18 @@ class WebrootPathProcessor(argparse.Action): # pylint: disable=missing-docstring
         Keep a record of --webroot-path / -w flags during processing, so that
         we know which apply to which -d flags
         """
-        if not config.webroot_path:
+        if config.webroot_path is None:      # first -w flag encountered
             config.webroot_path = []
             # if any --domain flags preceded the first --webroot-path flag,
             # apply that webroot path to those; subsequent entries in
             # config.webroot_map are filled in by cli.DomainFlagProcessor
             if config.domains:
-                config.webroot_map = dict([(d, webroot) for d in config.domains])
                 self.domain_before_webroot = True
-            else:
-                config.webroot_map = {}
+                for d in config.domains:
+                    config.webroot_map.setdefault(d, webroot)
         elif self.domain_before_webroot:
+            # FIXME if you set domains in a config file, you should get a different error
+            # here, pointing you to --webroot-map
             raise errors.Error("If you specify multiple webroot paths, one of "
                                "them must precede all domain flags")
         config.webroot_path.append(webroot)
@@ -1095,15 +1095,12 @@ class DomainFlagProcessor(argparse.Action): # pylint: disable=missing-docstring
         Process a new -d flag, helping the webroot plugin construct a map of
         {domain : webrootpath} if -w / --webroot-path is in use
         """
-        if not config.domains:
-            config.domains = []
-
-        for d in map(string.strip, domain_arg.split(",")): # pylint: disable=bad-builtin
-            if d not in config.domains:
-                config.domains.append(d)
+        for domain in (d.strip() for d in domain_arg.split(",")):
+            if domain not in config.domains:
+                config.domains.append(domain)
                 # Each domain has a webroot_path of the most recent -w flag
                 if config.webroot_path:
-                    config.webroot_map[d] = config.webroot_path[-1]
+                    config.webroot_map[domain] = config.webroot_path[-1]
 
 
 def setup_log_file_handler(args, logfile, fmt):
diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py
index 6709d0c87..705f08113 100644
--- a/letsencrypt/plugins/webroot.py
+++ b/letsencrypt/plugins/webroot.py
@@ -15,7 +15,6 @@ from letsencrypt.plugins import common
 logger = logging.getLogger(__name__)
 
 
-
 class Authenticator(common.Plugin):
     """Webroot Authenticator."""
     zope.interface.implements(interfaces.IAuthenticator)
@@ -72,9 +71,10 @@ to serve all files under specified web root ({0})."""
         return [self._perform_single(achall) for achall in achalls]
 
     def _path_for_achall(self, achall):
-        path = self.full_roots[achall.domain]
-        if not path:
-            raise errors.PluginError("Cannot find path {0} for domain: {1}"
+        try:
+            path = self.full_roots[achall.domain]
+        except IndexError:
+            raise errors.PluginError("Cannot find webroot path for domain: {1}"
                         .format(path, achall.domain))
         return os.path.join(path, achall.chall.encode("token"))
 

From f4dd66040351a075bf9a86d9e9b779e6fdf09722 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 1 Dec 2015 16:47:50 -0800
Subject: [PATCH 20/20] Oops!

- Finish a partial commit, providing what are perhaps excessively detailed and
  mystical errors in improbable cases.
---
 letsencrypt/plugins/webroot.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py
index 705f08113..e63cf31d4 100644
--- a/letsencrypt/plugins/webroot.py
+++ b/letsencrypt/plugins/webroot.py
@@ -74,7 +74,10 @@ to serve all files under specified web root ({0})."""
         try:
             path = self.full_roots[achall.domain]
         except IndexError:
-            raise errors.PluginError("Cannot find webroot path for domain: {1}"
+            raise errors.PluginError("Missing --webroot-path for domain: {1}"
+                        .format(achall.domain))
+        if not os.path.exists(path):
+            raise errors.PluginError("Mysteriously missing path {0} for domain: {1}"
                         .format(path, achall.domain))
         return os.path.join(path, achall.chall.encode("token"))