modoboa / modoboa-amavis

The amavis frontend of Modoboa
https://modoboa.org
MIT License
23 stars 14 forks source link

AttributeError at /quarantine/process/ - quarantined mails not being released #119

Closed psycotic2017 closed 4 years ago

psycotic2017 commented 5 years ago

Hi,

I'm running modoboa 1.14.0 and modoboa-amavis 1.2.3 under a Python 3.6 virtual environment.

On releasing quarantined mails I am receiving the following error:

AttributeError at /quarantine/process/
module 'string' has no attribute 'atoi'

Request Method:     POST
Request URL:        https://host.domain.tld/quarantine/process/
Django Version:     1.11.25
Exception Type:     AttributeError
Exception Value:    module 'string' has no attribute 'atoi'
Exception Location: /<modoboa_home>/env/lib/python3.6/site-packages/modoboa_amavis/lib.py in repl, line 76
Python Executable:  
Python Version:     3.6.9
Python Path:
['/<modoboa_home>/mx',
 '/<modoboa_home>/env/lib/python36.zip',
 '/<modoboa_home>/env/lib/python3.6',
 '/<modoboa_home>/env/lib/python3.6/lib-dynload',
 '/usr/local/lib/python3.6',
 '/<modoboa_home>/env/lib/python3.6/site-packages']

The actual line from modoboa_amavis/lib.py is:

return struct.pack("B", string.atoi(match.group(0)[1:], 16))

Unfortunately I have no python programming experience but a quick google suggests that string.atoi has been depreciated and doesn't exist in Python 3 and should possibly be replaced with int

The emails are not being released to the recipient although I'm not sure that the above is causing that.

Further down the traceback there is the following:

/<modoboa_home>/env/lib/python3.6/site-packages/modoboa_amavis/views.py in release

1. rcpt.mail.mail_id, rcpt.mail.secret_id, rcpt.rid.email

amr             <modoboa_amavis.lib.AMrelease object at 0x825806c88>
connector       <modoboa_amavis.sql_connector.SQLconnector object at 0x825844160>
error           None
i               'xV59S4bi1XHU'
mail_id         ['user@domain.tld xV59S4bi1XHU']
mid             'user@domain.tld xV59S4bi1XHU'
msgrcpts        [<Msgrcpt: Msgrcpt object>]
r               'user@domain.tld'
rcpt            <Msgrcpt: Msgrcpt object>
request         <WSGIRequest: POST '/quarantine/process/'>
valid_addresses []

Which all seems to be correct but then the next section of the traceback all the variables are prefixed with a letter "b":

/<modoboa_home>/env/lib/python3.6/site-packages/modoboa_amavis/lib.py in sendreq

1. answer = self.decode(answer)

answer     (b"setreply=450 4.5.0 Failure:%20Invalid%20mail_id%20'b'xV59S4bi1XHU''%20at%20("
            b'eval%2093)%20line%20281,%20<GEN40>%20line%206.\r\nreturn_value=tempfail\r\ne'
            b'xit_code=75\r\n\r\n')
mailid      b'xV59S4bi1XHU'
others     ()
recipient  b'user@domain.tld'
secretid   b'EzhMD31mQVqb'
self       <modoboa_amavis.lib.AMrelease object at 0x825806c88>

And in the mail server log there is the following message:

amavis[xxxxx]: (!!)policy_server FAILED: Invalid mail_id 'b'xV59S4bi1XHU'' at (eval 93) line 281, <GEN40> line 6.

Not sure if this is two separate issues but I've tried replacing:

return struct.pack("B", string.atoi(match.group(0)[1:], 16)) with return struct.pack("B", int(match.group(0)[1:], 16))

and the error goes away but equally the quarantined emails are still not being released and the error

amavis[xxxxx]: (!!)policy_server FAILED: Invalid mail_id 'b'xV59S4bi1XHU'' at (eval 93) line 281, <GEN40> line 6.

is still generated in the mail server log with the letter "b" incorrectly prepended to the mail_id

If you need any further info I will happily send it, would love to get this working.

lazynooblet commented 4 years ago

I'm also not a Python programmer, but I had a go at fixing this. I started with applying the int() fix you mentioned, which then led me to the incorrect mail_id error in Amavis logs you were seeing.

The 'b' symbol prefix is saying that the object is byte-encoded. Byte-encoded objects have a decode() function to convert back to string.

So to fix this, I changed the 'sendreq' function of the 'AMrelease' class to decode the information before it is passed to Amavis.

--- lib.py.orig 2020-01-05 18:44:29.605927959 +0000
+++ lib.py      2020-01-05 18:39:39.604364141 +0000
@@ -73,7 +73,7 @@

     def decode(self, answer):
         def repl(match):
-            return struct.pack("B", string.atoi(match.group(0)[1:], 16))
+            return struct.pack("B", int(match.group(0)[1:], 16))

         return re.sub(br"%([0-9a-fA-F]{2})", repl, answer)

@@ -87,7 +87,7 @@
 quar_type=Q
 recipient=%s

-""" % (mailid, secretid, recipient)))
+""" % (mailid.decode('utf-8'), secretid.decode('utf-8'), recipient.decode('utf-8'))))
         answer = self.sock.recv(1024)
         answer = self.decode(answer)
         if re.search(br"250 [\d\.]+ Ok", answer):
psycotic2017 commented 4 years ago

Thanks Stalks, your above mentioned fix works for me

Pleskan commented 4 years ago

That's patch works fine on MySQL backend, but doesn't work on PostgreSQL. Just for info @tonioo

tonioo commented 4 years ago

@Pleskan Can you give more information please? Like a stack trace?

Pleskan commented 4 years ago

Unfortunately no, for now. Just because I switched to MySQL in production environment. But as soon as I deploy test environment again (and have more time) I'll give you feedback ASAP.

tonioo commented 4 years ago

@Pleskan Thank you. I'll close this issue for now, please open a new one when you have more information.