From 105e0b9ef210002b2bf5b4499e1570b5e09b0718 Mon Sep 17 00:00:00 2001 From: "Chong Wang (cwg)" Date: Thu, 22 Sep 2022 11:51:02 +0000 Subject: [PATCH] [FIX] core: search translated fields with characters needed to be escaped The trigram index function jsonb_path_query_array("column_name", '$.*')::text uses all translations' representations to build the indexed text. So the original text needs to be JSON-escaped correctly to match it. X-original-commit: 7547df664945dddcb839e4903068f7f25ecfc08c Part-of: odoo/odoo#103031 --- odoo/addons/base/tests/test_translate.py | 36 +++++++++++- .../test_new_api/models/test_new_api.py | 7 +++ .../test_new_api/security/ir.model.access.csv | 1 + odoo/addons/test_new_api/tests/__init__.py | 1 + .../tests/test_indexed_translation.py | 40 +++++++++++++ odoo/osv/expression.py | 18 ++++-- odoo/tools/sql.py | 58 +++++++++++++++++++ 7 files changed, 154 insertions(+), 7 deletions(-) create mode 100644 odoo/addons/test_new_api/tests/test_indexed_translation.py diff --git a/odoo/addons/base/tests/test_translate.py b/odoo/addons/base/tests/test_translate.py index 0a9fdc4bee8..efb36b3a2fd 100644 --- a/odoo/addons/base/tests/test_translate.py +++ b/odoo/addons/base/tests/test_translate.py @@ -9,7 +9,7 @@ from psycopg2.extras import Json import io from odoo.exceptions import AccessError, ValidationError -from odoo.tools import trans_load_data +from odoo.tools import trans_load_data, sql from odoo.tools.translate import quote, unquote, xml_translate, html_translate from odoo.tests.common import TransactionCase, BaseCase, new_test_user, tagged @@ -854,3 +854,37 @@ class TestLanguageInstallPerformance(TransactionCase): fr_BE.toggle_active() t1 = time.time() _stats_logger.info("installed language fr_BE in %.3fs", t1 - t0) + + +class TestTranslationTrigramIndexPatterns(BaseCase): + def test_value_conversion(self): + cases = [ + # pylint: disable=bad-whitespace + ( 'abc', '%abc%', 'simple text is not escaped correctly'), + ( 'a"bc', r'%a\\"bc%', '" is not escaped correctly'), + (r'a\bc', r'%a\\\\bc%', r'\ is not escaped correctly'), + ( 'a\nbc', r'%a\\nbc%', r'\n is not escaped correctly'), + ( 'a_bc', r'%a\_bc%', '_ is not escaped correctly'), + ( 'a%bc', r'%a\%bc%', '% is not escaped correctly'), + ( 'a_', '%', 'values with less than 3 characters should be dropped'), + ] + for value, expected, message in cases: + self.assertEqual(sql.value_to_translated_trigram_pattern(value), expected, message) + + def test_pattern_conversion(self): + cases = [ + # pylint: disable=bad-whitespace + ( 'abc', '%abc%', 'simple pattern is not escaped correctly'), + ( 'a"bc', r'%a\\"bc%', '" is not escaped correctly'), + (r'a\\bc', r'%a\\\\bc%', r'\ is not escaped correctly'), + ( 'a\nbc', r'%a\\nbc%', r'\n is not escaped correctly'), + (r'a\_bc', r'%a\_bc%', r"\_ shouldn't be escaped"), + (r'a\%bc', r'%a\%bc%', r"\% shouldn't be escaped"), + ( 'abc_def', '%abc%def%', 'wildcard character _ should be changed to %'), + ( 'abc%def', '%abc%def%', "wildcard character % shouldn't be escaped"), + (r'a\bc', '%abc%', r'redundant \ for pattern should be removed'), + ( 'abc_de', '%abc%', 'sub patterns less than 3 characters should be dropped'), + ( 'ab', '%', 'patterns without trigram should be simplified'), + ] + for original_pattern, escaped_pattern, message in cases: + self.assertEqual(sql.pattern_to_translated_trigram_pattern(original_pattern), escaped_pattern, message) diff --git a/odoo/addons/test_new_api/models/test_new_api.py b/odoo/addons/test_new_api/models/test_new_api.py index 225e193b783..2330cd4eabd 100644 --- a/odoo/addons/test_new_api/models/test_new_api.py +++ b/odoo/addons/test_new_api/models/test_new_api.py @@ -1730,3 +1730,10 @@ class RelatedTranslation3(models.Model): parent_id = fields.Many2one('test_new_api.related_translation_2', string='Parent Model') name = fields.Char('Name Related', related='parent_id.name', readonly=False) html = fields.Html('HTML Related', related='parent_id.html', readonly=False) + + +class IndexedTranslation(models.Model): + _name = 'test_new_api.indexed_translation' + _description = 'A model to indexed translated fields' + + name = fields.Text('Name trigram', translate=True, index='trigram') diff --git a/odoo/addons/test_new_api/security/ir.model.access.csv b/odoo/addons/test_new_api/security/ir.model.access.csv index db6a005784e..137db627427 100644 --- a/odoo/addons/test_new_api/security/ir.model.access.csv +++ b/odoo/addons/test_new_api/security/ir.model.access.csv @@ -99,3 +99,4 @@ access_test_new_api_modified_line,access_test_new_api_modified_line,model_test_n access_test_new_api_related_translation_1,access_test_new_api_related_translation_1,model_test_new_api_related_translation_1,,1,1,1,1 access_test_new_api_related_translation_2,access_test_new_api_related_translation_2,model_test_new_api_related_translation_2,,1,1,1,1 access_test_new_api_related_translation_3,access_test_new_api_related_translation_3,model_test_new_api_related_translation_3,,1,1,1,1 +access_test_new_api_indexed_translation,access_test_new_api_indexed_translation,model_test_new_api_indexed_translation,,1,1,1,1 diff --git a/odoo/addons/test_new_api/tests/__init__.py b/odoo/addons/test_new_api/tests/__init__.py index c2a4b097bc9..db0523d2483 100644 --- a/odoo/addons/test_new_api/tests/__init__.py +++ b/odoo/addons/test_new_api/tests/__init__.py @@ -7,6 +7,7 @@ from . import test_one2many from . import test_many2many from . import test_properties from . import test_qweb_float +from . import test_indexed_translation from . import test_related_translation from . import test_ui from . import test_domain diff --git a/odoo/addons/test_new_api/tests/test_indexed_translation.py b/odoo/addons/test_new_api/tests/test_indexed_translation.py new file mode 100644 index 00000000000..90356750803 --- /dev/null +++ b/odoo/addons/test_new_api/tests/test_indexed_translation.py @@ -0,0 +1,40 @@ +# -*- coding: utf-8 -*- + +import odoo.tests + + +@odoo.tests.tagged('post_install', '-at_install') +class TestIndexedTranslation(odoo.tests.TransactionCase): + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.env['res.lang']._activate_lang('fr_FR') + cls.record_en = cls.env['test_new_api.indexed_translation'].with_context(lang='en_US').create({}) + cls.record_fr = cls.record_en.with_context(lang='fr_FR') + cls.record_en.name = '
hello
\n
' + cls.record_fr.name = '
%bonjour\\
\a
' + + def test_search_ilike(self): + self.assertEqual(self.record_en.name, '
hello
\n
') + self.assertEqual(self.record_fr.name, '
%bonjour\\
\a
') + + # matching double quotes + self.assertEqual(self.record_en.search([('name', 'ilike', 'class="my_class')]), self.record_en) + + # escaped and unescaped PG wildcards + self.assertEqual(self.record_en.search([('name', 'ilike', 'class%class')]), self.record_en) + self.assertEqual(self.record_en.search([('name', 'ilike', r'class="m_\_class')]), self.record_en) + self.assertEqual(self.record_en.search([('name', 'ilike', 'bonjour')]), self.record_en.browse()) + self.assertEqual(self.record_en.search([('name', 'ilike', '
\n
')]), self.record_en) + self.assertEqual(self.record_fr.search([('name', 'ilike', '
\a
')]), self.record_fr) + self.assertEqual(self.record_fr.search([('name', 'ilike', r'\%bonjour\\')]), self.record_fr) + + self.assertEqual( + self.record_en.search([('name', '=', '
hello
\n
')]), + self.record_en, + ) + self.assertEqual( + self.record_fr.search([('name', '=', '
%bonjour\\
\a
')]), + self.record_fr, + ) diff --git a/odoo/osv/expression.py b/odoo/osv/expression.py index 01a04f64d09..8a4a03e3228 100644 --- a/odoo/osv/expression.py +++ b/odoo/osv/expression.py @@ -124,7 +124,7 @@ from psycopg2.sql import Composable, SQL import odoo.modules from ..models import BaseModel -from odoo.tools import pycompat, Query, _generate_table_alias +from odoo.tools import pycompat, Query, _generate_table_alias, sql # Domain operators. @@ -968,11 +968,17 @@ class expression(object): if self._has_trigram and field.index == 'trigram' and sql_operator in ('=', 'like', 'ilike'): # a prefilter using trigram index to speed up '=', 'like', 'ilike' # '!=', '<=', '<', '>', '>=', 'in', 'not in', 'not like', 'not ilike' cannot use this trick - _unaccent = self._unaccent(field) - _left = _unaccent(f'''jsonb_path_query_array("{alias}"."{left}", '$.*')::text''') - _sql_operator = 'like' if sql_operator == '=' else sql_operator - expr = f"{_left} {_sql_operator} {_unaccent('%s')} AND " - params.append(f'%{right}%') + if sql_operator == '=': + _right = sql.value_to_translated_trigram_pattern(right) + else: + _right = sql.pattern_to_translated_trigram_pattern(right) + + if _right != '%': + _unaccent = self._unaccent(field) + _left = _unaccent(f'''jsonb_path_query_array("{alias}"."{left}", '$.*')::text''') + _sql_operator = 'like' if sql_operator == '=' else sql_operator + expr = f"{_left} {_sql_operator} {_unaccent('%s')} AND " + params.append(_right) unaccent = self._unaccent(field) if sql_operator.endswith('like') else lambda x: x lang = model.env.lang or 'en_US' diff --git a/odoo/tools/sql.py b/odoo/tools/sql.py index 0cb8168e684..99bfdf369a8 100644 --- a/odoo/tools/sql.py +++ b/odoo/tools/sql.py @@ -4,6 +4,8 @@ # pylint: disable=sql-injection import logging +import json +import re import psycopg2 from psycopg2.sql import SQL, Identifier @@ -363,3 +365,59 @@ def increment_fields_skiplock(records, *fields): cr = records._cr cr.execute(query, {'ids': records.ids}) return bool(cr.rowcount) + + +def value_to_translated_trigram_pattern(value): + """ Escape value to match a translated field's trigram index content + + The trigram index function jsonb_path_query_array("column_name", '$.*')::text + uses all translations' representations to build the indexed text. So the + original text needs to be JSON-escaped correctly to match it. + + :param str value: value provided in domain + :return: a pattern to match the indexed text + """ + if len(value) < 3: + # matching less than 3 characters will not take advantage of the index + return '%' + + # apply JSON escaping to value + json_escaped = json.dumps(value)[1:-1] + + # apply PG wildcard escaping to JSON-escaped text + wildcard_escaped = re.sub(r'(_|%|\\)', r'\\\1', json_escaped) + + # add wildcards around it to get the pattern + return f"%{wildcard_escaped}%" + + +def pattern_to_translated_trigram_pattern(pattern): + """ Escape pattern to match a translated field's trigram index content + + The trigram index function jsonb_path_query_array("column_name", '$.*')::text + uses all translations' representations to build the indexed text. So the + original pattern needs to be JSON-escaped correctly to match it. + + :param str pattern: value provided in domain + :return: a pattern to match the indexed text + """ + # find the parts around (non-escaped) wildcard characters (_, %) + sub_patterns = re.findall(r''' + ( + (?:.)*? # 0 or more charaters including the newline character + (?= 3] + + # apply PG wildcard escaping to JSON-escaped texts (% becomes \%) + wildcard_escaped = [re.sub(r'(_|%|\\)', r'\\\1', t) for t in json_escaped] + + # replace the original wildcard characters by % + return f"%{'%'.join(wildcard_escaped)}%" if wildcard_escaped else "%"