Repository: mjs/imapclient

Issue:
Investigate claims that UTF-7 encoding/decoding doesn't always work
Originally reported by: **Menno Smits (Bitbucket: [mjs0](https://bitbucket.org/mjs0))**

---

See:  https://github.com/MarechJ/py3_imap_utf7

If/when any problems are fixed, ask the author to update the page. Also add comments to SO where this is mentioned.

---
- Bitbucket: https://bitbucket.org/mjs0/imapclient/issue/190


Discussion:
NicolasLM: Awesome! Can you add a changelog entry?
mlorant: Changelog added (in 2.0.0, new section "Fixed")

Patch:
diff --git a/doc/src/api.rst b/doc/src/api.rst
index 3bbfebd..50a2a0e 100644
--- a/doc/src/api.rst
+++ b/doc/src/api.rst
@@ -21,8 +21,8 @@ are encountered during parsing.
 
 Exceptions
 ~~~~~~~~~~
-IMAPClient wraps exceptions raised by imaplib to ease the error handling.
-All the exceptions related to IMAP errors are defined in the module
+IMAPClient wraps exceptions raised by imaplib to ease the error handling. 
+All the exceptions related to IMAP errors are defined in the module 
 `imapclient.exceptions`. The following general exceptions may be raised:
 
 * IMAPClientError: the base class for IMAPClient's exceptions and the
@@ -51,13 +51,9 @@ malformed exception. In particular:
 * backports.ssl.CertificateError: raised when TLS certification
   verification fails. This is *not* a subclass of SSLError.
 
-Utilities
-~~~~~~~~~
-.. automodule:: imapclient.testable_imapclient
-   :members:
 
 TLS Support
 ~~~~~~~~~~~
 
 .. automodule:: imapclient.tls
-   :members:
+   :members:
\ No newline at end of file
diff --git a/doc/src/releases.rst b/doc/src/releases.rst
index 29e3ef7..2bc6734 100644
--- a/doc/src/releases.rst
+++ b/doc/src/releases.rst
@@ -6,7 +6,7 @@
 
 Added
 -----
-- Connection and read/write operations timeout can now be distinct, 
+- Connection and read/write operations timeout can now be distinct,
   using `imapclient.SocketTimeout` namedtuple as `timeout` parameter.
 - A context manager is introduced to automatically close connections to remote
   servers.
@@ -20,6 +20,12 @@ Changed
 - More precise exceptions available in `imapclient.exceptions` are raised when
   an error happens
 
+Fixed
+-----
+- Modified UTF-7 encoding function had quirks in its original algorithm,
+  leading to incorrect encoded output in some cases. The algorithm, described
+  in RFC 3501, has been reimplemented to fix #187 and is better documented.
+
 Other
 -----
 - Drop support of OAUTH(1)
diff --git a/imapclient/imap_utf7.py b/imapclient/imap_utf7.py
index c689055..eeb5668 100644
--- a/imapclient/imap_utf7.py
+++ b/imapclient/imap_utf7.py
@@ -1,37 +1,17 @@
-# The contents of this file has been derived code from the Twisted project
-# (http://twistedmatrix.com/). The original author is Jp Calderone.
-
-# Twisted project license follows:
-
-# Permission is hereby granted, free of charge, to any person obtaining
-# a copy of this software and associated documentation files (the
-# "Software"), to deal in the Software without restriction, including
-# without limitation the rights to use, copy, modify, merge, publish,
-# distribute, sublicense, and/or sell copies of the Software, and to
-# permit persons to whom the Software is furnished to do so, subject to
-# the following conditions:
+# This file contains two main methods used to encode and decode UTF-7
+# string, described in the RFC 3501. There are some variations specific
+# to IMAP4rev1, so the built-in Python UTF-7 codec can't be used instead.
 #
-# The above copyright notice and this permission notice shall be
-# included in all copies or substantial portions of the Software.
-#
-# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
-# EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
-# MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
-# NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
-# LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
-# OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
-# WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
-
+# The main difference is the shift character (used to switch from ASCII to
+# base64 encoding context), which is & in this modified UTF-7 convention,
+# since + is considered as mainly used in mailbox names. 
+# Other variations and examples can be found in the RFC 3501, section 5.1.3.
 from __future__ import unicode_literals
 
+import binascii
 from six import binary_type, text_type, byte2int, iterbytes, unichr
 
 
-PRINTABLE = set(range(0x20, 0x26)) | set(range(0x27, 0x7f))
-
-# TODO: module needs refactoring (e.g. variable names suck)
-
-
 def encode(s):
     """Encode a folder name using IMAP modified UTF-7 encoding.
 
@@ -41,27 +21,36 @@ def encode(s):
     if not isinstance(s, text_type):
         return s
 
-    r = []
-    _in = []
-
-    def extend_result_if_chars_buffered():
-        if _in:
-            r.extend([b'&', modified_utf7(''.join(_in)), b'-'])
-            del _in[:]
+    res = []
+    b64_buffer = []
+    def consume_b64_buffer(buf):
+        """
+        Consume the buffer by encoding it into a modified base 64 representation
+        and surround it with shift characters & and -
+        """
+        if b64_buffer:
+            res.extend([b'&', base64_utf7_encode(buf), b'-'])
+            del buf[:]
 
     for c in s:
-        if ord(c) in PRINTABLE:
-            extend_result_if_chars_buffered()
-            r.append(c.encode('latin-1'))
-        elif c == '&':
-            extend_result_if_chars_buffered()
-            r.append(b'&-')
+        # printable ascii case should not be modified
+        if 0x20 <= ord(c) <= 0x7e:
+            consume_b64_buffer(b64_buffer)
+            # Special case: & is used as shift character so we need to escape it in ASCII
+            if c == '&':
+                res.append(b'&-')
+            else:
+                res.append(c.encode('ascii'))
+
+        # Bufferize characters that will be encoded in base64 and append them later 
+        # in the result, when iterating over ASCII character or the end of string
         else:
-            _in.append(c)
+            b64_buffer.append(c)
 
-    extend_result_if_chars_buffered()
+    # Consume the remaining buffer if the string finish with non-ASCII characters
+    consume_b64_buffer(b64_buffer)
 
-    return b''.join(r)
+    return b''.join(res)
 
 
 AMPERSAND_ORD = byte2int(b'&')
@@ -75,35 +64,43 @@ def decode(s):
     unicode. If non-bytes/str input is provided, the input is returned
     unchanged.
     """
-
     if not isinstance(s, binary_type):
         return s
 
-    r = []
-    _in = bytearray()
+    res = []
+    # Store base64 substring that will be decoded once stepping on end shift character
+    b64_buffer = bytearray()
     for c in iterbytes(s):
-        if c == AMPERSAND_ORD and not _in:
-            _in.append(c)
-        elif c == DASH_ORD and _in:
-            if len(_in) == 1:
-                r.append('&')
+        # Shift character without anything in buffer -> starts storing base64 substring
+        if c == AMPERSAND_ORD and not b64_buffer:
+            b64_buffer.append(c)
+        # End shift char. -> append the decoded buffer to the result and reset it
+        elif c == DASH_ORD and b64_buffer:
+            # Special case &-, representing "&" escaped
+            if len(b64_buffer) == 1:
+                res.append('&')
             else:
-                r.append(modified_deutf7(_in[1:]))
-            _in = bytearray()
-        elif _in:
-            _in.append(c)
+                res.append(base64_utf7_decode(b64_buffer[1:]))
+            b64_buffer = bytearray()
+        # Still buffering between the shift character and the shift back to ASCII
+        elif b64_buffer:
+            b64_buffer.append(c)
+        # No buffer initialized yet, should be an ASCII printable char
         else:
-            r.append(unichr(c))
-    if _in:
-        r.append(modified_deutf7(_in[1:]))
-    return ''.join(r)
+            res.append(unichr(c))
+
+    # Decode the remaining buffer if any
+    if b64_buffer:
+        res.append(base64_utf7_decode(b64_buffer[1:]))
+
+    return ''.join(res)
 
 
-def modified_utf7(s):
-    s_utf7 = s.encode('utf-7')
-    return s_utf7[1:-1].replace(b'/', b',')
+def base64_utf7_encode(buffer):
+    s = ''.join(buffer).encode('utf-16be')
+    return binascii.b2a_base64(s).rstrip(b'\n=').replace(b'/', b',')
 
 
-def modified_deutf7(s):
+def base64_utf7_decode(s):
     s_utf7 = b'+' + s.replace(b',', b'/') + b'-'
     return s_utf7.decode('utf-7')


Test patch:
diff --git a/tests/imapclient_test.py b/tests/imapclient_test.py
index 4f6c406..7bef2dc 100644
--- a/tests/imapclient_test.py
+++ b/tests/imapclient_test.py
@@ -1,4 +1,4 @@
-from imapclient.testable_imapclient import TestableIMAPClient as IMAPClient
+from .testable_imapclient import TestableIMAPClient as IMAPClient
 from .util import unittest
 
 
diff --git a/tests/test_imap_utf7.py b/tests/test_imap_utf7.py
index 9fe64f1..29e7d0e 100644
--- a/tests/test_imap_utf7.py
+++ b/tests/test_imap_utf7.py
@@ -22,6 +22,7 @@ class IMAP4UTF7TestCase(unittest.TestCase):
         ['~peter/mail/\u65e5\u672c\u8a9e/\u53f0\u5317',
          b'~peter/mail/&ZeVnLIqe-/&U,BTFw-'],  # example from RFC 2060
         ['\x00foo', b'&AAA-foo'],
+        ['foo\r\n\nbar\n', b'foo&AA0ACgAK-bar&AAo-'] # see imapclient/#187 issue
     ]
 
     def test_encode(self):
diff --git a/tests/test_imapclient.py b/tests/test_imapclient.py
index 2ce58bf..4152725 100644
--- a/tests/test_imapclient.py
+++ b/tests/test_imapclient.py
@@ -15,8 +15,8 @@ import six
 from imapclient.exceptions import CapabilityError, IMAPClientError
 from imapclient.imapclient import IMAPlibLoggerAdapter
 from imapclient.fixed_offset import FixedOffset
-from imapclient.testable_imapclient import TestableIMAPClient as IMAPClient
 
+from .testable_imapclient import TestableIMAPClient as IMAPClient
 from .imapclient_test import IMAPClientTest
 from .util import patch, sentinel, Mock
 
diff --git a/imapclient/testable_imapclient.py b/tests/testable_imapclient.py
similarity index 54%
rename from imapclient/testable_imapclient.py
rename to tests/testable_imapclient.py
index 64d6411..6efe29d 100644
--- a/imapclient/testable_imapclient.py
+++ b/tests/testable_imapclient.py
@@ -4,28 +4,11 @@
 
 from __future__ import unicode_literals
 
-from .imapclient import IMAPClient
-
-try:
-    from unittest.mock import Mock
-except ImportError:
-    try:
-        from mock import Mock
-    except ImportError:
-        raise ImportError(
-            'mock library could not be loaded. Please install Python 3.3 or newer '
-            'or install the `mock` third-party package through PyPi.'
-        )
+from imapclient.imapclient import IMAPClient
+from .util import Mock
 
 
 class TestableIMAPClient(IMAPClient):
-    """Wrapper of :py:class:`imapclient.IMAPClient` that mocks all
-    interaction with real IMAP server.
-
-    This class should only be used in tests, where you can safely
-    interact with imapclient without running commands on a real
-    IMAP account.
-    """
 
     def __init__(self):
         super(TestableIMAPClient, self).__init__('somehost')