From e57e2ee378e34088ae937516c3c98c623e5ac3a2 Mon Sep 17 00:00:00 2001
From: Samuel Degueldre <sad@odoo.com>
Date: Wed, 22 Jun 2022 15:36:36 +0200
Subject: [PATCH] [FIX] blockdom: fix crash when class object key has leading
 spaces

Previously, having a leading or trailing space caused
HTMLElement.classList.add to be called with an empty string (because we
are splitting on whitespace), which is not allowed and caused a crash.
This commit fixes that by trimming the keys of the class object in the
same way that we already do it for class strings.
---
 src/runtime/blockdom/attributes.ts            |  4 +
 .../__snapshots__/attributes.test.ts.snap     | 84 +++++++++++++++++++
 tests/compiler/attributes.test.ts             | 30 +++++++
 3 files changed, 118 insertions(+)

diff --git a/src/runtime/blockdom/attributes.ts b/src/runtime/blockdom/attributes.ts
index d3466be9..f831547c 100644
--- a/src/runtime/blockdom/attributes.ts
+++ b/src/runtime/blockdom/attributes.ts
@@ -93,6 +93,10 @@ function toClassObj(expr: string | number | { [c: string]: any }) {
       for (let key in expr as any) {
         const value = (expr as any)[key];
         if (value) {
+          key = trim.call(key);
+          if (!key) {
+            continue;
+          }
           const words = split.call(key, wordRegexp);
           for (let word of words) {
             result[word] = value;
diff --git a/tests/compiler/__snapshots__/attributes.test.ts.snap b/tests/compiler/__snapshots__/attributes.test.ts.snap
index 60cc10cb..f4e51d84 100644
--- a/tests/compiler/__snapshots__/attributes.test.ts.snap
+++ b/tests/compiler/__snapshots__/attributes.test.ts.snap
@@ -168,6 +168,48 @@ exports[`attributes dynamic class attribute evaluating to 0 1`] = `
 }"
 `;
 
+exports[`attributes dynamic class attribute that starts and ends with a space 1`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  
+  let block1 = createBlock(\`<div block-attribute-0=\\"class\\"/>\`);
+  
+  return function template(ctx, node, key = \\"\\") {
+    let attr1 = ctx['c'];
+    return block1([attr1]);
+  }
+}"
+`;
+
+exports[`attributes dynamic class attribute which is only a space 1`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  
+  let block1 = createBlock(\`<div block-attribute-0=\\"class\\"/>\`);
+  
+  return function template(ctx, node, key = \\"\\") {
+    let attr1 = ctx['c'];
+    return block1([attr1]);
+  }
+}"
+`;
+
+exports[`attributes dynamic class attribute with multiple consecutive spaces 1`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  
+  let block1 = createBlock(\`<div block-attribute-0=\\"class\\"/>\`);
+  
+  return function template(ctx, node, key = \\"\\") {
+    let attr1 = ctx['c'];
+    return block1([attr1]);
+  }
+}"
+`;
+
 exports[`attributes dynamic empty class attribute 1`] = `
 "function anonymous(app, bdom, helpers
 ) {
@@ -470,6 +512,20 @@ exports[`attributes t-att-class with multiple classes 2`] = `
 }"
 `;
 
+exports[`attributes t-att-class with multiple classes 3`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  
+  let block1 = createBlock(\`<div block-attribute-0=\\"class\\"/>\`);
+  
+  return function template(ctx, node, key = \\"\\") {
+    let attr1 = {'a  b  c':ctx['value']};
+    return block1([attr1]);
+  }
+}"
+`;
+
 exports[`attributes t-att-class with multiple classes, some of which are duplicate 1`] = `
 "function anonymous(app, bdom, helpers
 ) {
@@ -498,6 +554,34 @@ exports[`attributes t-att-class with object 1`] = `
 }"
 `;
 
+exports[`attributes t-att-class with object 2`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  
+  let block1 = createBlock(\`<div block-attribute-0=\\"class\\"/>\`);
+  
+  return function template(ctx, node, key = \\"\\") {
+    let attr1 = {' a ':ctx['value']};
+    return block1([attr1]);
+  }
+}"
+`;
+
+exports[`attributes t-att-class with object 3`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  
+  let block1 = createBlock(\`<div block-attribute-0=\\"class\\"/>\`);
+  
+  return function template(ctx, node, key = \\"\\") {
+    let attr1 = {' ':ctx['value']};
+    return block1([attr1]);
+  }
+}"
+`;
+
 exports[`attributes t-attf-class 1`] = `
 "function anonymous(app, bdom, helpers
 ) {
diff --git a/tests/compiler/attributes.test.ts b/tests/compiler/attributes.test.ts
index 616e7d1e..a0df18ab 100644
--- a/tests/compiler/attributes.test.ts
+++ b/tests/compiler/attributes.test.ts
@@ -63,6 +63,24 @@ describe("attributes", () => {
     expect(result).toBe(`<div></div>`);
   });
 
+  test("dynamic class attribute which is only a space", () => {
+    const template = `<div t-att-class="c"/>`;
+    const result = renderToString(template, { c: " " });
+    expect(result).toBe(`<div></div>`);
+  });
+
+  test("dynamic class attribute with multiple consecutive spaces", () => {
+    const template = `<div t-att-class="c"/>`;
+    const result = renderToString(template, { c: "a  b" });
+    expect(result).toBe(`<div class="a b"></div>`);
+  });
+
+  test("dynamic class attribute that starts and ends with a space", () => {
+    const template = `<div t-att-class="c"/>`;
+    const result = renderToString(template, { c: " a " });
+    expect(result).toBe(`<div class="a"></div>`);
+  });
+
   test("dynamic undefined generic attribute", () => {
     const template = `<div t-att-thing="c"/>`;
     const result = renderToString(template, { c: undefined });
@@ -260,6 +278,14 @@ describe("attributes", () => {
     const template = `<div class="static" t-att-class="{a: b, c: d, e: f}"/>`;
     const result = renderToString(template, { b: true, d: false, f: true });
     expect(result).toBe(`<div class="static a e"></div>`);
+    // leading and trailing space in the key
+    expect(renderToString(`<div t-att-class="{' a ': value}" />`, { value: true })).toBe(
+      '<div class="a"></div>'
+    );
+    // whitespace only key
+    expect(renderToString(`<div t-att-class="{' ': value}" />`, { value: true })).toBe(
+      "<div></div>"
+    );
   });
 
   test("t-att-class with multiple classes", () => {
@@ -269,6 +295,10 @@ describe("attributes", () => {
     expect(renderToString(`<div t-att-class="{['a b c']: value}" />`, { value: true })).toBe(
       '<div class="a b c"></div>'
     );
+    // multiple spaces between classes
+    expect(renderToString(`<div t-att-class="{'a  b  c': value}" />`, { value: true })).toBe(
+      '<div class="a b c"></div>'
+    );
   });
 
   test("t-att-class with multiple classes, some of which are duplicate", () => {