From bc3765d6d0635efedf7b3162b9fa68f525c04e13 Mon Sep 17 00:00:00 2001
From: yomna <ynasser@users.noreply.github.com>
Date: Mon, 10 Jul 2017 19:05:52 -0700
Subject: [PATCH] No longer mask failed challenge errors with encoding errors
 (#4867)

*     no longer masker failed challenge errors with encoding errors

* simplifying through type-checking

* bytes
---
 acme/acme/messages.py        | 13 +++++++++----
 acme/acme/messages_test.py   | 17 ++++++++++-------
 certbot/achallenges.py       |  1 -
 certbot/tests/errors_test.py | 11 +++++++++++
 4 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/acme/acme/messages.py b/acme/acme/messages.py
index 5784e8e11..4b4fa5003 100644
--- a/acme/acme/messages.py
+++ b/acme/acme/messages.py
@@ -1,5 +1,6 @@
 """ACME protocol messages."""
 import collections
+import six
 
 from acme import challenges
 from acme import errors
@@ -36,9 +37,13 @@ ERROR_TYPE_DESCRIPTIONS.update(dict(  # add errors with old prefix, deprecate me
 
 def is_acme_error(err):
     """Check if argument is an ACME error."""
-    return (ERROR_PREFIX in str(err)) or (OLD_ERROR_PREFIX in str(err))
+    if isinstance(err, Error) and (err.typ is not None):
+        return (ERROR_PREFIX in err.typ) or (OLD_ERROR_PREFIX in err.typ)
+    else:
+        return False
 
 
+@six.python_2_unicode_compatible
 class Error(jose.JSONObjectWithFields, errors.Error):
     """ACME error.
 
@@ -92,10 +97,10 @@ class Error(jose.JSONObjectWithFields, errors.Error):
             return code
 
     def __str__(self):
-        return ' :: '.join(
-            part for part in
+        return b' :: '.join(
+            part.encode('ascii', 'backslashreplace') for part in
             (self.typ, self.description, self.detail, self.title)
-            if part is not None)
+            if part is not None).decode()
 
 
 class _Constant(jose.JSONDeSerializable, collections.Hashable):  # type: ignore
diff --git a/acme/acme/messages_test.py b/acme/acme/messages_test.py
index ab05a89b7..b4ce19a08 100644
--- a/acme/acme/messages_test.py
+++ b/acme/acme/messages_test.py
@@ -26,6 +26,7 @@ class ErrorTest(unittest.TestCase):
             'type': ERROR_PREFIX + 'malformed',
         }
         self.error_custom = Error(typ='custom', detail='bar')
+        self.empty_error = Error()
         self.jobj_custom = {'type': 'custom', 'detail': 'bar'}
 
     def test_default_typ(self):
@@ -45,12 +46,6 @@ class ErrorTest(unittest.TestCase):
             'The request message was malformed', self.error.description)
         self.assertTrue(self.error_custom.description is None)
 
-    def test_str(self):
-        self.assertEqual(
-            'urn:ietf:params:acme:error:malformed :: The request message was '
-            'malformed :: foo :: title', str(self.error))
-        self.assertEqual('custom :: bar', str(self.error_custom))
-
     def test_code(self):
         from acme.messages import Error
         self.assertEqual('malformed', self.error.code)
@@ -60,8 +55,16 @@ class ErrorTest(unittest.TestCase):
     def test_is_acme_error(self):
         from acme.messages import is_acme_error
         self.assertTrue(is_acme_error(self.error))
-        self.assertTrue(is_acme_error(str(self.error)))
         self.assertFalse(is_acme_error(self.error_custom))
+        self.assertFalse(is_acme_error(self.empty_error))
+        self.assertFalse(is_acme_error("must pet all the {dogs|rabbits}"))
+
+    def test_unicode_error(self):
+        from acme.messages import Error, ERROR_PREFIX, is_acme_error
+        arabic_error = Error(
+                detail=u'\u0639\u062f\u0627\u0644\u0629', typ=ERROR_PREFIX + 'malformed',
+            title='title')
+        self.assertTrue(is_acme_error(arabic_error))
 
     def test_with_code(self):
         from acme.messages import Error, is_acme_error
diff --git a/certbot/achallenges.py b/certbot/achallenges.py
index c2af45fdb..f39bb4cec 100644
--- a/certbot/achallenges.py
+++ b/certbot/achallenges.py
@@ -28,7 +28,6 @@ logger = logging.getLogger(__name__)
 
 # pylint: disable=too-few-public-methods
 
-
 class AnnotatedChallenge(jose.ImmutableMap):
     """Client annotated challenge.
 
diff --git a/certbot/tests/errors_test.py b/certbot/tests/errors_test.py
index aee1857a6..c8a6c4ac5 100644
--- a/certbot/tests/errors_test.py
+++ b/certbot/tests/errors_test.py
@@ -23,6 +23,17 @@ class FailedChallengesTest(unittest.TestCase):
         self.assertTrue(str(self.error).startswith(
             "Failed authorization procedure. example.com (dns-01): tls"))
 
+    def test_unicode(self):
+        from certbot.errors import FailedChallenges
+        arabic_detail = u'\u0639\u062f\u0627\u0644\u0629'
+        arabic_error = FailedChallenges(set([achallenges.DNS(
+            domain="example.com", challb=messages.ChallengeBody(
+                chall=acme_util.DNS01, uri=None,
+                error=messages.Error(typ="tls", detail=arabic_detail)))]))
+
+        self.assertTrue(str(arabic_error).startswith(
+            "Failed authorization procedure. example.com (dns-01): tls"))
+
 
 class StandaloneBindErrorTest(unittest.TestCase):
     """Tests for certbot.errors.StandaloneBindError."""