Skip to content

Commit

Permalink
[FIX] core: don't break import on files triggering UnicodeEncodeError
Browse files Browse the repository at this point in the history
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.

Issue 2480064

closes odoo#72395

X-original-commit: a24fe7f
Signed-off-by: Xavier Morel (xmo) <xmo@odoo.com>
  • Loading branch information
xmo-odoo committed Jun 21, 2021
1 parent 7872724 commit b758f79
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 2 deletions.
3 changes: 2 additions & 1 deletion odoo/addons/base/models/ir_fields.py
Expand Up @@ -31,7 +31,6 @@ class ImportWarning(Warning):
class ConversionNotFound(ValueError):
pass


class IrFieldsConverter(models.AbstractModel):
_name = 'ir.fields.converter'
_description = 'Fields Converter'
Expand Down Expand Up @@ -84,6 +83,8 @@ def fn(record, log):
# uniform handling
w = ImportWarning(w)
log(field, w)
except (UnicodeEncodeError, UnicodeDecodeError) as e:
log(field, ValueError(str(e)))
except ValueError as e:
log(field, e)
return converted
Expand Down
2 changes: 2 additions & 0 deletions odoo/addons/test_impex/ir.model.access.csv
Expand Up @@ -24,3 +24,5 @@ access_export_many2many_other,access_export_many2many_other,model_export_many2ma
access_export_selection_withdefault,access_export_selection_withdefault,model_export_selection_withdefault,,1,1,1,1
access_export_one2many_recursive,access_export_one2many_recursive,model_export_one2many_recursive,,1,1,1,1
access_export_unique,access_export_unique,model_export_unique,,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
11 changes: 11 additions & 0 deletions odoo/addons/test_impex/models.py
Expand Up @@ -163,3 +163,14 @@ class OnlyOne(models.Model):
('value_unique', 'unique (value)', "The value must be unique"),
('pair_unique', 'unique (value2, value3)', "The values must be unique"),
]


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 changes: 14 additions & 0 deletions odoo/addons/test_impex/tests/test_load.py
Expand Up @@ -704,6 +704,20 @@ def test_name_create_enabled_m2o(self):
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'
Expand Down
2 changes: 1 addition & 1 deletion odoo/models.py
Expand Up @@ -1166,7 +1166,7 @@ def _log(base, record, field, exception):
exc_vals = dict(base, record=record, field=field_names[field])
record = dict(base, type=type, record=record, field=field,
message=str(exception.args[0]) % exc_vals)
if len(exception.args) > 1 and exception.args[1]:
if len(exception.args) > 1 and isinstance(exception.args[1], dict):
record.update(exception.args[1])
log(record)

Expand Down

0 comments on commit b758f79

Please sign in to comment.