From 84abddfe904273daaa3be744b974c8cce98c30fb Mon Sep 17 00:00:00 2001 From: Mathieu Duckerts-Antoine Date: Tue, 12 Feb 2019 10:17:58 +0000 Subject: [PATCH] [FIX] web: quotes and backslashes in search view The commit 6d3ada1 aimed to solve problems occuring whith (single or double) quotes when creating new custom filters. Unfortunately, the point was missed and bugs were still present. The present commit solves the orginial problem satisfactorily. There was also a problem in the management of backslashes. Type '\\' in the search bar would lead to a domain like [('...', 'ilike', '\\\\\\\\\\\\\\\\')]. This has also been fixed. Note that it is necessary to input "\\" if one is looking for a backslash in db (as before). A remark on the _formatAST function (py_utils.js): We deescape only the chars `'`, `"`, and `\` because it does not seem possible to produce a string in the search view that would intend a search for a line return or a tab for example. A general remark: Domains should always be in normalized form. In that way they could be manipulated and combined together without a tokenization/parsing! closes odoo/odoo#31025 --- addons/web/static/lib/py.js/lib/py.js | 9 +-- addons/web/static/src/js/core/py_utils.js | 10 ++- .../tests/chrome/action_manager_tests.js | 2 +- .../web/static/tests/core/py_utils_tests.js | 35 ++++----- .../tests/views/search_filters_menu_tests.js | 71 +++++++++++++++++++ .../static/tests/views/search_view_tests.js | 10 +-- .../static/tests/views/view_dialogs_tests.js | 2 +- 7 files changed, 102 insertions(+), 37 deletions(-) diff --git a/addons/web/static/lib/py.js/lib/py.js b/addons/web/static/lib/py.js/lib/py.js index bd794323995..60b80a7f439 100644 --- a/addons/web/static/lib/py.js/lib/py.js +++ b/addons/web/static/lib/py.js/lib/py.js @@ -273,8 +273,7 @@ var py = {}; var Special = '[:;.,`@]'; var Funny = group(Operator, Bracket, Special); - var ContStr = group("([uU])?'([^']*)'", '([uU])?"([^"]*)"'); - + var ContStr = group("([uU])?'([^\n'\\\\]*(?:\\\\.[^\n'\\\\]*)*)'", '([uU])?"([^\n"\\\\]*(?:\\\\.[^\n"\\\\]*)*)"'); var PseudoToken = Whitespace + group(Number, Funny, ContStr, Name); var number_pattern = new RegExp('^' + Number + '$'); @@ -282,9 +281,6 @@ var py = {}; var name_pattern = new RegExp('^' + Name + '$'); var strip = new RegExp('^' + Whitespace); return function tokenize(s) { - s = s - .replace(/\\\"/g, '\\u0022') // for double quote - .replace(/\\\'/g, '\\u0027'); // for single quote var max=s.length, tokens = [], start, end; // /g flag makes repeated exec() have memory var pseudoprog = new RegExp(PseudoToken, 'g'); @@ -313,9 +309,6 @@ var py = {}; } else if (string_pattern.test(token)) { var m = string_pattern.exec(token); var value = (m[3] !== undefined ? m[3] : m[5]); - value - .replace(/\\u0022/g, '"') - .replace(/\\u0027/g, "'"); tokens.push(create(symbols['(string)'], { unicode: !!(m[2] || m[4]), value: value diff --git a/addons/web/static/src/js/core/py_utils.js b/addons/web/static/src/js/core/py_utils.js index 9bcd66864e5..296ab5c3089 100644 --- a/addons/web/static/src/js/core/py_utils.js +++ b/addons/web/static/src/js/core/py_utils.js @@ -415,7 +415,15 @@ function _formatAST(ast, lbp) { case "(number)": return String(ast.value); case "(string)": - return "'" + ast.value + "'"; + // ast.value is a string that may contain a mix of single and double quotes. + // But we need to return a string that represents the string ast.value. + // We use for that JSON.stringify. It will return a string with quotes correctly + // escaped. For instance, for x = `""` JSON.stringify(x) is `"\"\""`. + // But this means that if we pass several times here, escapes will be + // done again and again, leading to a profusion of backslashes that we don't want. + // For fun consider y = `\\` and y = JSON.stringify(y). + // This is why we first deescape particular characters in ast.value. + return JSON.stringify(ast.value.replace(/(\\(['"\\]))/g,"\$2")); case "(constant)": return ast.value; case "(name)": diff --git a/addons/web/static/tests/chrome/action_manager_tests.js b/addons/web/static/tests/chrome/action_manager_tests.js index cd123225357..45076903d9e 100644 --- a/addons/web/static/tests/chrome/action_manager_tests.js +++ b/addons/web/static/tests/chrome/action_manager_tests.js @@ -3071,7 +3071,7 @@ QUnit.module('ActionManager', { intercepts: { create_filter: function (event) { var filter = event.data.filter; - assert.deepEqual(filter.domain, "[('bar', '=', 1)]", + assert.deepEqual(filter.domain, `[("bar", "=", 1)]`, "should save the correct domain"); assert.deepEqual(filter.context, {shouldBeInFilterContext: true}, "should save the correct context"); diff --git a/addons/web/static/tests/core/py_utils_tests.js b/addons/web/static/tests/core/py_utils_tests.js index be5760fc6c7..fba8017cf01 100644 --- a/addons/web/static/tests/core/py_utils_tests.js +++ b/addons/web/static/tests/core/py_utils_tests.js @@ -1149,7 +1149,7 @@ QUnit.module('core', function () { assert.checkAST("1.4", "float value"); assert.checkAST("-12", "negative integer value"); assert.checkAST("True", "boolean"); - assert.checkAST("'some string'", "a string"); + assert.checkAST(`"some string"`, "a string"); assert.checkAST("None", "None"); }); @@ -1157,8 +1157,8 @@ QUnit.module('core', function () { assert.expect(3); assert.checkAST("{}", "empty dictionary"); - assert.checkAST("{'a': 1}", "dictionary with a single key"); - assert.checkAST("d['a']", "get a value in a dictionary"); + assert.checkAST(`{"a": 1}`, "dictionary with a single key"); + assert.checkAST(`d["a"]`, "get a value in a dictionary"); }); QUnit.test("list", function (assert) { @@ -1225,14 +1225,14 @@ QUnit.module('core', function () { QUnit.test("strftime", function (assert) { assert.expect(3); - assert.checkAST("time.strftime('%Y')", "strftime with year"); - assert.checkAST("time.strftime('%Y') + '-01-30'", "strftime with year"); - assert.checkAST("time.strftime('%Y-%m-%d %H:%M:%S')", "strftime with year"); + assert.checkAST(`time.strftime("%Y")`, "strftime with year"); + assert.checkAST(`time.strftime("%Y") + "-01-30"`, "strftime with year"); + assert.checkAST(`time.strftime("%Y-%m-%d %H:%M:%S")`, "strftime with year"); }); QUnit.test("context_today", function (assert) { assert.expect(1); - assert.checkAST("context_today().strftime('%Y-%m-%d')", "context today call"); + assert.checkAST(`context_today().strftime("%Y-%m-%d")`, "context today call"); }); @@ -1250,17 +1250,10 @@ QUnit.module('core', function () { assert.checkAST('(a - b).days', "substraction and .days"); assert.checkAST('a + day == date(2002, 3, 3)'); - var expr = "[('type', '=', 'in'), ('day', '<=', time.strftime('%Y-%m-%d')), ('day', '>', (context_today() - datetime.timedelta(days = 15)).strftime('%Y-%m-%d'))]"; + var expr = `[("type", "=", "in"), ("day", "<=", time.strftime("%Y-%m-%d")), ("day", ">", (context_today() - datetime.timedelta(days = 15)).strftime("%Y-%m-%d"))]`; assert.checkAST(expr); }); - QUnit.test("parse escaped quoted strings", function (assert) { - assert.expect(2); - - assert.checkAST(`'\"'`, "a string containing \""); - assert.checkAST(`'\'`, "a string containing \'"); - }); - QUnit.module('pyutils (_normalizeDomain)'); QUnit.assert.checkNormalization = function (domain, normalizedDomain) { @@ -1278,23 +1271,23 @@ QUnit.module('core', function () { assert.expect(3); assert.checkNormalization("[]"); - assert.checkNormalization("[('a', '=', 1)]"); - assert.checkNormalization("['!', ('a', '=', 1)]"); + assert.checkNormalization(`[("a", "=", 1)]`); + assert.checkNormalization(`["!", ("a", "=", 1)]`); }); QUnit.test("properly add the & in a non normalized domain", function (assert) { assert.expect(1); assert.checkNormalization( - "[('a', '=', 1), ('b', '=', 2)]", - "['&', ('a', '=', 1), ('b', '=', 2)]" + `[("a", "=", 1), ("b", "=", 2)]`, + `["&", ("a", "=", 1), ("b", "=", 2)]` ); }); QUnit.test("normalize domain with ! operator", function (assert) { assert.expect(1); assert.checkNormalization( - "['!', ('a', '=', 1), ('b', '=', 2)]", - "['&', '!', ('a', '=', 1), ('b', '=', 2)]" + `["!", ("a", "=", 1), ("b", "=", 2)]`, + `["&", "!", ("a", "=", 1), ("b", "=", 2)]` ); }); diff --git a/addons/web/static/tests/views/search_filters_menu_tests.js b/addons/web/static/tests/views/search_filters_menu_tests.js index 429cb6258be..e3b4fdb584a 100644 --- a/addons/web/static/tests/views/search_filters_menu_tests.js +++ b/addons/web/static/tests/views/search_filters_menu_tests.js @@ -1,7 +1,9 @@ odoo.define('web.search_filters_menu_tests', function (require) { "use strict"; +var Domain = require('web.Domain'); var FiltersMenu = require('web.FiltersMenu'); +var pyUtils = require('web.py_utils'); var testUtils = require('web.test_utils'); function createFiltersMenu(filters, fields, params) { @@ -171,5 +173,74 @@ QUnit.module('FiltersMenu', { filterMenu.destroy(); }); + + // DO NOT FORWARD PORT! THANK YOU. + QUnit.test('commit search with an extended proposition with field char does not cause a crash', function (assert) { + assert.expect(6); + + this.fields = {many2one_field: {string: "Trululu", type: "many2one", relation: 'partner', selectable: true, searchable: true}}; + + var expectedDomains = [ + [['many2one_field','ilike', `a`]], + [['many2one_field','ilike', `"a"`]], + [['many2one_field','ilike', `'a'`]], + [['many2one_field','ilike', `'`]], + [['many2one_field','ilike', `"`]], + [['many2one_field','ilike', `\\`]], + ]; + + var filterMenu = createFiltersMenu([], this.fields, { + intercepts: { + new_filters: function (ev) { + var filter = ev.data[0].filter; + Domain.prototype.stringToArray(filter.attrs.domain); + // this step combine a tokenization/parsing followed by a string formatting + var domain = pyUtils.assembleDomains([filter.attrs.domain]); + domain = Domain.prototype.stringToArray(domain); + assert.deepEqual(domain, expectedDomains.shift()); + }, + } + }); + + // open menu dropdown and custom filter submenu, select trululu field and enter string `a`, then click apply + testUtils.dom.click(filterMenu.$('span.fa-filter')); + testUtils.dom.click(filterMenu.$('.o_add_custom_filter')); + testUtils.fields.editSelect(filterMenu.$('.o_filter_condition select.o_input.o_searchview_extended_prop_field'), 'many2one_field'); + testUtils.fields.editInput(filterMenu.$('.o_filter_condition .o_searchview_extended_prop_value input'), `a`); + testUtils.dom.click(filterMenu.$('.o_apply_filter')); + + + // open custom filter submenu, select trululu field and enter string `"a"`, then click apply + testUtils.dom.click(filterMenu.$('.o_add_custom_filter')); + testUtils.fields.editSelect(filterMenu.$('.o_filter_condition select.o_input.o_searchview_extended_prop_field'), 'many2one_field'); + testUtils.fields.editInput(filterMenu.$('.o_filter_condition .o_searchview_extended_prop_value input'), `"a"`); + testUtils.dom.click(filterMenu.$('.o_apply_filter')); + + // open custom filter submenu, select trululu field and enter string `'a'`, then click apply + testUtils.dom.click(filterMenu.$('.o_add_custom_filter')); + testUtils.fields.editSelect(filterMenu.$('.o_filter_condition select.o_input.o_searchview_extended_prop_field'), 'many2one_field'); + testUtils.fields.editInput(filterMenu.$('.o_filter_condition .o_searchview_extended_prop_value input'), `'a'`); + testUtils.dom.click(filterMenu.$('.o_apply_filter')); + + // open custom filter submenu, select trululu field and enter string `'`, then click apply + testUtils.dom.click(filterMenu.$('.o_add_custom_filter')); + testUtils.fields.editSelect(filterMenu.$('.o_filter_condition select.o_input.o_searchview_extended_prop_field'), 'many2one_field'); + testUtils.fields.editInput(filterMenu.$('.o_filter_condition .o_searchview_extended_prop_value input'), `'`); + testUtils.dom.click(filterMenu.$('.o_apply_filter')); + + // open custom filter submenu, select trululu field and enter string `"`, then click apply + testUtils.dom.click(filterMenu.$('.o_add_custom_filter')); + testUtils.fields.editSelect(filterMenu.$('.o_filter_condition select.o_input.o_searchview_extended_prop_field'), 'many2one_field'); + testUtils.fields.editInput(filterMenu.$('.o_filter_condition .o_searchview_extended_prop_value input'), `"`); + testUtils.dom.click(filterMenu.$('.o_apply_filter')); + + // open custom filter submenu, select trululu field and enter string `\`, then click apply + testUtils.dom.click(filterMenu.$('.o_add_custom_filter')); + testUtils.fields.editSelect(filterMenu.$('.o_filter_condition select.o_input.o_searchview_extended_prop_field'), 'many2one_field'); + testUtils.fields.editInput(filterMenu.$('.o_filter_condition .o_searchview_extended_prop_value input'), `\\`); + testUtils.dom.click(filterMenu.$('.o_apply_filter')); + + filterMenu.destroy(); + }); }); }); diff --git a/addons/web/static/tests/views/search_view_tests.js b/addons/web/static/tests/views/search_view_tests.js index c5c46311c65..5d58e8d9cde 100644 --- a/addons/web/static/tests/views/search_view_tests.js +++ b/addons/web/static/tests/views/search_view_tests.js @@ -663,10 +663,10 @@ QUnit.module('Search View', { console.log(ev.data); assert.equal( ev.data.filter.domain, - "['&', " + - "('date_field', '>=', (context_today() + relativedelta()).strftime('%Y-%m-%d')), " + - "('date_field', '<', (context_today() + relativedelta(days = 1)).strftime('%Y-%m-%d'))"+ - "]"); + `["&", ` + + `("date_field", ">=", (context_today() + relativedelta()).strftime("%Y-%m-%d")), ` + + `("date_field", "<", (context_today() + relativedelta(days = 1)).strftime("%Y-%m-%d"))`+ + `]`); }, }, }); @@ -757,7 +757,7 @@ QUnit.module('Search View', { data: this.data, intercepts: { create_filter: function (ev) { - assert.ok(ev.data.filter.domain === "[['foo', 'ilike', 'a']]"); + assert.ok(ev.data.filter.domain === `[["foo", "ilike", "a"]]`); }, }, }); diff --git a/addons/web/static/tests/views/view_dialogs_tests.js b/addons/web/static/tests/views/view_dialogs_tests.js index 3d1942b5e22..332d0142faa 100644 --- a/addons/web/static/tests/views/view_dialogs_tests.js +++ b/addons/web/static/tests/views/view_dialogs_tests.js @@ -446,7 +446,7 @@ QUnit.module('Views', { intercepts: { create_filter: function (event) { var filter = event.data.filter; - assert.deepEqual(filter.domain, "[('bar', '=', True)]", + assert.deepEqual(filter.domain, `[("bar", "=", True)]`, "should save the correct domain"); assert.deepEqual(filter.context, {shouldBeInFilterContext: true}, "should save the correct context");