[FIX] core: don't break import on files triggering UnicodeEncodeError

The import logging (ish) assumes that if an exception has at least 2
args the second arg is metadata added by the callee.

As it turns out, `UnicodeEncodeError` has *five* arguments, none of
which is added by us. So if encoding something fails during the
process (e.g. because the file contains a lone surrogate, which leads
to the database insert failing when psycopg2 tries to encode the query
to UTF8), then the `_log` function itself will fail, yielding a very
unhelpful error of:

   dictionary update sequence element #0 has length 1; 2 is required

(because we tried to update a dict using a string).

This issue occurs only during *field conversion* and most fields have
no need to interact with the database (so don't need to encode the
value, which is what fails), however it is a problem when the invalid
string is used as a record name to look for (e.g. an m2o).

Further improve the experience by converting the UnicodeEncodeError to
a ValueError using the stringified UEE: `_log` assumes the first
argument to the exception is an error message of some sort, but for
UnicodeError subclasses it's just the encoding involved in the
error (here `utf-8`), which doesn't really serve as an error message.

Stringifying the exception generates a complete error message which is
quite a bit more helpful.

Specific update notes:

* avoid modifying the exception in-place, doesn't seem useful
* not sure why `field_name` was added as part of the augmentation
  rather than up-front when `record` is created

Issue 2480064

closes odoo/odoo#72517

X-original-commit: 6c3c500929cd463cd3a1749f4ede8f3a8afd5748
Signed-off-by: Xavier Morel (xmo) <xmo@odoo.com>
This commit is contained in:
Xavier Morel
2021-06-22 09:33:43 +00:00
parent 0efc1b3384
commit 7ac3a391a1
5 changed files with 35 additions and 7 deletions
+2 -1
View File
@@ -24,7 +24,6 @@ class ImportWarning(Warning):
class ConversionNotFound(ValueError):
pass
class IrFieldsConverter(models.AbstractModel):
_name = 'ir.fields.converter'
_description = 'Fields Converter'
@@ -113,6 +112,8 @@ class IrFieldsConverter(models.AbstractModel):
# uniform handling
w = ImportWarning(w)
log(field, w)
except (UnicodeEncodeError, UnicodeDecodeError) as e:
log(field, ValueError(str(e)))
except ValueError as e:
if import_file_context:
# if the error is linked to a matching error, the error is a tuple
@@ -26,3 +26,5 @@ access_export_one2many_recursive,access_export_one2many_recursive,model_export_o
access_export_unique,access_export_unique,model_export_unique,,1,1,1,1
access_export_inherits_parent,access_export_inherits_parent,model_export_inherits_parent,,1,1,1,1
access_export_inherits_child,access_export_inherits_child,model_export_inherits_child,,1,1,1,1
access_export_m2o_str,access_export_m2o_str,model_export_m2o_str,,1,1,1,1
access_export_m2o_str_child,access_export_m2o_str_child,model_export_m2o_str_child,,1,1,1,1
1 id name model_id:id group_id:id perm_read perm_write perm_create perm_unlink
26 access_export_unique access_export_unique model_export_unique 1 1 1 1
27 access_export_inherits_parent access_export_inherits_parent model_export_inherits_parent 1 1 1 1
28 access_export_inherits_child access_export_inherits_child model_export_inherits_child 1 1 1 1
29 access_export_m2o_str access_export_m2o_str model_export_m2o_str 1 1 1 1
30 access_export_m2o_str_child access_export_m2o_str_child model_export_m2o_str_child 1 1 1 1
+10
View File
@@ -174,3 +174,13 @@ class InheritsChild(models.Model):
parent_id = fields.Many2one('export.inherits.parent', required=True, ondelete='cascade')
value = fields.Integer()
class Many2String(models.Model):
_name = _description = 'export.m2o.str'
child_id = fields.Many2one('export.m2o.str.child')
class ChidToString(models.Model):
_name = _description = 'export.m2o.str.child'
name = fields.Char()
+14
View File
@@ -737,6 +737,20 @@ class test_m2o(ImporterCase):
self.assertFalse(result['messages'])
self.assertEqual(len(result['ids']), 1)
class TestInvalidStrings(ImporterCase):
model_name = 'export.m2o.str'
@mute_logger('odoo.sql_db')
def test_fail_unpaired_surrogate(self):
result = self.import_(['child_id'], [['\uddff']])
self.assertTrue(result['messages'])
self.assertIn('surrogates', result['messages'][0]['message'])
@mute_logger('odoo.sql_db')
def test_fail_nul(self):
result = self.import_(['child_id'], [['\x00']])
self.assertTrue(result['messages'])
self.assertIn('NUL', result['messages'][0]['message'])
class test_m2m(ImporterCase):
model_name = 'export.many2many'
+7 -6
View File
@@ -1312,13 +1312,14 @@ class BaseModel(metaclass=MetaModel):
exc_vals = dict(base, record=record, field=field_name)
record = dict(base, type=type, record=record, field=field,
message=str(exception.args[0]) % exc_vals)
# ensure to add field_name to the exception. Used in import to concatenate multiple errors in the same block
if len(exception.args) > 1:
if not exception.args[1]:
exception.args = (exception.args[0], {'field_name': field_name})
else:
exception.args[1]['field_name'] = field_name
record.update(exception.args[1])
info = {}
if exception.args[1] and isinstance(exception.args[1], dict):
info = exception.args[1]
# ensure field_name is added to the exception. Used in import to
# concatenate multiple errors in the same block
info['field_name'] = field_name
record.update(info)
log(record)
stream = CountingStream(records)