From b5214e1d402b4cd34ad2d0adb8c7680607dea059 Mon Sep 17 00:00:00 2001
From: Julien Van Roy <juvr@odoo.com>
Date: Tue, 25 Apr 2023 07:54:12 +0000
Subject: [PATCH] [FIX] mail,account_edi: fix creation of invoice upon email
 reception
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When receiving an email on a mailbox with an alias that triggers the
creation of invoices, 4 bugs could occur.

1. If the xml received contains replacement characters (U+FFFD �), and
the charset of the part of the email is "US-ASCII" the encoding of the
string will fail, preventing the rest of the flow to be completed. Be
more resilient, encode the string and ignores these characters if this
case occurs.

NB: sometimes, the charset is omitted for a Content-type: text/xml. This
is valid but not recommended (see:
https://www.ietf.org/rfc/rfc2376.txt). In this case, the default used is
"US-ASCII". This means that any non-ascii char will be lost (they are
replaced by the replacement character: �, see:
https://github.com/python/cpython/blob/3.10/Lib/email/contentmanager.py#L67)
when decoding the attachment.

2. When the xml attachment is created in Odoo, the mimetype is
'text/plain' (rather than 'application/xml'). Thus, the
`_decode_attachment` needs to be more flexible when guessing the type of
the attachment (to know which function to use to read the content of the
attachment and create the invoice).

3. When creating an invoice from an email with an xml attachment, the
xml is attached as the `message_main_attachment_id`. It's only later on
that the content of the xml is read and we possibly find the PDF in
base64 inside. When creating the PDF attachment, it was not set as the
`message_main_attachment_id`, so the PDF was not rendered on the right
part of the invoice form view. Add a clause to replace the
`message_main_attachment_id` in such a case.

4. When the xml attachment represents a credit note, the move_type of
the invoice created by the email alias needs to be changed. Indeed, the
invoice is created before decoding the attachment, so we can only change
the `move_type` later.

opw-3144519
opw-3149649

closes odoo/odoo#119602

Signed-off-by: Laurent Smet <las@odoo.com>
---
 .../account_edi/models/account_edi_format.py  |  6 +--
 .../models/account_edi_common.py              | 18 ++++++-
 .../tests/test_xml_ubl_be.py                  | 15 ++++++
 addons/mail/models/mail_thread.py             |  5 +-
 addons/test_mail/data/test_mail_data.py       | 50 +++++++++++++++++++
 addons/test_mail/tests/test_mail_gateway.py   | 21 ++++++++
 6 files changed, 109 insertions(+), 6 deletions(-)

diff --git a/addons/account_edi/models/account_edi_format.py b/addons/account_edi/models/account_edi_format.py
index d7babc97a467..70f8d9c02289 100644
--- a/addons/account_edi/models/account_edi_format.py
+++ b/addons/account_edi/models/account_edi_format.py
@@ -439,9 +439,9 @@ class AccountEdiFormat(models.Model):
         content = base64.b64decode(attachment.with_context(bin_size=False).datas)
         to_process = []
 
-        # XML attachments received by mail have a 'text/plain' mimetype.
-        # Therefore, if content start with '<?xml', it is considered as XML.
-        is_text_plain_xml = 'text/plain' in attachment.mimetype and content.startswith(b'<?xml')
+        # XML attachments received by mail have a 'text/plain' mimetype (cfr. context key: 'attachments_mime_plainxml')
+        # Therefore, if content start with '<?xml', or if the filename ends with '.xml', it is considered as XML.
+        is_text_plain_xml = 'text/plain' in attachment.mimetype and (content.startswith(b'<?xml') or attachment.name.endswith('.xml'))
         if 'pdf' in attachment.mimetype:
             to_process.extend(self._decode_pdf(attachment.name, content))
         elif attachment.mimetype.endswith('/xml') or is_text_plain_xml:
diff --git a/addons/account_edi_ubl_cii/models/account_edi_common.py b/addons/account_edi_ubl_cii/models/account_edi_common.py
index 81915b6db80a..af1eff773069 100644
--- a/addons/account_edi_ubl_cii/models/account_edi_common.py
+++ b/addons/account_edi_ubl_cii/models/account_edi_common.py
@@ -271,7 +271,14 @@ class AccountEdiCommon(models.AbstractModel):
         else:
             return
         if existing_invoice and existing_invoice.move_type != move_type:
-            return
+            # with an email alias to create account_move, first the move is created (using alias_defaults, which
+            # contains move_type = 'out_invoice') then the attachment is decoded, if it represents a credit note,
+            # the move type needs to be changed to 'out_refund'
+            types = {move_type, existing_invoice.move_type}
+            if types == {'out_invoice', 'out_refund'} or types == {'in_invoice', 'in_refund'}:
+                existing_invoice.move_type = move_type
+            else:
+                return
 
         invoice = existing_invoice or self.env['account.move']
         invoice_form = Form(invoice.with_context(
@@ -306,7 +313,7 @@ class AccountEdiCommon(models.AbstractModel):
                 # (Windows or Linux style) and/or the name of the xml instead of the pdf.
                 # Get only the filename with a pdf extension.
                 name = attachment_name.text.split('\\')[-1].split('/')[-1].split('.')[0] + '.pdf'
-                attachments |= self.env['ir.attachment'].create({
+                attachment = self.env['ir.attachment'].create({
                     'name': name,
                     'res_id': invoice.id,
                     'res_model': 'account.move',
@@ -314,6 +321,13 @@ class AccountEdiCommon(models.AbstractModel):
                     'type': 'binary',
                     'mimetype': 'application/pdf',
                 })
+                # Upon receiving an email (containing an xml) with a configured alias to create invoice, the xml is
+                # set as the main_attachment. To be rendered in the form view, the pdf should be the main_attachment.
+                if invoice.message_main_attachment_id and \
+                        invoice.message_main_attachment_id.name.endswith('.xml') and \
+                        'pdf' not in invoice.message_main_attachment_id.mimetype:
+                    invoice.message_main_attachment_id = attachment
+                attachments |= attachment
         if attachments:
             invoice.with_context(no_new_invoice=True).message_post(attachment_ids=attachments.ids)
 
diff --git a/addons/l10n_account_edi_ubl_cii_tests/tests/test_xml_ubl_be.py b/addons/l10n_account_edi_ubl_cii_tests/tests/test_xml_ubl_be.py
index 0d6a3e504b55..f16987ef7b17 100644
--- a/addons/l10n_account_edi_ubl_cii_tests/tests/test_xml_ubl_be.py
+++ b/addons/l10n_account_edi_ubl_cii_tests/tests/test_xml_ubl_be.py
@@ -347,3 +347,18 @@ class TestUBLBE(TestUBLCommon):
         # source: vat-category-E.xml
         self._assert_imported_invoice_from_file(subfolder=subfolder, filename='bis3_tax_exempt_gbp.xml',
             amount_total=1200, amount_tax=0, list_line_subtotals=[1200], currency_id=self.env.ref('base.GBP').id)
+
+    def test_import_existing_invoice_flip_move_type(self):
+        """ Tests whether the move_type of an existing invoice can be flipped when importing an attachment
+        For instance: with an email alias to create account_move, first the move is created (using alias_defaults,
+        which contains move_type = 'out_invoice') then the attachment is decoded, if it represents a credit note,
+        the move type needs to be changed to 'out_refund'
+        """
+        invoice = self.env['account.move'].create({'move_type': 'out_invoice'})
+        self.update_invoice_from_file(
+            'l10n_account_edi_ubl_cii_tests',
+            'tests/test_files/from_odoo',
+            'bis3_out_refund.xml',
+            invoice,
+        )
+        self.assertRecordValues(invoice, [{'move_type': 'out_refund', 'amount_total': 3164.22}])
diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py
index b31125792fbf..9ac155e6db4a 100644
--- a/addons/mail/models/mail_thread.py
+++ b/addons/mail/models/mail_thread.py
@@ -1745,7 +1745,10 @@ class MailThread(models.AbstractModel):
                     continue
                 if isinstance(content, str):
                     encoding = info and info.get('encoding')
-                    content = content.encode(encoding or 'utf-8')
+                    try:
+                        content = content.encode(encoding or "utf-8")
+                    except UnicodeEncodeError:
+                        content = content.encode("utf-8")
                 elif isinstance(content, EmailMessage):
                     content = content.as_bytes()
                 elif content is None:
diff --git a/addons/test_mail/data/test_mail_data.py b/addons/test_mail/data/test_mail_data.py
index 8c30bd2778f2..2b5695d59421 100644
--- a/addons/test_mail/data/test_mail_data.py
+++ b/addons/test_mail/data/test_mail_data.py
@@ -276,6 +276,56 @@ SGVsbG8gd29ybGQK
 --Apple-Mail=_9331E12B-8BD2-4EC7-B53E-01F3FBEC9227--
 """
 
+MAIL_MULTIPART_INVALID_ENCODING = """Return-Path: <whatever-2a840@postmaster.twitter.com>
+To: {to}
+cc: {cc}
+Received: by mail1.openerp.com (Postfix, from userid 10002)
+    id 5DF9ABFB2A; Fri, 10 Aug 2012 16:16:39 +0200 (CEST)
+From: {email_from}
+Subject: {subject}
+MIME-Version: 1.0
+Content-Type: multipart/alternative;
+    boundary="00000000000005d9da05fa394cc0"
+Date: Fri, 10 Aug 2012 14:16:26 +0000
+Message-ID: {msg_id}
+{extra}
+
+--00000000000005d9da05fa394cc0
+Content-Type: multipart/alternative; boundary="00000000000005d9d905fa394cbe"
+
+--00000000000005d9d905fa394cbe
+Content-Type: text/plain; charset="UTF-8"
+
+Dear customer,
+
+Please find attached the Peppol Bis 3 attachment of your invoice (with an
+encoding error in the address)
+
+Cheers,
+
+--00000000000005d9d905fa394cbe
+Content-Type: text/html; charset="UTF-8"
+
+<div dir="ltr">Dear customer,<div><br></div><div>Please find attached the Peppol Bis 3 attachment of your invoice (with an encoding error in the address)</div><div><br></div><div>Cheers,</div></div>
+
+--00000000000005d9d905fa394cbe--
+
+--00000000000005d9da05fa394cc0
+Content-Type: text/xml; charset="US-ASCII";
+ name="bis3_with_error_encoding_address.xml"
+Content-Disposition: attachment; 
+	filename="bis3_with_error_encoding_address.xml"
+Content-Transfer-Encoding: base64
+Content-ID: <f_lgxgdqx40>
+X-Attachment-Id: f_lgxgdqx40
+
+PEludm9pY2UgeG1sbnM6Y2JjPSJ1cm46b2FzaXM6bmFtZXM6c3BlY2lmaWNhdGlvbjp1Ymw6c2No
+ZW1hOnhzZDpDb21tb25CYXNpY0NvbXBvbmVudHMtMiIgeG1sbnM9InVybjpvYXNpczpuYW1lczpz
+cGVjaWZpY2F0aW9uOnVibDpzY2hlbWE6eHNkOkludm9pY2UtMiI+DQo8Y2JjOlN0cmVldE5hbWU+
+Q2hhdXNz77+977+9ZSBkZSBCcnV4ZWxsZXM8L2NiYzpTdHJlZXROYW1lPg0KPC9JbnZvaWNlPg0K
+--00000000000005d9da05fa394cc0--
+"""
+
 
 MAIL_SINGLE_BINARY = """X-Original-To: raoul@grosbedon.fr
 Delivered-To: raoul@grosbedon.fr
diff --git a/addons/test_mail/tests/test_mail_gateway.py b/addons/test_mail/tests/test_mail_gateway.py
index f0b381ce615e..1ad2b01ce8c5 100644
--- a/addons/test_mail/tests/test_mail_gateway.py
+++ b/addons/test_mail/tests/test_mail_gateway.py
@@ -1486,6 +1486,27 @@ class TestMailgateway(TestMailCommon):
             if encoding not in ['', 'UTF-8']:
                 self.assertNotEqual(file_content, attachment.raw.decode('utf-8'))
 
+    # --------------------------------------------------
+    # Corner cases / Bugs during message process
+    # --------------------------------------------------
+
+    def test_message_process_file_encoding_ascii(self):
+        """ Incoming email containing an xml attachment with unknown characters (�) but an ASCII charset should not
+        raise an Exception. UTF-8 is used as a safe fallback.
+        """
+        record = self.format_and_process(test_mail_data.MAIL_MULTIPART_INVALID_ENCODING, self.email_from, 'groups@test.com')
+
+        self.assertEqual(record.message_main_attachment_id.name, 'bis3_with_error_encoding_address.xml')
+        # NB: the xml received by email contains b"Chauss\xef\xbf\xbd\xef\xbf\xbde" with "\xef\xbf\xbd" being the
+        # replacement character � in UTF-8.
+        # When calling `_message_parse_extract_payload`, `part.get_content()` will be called on the attachment part of
+        # the email, triggering the decoding of the base64 attachment, so b"Chauss\xef\xbf\xbd\xef\xbf\xbde" is
+        # first retrieved. Then, `get_text_content` in `email` tries to decode this using the charset of the email
+        # part, i.e: `content.decode('us-ascii', errors='replace')`. So the errors are replaced using the Unicode
+        # replacement marker and the string "Chauss������e" is used to create the attachment.
+        # This explains the multiple "�" in the attachment.
+        self.assertIn("Chauss������e de Bruxelles", record.message_main_attachment_id.raw.decode())
+
 
 class TestMailThreadCC(TestMailCommon):
 
-- 
GitLab