[FIX] qweb: renderToString now properly escape

closes #565

src/qweb/qweb.ts

@@ -1,7 +1,7 @@
 import { EventBus } from "../core/event_bus";
 import { h, patch, VNode } from "../vdom/index";
 import { CompilationContext } from "./compilation_context";
-import { shallowEqual } from "../utils";
+import { shallowEqual, escape } from "../utils";
 import { addNS } from "../vdom/vdom";
 
 /**
@@ -348,8 +348,15 @@ export class QWeb extends EventBus {
       return vnode.text!;
     }
     const node = document.createElement(vnode.sel);
-    const result = patch(node, vnode);
+    const elem = patch(node, vnode).elm as HTMLElement;
-    return (<HTMLElement>result.elm).outerHTML;
+    function escapeTextNodes(node) {
+      if (node.nodeType === 3) {
+        node.textContent = escape(node.textContent);
+      }
+      for (let n of node.childNodes) { escapeTextNodes(n) }
+    }
+    escapeTextNodes(elem);
+    return elem.outerHTML;
   }
 
   /**

src/utils.ts

@@ -58,12 +58,9 @@ export function escape(str: string | number | undefined): string {
   if (typeof str === "number") {
     return String(str);
   }
-  return str
+  const p = document.createElement('p');
-    .replace(/&/g, "&")
+  p.textContent = str;
-    .replace(/</g, "&lt;")
+  return p.innerHTML;
-    .replace(/>/g, "&gt;")
-    .replace(/"/g, "&#x27;")
-    .replace(/`/g, "&#x60;");
 }
 
 /**

tests/helpers.ts

@@ -123,11 +123,8 @@ export function renderToString(
   context: EvalContext = {},
   extra?: any
 ): string {
-  const node = renderToDOM(qweb, t, context, extra);
+  const result = qweb.renderToString(t, context, extra);
-  const result = node instanceof Text ? node.textContent! : node.outerHTML;
+  expect(qweb.templates[t].fn.toString()).toMatchSnapshot();
-  if (result !== qweb.renderToString(t, context, extra)) {
-    throw new Error("HTML string returned by renderToString helper does not match QWeb render");
-  }
   return result;
 }
 

tests/qweb/__snapshots__/qweb.test.ts.snap

@@ -347,6 +347,29 @@ exports[`attributes tuple variable 1`] = `
 }"
 `;
 
+exports[`attributes various escapes 1`] = `
+"function anonymous(context, extra
+) {
+    // Template name: \\"test\\"
+    let scope = Object.create(context);
+    var h = this.h;
+    var _1 = '<foo';
+    var _2 = scope['bar'];
+    var _3 = \`<\${scope['baz']}>\`;
+    var _4 = scope['qux'];
+    let c5 = [], p5 = {key:5,attrs:{foo: _1,bar: _2,baz: _3}};
+    if (_4 instanceof Array) {
+        p5.attrs[_4[0]] = _4[1];
+    } else {
+        for (let key in _4) {
+            p5.attrs[key] = _4[key];
+        }
+    }
+    var vn5 = h('div', p5, c5);
+    return vn5;
+}"
+`;
+
 exports[`debugging t-debug 1`] = `
 "function anonymous(context, extra
 ) {
@@ -1553,6 +1576,22 @@ exports[`t-call (template calling with used set body 1`] = `
 }"
 `;
 
+exports[`t-esc escaping 1`] = `
+"function anonymous(context, extra
+) {
+    // Template name: \\"test\\"
+    let scope = Object.create(context);
+    var h = this.h;
+    let c1 = [], p1 = {key:1};
+    var vn1 = h('span', p1, c1);
+    var _2 = scope['var'];
+    if (_2 || _2 === 0) {
+        c1.push({text: _2});
+    }
+    return vn1;
+}"
+`;
+
 exports[`t-esc escaping on a node 1`] = `
 "function anonymous(context, extra
 ) {

tests/qweb/qweb.test.ts

@@ -120,9 +120,9 @@ describe("t-esc", () => {
     expect(renderToString(qweb, "test", { var: "ok" })).toBe("<span>ok</span>");
   });
 
-  test.skip("escaping", () => {
+  test("escaping", () => {
     qweb.addTemplate("test", `<span><t t-esc="var"/></span>`);
-    expect(renderToString(qweb, "test", { var: "<ok>" })).toBe("<span>&lt;ok&gt;</span>");
+    expect(renderToString(qweb, "test", { var: "<ok>abc</ok>" })).toBe("<span>&amp;lt;ok&amp;gt;abc&amp;lt;/ok&amp;gt;</span>");
   });
 
   test("escaping on a node", () => {
@@ -620,7 +620,7 @@ describe("attributes", () => {
     expect(result).toBe(`<div foo="a 0 is 1 of 2 ]"></div>`);
   });
 
-  test.skip("various escapes", () => {
+  test("various escapes", () => {
     // not needed??
     qweb.addTemplate(
       "test",
@@ -636,7 +636,7 @@ describe("attributes", () => {
       baz: 1,
       qux: { qux: "<>" }
     });
-    const expected = `<div foo="&lt;foo" bar="&lt;bar&gt;" baz="&lt;&quot;&lt;baz&gt;&quot;&gt;" qux="&lt;&gt;"></div>`;
+    const expected =  "<div foo=\"<foo\" bar=\"0\" baz=\"<1>\" qux=\"<>\"></div>";
     expect(result).toBe(expected);
   });