deprecate unicode input for RFC822Name (#3836)

* deprecate unicode input for RFC822Name

* pep8...?

5 files changed, 108 insertions, 41 deletions

diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 254402d7..5fa545fa 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -21,7 +21,13 @@ Changelog
   necessary. In addition, the
   :attr:`~cryptography.x509.UniformResourceIdentifier.value` attribute was
   deprecated, users should use
-  :attr:`~cryptography.x509.UniformResourceIdentifier.bytes_value` to access the
+  :attr:`~cryptography.x509.UniformResourceIdentifier.bytes_value` to access
+  the raw value.
+* Deprecated passing unicode to the :class:`~cryptography.x509.RFC822Name`
+  constructor. Instead, users should pass email addresses as ``bytes``, with
+  ``idna`` encoding of the hostname if necessary. In addition, the
+  :attr:`~cryptography.x509.RFC822Name.value` attribute was deprecated, users
+  should use :attr:`~cryptography.x509.RFC822Name.bytes_value` to access the
   raw value.
 
 2.0.2 - 2017-07-27
diff --git a/docs/x509/reference.rst b/docs/x509/reference.rst
index 5b0bcd45..8b976119 100644
--- a/docs/x509/reference.rst
+++ b/docs/x509/reference.rst
@@ -1232,8 +1232,24 @@ General Name Classes
 
     This corresponds to an email address. For example, ``user@example.com``.
 
+    ..note::
+
+        Starting with version 2.1 unicode input is deprecated. If passing an
+        internationalized domain name (IDN) you should first IDNA encode the
+        hostname and then pass the resulting bytes.
+
+    .. attribute:: bytes_value
+
+        .. versionadded:: 2.1
+
+        :type: bytes
+
     .. attribute:: value
 
+        .. deprecated:: 2.1
+
+        Deprecated accessor for the idna-decoded value of :attr:`bytes_value`
+
         :type: :term:`text`
 
 .. class:: DNSName(value)
diff --git a/src/cryptography/hazmat/backends/openssl/encode_asn1.py b/src/cryptography/hazmat/backends/openssl/encode_asn1.py
index 78ae21a2..6d9f9567 100644
--- a/src/cryptography/hazmat/backends/openssl/encode_asn1.py
+++ b/src/cryptography/hazmat/backends/openssl/encode_asn1.py
@@ -441,7 +441,7 @@ def _encode_general_name(backend, name):
         gn = backend._lib.GENERAL_NAME_new()
         backend.openssl_assert(gn != backend._ffi.NULL)
         asn1_str = _encode_asn1_str(
-            backend, name._encoded, len(name._encoded)
+            backend, name.bytes_value, len(name.bytes_value)
         )
         gn.type = backend._lib.GEN_EMAIL
         gn.d.rfc822Name = asn1_str
diff --git a/src/cryptography/x509/general_name.py b/src/cryptography/x509/general_name.py
index 40525a01..114b43ae 100644
--- a/src/cryptography/x509/general_name.py
+++ b/src/cryptography/x509/general_name.py
@@ -51,31 +51,67 @@ class GeneralName(object):
 @utils.register_interface(GeneralName)
 class RFC822Name(object):
     def __init__(self, value):
-        if not isinstance(value, six.text_type):
-            raise TypeError("value must be a unicode string")
+        if isinstance(value, six.text_type):
+            try:
+                value = value.encode("ascii")
+            except UnicodeEncodeError:
+                value = self._idna_encode(value)
+                warnings.warn(
+                    "RFC822Name values should be passed as bytes, not strings."
+                    " Support for passing unicode strings will be removed in a"
+                    " future version.",
+                    utils.DeprecatedIn21,
+                    stacklevel=2,
+                )
+            else:
+                warnings.warn(
+                    "RFC822Name values should be passed as bytes, not strings."
+                    " Support for passing unicode strings will be removed in a"
+                    " future version.",
+                    utils.DeprecatedIn21,
+                    stacklevel=2,
+                )
+        elif not isinstance(value, bytes):
+            raise TypeError("value must be bytes")
 
-        name, address = parseaddr(value)
-        parts = address.split(u"@")
+        name, address = parseaddr(value.decode("ascii"))
         if name or not address:
             # parseaddr has found a name (e.g. Name <email>) or the entire
             # value is an empty string.
             raise ValueError("Invalid rfc822name value")
-        elif len(parts) == 1:
+
+        self._bytes_value = value
+
+    bytes_value = utils.read_only_property("_bytes_value")
+
+    def _idna_encode(self, value):
+        _, address = parseaddr(value)
+        parts = address.split(u"@")
+        return parts[0].encode("ascii") + b"@" + idna.encode(parts[1])
+
+    @property
+    def value(self):
+        warnings.warn(
+            "RFC822Name.bytes_value should be used instead of RFC822Name.value"
+            "; it contains the name as raw bytes, instead of as an idna-"
+            "decoded unicode string. RFC822Name.value will be removed in a "
+            "future version.",
+            utils.DeprecatedIn21,
+            stacklevel=2
+        )
+        _, address = parseaddr(self.bytes_value.decode("ascii"))
+        parts = address.split(u"@")
+        if len(parts) == 1:
             # Single label email name. This is valid for local delivery.
-            # No IDNA encoding needed since there is no domain component.
-            encoded = address.encode("ascii")
+            # No IDNA decoding needed since there is no domain component.
+            return address
         else:
             # A normal email of the form user@domain.com. Let's attempt to
             # encode the domain component and reconstruct the address.
-            encoded = parts[0].encode("ascii") + b"@" + idna.encode(parts[1])
-
-        self._value = value
-        self._encoded = encoded
-
-    value = utils.read_only_property("_value")
+            return parts[0] + u"@" + idna.decode(parts[1])
 
     def __repr__(self):
-        return "<RFC822Name(value={0})>".format(self.value)
+        return "<RFC822Name(bytes_value={0!r})>".format(self.bytes_value)
 
     def __eq__(self, other):
         if not isinstance(other, RFC822Name):
diff --git a/tests/test_x509_ext.py b/tests/test_x509_ext.py
index 93538bbc..ee94faaf 100644
--- a/tests/test_x509_ext.py
+++ b/tests/test_x509_ext.py
@@ -166,7 +166,7 @@ class TestCertificateIssuer(object):
             x509.DNSName(b"cryptography.io"),
             x509.DNSName(b"crypto.local"),
             x509.DNSName(b"another.local"),
-            x509.RFC822Name(u"email@another.local"),
+            x509.RFC822Name(b"email@another.local"),
             x509.UniformResourceIdentifier(b"http://another.local"),
         ])
         assert ci[-1] == ci[4]
@@ -1511,44 +1511,53 @@ class TestDirectoryName(object):
 
 class TestRFC822Name(object):
     def test_repr(self):
-        gn = x509.RFC822Name(u"string")
-        assert repr(gn) == "<RFC822Name(value=string)>"
+        gn = x509.RFC822Name(b"string")
+        if six.PY3:
+            assert repr(gn) == "<RFC822Name(bytes_value=b'string')>"
+        else:
+            assert repr(gn) == "<RFC822Name(bytes_value='string')>"
 
     def test_equality(self):
-        gn = x509.RFC822Name(u"string")
-        gn2 = x509.RFC822Name(u"string2")
-        gn3 = x509.RFC822Name(u"string")
+        gn = x509.RFC822Name(b"string")
+        gn2 = x509.RFC822Name(b"string2")
+        gn3 = x509.RFC822Name(b"string")
         assert gn != gn2
         assert gn != object()
         assert gn == gn3
 
-    def test_not_text(self):
-        with pytest.raises(TypeError):
-            x509.RFC822Name(b"notaunicodestring")
-
+    def test_not_text_or_bytes(self):
         with pytest.raises(TypeError):
             x509.RFC822Name(1.3)
 
     def test_invalid_email(self):
         with pytest.raises(ValueError):
             x509.RFC822Name(u"Name <email>")
+        with pytest.raises(ValueError):
+            x509.RFC822Name(b"Name <email>")
 
         with pytest.raises(ValueError):
-            x509.RFC822Name(u"")
+            x509.RFC822Name(b"")
 
     def test_single_label(self):
-        gn = x509.RFC822Name(u"administrator")
-        assert gn.value == u"administrator"
+        gn = x509.RFC822Name(b"administrator")
+        with pytest.warns(utils.DeprecatedIn21):
+            assert gn.value == u"administrator"
+
+        assert gn.bytes_value == b"administrator"
 
     def test_idna(self):
-        gn = x509.RFC822Name(u"email@em\xe5\xefl.com")
-        assert gn.value == u"email@em\xe5\xefl.com"
-        assert gn._encoded == b"email@xn--eml-vla4c.com"
+        with pytest.warns(utils.DeprecatedIn21):
+            gn = x509.RFC822Name(u"email@em\xe5\xefl.com")
+
+        with pytest.warns(utils.DeprecatedIn21):
+            assert gn.value == u"email@em\xe5\xefl.com"
+
+        assert gn.bytes_value == b"email@xn--eml-vla4c.com"
 
     def test_hash(self):
-        g1 = x509.RFC822Name(u"email@host.com")
-        g2 = x509.RFC822Name(u"email@host.com")
-        g3 = x509.RFC822Name(u"admin@host.com")
+        g1 = x509.RFC822Name(b"email@host.com")
+        g2 = x509.RFC822Name(b"email@host.com")
+        g3 = x509.RFC822Name(b"admin@host.com")
 
         assert hash(g1) == hash(g2)
         assert hash(g1) != hash(g3)
@@ -1766,7 +1775,7 @@ class TestGeneralNames(object):
             x509.DNSName(b"cryptography.io"),
             x509.DNSName(b"crypto.local"),
             x509.DNSName(b"another.local"),
-            x509.RFC822Name(u"email@another.local"),
+            x509.RFC822Name(b"email@another.local"),
             x509.UniformResourceIdentifier(b"http://another.local"),
         ])
         assert gn[-1] == gn[4]
@@ -1807,7 +1816,7 @@ class TestGeneralNames(object):
             [x509.DNSName(b"cryptography.io")]
         )
         gns2 = x509.GeneralNames(
-            [x509.RFC822Name(u"admin@cryptography.io")]
+            [x509.RFC822Name(b"admin@cryptography.io")]
         )
         assert gns != gns2
         assert gns != object()
@@ -1837,7 +1846,7 @@ class TestIssuerAlternativeName(object):
             x509.DNSName(b"cryptography.io"),
             x509.DNSName(b"crypto.local"),
             x509.DNSName(b"another.local"),
-            x509.RFC822Name(u"email@another.local"),
+            x509.RFC822Name(b"email@another.local"),
             x509.UniformResourceIdentifier(b"http://another.local"),
         ])
         assert ian[-1] == ian[4]
@@ -1880,7 +1889,7 @@ class TestIssuerAlternativeName(object):
             [x509.DNSName(b"cryptography.io")]
         )
         san2 = x509.IssuerAlternativeName(
-            [x509.RFC822Name(u"admin@cryptography.io")]
+            [x509.RFC822Name(b"admin@cryptography.io")]
         )
         assert san != san2
         assert san != object()
@@ -1953,7 +1962,7 @@ class TestSubjectAlternativeName(object):
             x509.DNSName(b"cryptography.io"),
             x509.DNSName(b"crypto.local"),
             x509.DNSName(b"another.local"),
-            x509.RFC822Name(u"email@another.local"),
+            x509.RFC822Name(b"email@another.local"),
             x509.UniformResourceIdentifier(b"http://another.local"),
         ])
         assert san[-1] == san[4]
@@ -1996,7 +2005,7 @@ class TestSubjectAlternativeName(object):
             [x509.DNSName(b"cryptography.io")]
         )
         san2 = x509.SubjectAlternativeName(
-            [x509.RFC822Name(u"admin@cryptography.io")]
+            [x509.RFC822Name(b"admin@cryptography.io")]
         )
         assert san != san2
         assert san != object()