jazzband / imaplib2

Fork of Piers Lauder's imaplib2 library for Python.
https://imaplib2.readthedocs.io/
MIT License
34 stars 29 forks source link

Reappearance of an old issue, in a different place #14

Open lelit opened 3 years ago

lelit commented 3 years ago

I'm re-experiencing a problem that I reported years ago: I'm on a Debian sid system and I use offlineimap to keep a local copy of my incoming email. As reported in the references issue, fetching from one of my providers gives the following traceback:

2021-02-01 14:29:00 WARNING: ERROR: While attempting to sync account 'pec'
  Incorrect padding
2021-02-01 14:29:00 WARNING: 
Traceback:
  File "/usr/share/offlineimap3/offlineimap/accounts.py", line 298, in syncrunner
    self.__sync()
  File "/usr/share/offlineimap3/offlineimap/accounts.py", line 374, in __sync
    remoterepos.getfolders()
  File "/usr/share/offlineimap3/offlineimap/repository/IMAP.py", line 648, in getfolders
    imapobj = self.imapserver.acquireconnection()
  File "/usr/share/offlineimap3/offlineimap/imapserver.py", line 592, in acquireconnection
    self.__authn_helper(imapobj)
  File "/usr/share/offlineimap3/offlineimap/imapserver.py", line 449, in __authn_helper
    if func(imapobj):
  File "/usr/share/offlineimap3/offlineimap/imapserver.py", line 375, in __authn_plain
    imapobj.authenticate('PLAIN', self.__plainhandler)
  File "/usr/lib/python3/dist-packages/imaplib2.py", line 691, in authenticate
    typ, dat = self._simple_command('AUTHENTICATE', mechanism.upper())
  File "/usr/lib/python3/dist-packages/imaplib2.py", line 1684, in _simple_command
    return self._command_complete(self._command(name, *args), kw)
  File "/usr/lib/python3/dist-packages/imaplib2.py", line 1404, in _command
    literal = literator(data, rqb)
  File "/usr/lib/python3/dist-packages/imaplib2.py", line 2247, in process
    ret = self.mech(self.decode(data))
  File "/usr/lib/python3/dist-packages/imaplib2.py", line 2279, in decode
    return binascii.a2b_base64(inp)

Inspecting the case, I found that the bad value triggering the error is go ahead, that is, the very same case cured in the commit 3e87a66750b4fe79ffafdb1cce8509801720d125.

lelit commented 3 years ago

Sorry, I left out the details on the two packages:

$ apt show offlineimap python3-imaplib2
Package: offlineimap
Version: 7.3.3+dfsg1-1+0.0~git20210105.00d395b+dfsg-2
...

Package: python3-imaplib2
Version: 2.57-5.2
...
RonnyPfannschmidt commented 3 years ago

at first glance i'm under the impression this is a downstream issue

im not familiar with the debian patches applied on top

RonnyPfannschmidt commented 3 years ago

unfortunately the bugfix back then didn't include a test to validate against different cases/misbehaviours

lelit commented 3 years ago

at first glance i'm under the impression this is a downstream issue

im not familiar with the debian patches applied on top

Not sure to understand what you mean, but Debian patches do not seem to touch related code. Can you explain your doubt?

miseran commented 2 years ago

I'm having the same issue, which is fixed in my case by changing 'go ahead' to b'go ahead' in the referenced fix. But I don't know if data is always a bytes object, or if this change will break some other situation.

diff --git a/imaplib2/imaplib2.py b/imaplib2/imaplib2.py
index cfa108f..373e240 100644
--- a/imaplib2/imaplib2.py
+++ b/imaplib2/imaplib2.py
@@ -1414,8 +1414,8 @@ class IMAP4(object):
             if not ok:
                 break

-            if data == 'go ahead':  # Apparently not uncommon broken IMAP4 server response to AUTHENTICATE command
-                data = ''
+            if data == b'go ahead': # Apparently not uncommon broken IMAP4 server response to AUTHENTICATE command
+                data = b''

             # Send literal
Shamar commented 1 year ago

Hi, an Italian friend got the same issue reading from a PEC few days ago. 🤷‍♂️

He was running offlineimap3 with a system imaplib2 installed on an updated Ubuntu. imaplib2.py didn't look much updated though:

__version__ = "3.05"
__release__ = "3"
__revision__ = "05"
__credits__ = """
[...]
Fix for shutown exception by Sebastien Gross <seb@chezwam.org> November 2015."""
__author__ = "Piers Lauder <piers@janeelix.com>"
__URL__ = "http://imaplib2.sourceforge.net"
__license__ = "Python License"

The workaround we used was to hack _Authenticator.decode like this:

    def decode(self, inp):
        if not inp:
            return b''

        try:        
            return binascii.a2b_base64(inp)
        except Exception as e:
            print(f"Error decoding {inp}", e)
            if inp == b'go ahead':
                return b''
            raise

I didn't try @miseran's fix because I wasn't aware if it back then.

So looks like we have two issues here: