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

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

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

certbot/achallenges.py

@@ -28,7 +28,6 @@ logger = logging.getLogger(__name__)
 
 # pylint: disable=too-few-public-methods
 
-
 class AnnotatedChallenge(jose.ImmutableMap):
     """Client annotated challenge.
 

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."""