From e95ccf0908c263f6cf23c91c477a8e252e936ff1 Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Wed, 5 Feb 2020 10:34:51 +0000 Subject: [FIX] *: domains should not have as many binary operator as there are terms * modify client-side domain normalisation to error if the domain is invalid (not enough terms / segments for the number of operators) * add better error reporting to attrs / modifiers parsing * add a few layered error augmentation to provide clearer context e.g. "error: invalid domain " is helpful but "error while parsing modifiers for field foo: modifier invisible: invalid domain " is much more helpful * rework _evalModifiers to deduplicate it in order to more easily implement this contextual augmentation * test that improper domains are properly found improper * fix a bunch of incorrect attrs domains * also removed an apparently undefined (& unused) "options" argument to a _applyModifiers call closes odoo/odoo#44642 Related: odoo/enterprise#8175 Signed-off-by: Xavier Morel (xmo) --- addons/web/static/src/js/core/domain.js | 7 +++++ .../web/static/src/js/views/basic/basic_model.js | 36 ++++++++++------------ .../static/src/js/views/basic/basic_renderer.js | 13 ++++++-- 3 files changed, 34 insertions(+), 22 deletions(-) (limited to 'addons/web/static/src/js') diff --git a/addons/web/static/src/js/core/domain.js b/addons/web/static/src/js/core/domain.js index b54dd54d0b6..0db9847645d 100644 --- a/addons/web/static/src/js/core/domain.js +++ b/addons/web/static/src/js/core/domain.js @@ -383,8 +383,10 @@ var Domain = collections.Tree.extend({ * to normalize (! will be normalized in-place) * @returns {Array} the normalized JS prefix-array representation of the * given domain + * @throws {Error} if the domain is invalid and can't be normalised */ normalizeArray: function (domain) { + if (domain.length === 0) { return domain; } var expected = 1; _.each(domain, function (item) { if (item === "&" || item === "|") { @@ -395,6 +397,11 @@ var Domain = collections.Tree.extend({ }); if (expected < 0) { domain.unshift.apply(domain, _.times(Math.abs(expected), _.constant("&"))); + } else if (expected > 0) { + throw new Error(_.str.sprintf( + "invalid domain %s (missing %d segment(s))", + JSON.stringify(domain), expected + )); } return domain; }, diff --git a/addons/web/static/src/js/views/basic/basic_model.js b/addons/web/static/src/js/views/basic/basic_model.js index 1ed8c36bb98..532af65744c 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -2259,31 +2259,27 @@ var BasicModel = AbstractModel.extend({ * context. * @param {Object} modifiers * @returns {Object} + * @throws {Error} if one of the modifier domains is invalid */ _evalModifiers: function (element, modifiers) { - var result = {}; - var self = this; - var evalContext; - function evalModifier(mod) { + let evalContext = null; + const evaluated = {}; + for (const k of ['invisible', 'column_invisible', 'readonly', 'required']) { + const mod = modifiers[k]; if (mod === undefined || mod === false || mod === true) { - return !!mod; + if (k in modifiers) { + evaluated[k] = !!mod; + } + continue; + } + try { + evalContext = evalContext || this._getEvalContext(element); + evaluated[k] = new Domain(mod, evalContext).compute(evalContext); + } catch (e) { + throw new Error(_.str.sprintf('for modifier "%s": %s', k, e.message)); } - evalContext = evalContext || self._getEvalContext(element); - return new Domain(mod, evalContext).compute(evalContext); - } - if ('invisible' in modifiers) { - result.invisible = evalModifier(modifiers.invisible); - } - if ('column_invisible' in modifiers) { - result.column_invisible = evalModifier(modifiers.column_invisible); - } - if ('readonly' in modifiers) { - result.readonly = evalModifier(modifiers.readonly); - } - if ('required' in modifiers) { - result.required = evalModifier(modifiers.required); } - return result; + return evaluated; }, /** * Fetch all name_gets for the many2ones in a group diff --git a/addons/web/static/src/js/views/basic/basic_renderer.js b/addons/web/static/src/js/views/basic/basic_renderer.js index 046d10a741f..ba407b766c1 100644 --- a/addons/web/static/src/js/views/basic/basic_renderer.js +++ b/addons/web/static/src/js/views/basic/basic_renderer.js @@ -547,6 +547,7 @@ var BasicRenderer = AbstractRenderer.extend(WidgetAdapterMixin, { * value (if not given, it is set to this.mode, the mode of the renderer) * @returns {Object} for code efficiency, returns the last evaluated * modifiers for the given node and record. + * @throws {Error} if one of the modifier domains is not valid */ _registerModifiers: function (node, record, element, options) { options = options || {}; @@ -575,7 +576,15 @@ var BasicRenderer = AbstractRenderer.extend(WidgetAdapterMixin, { // Evaluate if necessary if (!modifiersData.evaluatedModifiers[record.id]) { - modifiersData.evaluatedModifiers[record.id] = record.evalModifiers(modifiersData.modifiers); + try { + modifiersData.evaluatedModifiers[record.id] = record.evalModifiers(modifiersData.modifiers); + } catch (e) { + throw new Error(_.str.sprintf( + "While parsing modifiers for %s%s: %s", + node.tag, node.tag === 'field' ? ' ' + node.attrs.name : '', + e.message + )); + } } // Element might not be given yet (a second call to the function can @@ -597,7 +606,7 @@ var BasicRenderer = AbstractRenderer.extend(WidgetAdapterMixin, { } modifiersData.elementsByRecord[record.id].push(newElement); - this._applyModifiers(modifiersData, record, newElement, options); + this._applyModifiers(modifiersData, record, newElement); } return modifiersData.evaluatedModifiers[record.id]; -- cgit v1.2.3