From 31d7b5f6d70d3b58cc0315762e71ecad7ad0cf95 Mon Sep 17 00:00:00 2001
From: Nick Fong <nickfong@users.noreply.github.com>
Date: Mon, 9 Jan 2017 18:59:48 -0800
Subject: [PATCH] Fix Error Message for invalid FQDNs (#3994)

* Add better error handling for invalid FQDNs

Add explicit error handling for labels that are empty.  Also add test
cases to test invalid domains.

* Add more thorough tests
---
 certbot/tests/util_test.py | 38 ++++++++++++++++++++++++++++++++++++++
 certbot/util.py            | 10 ++++++----
 2 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/certbot/tests/util_test.py b/certbot/tests/util_test.py
index cd02d835d..6dc839025 100644
--- a/certbot/tests/util_test.py
+++ b/certbot/tests/util_test.py
@@ -374,6 +374,44 @@ class EnforceDomainSanityTest(unittest.TestCase):
         self.assertRaises(errors.ConfigurationError, self._call,
                           u"eichh\u00f6rnchen.example.com")
 
+    def test_too_long(self):
+        long_domain = u"a"*256
+        self.assertRaises(errors.ConfigurationError, self._call,
+                          long_domain)
+
+    def test_not_too_long(self):
+        not_too_long_domain = u"{0}.{1}.{2}.{3}".format("a"*63, "b"*63, "c"*63, "d"*63)
+        self._call(not_too_long_domain)
+
+    def test_empty_label(self):
+        empty_label_domain = u"fizz..example.com"
+        self.assertRaises(errors.ConfigurationError, self._call,
+                          empty_label_domain)
+
+    def test_empty_trailing_label(self):
+        empty_trailing_label_domain = u"example.com.."
+        self.assertRaises(errors.ConfigurationError, self._call,
+                          empty_trailing_label_domain)
+
+    def test_long_label_1(self):
+        long_label_domain = u"a"*64
+        self.assertRaises(errors.ConfigurationError, self._call,
+                          long_label_domain)
+
+    def test_long_label_2(self):
+        long_label_domain = u"{0}.{1}.com".format(u"a"*64, u"b"*63)
+        self.assertRaises(errors.ConfigurationError, self._call,
+                          long_label_domain)
+
+    def test_not_long_label(self):
+        not_too_long_label_domain = u"{0}.{1}.com".format(u"a"*63, u"b"*63)
+        self._call(not_too_long_label_domain)
+
+    def test_empty_domain(self):
+        empty_domain = u""
+        self.assertRaises(errors.ConfigurationError, self._call,
+                          empty_domain)
+
     def test_punycode_ok(self):
         # Punycode is now legal, so no longer an error; instead check
         # that it's _not_ an error (at the initial sanity check stage)
diff --git a/certbot/util.py b/certbot/util.py
index cc0a74bd2..1b4877530 100644
--- a/certbot/util.py
+++ b/certbot/util.py
@@ -479,12 +479,14 @@ def enforce_domain_sanity(domain):
     # octets (inclusive). And each label is 1 - 63 octets (inclusive).
     # https://tools.ietf.org/html/rfc2181#section-11
     msg = "Requested domain {0} is not a FQDN because ".format(domain)
-    labels = domain.split('.')
-    for l in labels:
-        if not 0 < len(l) < 64:
-            raise errors.ConfigurationError(msg + "label {0} is too long.".format(l))
     if len(domain) > 255:
         raise errors.ConfigurationError(msg + "it is too long.")
+    labels = domain.split('.')
+    for l in labels:
+        if not l:
+            raise errors.ConfigurationError("{0} it contains an empty label.".format(msg))
+        elif len(l) > 63:
+            raise errors.ConfigurationError("{0} label {1} is too long.".format(msg, l))
 
     return domain