Review: Disapprove
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.
--
https://code.launchpad.net/~openerp-dev/openobject-addons/trunk-bug-921442-nco/+merge/93940
Your team OpenERP R&D Team is subscribed to branch
lp:~openerp-dev/openobject-addons/trunk-bug-921442-nco.
_______________________________________________
Mailing list: https://launchpad.net/~openerp-dev-gtk
Post to : [email protected]
Unsubscribe : https://launchpad.net/~openerp-dev-gtk
More help : https://help.launchpad.net/ListHelp
