Merge lp:~openerp-dev/openobject-addons/trunk-bug-921442-nco into lp:openobject-addons

[Nimesh Contractor(Open ERP)]

Hello,

     For encoding trouble: encode the string in unicode, I have applied patch of Joël Grand-Guillaume @ CampToCamp (jgrandguillaume-c2c) and its working fine.

Thanks,
 NCO.

[Raphael Collet (OpenERP) (rco-openerp)]

Thanks!
Raphael

review: Approve

[Raphael Collet (OpenERP) (rco-openerp)]

Oh, I just noticed that you changed history.name into history.subject.
Why?

review: Needs Information

[Olivier Dony (Odoo) (odo-openerp)]

The fix with the missing ustr() call is fine, but it was already correct in the branch from kjo, so I'll merge that in the 6.1 branch separately.

The other fix from Joel is unfortunately not correct for multiple reasons:

1. There is still no proper way to reproduce it that does not involve injecting the error directly in the OpenERP code. We need a way to reproduce this by passing an email message to the parse_message() method, anything else is flawed: when you directly patch the code to produce an error you can do anything that is impossible to trigger in real cases.

2. This is not a proper way to avoid unicode errors. When you call unicode(txt, errors='ignore') you *explicitly* drop anything that is not ASCII within `txt` in the process, because anything else is considered invalid in the default ASCII encoding! e.g.:
  >>> unicode('abcdé', errors='ignore')
  u'abcd'
A cleaner way to do it would be to use errors='replace' and pass the original encoding so only invalid bytes *in the specified encoding* will be replaced. i.e.
  >>> unicode('abcdé' + chr(255), 'utf8', errors='replace')
  u'abcd\xe9\ufffd''

3. This patch is attempting to spot-patch just one out of the dozens of many conversions that are performed in the parse_message() method, which makes little sense. If the code is supposed to be tolerant to malformed email parts, it should not do it for only of the possible places that can be malformed.

review: Disapprove

=== modified file 'base_action_rule/base_action_rule.py'
--- base_action_rule/base_action_rule.py	2011-12-27 09:27:00 +0000
+++ base_action_rule/base_action_rule.py	2012-02-21 09:12:25 +0000
@@ -28,6 +28,7 @@
 import re
 import time
 import tools
+from openerp.loglevels import ustr 
 
 
 def get_datetime(date_field):
@@ -369,8 +370,8 @@
         reg_name = action.regex_name
         result_name = True
         if reg_name:
-            ptrn = re.compile(str(reg_name))
-            _result = ptrn.search(str(obj.name))
+            ptrn = re.compile(ustr(reg_name))
+            _result = ptrn.search(ustr(obj.name))
             if not _result:
                 result_name = False
         regex_n = not reg_name or result_name
 
=== modified file 'crm/crm_action_rule.py'
--- crm/crm_action_rule.py	2011-12-20 15:29:31 +0000
+++ crm/crm_action_rule.py	2012-02-21 09:12:25 +0000
@@ -27,6 +27,7 @@
 from osv import osv
 
 import crm
+from openerp.loglevels import ustr 
 
 class base_action_rule(osv.osv):
     """ Base Action Rule """
@@ -73,9 +74,9 @@
         regex = action.regex_history
         if regex:
             res = False
-            ptrn = re.compile(str(regex))
+            ptrn = re.compile(ustr(regex))
             for history in obj.message_ids:
-                _result = ptrn.search(str(history.name))
+                _result = ptrn.search(ustr(history.subject))
                 if _result:
                     res = True
                     break
 
=== modified file 'mail/mail_message.py'
--- mail/mail_message.py	2012-01-31 13:36:57 +0000
+++ mail/mail_message.py	2012-02-21 09:12:25 +0000
@@ -453,6 +453,9 @@
 
             msg['body_text'] = body
         msg['attachments'] = attachments
+        # For encoding trouble: encode the string in unicode, ignoring unkown byte
+        if 'body_html' in msg and isinstance(msg['body_html'],str):
+            msg['body_html'] = unicode(msg['body_html'], errors='ignore')
 
         # for backwards compatibility:
         msg['body'] = msg['body_text']