[IMP] web: prepare for de-traw-ification

A note concerning the attributes which I will probably need to take a
look at in the vdom version: the Python version has to process attf in
order to stringify individual elements, and separately stringify the
attribute value so it gets forcefully escaped even if it's
markup-safe (because markup-safety and attributes-safety are
different).

The first should not need to be performed on the JS side, because
Markup can not overload addition, therefore in JS String + Markup is
String whereas in Python it's Markup (and the String gets forcefully
escaped). In general, js!markup is currently much simpler than
py!markup, both by necessity (can't overload operators) and
simplicity (we might want to overload some of the operations
e.g. String#replace, but that's complicated and it's not been strictly
necessary for now).

Also wrt Markup / _Markup: `class` ctors can only be invoked with
`new` meaning they can't be used as template strings or regular
functions. Here `class` is useful to avoid the mess of calling the
super's constructor explicitly (which may not even be possible for
`String`), however it means we need a facade function to support our
use-cases.

Also update `utils.sprintf` to be Markup-aware: if the format string
is a Markup object, interpolated values get automatically escaped (if
necessary) and the result remains a Markup object.

If we need to perform explicit instance check we can always set
`Markup.prototype = _Markup.prototype` (I think), however in theory
that's not necessary: there are protocols in place for the relevant
pseudo-escaping operations and they ought suffice.

qweb/js divergence from qweb/py
===============================

Unlike qweb/py, qweb/js will *not* return a Markup object. That is
because in js a primitive `string` and a boxed `String` object don't
match when typechecking, and while `markup instanceof String` passes,
`typeof markup === 'string'` does not.

The overwhelming majority of string typechecks are the latter: there
are all of 6 `instanceof String` in the entire codebase, all in
dependencies, while there are hundreds of `typeof $X === 'string'`,
several of which get fed the output of template rendering
e.g. `jQuery.parseXML` or `AbstractView#init` (some widgets will
render a template then use it as the `arch` of a subview, so
`viewInfo.arch` can be the output of a qweb template rendering). This
makes for very annoying and somewhat gnarly debugging.

Plus jQuery in particular really doesn't like being fed a boxed
String, as it will interpret said boxed string as an array, and assume
it's an array of DOM elements to wrap, leading to a rather strange
jQuery object as output. Since feeding the result of a template
rendering to jQuery is a major use-case in non-vdom widgets... that's
a bit of an issue.

For the same reason while `_.escape` is `Markup`-aware, unlike
`markupsafe-escape` it does not *produce*, though it is
`Markup`-transparent.
This commit is contained in:
Xavier Morel
2021-07-20 05:37:42 +00:00
parent 7a8c4068de
commit 7e1827a011
3 changed files with 296 additions and 111 deletions
+26 -13
View File
@@ -28,7 +28,7 @@ WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
var QWeb2 = {
expressions_cache: { },
RESERVED_WORDS: 'true,false,NaN,null,undefined,debugger,console,window,in,instanceof,new,function,return,this,typeof,eval,void,Math,RegExp,Array,Object,Date'.split(','),
ACTIONS_PRECEDENCE: 'foreach,if,elif,else,call,set,tag,esc,raw,js,debug,log'.split(','),
ACTIONS_PRECEDENCE: 'foreach,if,elif,else,call,set,tag,out,esc,raw,js,debug,log'.split(','),
WORD_REPLACEMENT: {
'and': '&&',
'or': '||',
@@ -65,15 +65,14 @@ var QWeb2 = {
js_escape: function(s, noquotes) {
return (noquotes ? '' : "'") + s.replace(/\r?\n/g, "\\n").replace(/'/g, "\\'") + (noquotes ? '' : "'");
},
html_escape: function(s, attribute) {
html_escape: function(s) {
if (s == null) {
return '';
}
s = String(s).replace(/&/g, '&amp;').replace(/</g, '&lt;').replace(/>/g, '&gt;');
if (attribute) {
s = s.replace(/"/g, '&quot;');
}
return s;
return _.escape(s);
},
markup(s) {
return new _Markup(s);
},
gen_attribute: function(o) {
if (o !== null && o !== undefined) {
@@ -94,7 +93,10 @@ var QWeb2 = {
return '';
},
format_attribute: function(name, value) {
return ' ' + name + '="' + this.html_escape(value, true) + '"';
// we want to ensure `value` is *not* a `Markup`, because markup-safe
// strings are not necessarily attributes-safe.
const attrvalue = value == null ? '' : this.html_escape(String(value));
return ` ${name}="${attrvalue}"`;
},
extend: function(dst, src, exclude) {
for (var p in src) {
@@ -149,9 +151,9 @@ var QWeb2 = {
var new_dict = this.extend({}, old_dict);
new_dict['__caller__'] = old_dict['__template__'];
if (callback) {
new_dict[0] = callback(context, new_dict);
new_dict[0] = this.markup(callback(context, new_dict));
}
return context.engine._render(template, new_dict);
return this.markup(context.engine._render(template, new_dict));
},
foreach: function(context, enu, as, old_dict, callback) {
if (enu != null) {
@@ -689,7 +691,7 @@ QWeb2.Element = (function() {
s && r.push(_this.engine.tools.js_escape(s));
}
var re = /(?:#{(.+?)}|{{(.+?)}})/g, start = 0, r = [], m;
var re = /#{(.+?)}|{{(.+?)}}/g, start = 0, r = [], m;
while (m = re.exec(s)) {
// extract literal string between previous and current match
append_literal(s.slice(start, re.lastIndex - m[0].length));
@@ -800,7 +802,7 @@ QWeb2.Element = (function() {
this.bottom("}));");
this.indent();
this.top("var r = [];");
return this.bottom("return r.join('');");
return this.bottom("return context.engine.tools.markup(r.join(''));");
}
},
compile_action_set : function(value) {
@@ -822,11 +824,14 @@ QWeb2.Element = (function() {
this.bottom("})(dict);");
this.indent();
this.top("var r = [];");
this.bottom("return r.join('');");
this.bottom("return context.engine.tools.markup(r.join(''));");
}
}
},
compile_action_esc : function(value) {
return this.compile_action_out(value);
},
compile_action_out(value) {
this.top("var t = " + this.format_str(value) + ";");
this.top("if (t != null) r.push(context.engine.tools.html_escape(t));");
this.top("else {");
@@ -834,6 +839,14 @@ QWeb2.Element = (function() {
this.indent();
},
compile_action_raw : function(value) {
let e = this.node;
while (e.parentElement && !e.getAttribute('t-name')) {
e = e.parentElement;
}
console.warn(
"Found deprecated directive '@t-raw=\"%s\"' in "
+ "template '%s'. Replace by @t-out, and explicitely wrap content in "
+ "utils.Markup if necessary.", value, e.getAttribute('t-name'));
this.top("var t = " + this.format_str(value) + ";");
this.top("if (t != null) r.push(t);");
this.top("else {");
+97 -10
View File
@@ -240,9 +240,81 @@ class UnknownPatchError extends Error {
}
}
const __escape = _.escape;
_.escapeMethod = Symbol('html')
_.escape = function escape(s) {
return s[_.escapeMethod] ? s[_.escapeMethod]() : __escape(s);
}
// notable issues:
// * objects can't be negative in JS, so !!"" -> false but
// !!(new String) -> true, likewise markup
// TODO (?)
// * Markup.join / Markup#join => escapes items and returns a Markup
// * Markup#replace => automatically escapes the replacements (difficult impl)
class _Markup extends String {
[_.escapeMethod]() {
return this;
}
}
// exposed for qweb2.js
window._Markup = _Markup;
/**
* Returns a markup object, which acts like a String but is considered safe by
* `_.escape`, and will therefore be injected as-is (without additional
* escaping) in templates. Can be used to inject dynamic HTML in templates
* (where the template itself can't), see first example.
*
* Can also be used as a *template tag*, in which case the literal content
* won't be escaped but the substitutions which are not already markup objects
* will be.
*
* ## WARNINGS:
* * A markup object is a `String` (boxed) but not a `string` (primitive), they
* typecheck differently which can be relevant.
* * To strip out the "markupness", just call `String(markup)`.
* * Most string operations (e.g. concatenation, `String#replace`, ...) will
* also strip out markupness
* * If the input is empty, returns a regular string (that way boolean tests
* work as expected).
*
* @returns a markup object
*
* @example regular function
* let h;
* if (someTest) {
* h = Markup(_t("This is a <strong>success</strong>"));
* } else {
* h = Markup(_t("Things did <strong>not</strong> work out"));
* }
* qweb.render("some_template", { message: h });
*
* @example template tag
* const escaped = "<some> text";
* const asis = Markup`some <b>text</b>`;
* const h = Markup`Regular strings get ${escaped} but markup is injected ${asis}`;
*/
function Markup(v, ...exprs) {
if (!(v instanceof Array)) {
return v ? new _Markup(v) : '';
}
const elements = [];
let i = 0;
for(; i < exprs.length; ++i) {
elements.push(v[i], _.escape(exprs[i]));
}
elements.push(v[i]);
const s = elements.join('');
if (!s) { return '' }
return new _Markup(s);
}
var utils = {
AlreadyDefinedPatchError,
UnknownPatchError,
Markup,
/**
* Throws an error if the given condition is not true
@@ -818,25 +890,40 @@ var utils = {
},
/**
* Returns a string formatted using given values.
*
* If the value is an object, its keys will replace `%(key)s` expressions.
* If the values are a set of strings, they will replace `%s` expressions.
* If no value is given, the string will not be formatted.
*
* @param {string} string
* @param {(Object|...string)} values
* `Markup`-aware: if a `Markup` object is provided as format string,
* automatically escapes injected values and returns a new `Markup`
* object.
*
* @param {string|Markup} string format string
* @param values values injected into the format string, can be either a
* sequence of positional parameters (for positional
* placeholders) or a single Object acting a as map (for named
* placeholders).
* @returns {string|Markup}
*/
sprintf: function (string, ...values) {
sprintf(string, ...values) {
let finalizer, mapper;
finalizer = mapper = a => a;
if (string instanceof _Markup) {
string = String(string);
finalizer = Markup;
mapper = _.escape;
}
if (values.length === 1 && typeof values[0] === 'object') {
const valuesDict = values[0];
for (const value in valuesDict) {
string = string.replace(`%(${value})s`, valuesDict[value]);
}
string = string.replace(
/%\((\w+)\)s/g,
(_, group) => mapper(valuesDict[group]));
} else {
for (const value of values) {
string = string.replace(/%s/, value);
}
let i = 0;
string = string.replace(/%s/g, () => mapper(values[i++]));
}
return string;
return finalizer(string);
},
/**
* Sort an array in place, keeping the initial order for identical values.
+173 -88
View File
@@ -1,6 +1,8 @@
odoo.define('web.qweb_tests', function (require) {
"use strict";
const {Markup} = require('web.utils');
var qwebPath = '/web/static/lib/qweb/';
const {hushConsole} = require('@web/../tests/helpers/utils');
@@ -9,99 +11,182 @@ function trim(s) {
}
/**
* Loads the template file, and executes all the test template in a
* qunit module $title
* Promise-based wrapper for QWeb2.Engine#add_template
*
* The base version is callbacks-based which is a bit shit, and it also has
* variable asynchronicity: it'll be async if passed a URL but not if passed
* either a Document or an XML string.
*
* Either way this converts the `(error, doc)` callback to a `Promise<Document>`
* as in it literally returns the parsed DOM Document.
*
* @param qweb the qweb instance to load the template into
* @param {String|Document} template qweb template (Document, template file, or template URL)
*/
function loadTest(assert, template, context) {
var done = assert.async();
assert.expect(1);
var qweb = new window.QWeb2.Engine();
var prom = new Promise(function (resolve, reject) {
qweb.add_template(qwebPath + template, function (error, doc) {
if (error) {
return reject(error);
}
resolve({
qweb: qweb,
doc: doc
});
});
function add_template(qweb, template) {
return new Promise(function (resolve, reject) {
qweb.add_template(template, (error, doc) => error ? reject(error) : resolve(doc));
});
prom.then(function (r) {
var qweb = r.qweb;
var doc = r.doc;
assert.expect(doc.querySelectorAll('result').length);
var templates = qweb.templates;
for (var template in templates) {
try {
if (!templates.hasOwnProperty(template)) {
continue;
}
// ignore templates whose name starts with _, they're
// helpers/internal
if (/^_/.test(template)) {
continue;
}
var params = doc.querySelector('params#' + template);
var args = params ? JSON.parse(params.textContent) : (context ? _.clone(context) : {});
var results = doc.querySelector('result#' + template);
assert.equal(
trim(qweb.render(template, args)),
trim(results.textContent.replace(new RegExp(String.fromCharCode(13), 'g'), '')),
template);
} catch (error) {
assert.notOk(error.stack || error, 'Rendering error');
}
}
done();
});
return prom;
}
QUnit.module('QWeb', {
before() {
this.oldConsole = window.console;
window.console = hushConsole;
/**
* Loads the template file, and executes all the test template in a
* qunit module $title
*
* @param assert QUnit assertion module
* @param {String|Document} templateFile template container to load
* @param {Object} [context] additional rendering context
*/
async function loadTest(assert, templateFile, context) {
const qweb = new window.QWeb2.Engine();
const doc = await add_template(qweb, qwebPath + templateFile);
assert.expect(doc.querySelectorAll('result').length);
const templates = qweb.templates;
for (const template in templates) {
if (!templates.hasOwnProperty(template)) {
continue;
}
// ignore templates whose name starts with _, they're
// helpers/internal
if (/^_/.test(template)) {
continue;
}
const results = doc.querySelector(`result#${template}`).textContent.replace(/\r/g, '');
const params = doc.querySelector(`params#${template}`) || {textContent: 'null'};
const args = {...JSON.parse(params.textContent), ...context};
try {
assert.equal(trim(qweb.render(template, args)), trim(results), template);
} catch (error) {
assert.notOk(error.stack || error, `Rendering error for ${template} (in ${templateFile} with context ${JSON.stringify(args)}).`);
}
}
}
const table = [
{name: 'Output', file: 'qweb-test-output.xml'},
{name: 'Context-setting', file: 'qweb-test-set.xml'},
{name: 'Conditionals', file: 'qweb-test-conditionals.xml'},
{name: 'Attributes manipulation', file: 'qweb-test-attributes.xml'},
{name: 'Templates calling (to the faraway pages)', file: 'qweb-test-call.xml', context: {True: true}},
{name: 'Foreach', file: 'qweb-test-foreach.xml'},
{
name: 'Global', file: 'qweb-test-global.xml',
// test uses python syntax
context: {bool: (v) => !!v ? 'True' : 'False'},
fixture: {
before() {
this.WORD_REPLACEMENT = window.QWeb2.WORD_REPLACEMENT;
window.QWeb2.WORD_REPLACEMENT = _.extend(
{not: '!', None: 'undefined'},
this.WORD_REPLACEMENT
)
},
after() {
window.QWeb2.WORD_REPLACEMENT = this.WORD_REPLACEMENT;
}
}
},
after() {window.console = this.oldConsole;}
}, function () {
QUnit.test('Output', function (assert) {
loadTest(assert, 'qweb-test-output.xml');
});
QUnit.test('Context-setting', function (assert) {
loadTest(assert, 'qweb-test-set.xml');
});
QUnit.test('Conditionals', function (assert) {
loadTest(assert, 'qweb-test-conditionals.xml');
});
QUnit.test('Attributes manipulation', function (assert) {
loadTest(assert, 'qweb-test-attributes.xml');
});
QUnit.test('Template calling (to the faraway pages)', function (assert) {
loadTest(assert, 'qweb-test-call.xml', {True: true});
});
QUnit.test('Foreach', function (assert) {
loadTest(assert, 'qweb-test-foreach.xml');
});
QUnit.test('Global', function (assert) {
// test use python syntax
var WORD_REPLACEMENT = window.QWeb2.WORD_REPLACEMENT;
window.QWeb2.WORD_REPLACEMENT = _.extend({not: '!', None: 'undefined'}, WORD_REPLACEMENT);
loadTest(assert, 'qweb-test-global.xml', {bool: function (v) { return !!v ? 'True' : 'False';}})
.then(function () {
window.QWeb2.WORD_REPLACEMENT = WORD_REPLACEMENT;
}, function () {
window.QWeb2.WORD_REPLACEMENT = WORD_REPLACEMENT;
});
});
QUnit.test('Template inheritance', function (assert) {
loadTest(assert, 'qweb-test-extend.xml');
{name: 'Template Inheritance', file: 'qweb-test-extend.xml'},
];
QUnit.module('QWeb', {
beforeEach() {
this.oldConsole = window.console;
window.console = hushConsole;
},
afterEach() {window.console = this.oldConsole;}
}, () => {
for(const {name, file, context, fixture} of table) {
QUnit.test(name, async assert => {
// fake expect to avoid qunit being a pain in the ass, loadTest will
// update it
assert.expect(1);
if (fixture && 'before' in fixture) { await fixture.before(); }
try {
await loadTest(assert, file, context);
} finally {
if (fixture && 'after' in fixture) { await fixture.after(); }
}
});
}
QUnit.test('escape', assert => {
// not strictly about qweb...
assert.expect(8);
assert.equal(_.escape('a'), 'a');
assert.equal(_.escape('<a>'), '&lt;a&gt;');
assert.equal(_.escape({[_.escapeMethod]() { return 'a'; }}), 'a');
assert.equal(_.escape({[_.escapeMethod]() { return '<a>'; }}), '<a>');
assert.equal(_.escape(Markup('a')), 'a');
assert.equal(_.escape(Markup('<a>')), '<a>');
assert.equal(_.escape(Markup`a`), 'a');
assert.equal(_.escape(Markup`<a>`), '<a>');
});
QUnit.module('t-out', {}, () => {
QUnit.test("basics", async assert => {
assert.expect(5);
const qweb = new QWeb2.Engine;
await add_template(qweb, `<templates>
<t t-name="t-out"><p><t t-out="value"/></p></t>
</templates>`);
assert.equal(
qweb.render('t-out', {value: '<i>test</i>'}),
'<p>&lt;i&gt;test&lt;/i&gt;</p>',
"regular t-out should just escape the contents"
);
assert.equal(
qweb.render('t-out', {value: Markup('<i>test</i>')}),
'<p><i>test</i></p>',
"Markup contents should not be escaped"
);
assert.equal(
qweb.render('t-out', {value: Markup`<i>test ${1}</i>`}),
'<p><i>test 1</i></p>',
"markup template string should not be escaped"
);
const teststr = '<i>test</i>';
assert.equal(
qweb.render('t-out', {value: Markup`<b>${teststr}</b>`}),
'<p><b>&lt;i&gt;test&lt;/i&gt;</b></p>',
"the markup template should not be escaped but what it uses should be"
);
const testMarkup = Markup(teststr);
assert.equal(
qweb.render('t-out', {value: Markup`<b>${testMarkup}</b>`}),
'<p><b><i>test</i></b></p>',
"markupception"
);
});
QUnit.test("Set", async assert => {
assert.expect(4);
const qweb = new QWeb2.Engine;
await add_template(qweb, `<templates>
<t t-name="litval">
<t t-set="x" t-value="'&lt;a/&gt;'"/>
<x><t t-out="x"/></x>
</t>
<t t-name="body">
<t t-set="x"><a/></t>
<x><t t-out="x"/></x>
</t>
<t t-name="value">
<t t-set="x" t-value="val"/>
<x><t t-out="x"/></x>
</t>
<t t-name="bodyout">
<t t-set="x"><t t-out="val"/></t>
<x><t t-out="x"/></x>
</t>
</templates>`);
assert.equal(trim(qweb.render('litval')), '<x>&lt;a/&gt;</x>');
assert.equal(trim(qweb.render('body')), '<x><a></a></x>');
assert.equal(trim(qweb.render('value', {val: '<a/>'})), '<x>&lt;a/&gt;</x>');
assert.equal(trim(qweb.render('bodyout', {val: '<a/>'})), '<x>&lt;a/&gt;</x>');
});
})
});
});