From 2758aaa6f87311f01c0baf82ad9c1f0ef4535ab5 Mon Sep 17 00:00:00 2001 From: Martin Trigaux Date: Wed, 1 Jul 2015 12:36:17 +0200 Subject: [PATCH] [IMP] models: sanitize error messages in import Revert 83282f2d for a cleaner sanitizing earlier in the generation of the error message. If the import is failing, the error message contains the value that is problematic. Escape this value in case it contains '%' --- openerp/addons/base/ir/ir_fields.py | 91 ++++++++++++------- .../addons/test_impex/tests/test_import.py | 13 ++- openerp/models.py | 7 +- 3 files changed, 73 insertions(+), 38 deletions(-) diff --git a/openerp/addons/base/ir/ir_fields.py b/openerp/addons/base/ir/ir_fields.py index 77c0c830cba..69dbe144f87 100644 --- a/openerp/addons/base/ir/ir_fields.py +++ b/openerp/addons/base/ir/ir_fields.py @@ -37,6 +37,19 @@ class ConversionNotFound(ValueError): pass class ir_fields_converter(models.Model): _name = 'ir.fields.converter' + @api.model + def _format_import_error(self, error_type, error_msg, error_params=(), error_args=None): + # sanitize error params for later formatting by the import system + sanitize = lambda p: p.replace('%', '%%') if isinstance(p, basestring) else p + if error_params: + if isinstance(error_params, basestring): + error_params = sanitize(error_params) + elif isinstance(error_params, dict): + error_params = dict((k, sanitize(v)) for k, v in error_params.iteritems()) + elif isinstance(error_params, tuple): + error_params = tuple(map(sanitize, error_params)) + return error_type(error_msg % error_params, error_args) + @api.model def for_model(self, model, fromtype=str): """ Returns a converter object for the model. A converter is a @@ -148,29 +161,34 @@ class ir_fields_converter(models.Model): if value.lower() in falses: return False, [] - return True, [ImportWarning( - _(u"Unknown value '%s' for boolean field '%%(field)s', assuming '%s'") - % (value, yes), { - 'moreinfo': _(u"Use '1' for yes and '0' for no") - })] + return True, [self._format_import_error( + ImportWarning, + _(u"Unknown value '%s' for boolean field '%%(field)s', assuming '%s'"), + (value, yes), + {'moreinfo': _(u"Use '1' for yes and '0' for no")} + )] @api.model def _str_to_integer(self, model, field, value): try: return int(value), [] except ValueError: - raise ValueError( - _(u"'%s' does not seem to be an integer for field '%%(field)s'") - % value) + raise self._format_import_error( + ValueError, + _(u"'%s' does not seem to be an integer for field '%%(field)s'"), + value + ) @api.model def _str_to_float(self, model, field, value): try: return float(value), [] except ValueError: - raise ValueError( - _(u"'%s' does not seem to be a number for field '%%(field)s'") - % value) + raise self._format_import_error( + ValueError, + _(u"'%s' does not seem to be a number for field '%%(field)s'"), + value + ) @api.model def _str_id(self, model, field, value): @@ -184,10 +202,12 @@ class ir_fields_converter(models.Model): time.strptime(value, DEFAULT_SERVER_DATE_FORMAT) return value, [] except ValueError: - raise ValueError( - _(u"'%s' does not seem to be a valid date for field '%%(field)s'") % value, { - 'moreinfo': _(u"Use the format '%s'") % u"2012-12-31" - }) + raise self._format_import_error( + ValueError, + _(u"'%s' does not seem to be a valid date for field '%%(field)s'"), + value, + {'moreinfo': _(u"Use the format '%s'") % u"2012-12-31"} + ) @api.model def _input_tz(self): @@ -215,10 +235,12 @@ class ir_fields_converter(models.Model): parsed_value = datetime.datetime.strptime( value, DEFAULT_SERVER_DATETIME_FORMAT) except ValueError: - raise ValueError( - _(u"'%s' does not seem to be a valid datetime for field '%%(field)s'") % value, { - 'moreinfo': _(u"Use the format '%s'") % u"2012-12-31 23:59:59" - }) + raise self._format_import_error( + ValueError, + _(u"'%s' does not seem to be a valid datetime for field '%%(field)s'"), + value, + {'moreinfo': _(u"Use the format '%s'") % u"2012-12-31 23:59:59"} + ) input_tz = self._input_tz()# Apply input tz to the parsed naive datetime dt = input_tz.localize(parsed_value, is_dst=False) @@ -251,12 +273,12 @@ class ir_fields_converter(models.Model): if value == unicode(item) or value in labels: return item, [] - raise ValueError( - _(u"Value '%s' not found in selection field '%%(field)s'") % ( - value), { - 'moreinfo': [_label or unicode(item) for item, _label in selection - if _label or item] - }) + raise self._format_import_error( + ValueError, + _(u"Value '%s' not found in selection field '%%(field)s'"), + value, + {'moreinfo': [_label or unicode(item) for item, _label in selection if _label or item]} + ) @api.model def db_id_for(self, model, field, subfield, value): @@ -298,8 +320,10 @@ class ir_fields_converter(models.Model): id = tentative_id except psycopg2.DataError: # type error - raise ValueError( - _(u"Invalid database id '%s' for the field '%%(field)s'") % value, + raise self._format_import_error( + ValueError, + _(u"Invalid database id '%s' for the field '%%(field)s'"), + value, {'moreinfo': action}) elif subfield == 'id': field_type = _(u"external id") @@ -321,12 +345,17 @@ class ir_fields_converter(models.Model): % (len(ids)))) id, _name = ids[0] else: - raise Exception(_(u"Unknown sub-field '%s'") % subfield) + raise self._format_import_error( + Exception, + _(u"Unknown sub-field '%s'"), + subfield + ) if id is None: - raise ValueError( - _(u"No matching record found for %(field_type)s '%(value)s' in field '%%(field)s'") - % {'field_type': field_type, 'value': value}, + raise self._format_import_error( + ValueError, + _(u"No matching record found for %(field_type)s '%(value)s' in field '%%(field)s'"), + {'field_type': field_type, 'value': value}, {'moreinfo': action}) return id, field_type, warnings diff --git a/openerp/addons/test_impex/tests/test_import.py b/openerp/addons/test_impex/tests/test_import.py index 11bacf6cce1..eab986fb29a 100644 --- a/openerp/addons/test_impex/tests/test_import.py +++ b/openerp/addons/test_impex/tests/test_import.py @@ -111,6 +111,12 @@ class test_ids_stuff(ImporterCase): self.import_(['id', 'value'], [['somexmlid', '1234567']]) self.assertEqual([1234567], values(self.read())) + def test_wrong_format(self): + self.assertEqual( + self.import_(['value'], [['50%']]), + error(1, u"'50%' does not seem to be an integer for field 'unknown'")) + + class test_boolean_field(ImporterCase): model_name = 'export.boolean' @@ -163,10 +169,13 @@ class test_boolean_field(ImporterCase): # Problem: OpenOffice (and probably excel) output localized booleans ['VRAI'], [u'OFF'], + [u'是的'], + ['!&%#${}'], + ['%(field)s'], ]), - ok(8)) + ok(11)) self.assertEqual( - [True] * 8, + [True] * 11, values(self.read())) class test_integer_field(ImporterCase): diff --git a/openerp/models.py b/openerp/models.py index 83a212096ac..c72bdd1a0ea 100644 --- a/openerp/models.py +++ b/openerp/models.py @@ -1199,11 +1199,8 @@ class BaseModel(object): type = 'warning' if isinstance(exception, Warning) else 'error' # logs the logical (not human-readable) field name for automated # processing of response, but injects human readable in message - # Before applying string substitution, we need to escape all '%' - # characters that are not part of substitutable reference. - regex = '%%(?!\((%s)\)s)' % '|'.join(base.keys()) - message = unicode(re.sub(regex, '%%', exception.args[0])) % base - record = dict(base, type=type, field=field, message=message) + record = dict(base, type=type, field=field, + message=unicode(exception.args[0]) % base) if len(exception.args) > 1 and exception.args[1]: record.update(exception.args[1]) log(record)